[UX] Allow to redirect logged-in user on any routes and add a redirection for registration page #15418

Jibbarth · 2023-10-08T16:10:51Z

Q	A
Branch?	1.14
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Related tickets	fixes #9984
License	MIT

When a logged-in user visit registration page, he is now redirected directly to account dashboard page, like for /login.

I targetted the sylius.customer.initialize_register dispatched by the resource controller to not use a Request event that will be triggered on every page.

However, while coding, as I added the logged_in_route in register route configuration, I was wondering if it wouldn't be better to add a generic event that check every route for the logged_in_route attribute, and in such case, we can perform the redirection.

I think it would be nice to have this ability to define a logged_in_route to redirect on every route. WDYT ?

EDIT

We can now add a logged_in_route in any route, and the redirection will occur as soon as we are logged_in.

Two way to add the attribute :

any_route:
    # ...
    defaults:
        _sylius:
            logged_in_route: sylius_shop_account_dashboard

any_route:
    # ...
    defaults:
        _sylius:
            logged_in_route: 
                name: sylius_shop_account_dashboard
                parameters: 
                    param1: value1

github-actions · 2023-10-08T16:21:33Z

Bunnyshell Preview Environment deployment failed

Check https://github.com/Sylius/Sylius/actions/runs/7862012607 for details.

Available commands:

/bns:deploy to redeploy the environment
/bns:delete to remove the environment

Prometee · 2023-10-08T16:35:29Z

@Jibbarth I test a different approach here because I use a different security config than the one present in sylius/sylius-standard :

security:
    firewalls:
        shop:
            form_login:
                default_target_path: sylius_shop_account_dashboard

In the case I'm a registered customer and I register a second one I'm redirected to the dashboard.

One drawback I saw : if your channel is configured to required the account verification then after the registration you are not logged in as the new customer.

Jibbarth · 2023-10-08T16:56:05Z

@Prometee 🤔 I'm not sure to follow you. The snippet you provide is for redirecting after login, right ?

Here, we'll make a redirection for an already logged-in user that try to reach /register page, coming from google or directly from url.

There is already such things for /login page here: https://github.com/Sylius/Sylius/blob/1.13/src/Sylius/Bundle/UiBundle/Controller/SecurityController.php#L39-L43

But maybe I missed something 😅

Jibbarth · 2023-10-08T17:05:10Z

😱 oh no, my behat test seems no valid. Any clue to fix this?

Prometee · 2023-10-08T17:31:13Z

@Jibbarth nevermind I though you wanted to allow redirecting to the dashboard when already logged in. I didn't understand that you wanted to avoid this behaviour, sorry for that.

diimpp

I think it would be nice to have this ability to define a logged_in_route to redirect on every route. WDYT ?

Sounds nice, though I didn't work with html shop for years and don't have an opinion.

src/Sylius/Bundle/ShopBundle/EventListener/AlreadyLoggedInUserRegistrationListener.php

features/account/customer_account/redirect_when_already_signed_in.feature

oallain

👍🏼

src/Sylius/Bundle/ShopBundle/EventListener/AlreadyLoggedInUserRegistrationListener.php

features/account/customer_account/redirect_when_already_signed_in.feature

src/Sylius/Bundle/ShopBundle/EventListener/AlreadyLoggedInUserRegistrationListener.php

src/Sylius/Bundle/UiBundle/Controller/SecurityController.php

Jibbarth · 2024-02-05T12:36:57Z

Sorry @Rafikooo, i didn't see your comments. I'll try to address them as soon as possible 😉

… page This fix Sylius#9984

Co-authored-by: Dmitri Perunov <diimpp@gmail.com>

…h login page

…tyController

…dmin login page

Jibbarth · 2024-02-11T12:44:01Z

Comments addressed @Rafikooo 😉

To handle both Login and registration behavior in the Listener, I moved it in UiBundle, cause the SecurityController act also for admin login.

Moreover, the event is now more flexible.

We can add a logged_in_route in any route, and the redirection will occur as soon as we are logged_in.

Two way to add the attribute :

any_route:
    # ...
    defaults:
        _sylius:
            logged_in_route: sylius_shop_account_dashboard

any_route:
    # ...
    defaults:
        _sylius:
            logged_in_route: 
                name: sylius_shop_account_dashboard
                parameters: 
                    param1: value1

Jibbarth · 2024-02-11T12:45:15Z

src/Sylius/Bundle/UiBundle/EventListener/AlreadyLoggedInUserRedirectionListener.php

+        // case logged_in_route:
+        //     name: 'app_route'
+        //     parameters:
+        //         param1: 'value1'


tell me if such comment are not needed

Jibbarth · 2024-02-11T12:46:37Z

src/Sylius/Behat/Context/Ui/Shop/AccountContext.php

