Refactoring: Move flash messages from Task services to Controller actions (Case 212955)

src/ConfirmRegistration/Task.php

@@ -3,9 +3,6 @@
 namespace Webfactory\NewsletterRegistrationBundle\ConfirmRegistration;
 
 use DateTimeImmutable;
-use Symfony\Component\HttpFoundation\RequestStack;
-use Symfony\Component\HttpFoundation\Session\FlashBagAwareSessionInterface;
-use Symfony\Contracts\Translation\TranslatorInterface;
 use Webfactory\NewsletterRegistrationBundle\Entity\EmailAddressFactoryInterface;
 use Webfactory\NewsletterRegistrationBundle\Entity\PendingOptInInterface;
 use Webfactory\NewsletterRegistrationBundle\Entity\PendingOptInRepositoryInterface;
@@ -22,25 +19,19 @@ class Task implements TaskInterface
     protected EmailAddressFactoryInterface $emailAddressFactory;
     protected RecipientFactoryInterface $recipientFactory;
     protected RecipientRepositoryInterface $recipientRepo;
-    protected RequestStack $requestStack;
-    protected TranslatorInterface $translator;
 
     public function __construct(
         PendingOptInRepositoryInterface $pendingOptInRepo,
         int $timeLimitForOptInInHours,
         EmailAddressFactoryInterface $emailAddressFactory,
         RecipientFactoryInterface $recipientFactory,
-        RecipientRepositoryInterface $recipientRepo,
-        RequestStack $requestStack,
-        TranslatorInterface $translator
+        RecipientRepositoryInterface $recipientRepo
     ) {
         $this->pendingOptInRepo = $pendingOptInRepo;
         $this->timeLimitForOptInInHours = $timeLimitForOptInInHours;
         $this->emailAddressFactory = $emailAddressFactory;
         $this->recipientFactory = $recipientFactory;
         $this->recipientRepo = $recipientRepo;
-        $this->requestStack = $requestStack;
-        $this->translator = $translator;
     }
 
     /**
@@ -63,13 +54,6 @@ public function confirmRegistration(
         $this->recipientRepo->save($recipient);
         $this->pendingOptInRepo->remove($pendingOptIn);
 
-        $session = $this->requestStack->getSession();
-        \assert($session instanceof FlashBagAwareSessionInterface);
-        $session->getFlashBag()->add(
-            'success',
-            $this->translator->trans('confirm.registration.complete', [], 'webfactory-newsletter-registration')
-        );
-
         return $recipient;
     }
 

src/Controller.php

@@ -8,8 +8,10 @@
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpFoundation\Session\FlashBagAwareSessionInterface;
 use Symfony\Component\Routing\Attribute\Route;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+use Symfony\Contracts\Translation\TranslatorInterface;
 use Twig\Environment;
 use Webfactory\NewsletterRegistrationBundle\BlockEmails\TaskInterface as BlockEmailsTaskInterface;
 use Webfactory\NewsletterRegistrationBundle\ConfirmRegistration\TaskInterface as ConfirmRegistrationTaskInterface;
@@ -37,6 +39,7 @@ class Controller
     protected BlockEmailsTaskInterface $blockEmailsTask;
     protected PendingOptInRepositoryInterface $pendingOptInRepository;
     protected RecipientRepositoryInterface $recipientRepository;
+    protected TranslatorInterface $translator;
 
     public function __construct(
         FormFactoryInterface $formFactory,
@@ -48,7 +51,8 @@ public function __construct(
         DeleteRegistrationTaskInterface $deleteRegistrationTask,
         BlockEmailsTaskInterface $blockEmailsTask,
         PendingOptInRepositoryInterface $pendingOptInRepository,
-        RecipientRepositoryInterface $recipientRepository
+        RecipientRepositoryInterface $recipientRepository,
+        TranslatorInterface $translator
     ) {
         $this->formFactory = $formFactory;
         $this->twig = $twig;
@@ -60,6 +64,7 @@ public function __construct(
         $this->blockEmailsTask = $blockEmailsTask;
         $this->pendingOptInRepository = $pendingOptInRepository;
         $this->recipientRepository = $recipientRepository;
+        $this->translator = $translator;
     }
 
     #[Route('/', name: 'newsletter-registration-start')]
@@ -96,7 +101,7 @@ public function startRegistrationPartial(): Response
     }
 
     #[Route('/{uuid}/{emailAddress}/', name: 'newsletter-registration-confirm', requirements: ['uuid' => '([a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}){1}', 'emailAddress' => '.*@((?!\/).)*'])]
-    public function confirmRegistration(string $uuid, string $emailAddress): Response
+    public function confirmRegistration(string $uuid, string $emailAddress, FlashBagAwareSessionInterface $session): Response
     {
         $pendingOptIn = $this->pendingOptInRepository->findByUuid($uuid);
         if (null === $pendingOptIn) {
@@ -134,13 +139,18 @@ public function confirmRegistration(string $uuid, string $emailAddress): Respons
             );
         }
 
+        $session->getFlashBag()->add(
+            'success',
+            $this->translator->trans('confirm.registration.complete', [], 'webfactory-newsletter-registration')
+        );
+
         return new RedirectResponse(
             $this->urlGenerator->generate('newsletter-registration-edit', ['uuid' => $recipient->getUuid()])
         );
     }
 
     #[Route('/{uuid}/', name: 'newsletter-registration-edit', requirements: ['uuid' => '([a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}){1}'])]
-    public function editRegistration(string $uuid, Request $request): Response
+    public function editRegistration(string $uuid, Request $request, FlashBagAwareSessionInterface $session): Response
     {
         $recipient = $this->recipientRepository->findByUuid($uuid);
         if (null === $recipient) {
@@ -159,6 +169,13 @@ public function editRegistration(string $uuid, Request $request): Response
 
         if ($editForm->isSubmitted() && $editForm->isValid()) {
             $this->editRegistrationTask->editRegistration($recipient);
+            $messageKey = \count($recipient->getNewsletters()) > 0
+                ? 'edit.registration.updated'
+                : 'edit.registration.updated.no.newsletters.chosen';
+            $session->getFlashBag()->add(
+                'success',
+                $this->translator->trans($messageKey, [], 'webfactory-newsletter-registration')
+            );
         }
 
         $deleteForm = $this->formFactory->createNamed(
@@ -181,7 +198,7 @@ public function editRegistration(string $uuid, Request $request): Response
     }
 
     #[Route('/{uuid}/delete/', name: 'newsletter-registration-delete', methods: ['POST'], requirements: ['uuid' => '([a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}){1}'])]
-    public function deleteRegistration(string $uuid): Response
+    public function deleteRegistration(string $uuid, FlashBagAwareSessionInterface $session): Response
     {
         $recipient = $this->recipientRepository->findByUuid($uuid);
         if (null === $recipient) {
@@ -192,6 +209,10 @@ public function deleteRegistration(string $uuid): Response
         }
 
         $this->deleteRegistrationTask->deleteRegistration($recipient);
+        $session->getFlashBag()->add(
+            'success',
+            $this->translator->trans('delete.registration.success', [], 'webfactory-newsletter-registration')
+        );
 
         return new RedirectResponse(
             $this->urlGenerator->generate('newsletter-registration-start')

src/DeleteRegistration/Task.php

@@ -2,37 +2,20 @@
 
 namespace Webfactory\NewsletterRegistrationBundle\DeleteRegistration;
 
-use Symfony\Component\HttpFoundation\RequestStack;
-use Symfony\Component\HttpFoundation\Session\FlashBagAwareSessionInterface;
-use Symfony\Contracts\Translation\TranslatorInterface;
 use Webfactory\NewsletterRegistrationBundle\Entity\RecipientInterface;
 use Webfactory\NewsletterRegistrationBundle\Entity\RecipientRepositoryInterface;
 
 class Task implements TaskInterface
 {
     protected RecipientRepositoryInterface $recipientRepo;
-    protected RequestStack $requestStack;
-    protected TranslatorInterface $translator;
 
-    public function __construct(
-        RecipientRepositoryInterface $recipientRepo,
-        RequestStack $requestStack,
-        TranslatorInterface $translator
-    ) {
+    public function __construct(RecipientRepositoryInterface $recipientRepo)
+    {
         $this->recipientRepo = $recipientRepo;
-        $this->requestStack = $requestStack;
-        $this->translator = $translator;
     }
 
     public function deleteRegistration(RecipientInterface $recipient): void
     {
         $this->recipientRepo->remove($recipient);
-
-        $session = $this->requestStack->getSession();
-        \assert($session instanceof FlashBagAwareSessionInterface);
-        $session->getFlashBag()->add(
-            'success',
-            $this->translator->trans('delete.registration.success', [], 'webfactory-newsletter-registration')
-        );
     }
 }

src/DependencyInjection/services.yml

@@ -11,6 +11,7 @@ services:
             - '@Webfactory\NewsletterRegistrationBundle\BlockEmails\TaskInterface'
             - '@Webfactory\NewsletterRegistrationBundle\Entity\PendingOptInRepositoryInterface'
             - '@Webfactory\NewsletterRegistrationBundle\Entity\RecipientRepositoryInterface'
+            - '@Symfony\Contracts\Translation\TranslatorInterface'
         tags: ['controller.service_arguments']
 
     Webfactory\NewsletterRegistrationBundle\StartRegistration\Type:
@@ -96,8 +97,6 @@ services:
             - '@Webfactory\NewsletterRegistrationBundle\Entity\EmailAddressFactoryInterface'
             - '@Webfactory\NewsletterRegistrationBundle\Entity\RecipientFactoryInterface'
             - '@Webfactory\NewsletterRegistrationBundle\Entity\RecipientRepositoryInterface'
-            - '@request_stack'
-            - '@Symfony\Contracts\Translation\TranslatorInterface'
 
     Webfactory\NewsletterRegistrationBundle\ConfirmRegistration\TaskInterface:
         alias: 'Webfactory\NewsletterRegistrationBundle\ConfirmRegistration\Task'
@@ -110,17 +109,13 @@ services:
     Webfactory\NewsletterRegistrationBundle\EditRegistration\Task:
         arguments:
             - '@Webfactory\NewsletterRegistrationBundle\Entity\RecipientRepositoryInterface'
-            - '@request_stack'
-            - '@Symfony\Contracts\Translation\TranslatorInterface'
 
     Webfactory\NewsletterRegistrationBundle\EditRegistration\TaskInterface:
         alias: 'Webfactory\NewsletterRegistrationBundle\EditRegistration\Task'
 
     Webfactory\NewsletterRegistrationBundle\DeleteRegistration\Task:
         arguments:
             - '@Webfactory\NewsletterRegistrationBundle\Entity\RecipientRepositoryInterface'
-            - '@request_stack'
-            - '@Symfony\Contracts\Translation\TranslatorInterface'
 
     Webfactory\NewsletterRegistrationBundle\DeleteRegistration\TaskInterface:
         alias: 'Webfactory\NewsletterRegistrationBundle\DeleteRegistration\Task'

src/EditRegistration/Task.php

@@ -2,40 +2,20 @@
 
 namespace Webfactory\NewsletterRegistrationBundle\EditRegistration;
 
-use Symfony\Component\HttpFoundation\RequestStack;
-use Symfony\Component\HttpFoundation\Session\FlashBagAwareSessionInterface;
-use Symfony\Contracts\Translation\TranslatorInterface;
 use Webfactory\NewsletterRegistrationBundle\Entity\RecipientInterface;
 use Webfactory\NewsletterRegistrationBundle\Entity\RecipientRepositoryInterface;
 
 class Task implements TaskInterface
 {
     protected RecipientRepositoryInterface $recipientRepo;
-    protected RequestStack $requestStack;
-    protected TranslatorInterface $translator;
 
-    public function __construct(
-        RecipientRepositoryInterface $recipientRepo,
-        RequestStack $requestStack,
-        TranslatorInterface $translator
-    ) {
+    public function __construct(RecipientRepositoryInterface $recipientRepo)
+    {
         $this->recipientRepo = $recipientRepo;
-        $this->requestStack = $requestStack;
-        $this->translator = $translator;
     }
 
     public function editRegistration(RecipientInterface $recipient): void
     {
         $this->recipientRepo->save($recipient);
-
-        $messageKey = \count($recipient->getNewsletters()) > 0
-            ? 'edit.registration.updated'
-            : 'edit.registration.updated.no.newsletters.chosen';
-        $session = $this->requestStack->getSession();
-        \assert($session instanceof FlashBagAwareSessionInterface);
-        $session->getFlashBag()->add(
-            'success',
-            $this->translator->trans($messageKey, [], 'webfactory-newsletter-registration')
-        );
     }
 }

tests/ConfirmRegistration/TaskTest.php

@@ -6,10 +6,6 @@
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\HttpFoundation\RequestStack;
-use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface;
-use Symfony\Component\HttpFoundation\Session\FlashBagAwareSessionInterface;
-use Symfony\Contracts\Translation\TranslatorInterface;
 use Webfactory\NewsletterRegistrationBundle\ConfirmRegistration\Task;
 use Webfactory\NewsletterRegistrationBundle\Entity\EmailAddressFactory;
 use Webfactory\NewsletterRegistrationBundle\Entity\EmailAddressFactoryInterface;
@@ -28,9 +24,6 @@ class TaskTest extends TestCase
     protected RecipientFactoryInterface&MockObject $recipientFactory;
     protected RecipientRepositoryInterface&MockObject $recipientRepo;
     protected PendingOptInRepositoryInterface&MockObject $pendingOptInRepo;
-    protected RequestStack&MockObject $requestStack;
-    protected FlashBagInterface&MockObject $flashBag;
-    protected TranslatorInterface&MockObject $translator;
     protected Task $task;
 
     protected function setUp(): void
@@ -41,20 +34,12 @@ protected function setUp(): void
         $this->recipientFactory = $this->createMock(RecipientFactoryInterface::class);
         $this->recipientRepo = $this->createMock(RecipientRepositoryInterface::class);
         $this->pendingOptInRepo = $this->createMock(PendingOptInRepositoryInterface::class);
-        $this->flashBag = $this->createMock(FlashBagInterface::class);
-        $session = $this->createMock(FlashBagAwareSessionInterface::class);
-        $session->method('getFlashBag')->willReturn($this->flashBag);
-        $this->requestStack = $this->createMock(RequestStack::class);
-        $this->requestStack->method('getSession')->willReturn($session);
-        $this->translator = $this->createMock(TranslatorInterface::class);
         $this->task = new Task(
             $this->pendingOptInRepo,
             self::TIME_LIMIT_FOR_OPT_IN_IN_HOURS,
             $this->emailAddressFactory,
             $this->recipientFactory,
-            $this->recipientRepo,
-            $this->requestStack,
-            $this->translator
+            $this->recipientRepo
         );
     }
 
@@ -98,13 +83,4 @@ public function removes_pending_opt_in()
 
         $this->task->confirmRegistration($pendingOptIn, 'webfactory@example.com');
     }
-
-    #[Test]
-    public function writes_success_flash()
-    {
-        $pendingOptIn = new PendingOptIn('uuid', $this->emailAddressFactory->fromString('webfactory@example.com'));
-        $this->flashBag->expects($this->once())->method('add');
-
-        $this->task->confirmRegistration($pendingOptIn, 'webfactory@example.com');
-    }
 }

tests/DeleteRegistration/TaskTest.php

@@ -5,10 +5,6 @@
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\HttpFoundation\RequestStack;
-use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface;
-use Symfony\Component\HttpFoundation\Session\FlashBagAwareSessionInterface;
-use Symfony\Contracts\Translation\TranslatorInterface;
 use Webfactory\NewsletterRegistrationBundle\DeleteRegistration\Task;
 use Webfactory\NewsletterRegistrationBundle\Entity\EmailAddress;
 use Webfactory\NewsletterRegistrationBundle\Entity\RecipientRepositoryInterface;
@@ -17,23 +13,14 @@
 class TaskTest extends TestCase
 {
     protected RecipientRepositoryInterface&MockObject $recipientRepo;
-    protected RequestStack&MockObject $requestStack;
-    protected FlashBagInterface&MockObject $flashBag;
     protected Task $task;
-    protected TranslatorInterface&MockObject $translator;
 
     protected function setUp(): void
     {
         parent::setUp();
 
         $this->recipientRepo = $this->createMock(RecipientRepositoryInterface::class);
-        $this->flashBag = $this->createMock(FlashBagInterface::class);
-        $session = $this->createMock(FlashBagAwareSessionInterface::class);
-        $session->method('getFlashBag')->willReturn($this->flashBag);
-        $this->requestStack = $this->createMock(RequestStack::class);
-        $this->requestStack->method('getSession')->willReturn($session);
-        $this->translator = $this->createMock(TranslatorInterface::class);
-        $this->task = new Task($this->recipientRepo, $this->requestStack, $this->translator);
+        $this->task = new Task($this->recipientRepo);
     }
 
     #[Test]
@@ -44,13 +31,4 @@ public function removes_recipient()
 
         $this->task->deleteRegistration($recipient);
     }
-
-    #[Test]
-    public function writes_success_flash()
-    {
-        $recipient = new Recipient('uuid', new EmailAddress('webfactory@example.com', null));
-        $this->flashBag->expects($this->once())->method('add');
-
-        $this->task->deleteRegistration($recipient);
-    }
 }