[New routing] Refactoring verify shop user by loic425 · Pull Request #18879 · Sylius/Sylius

loic425 · 2026-02-26T19:57:05Z

Q	A
Branch?	new-routing
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Related tickets	partially #18212
License	MIT

coderabbitai · 2026-02-26T19:57:16Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1faee830-b615-4472-a41c-00fc90b24724

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2026-02-26T19:58:50Z

❗ Preview Environment deployment failed on Bunnyshell

See: Environment Details | Pipeline Logs

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

Available commands:

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

...Sylius/Bundle/CoreBundle/CommandHandler/Shop/Account/SendAccountRegistrationEmailHandler.php

NoResponseMate · 2026-02-27T10:05:30Z

src/Sylius/Bundle/CoreBundle/CommandHandler/Shop/Account/VerifyShopUserHandler.php

+
+namespace Sylius\Bundle\CoreBundle\CommandHandler\Shop\Account;
+
+use InvalidArgumentException;


Suggested change

use InvalidArgumentException;

src/Sylius/Bundle/CoreBundle/CommandHandler/Shop/Account/VerifyShopUserHandler.php

NoResponseMate · 2026-02-27T10:07:52Z

src/Sylius/Bundle/ShopBundle/Controller/VerifyShopUserAction.php

+use Sylius\Bundle\CoreBundle\Command\Shop\Account\VerifyShopUser;
+use Sylius\Bundle\CoreBundle\CommandDispatcher\ResetPasswordDispatcherInterface;
+use Sylius\Bundle\CoreBundle\Provider\FlashBagProvider;
+use Sylius\Bundle\UserBundle\Form\Model\PasswordReset;
+use Sylius\Bundle\UserBundle\Form\Type\UserResetPasswordType;
+use Sylius\Component\Channel\Context\ChannelContextInterface;
+use Sylius\Component\Locale\Context\LocaleContextInterface;
+use Sylius\Component\User\Model\UserInterface;
+use Sylius\Component\User\Repository\UserRepositoryInterface;
+use Symfony\Component\Form\FormFactoryInterface;
+use Symfony\Component\HttpFoundation\RedirectResponse;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+use Symfony\Component\Messenger\MessageBusInterface;
+use Symfony\Component\Routing\RouterInterface;
+use Symfony\Contracts\Translation\TranslatorInterface;
+use Twig\Environment;


Some of these aren't used at all

Co-authored-by: Jan Góralski <jan.wojciech.goralski@gmail.com>

loic425 · 2026-02-27T12:52:37Z

@NoResponseMate Flash messages are already translated in the UI.

<div {{ sylius_test_html_attribute('flash-messages') }}>
    {% if flash is iterable %}
        {{ flash.message|trans(flash.parameters, 'flashes') }}
    {% else %}
        {{ flash|trans({}, 'flashes') }}
    {% endif %}
</div>

src/Sylius/Bundle/ShopBundle/Controller/VerifyShopUserAction.php

diimpp · 2026-03-05T12:41:02Z

src/Sylius/Bundle/CoreBundle/Command/Shop/Account/SendAccountRegistrationEmail.php

Would be cool to sync arguments order between VerifyShopUser and SendAccountRegistrationEmail and future commands.

Right now it's like so, but putting subject on first line and channelCode, localeCode in this order would look better.

--- public readonly string $shopUserEmail, --- public readonly string $localeCode, --- public readonly string $channelCode, +++ public readonly string $channelCode, +++ public readonly string $localeCode, +++ public readonly string $token,

diimpp · 2026-03-05T12:41:19Z

src/Sylius/Bundle/CoreBundle/Command/Shop/Account/SendAccountRegistrationEmail.php

+        public readonly string $shopUserEmail,
+        public readonly string $localeCode,
+        public readonly string $channelCode,


Suggested change

public readonly string $shopUserEmail,

public readonly string $localeCode,

public readonly string $channelCode,

public readonly string $shopUserEmail,

public readonly string $channelCode,

public readonly string $localeCode,

diimpp · 2026-03-05T12:41:46Z

src/Sylius/Bundle/CoreBundle/Command/Shop/Account/VerifyShopUser.php

+        public readonly string $channelCode,
+        public readonly string $localeCode,
+        public readonly string $token,


Suggested change

public readonly string $channelCode,

public readonly string $localeCode,

public readonly string $token,

public readonly string $token,

public readonly string $channelCode,

public readonly string $localeCode,

diimpp · 2026-03-05T12:43:56Z

...Sylius/Bundle/CoreBundle/CommandHandler/Shop/Account/SendAccountRegistrationEmailHandler.php

+use Symfony\Component\Messenger\Attribute\AsMessageHandler;
+
+#[AsMessageHandler]
+final class SendAccountRegistrationEmailHandler


