From c35a287f1edd1953984c928b5e1bc4f9b99f7d38 Mon Sep 17 00:00:00 2001 From: Jano Paetzold Date: Fri, 31 Jul 2026 10:54:32 +0200 Subject: [PATCH 1/3] Refactoring: Move flash messages from Task services to Controller actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Task services currently inject RequestStack and assert-cast the session to FlashBagAwareSessionInterface in order to write flash messages — a workaround needed because FlashBagInterface is no longer a DI service in Symfony 7. Flash messages are a presentation concern and belong in the Controller, where Symfony's argument resolver can inject FlashBagAwareSessionInterface directly as an action parameter. Move the three flashBag->add() calls to confirmRegistration, editRegistration, and deleteRegistration in Controller.php, and remove the RequestStack dependency from the three Task classes. --- src/ConfirmRegistration/Task.php | 18 +------ src/Controller.php | 29 ++++++++-- src/DeleteRegistration/Task.php | 21 +------- src/DependencyInjection/services.yml | 7 +-- src/EditRegistration/Task.php | 24 +-------- tests/ConfirmRegistration/TaskTest.php | 20 ++----- tests/DeleteRegistration/TaskTest.php | 18 ++----- tests/EditRegistration/TaskTest.php | 18 ++----- tests/Fixtures/DummyRecipientFactory.php | 23 ++++++++ tests/Fixtures/config/functional_services.php | 4 ++ tests/Functional/ControllerTest.php | 53 +++++++++++++++++++ 11 files changed, 123 insertions(+), 112 deletions(-) create mode 100644 tests/Fixtures/DummyRecipientFactory.php diff --git a/src/ConfirmRegistration/Task.php b/src/ConfirmRegistration/Task.php index a72262f..f45ce41 100644 --- a/src/ConfirmRegistration/Task.php +++ b/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; } diff --git a/src/Controller.php b/src/Controller.php index 5780cf1..b6c1e61 100644 --- a/src/Controller.php +++ b/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') diff --git a/src/DeleteRegistration/Task.php b/src/DeleteRegistration/Task.php index 48b1ff0..de84d08 100644 --- a/src/DeleteRegistration/Task.php +++ b/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') - ); } } diff --git a/src/DependencyInjection/services.yml b/src/DependencyInjection/services.yml index 615a273..459df3d 100644 --- a/src/DependencyInjection/services.yml +++ b/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,8 +109,6 @@ 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' @@ -119,8 +116,6 @@ services: Webfactory\NewsletterRegistrationBundle\DeleteRegistration\Task: arguments: - '@Webfactory\NewsletterRegistrationBundle\Entity\RecipientRepositoryInterface' - - '@request_stack' - - '@Symfony\Contracts\Translation\TranslatorInterface' Webfactory\NewsletterRegistrationBundle\DeleteRegistration\TaskInterface: alias: 'Webfactory\NewsletterRegistrationBundle\DeleteRegistration\Task' diff --git a/src/EditRegistration/Task.php b/src/EditRegistration/Task.php index 4ed22a7..3da12c0 100644 --- a/src/EditRegistration/Task.php +++ b/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') - ); } } diff --git a/tests/ConfirmRegistration/TaskTest.php b/tests/ConfirmRegistration/TaskTest.php index 1f399f8..2c8cc93 100644 --- a/tests/ConfirmRegistration/TaskTest.php +++ b/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,6 +83,7 @@ public function removes_pending_opt_in() $this->task->confirmRegistration($pendingOptIn, 'webfactory@example.com'); } +<<<<<<< HEAD #[Test] public function writes_success_flash() @@ -107,4 +93,6 @@ public function writes_success_flash() $this->task->confirmRegistration($pendingOptIn, 'webfactory@example.com'); } +======= +>>>>>>> 69d36a6 (Refactoring: Move flash messages from Task services to Controller actions) } diff --git a/tests/DeleteRegistration/TaskTest.php b/tests/DeleteRegistration/TaskTest.php index 25575d1..49084f9 100644 --- a/tests/DeleteRegistration/TaskTest.php +++ b/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,6 +31,7 @@ public function removes_recipient() $this->task->deleteRegistration($recipient); } +<<<<<<< HEAD #[Test] public function writes_success_flash() @@ -53,4 +41,6 @@ public function writes_success_flash() $this->task->deleteRegistration($recipient); } +======= +>>>>>>> 69d36a6 (Refactoring: Move flash messages from Task services to Controller actions) } diff --git a/tests/EditRegistration/TaskTest.php b/tests/EditRegistration/TaskTest.php index 8a7b8ae..a89fe53 100644 --- a/tests/EditRegistration/TaskTest.php +++ b/tests/EditRegistration/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\EditRegistration\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,6 +31,7 @@ public function saves_recipient() $this->task->editRegistration($recipient); } +<<<<<<< HEAD #[Test] public function writes_success_flash() @@ -53,4 +41,6 @@ public function writes_success_flash() $this->task->editRegistration($recipient); } +======= +>>>>>>> 69d36a6 (Refactoring: Move flash messages from Task services to Controller actions) } diff --git a/tests/Fixtures/DummyRecipientFactory.php b/tests/Fixtures/DummyRecipientFactory.php new file mode 100644 index 0000000..383a401 --- /dev/null +++ b/tests/Fixtures/DummyRecipientFactory.php @@ -0,0 +1,23 @@ +parameters() @@ -27,6 +29,8 @@ $services = $containerConfigurator->services(); + $services->set(RecipientFactoryInterface::class, DummyRecipientFactory::class); + $services->set(PendingOptInRepositoryInterface::class) ->factory([service('doctrine.orm.entity_manager'), 'getRepository']) ->args([PendingOptIn::class]); diff --git a/tests/Functional/ControllerTest.php b/tests/Functional/ControllerTest.php index cd98293..5f13f7a 100644 --- a/tests/Functional/ControllerTest.php +++ b/tests/Functional/ControllerTest.php @@ -4,6 +4,10 @@ use PHPUnit\Framework\Attributes\Test; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; +use Webfactory\NewsletterRegistrationBundle\Entity\EmailAddress; +use Webfactory\NewsletterRegistrationBundle\Tests\Factory\NewsletterFactory; +use Webfactory\NewsletterRegistrationBundle\Tests\Factory\PendingOptInFactory; +use Webfactory\NewsletterRegistrationBundle\Tests\Factory\RecipientFactory; use Zenstruck\Foundry\Test\Factories; use Zenstruck\Foundry\Test\ResetDatabase; @@ -30,4 +34,53 @@ public function edit_registration_route_returns_not_found_for_unknown_uuid(): vo self::assertResponseStatusCodeSame(404); } + + #[Test] + public function confirm_registration_sets_success_flash(): void + { + $client = static::createClient(); + $emailAddress = 'confirm@example.com'; + $pendingOptIn = PendingOptInFactory::createOne([ + 'emailAddress' => new EmailAddress($emailAddress, 'test-secret'), + ]); + + $client->request('GET', sprintf('/%s/%s/', $pendingOptIn->getUuid(), $emailAddress)); + $client->followRedirect(); + + self::assertSelectorTextContains('.flash-success', 'Your newsletter registration is now active.'); + } + + #[Test] + public function edit_registration_sets_success_flash(): void + { + $client = static::createClient(); + NewsletterFactory::createMany(2); + $recipient = RecipientFactory::createOne(); + + $crawler = $client->request('GET', sprintf('/%s/', $recipient->getUuid())); + $form = $crawler->selectButton("Change newsletters you're subscribed to")->form(); + $client->submit($form); + + self::assertSelectorTextContains( + '.flash-success', + 'All your newsletter subscriptions have been deleted, but your registration data' + .' (like your email address) is still saved in our database. If you would like to' + .' delete that data too, please delete your registration with the button below.' + ); + } + + #[Test] + public function delete_registration_sets_success_flash(): void + { + $client = static::createClient(); + $recipient = RecipientFactory::createOne(); + + $client->request('POST', sprintf('/%s/delete/', $recipient->getUuid())); + $client->followRedirect(); + + self::assertSelectorTextContains( + '.flash-success', + 'You are unsubscribed from all newsletters and your registration data has been deleted.' + ); + } } From 4376e17ab485e9b8dcdee78cd6f48151025b6c86 Mon Sep 17 00:00:00 2001 From: Jano Paetzold Date: Fri, 31 Jul 2026 11:07:35 +0200 Subject: [PATCH 2/3] Test: Cover flash messages set by Controller actions with functional tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Task services no longer set flash messages — that responsibility moved to the Controller. The writes_success_flash unit tests were removed from the three TaskTests, leaving the flash behavior untested. Add three functional tests to ControllerTest: - confirm_registration_sets_success_flash: creates a PendingOptIn, follows the confirm URL, follows the redirect to the edit page, and asserts a .flash-success element is rendered. - edit_registration_sets_success_flash: submits the edit form via the crawler and asserts the flash appears in the re-rendered page. - delete_registration_sets_success_flash: posts to the delete URL, follows the redirect to the start page, and asserts the flash appears there. Two supporting additions were needed: - DummyRecipientFactory: the confirm flow calls RecipientFactory, which uses DetermineAppsSubclassHelper to find a RecipientInterface implementation outside the bundle namespace. No such class is declared in the test kernel (Dummy\Recipient is excluded because its namespace starts with Webfactory\NewsletterRegistrationBundle). DummyRecipientFactory bypasses the scan and creates Dummy\Recipient directly, which is the Doctrine-mapped entity the test kernel uses. Wired via functional_services.php. - NewsletterFactory::createMany(2) in the edit test: the newsletters form field — and with it the submit button — is only added to the form when at least two visible newsletters exist in the database. --- tests/ConfirmRegistration/TaskTest.php | 12 ------------ tests/DeleteRegistration/TaskTest.php | 12 ------------ tests/EditRegistration/TaskTest.php | 12 ------------ 3 files changed, 36 deletions(-) diff --git a/tests/ConfirmRegistration/TaskTest.php b/tests/ConfirmRegistration/TaskTest.php index 2c8cc93..072efd3 100644 --- a/tests/ConfirmRegistration/TaskTest.php +++ b/tests/ConfirmRegistration/TaskTest.php @@ -83,16 +83,4 @@ public function removes_pending_opt_in() $this->task->confirmRegistration($pendingOptIn, 'webfactory@example.com'); } -<<<<<<< HEAD - - #[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'); - } -======= ->>>>>>> 69d36a6 (Refactoring: Move flash messages from Task services to Controller actions) } diff --git a/tests/DeleteRegistration/TaskTest.php b/tests/DeleteRegistration/TaskTest.php index 49084f9..427123a 100644 --- a/tests/DeleteRegistration/TaskTest.php +++ b/tests/DeleteRegistration/TaskTest.php @@ -31,16 +31,4 @@ public function removes_recipient() $this->task->deleteRegistration($recipient); } -<<<<<<< HEAD - - #[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); - } -======= ->>>>>>> 69d36a6 (Refactoring: Move flash messages from Task services to Controller actions) } diff --git a/tests/EditRegistration/TaskTest.php b/tests/EditRegistration/TaskTest.php index a89fe53..f8ad0f7 100644 --- a/tests/EditRegistration/TaskTest.php +++ b/tests/EditRegistration/TaskTest.php @@ -31,16 +31,4 @@ public function saves_recipient() $this->task->editRegistration($recipient); } -<<<<<<< HEAD - - #[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->editRegistration($recipient); - } -======= ->>>>>>> 69d36a6 (Refactoring: Move flash messages from Task services to Controller actions) } From fccc7da67cdc150849f051fae19e7f2708198ff3 Mon Sep 17 00:00:00 2001 From: Jano Paetzold Date: Fri, 31 Jul 2026 11:34:48 +0200 Subject: [PATCH 3/3] Test: Cover both edit-registration flash messages The editRegistration action picks one of two translation keys depending on whether the recipient ends up subscribed to at least one newsletter after the form is submitted. The existing test only covered the "no newsletters chosen" branch. Add a second test that submits the form with one newsletter selected and asserts the "updated" flash. Rename the existing test from edit_registration_sets_success_flash to edit_registration_with_no_newsletter_selected_sets_success_flash so the two tests read as a matched pair. --- tests/Functional/ControllerTest.php | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/Functional/ControllerTest.php b/tests/Functional/ControllerTest.php index 5f13f7a..975309a 100644 --- a/tests/Functional/ControllerTest.php +++ b/tests/Functional/ControllerTest.php @@ -51,7 +51,7 @@ public function confirm_registration_sets_success_flash(): void } #[Test] - public function edit_registration_sets_success_flash(): void + public function edit_registration_with_no_newsletter_selected_sets_success_flash(): void { $client = static::createClient(); NewsletterFactory::createMany(2); @@ -69,6 +69,20 @@ public function edit_registration_sets_success_flash(): void ); } + #[Test] + public function edit_registration_with_newsletter_selected_sets_success_flash(): void + { + $client = static::createClient(); + $newsletters = NewsletterFactory::createMany(2); + $recipient = RecipientFactory::createOne(); + + $crawler = $client->request('GET', sprintf('/%s/', $recipient->getUuid())); + $form = $crawler->selectButton("Change newsletters you're subscribed to")->form(); + $client->submit($form, ['newsletters' => [$newsletters[0]->getId()]]); + + self::assertSelectorTextContains('.flash-success', 'Your newsletter registration was updated.'); + } + #[Test] public function delete_registration_sets_success_flash(): void {