@@ -40,6 +41,7 @@ public function __construct(
        private LoginPageInterface $loginPage,
        private NotificationCheckerInterface $notificationChecker,
        private SharedStorageInterface $sharedStorage,
+        private RegisterPageInterface $registerPage,


Tell me if we should gather PageInterface together in order's declaration

Jibbarth requested a review from a team as a code owner October 8, 2023 16:10

probot-autolabeler bot added the Shop ShopBundle related issues and PRs. label Oct 8, 2023

Jibbarth changed the title ~~[UX][Shop] Redirect to dashboard a logged-in user that reach register page~~ [UX][Shop] Redirect to dashboard a logged-in user who reach register page Oct 8, 2023

diimpp reviewed Oct 12, 2023

View reviewed changes

src/Sylius/Bundle/ShopBundle/EventListener/AlreadyLoggedInUserRegistrationListener.php Outdated Show resolved Hide resolved

Jibbarth force-pushed the feature/redirect-logged-in-user-when-visit-registration-page branch from 777684f to 74f1b64 Compare October 14, 2023 10:27

diimpp reviewed Oct 14, 2023

View reviewed changes

diimpp previously approved these changes Oct 22, 2023

View reviewed changes

diimpp reviewed Nov 24, 2023

View reviewed changes

features/account/customer_account/redirect_when_already_signed_in.feature Outdated Show resolved Hide resolved

oallain previously approved these changes Nov 24, 2023

View reviewed changes

Rafikooo reviewed Nov 24, 2023

View reviewed changes

Jibbarth and others added 10 commits February 11, 2024 11:57

[UX][Shop] Redirect to dashboard a logged-in user that reach register…

d214510

… page This fix Sylius#9984

Switch to a less strict "role"

586e3f0

Co-authored-by: Dmitri Perunov <diimpp@gmail.com>

Switch also to less strict role in SecurityController to prevent reac…

290b57b

…h login page

Fix behat tests

4182590

Use Property promotion constructor, remove EventSubscriberInterface

d9d8195

Behat - stick to customer_account tag for visit registration page

d8d19b1

Move Listener in UiBundle as it can concern both Admin and Shop routes

6d3e16c

Delegate redirection to Event listener instead of having it in Securi…

a616bb0

…tyController

Behat - Assert we have a redirection to dashboard when try to reach a…

1d72f12

…dmin login page

Spec: add AlreadyLoggedInUserRedirectionListenerSpec

b6f21d7

Jibbarth dismissed stale reviews from oallain and diimpp via b6f21d7 February 11, 2024 12:38

Jibbarth force-pushed the feature/redirect-logged-in-user-when-visit-registration-page branch from d446e17 to b6f21d7 Compare February 11, 2024 12:38

Jibbarth requested a review from a team as a code owner February 11, 2024 12:38

Jibbarth commented Feb 11, 2024

View reviewed changes

Jibbarth changed the title ~~[UX][Shop] Redirect to dashboard a logged-in user who reach register page~~ [UX] Allow to redirect logged-in user on any routes Feb 11, 2024

Jibbarth changed the title ~~[UX] Allow to redirect logged-in user on any routes~~ [UX] Allow to redirect logged-in user on any routes and add a redirection for registration page Feb 11, 2024

Jibbarth requested a review from Rafikooo March 11, 2024 12:31

Rafikooo self-assigned this Mar 12, 2024

Jibbarth changed the base branch from 1.13 to 1.14 June 11, 2024 14:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UX] Allow to redirect logged-in user on any routes and add a redirection for registration page #15418

[UX] Allow to redirect logged-in user on any routes and add a redirection for registration page #15418

Jibbarth commented Oct 8, 2023 •

edited

Loading

github-actions bot commented Oct 8, 2023 •

edited

Loading

Prometee commented Oct 8, 2023 •

edited

Loading

Jibbarth commented Oct 8, 2023

Jibbarth commented Oct 8, 2023

Prometee commented Oct 8, 2023

diimpp left a comment

oallain left a comment

Jibbarth commented Feb 5, 2024

Jibbarth commented Feb 11, 2024

Jibbarth Feb 11, 2024

Jibbarth Feb 11, 2024

[UX] Allow to redirect logged-in user on any routes and add a redirection for registration page #15418

Are you sure you want to change the base?

[UX] Allow to redirect logged-in user on any routes and add a redirection for registration page #15418

Conversation

Jibbarth commented Oct 8, 2023 • edited Loading

github-actions bot commented Oct 8, 2023 • edited Loading

Bunnyshell Preview Environment deployment failed

Prometee commented Oct 8, 2023 • edited Loading

Jibbarth commented Oct 8, 2023

Jibbarth commented Oct 8, 2023

Prometee commented Oct 8, 2023

diimpp left a comment

Choose a reason for hiding this comment

oallain left a comment

Choose a reason for hiding this comment

Jibbarth commented Feb 5, 2024

Jibbarth commented Feb 11, 2024

Jibbarth Feb 11, 2024

Choose a reason for hiding this comment

Jibbarth Feb 11, 2024

Choose a reason for hiding this comment

Jibbarth commented Oct 8, 2023 •

edited

Loading

github-actions bot commented Oct 8, 2023 •

edited

Loading

Prometee commented Oct 8, 2023 •

edited

Loading