Suggested change

final class SendAccountRegistrationEmailHandler

final readonly class SendAccountRegistrationEmailHandler

diimpp · 2026-03-05T12:45:54Z

...Sylius/Bundle/CoreBundle/CommandHandler/Shop/Account/SendAccountRegistrationEmailHandler.php

+        if (null === $shopUser) {
+            throw new \InvalidArgumentException(
+                sprintf('There is no shop user with %s email', $command->shopUserEmail),
+            );
+        }


Why not use Assert as usual?

Suggested change

if (null === $shopUser) {

throw new \InvalidArgumentException(

sprintf('There is no shop user with %s email', $command->shopUserEmail),

);

}

Assert::notNull($shopUser, sprintf('There is no shop user with %s email', $command->shopUserEmail));

diimpp · 2026-03-05T12:46:28Z

...Sylius/Bundle/CoreBundle/CommandHandler/Shop/Account/SendAccountRegistrationEmailHandler.php

+        if (null === $channel) {
+            throw new \InvalidArgumentException(
+                sprintf('There is no channel with %s code', $command->channelCode),
+            );
+        }


Assert usage would be shorter.

diimpp · 2026-03-05T12:47:20Z

...Sylius/Bundle/CoreBundle/CommandHandler/Shop/Account/SendAccountRegistrationEmailHandler.php

+            return;
+        }
+
+        $this->accountRegistrationEmailManager->sendAccountRegistrationEmail($shopUser, $channel, $command->localeCode);


Missing check, that localeCode matches provided channel.

diimpp · 2026-03-05T12:48:10Z

src/Sylius/Bundle/CoreBundle/CommandHandler/Shop/Account/VerifyShopUserHandler.php

+        if (null === $user) {
+            throw new \InvalidArgumentException(
+                sprintf('There is no shop user with %s email verification token', $command->token),
+            );
+        }


Assert would be shorter

diimpp · 2026-03-05T12:49:25Z

src/Sylius/Bundle/CoreBundle/CommandHandler/Shop/Account/VerifyShopUserHandler.php

+        $user->enable();
+
+        $this->commandBus->dispatch(
+            new SendAccountRegistrationEmail($user->getEmail(), $command->localeCode, $command->channelCode),


It's okay to skip channelCode/localeCode asserts as long as SendAccountRegistrationEmail checks them.

P.S. We should dispatch events, like ShopUserVerified, not hardcode command calls.

loic425 requested review from a team as code owners February 26, 2026 19:57

probot-autolabeler bot added the Shop ShopBundle related issues and PRs. label Feb 26, 2026

loic425 mentioned this pull request Feb 26, 2026

Using new routing system from Sylius resource bundle #18212

Open

loic425 force-pushed the verify-shop-user branch 3 times, most recently from 464301f to 006bf6d Compare February 27, 2026 08:43

loic425 commented Feb 27, 2026

View reviewed changes

...Sylius/Bundle/CoreBundle/CommandHandler/Shop/Account/SendAccountRegistrationEmailHandler.php Outdated Show resolved Hide resolved

[New routing] Refactoring verify shop user

939ccd6

loic425 force-pushed the verify-shop-user branch from 006bf6d to 939ccd6 Compare February 27, 2026 08:55

NoResponseMate reviewed Feb 27, 2026

View reviewed changes

loic425 force-pushed the verify-shop-user branch 2 times, most recently from 8833ef1 to 1835a6c Compare February 27, 2026 12:44

Apply suggestions from code review

4e91066

Co-authored-by: Jan Góralski <jan.wojciech.goralski@gmail.com>

loic425 force-pushed the verify-shop-user branch from 1835a6c to 4e91066 Compare February 27, 2026 12:50

diimpp reviewed Mar 3, 2026

View reviewed changes

src/Sylius/Bundle/ShopBundle/Controller/VerifyShopUserAction.php Outdated Show resolved Hide resolved

Apply code review suggestions

e688854

diimpp reviewed Mar 5, 2026

View reviewed changes


		namespace Sylius\Bundle\CoreBundle\CommandHandler\Shop\Account;

		use InvalidArgumentException;

	final class SendAccountRegistrationEmailHandler
	final readonly class SendAccountRegistrationEmailHandler

Uh oh!

Conversation

loic425 commented Feb 26, 2026

Uh oh!

coderabbitai bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

github-actions bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❗ Preview Environment deployment failed on Bunnyshell

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic425 commented Feb 27, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diimpp Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diimpp Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai bot commented Feb 26, 2026 •

edited

Loading

github-actions bot commented Feb 26, 2026 •

edited

Loading

diimpp Mar 5, 2026 •

edited

Loading

diimpp Mar 6, 2026 •

edited

Loading