Issue #3432109: Add new route /my-settings to redirect users to their settings page

[tregismoreira]

Problem

Currently, there is no easy way for users to go to their settings page unless they know their User ID, given that the path to the original route is /user/{user}/edit. It makes the UX to be somehow degraded.

Solution

The solution is to create a custom route /my-settings which redirects the users to their settings page, so they don't need to think about it.

The same approach is already applied to other extensions (eg. /my-invites and /my-events).

Issue tracker

• https://www.drupal.org/project/social/issues/3432109
• https://getopensocial.atlassian.net/browse/PROD-28573

Theme issue tracker

N/A

How to test

• Checkout branch feature/3432109-new-route-my-settings of Open Social
• As an authenticated user
• Go to /my-settings page
• The expected result is that the user is redirected to their /user/{user}/edit page

Definition of done

Before merge

• Code/peer review is completed
• All commit messages are clear and clean. If applicable a rebase was performed
• All automated tests are green
• Functional/manual tests of the acceptance criteria are approved
• All acceptance criteria were met
• New features or changes to existing features are covered by tests, either unit (preferably) or behat
• Update path is tested. New hook_updates should respect update order, right naming convention and consider hook_post_update code
• Module can be safely uninstalled. Update/implement hook_uninstall and make sure that removed configuration or dependencies are removed/uninstalled
• This pull request has all required labels (team/type/priority)
• This pull request has a milestone
• This pull request has an assignee (if applicable)
• Any front end changes are tested on all major browsers
• New UI elements, or changes on UI elements are approved by the design team
• New features, or feature changes are approved by the product owner

After merge

• Code is tested on all branches that it has been cherry-picked
• Update hook number might need adjustment, make sure they have the correct order
• The Drupal.org ticket(s) are updated according to this pull request status

Screenshots

N/A

Release notes

N/A

Change Record

N/A

Translations

N/A

[mergeable]

Thanks for contributing towards Open Social! A maintainer from the @goalgorilla/maintainers group might not review all changes from all teams/contributors. Please don't be discouraged if it takes a while. In the meantime, we have some automated checks running and it might be that you will see our comments with some tips or requests to speed up the review process. 😊

[Kingdutch]

Hey Thiago,

This looks like a good improvement to add consistency to the set of /my-* routes we already have! :D

To make sure this keeps working (since people may bookmark it for example) I think we might benefit from a Behat test that ensures that when you're logged in and you go to /my-settings you end up on your settings edit page.

With that test we can also do some refactoring and remove the need for a custom route in social_user.routing.yml and the method in SocialUserController. The redirect behaviour we have here is provided for us by Drupal Core in the path_alias module which is already enabled in Open Social by default. We could create a path alias in hook_install to let that module take care of it: https://drupal.stackexchange.com/a/291584/2166

Finally, while I'm excited to see code improvements that remove lines from the PHPStan baseline 🎉 it's usually a good idea to pull those out to a separate PR. This makes reviewing the PRs easier because they're more focused on a specific change and don't need to check whether the other change has impact on what's being described in the PR message.

Let me know if you need any help! :D

[tregismoreira]

Hi @Kingdutch. Thanks for your feedback :)

As we discussed internally, instead of using a path alias, I've replaced the custom route controller by the core \Drupal\user\Controller\UserController::userEditPage which is the same as used in the user.edit route.

Besides, I've also added a Behat test to make sure the /my-settings path will be redirected to /user/*/edit.

Looking forward to your feedback.

[bramtenhove]

Great! Thanks Thiago 💪

Final point will be picked up in new PRs.

[Kingdutch]

The only thing left to do before we merge this is to clean up the commits :D We currently rebase all our PRs onto main when merging which causes a linear history. To ensure that history stays readable we try to avoid commits that simply say "Fix " since those commits don't really communicate intention and they're really all part of the work for the first commit :)

You can git rebase the branch locally to squash all commits into a small number of commits that document the intended changes (for this PR that could be just the first one).

Your initial commit "Issue #3432109: Add new route /my-settings to redirect users to their settings page" communicates what we do very well. I think on Slack we've had some discussion on why we ended up choosing to create the route and have it point to the existing controller. Summarising that in the body of the commit message will help developers in the future (which might be us) to see why we choose a certain solution and make an informed decision of whether those reasons have perhaps changed :)

[tregismoreira]

@Kingdutch the PHPStan fix was actually only needed because in my first solution I was creating a custom method to use in the route controller. Now that we are not changing \Drupal\social_user\Controller\SocialUserController anymore, PHPStan won't alert the anymore.

So I've removed the fix from this PR, and squashed the other commits to make it cleaner. We can create a separate PR with this fix later.

[Kingdutch]

Focused and tested PR 🎉

[Kingdutch]

🍒 picked to 12.4.x as 379c1f3