From d68b6b156cca13bba27e0742ef6f1c0d00af8bf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Thu, 15 Oct 2026 10:49:09 +0200 Subject: [PATCH] feat: let admins notify all users about updated access codes --- config/packages/messenger.yaml | 2 +- config/services.yaml | 5 +- .../Admin/AccessCode/NotifyController.php | 64 +++++++++++ src/Email/Mailer.php | 80 ++++++++++++++ src/Email/SendReport.php | 18 +++ src/Repository/UserRepository.php | 16 +++ src/Service/AccessCodeNotifier.php | 46 ++++++++ src/Service/AuditLogger.php | 12 ++ src/Service/BookingNotifier.php | 56 +--------- .../admin/_modal_access_code_notify.html.twig | 15 +++ templates/admin/access_codes.html.twig | 14 ++- .../email/access_codes_updated.html.twig | 14 +++ templates/email/access_codes_updated.txt.twig | 7 ++ .../Admin/AccessCodeAdminControllerTest.php | 57 ++++++++++ tests/Email/MailerTest.php | 103 ++++++++++++++++++ tests/Service/AuditLoggerTest.php | 8 ++ 16 files changed, 458 insertions(+), 59 deletions(-) create mode 100644 src/Controller/Admin/AccessCode/NotifyController.php create mode 100644 src/Email/Mailer.php create mode 100644 src/Email/SendReport.php create mode 100644 src/Service/AccessCodeNotifier.php create mode 100644 templates/admin/_modal_access_code_notify.html.twig create mode 100644 templates/email/access_codes_updated.html.twig create mode 100644 templates/email/access_codes_updated.txt.twig create mode 100644 tests/Email/MailerTest.php diff --git a/config/packages/messenger.yaml b/config/packages/messenger.yaml index 06be806..5a42470 100644 --- a/config/packages/messenger.yaml +++ b/config/packages/messenger.yaml @@ -21,7 +21,7 @@ framework: # Mail is deliberately NOT routed to async. Nothing starts a worker on the server -- # deploy.php only stops them -- so a queued SendEmailMessage would be written to # messenger_messages and stay there, with no error anywhere and nobody notified. - # Sending happens in the request instead; BookingNotifier catches a transport + # Sending happens in the request instead; App\Email\Mailer catches a transport # failure so it can never fail the booking that triggered it. Route it back here # once a `messenger:consume async` worker actually runs. # Symfony\Component\Mailer\Messenger\SendEmailMessage: async diff --git a/config/services.yaml b/config/services.yaml index df5b2d6..2325d6d 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -26,9 +26,8 @@ services: arguments: $logger: '@monolog.logger.audit' - # Booking notifications. The sender is also the To of every notification -- the actual - # recipients are Bcc'd -- so it has to be an address that may both send and receive. - App\Service\BookingNotifier: + # Every mail goes out from this address, one per recipient -- see App\Email\Mailer. + App\Email\Mailer: arguments: $sender: '%env(string:MAILER_FROM)%' diff --git a/src/Controller/Admin/AccessCode/NotifyController.php b/src/Controller/Admin/AccessCode/NotifyController.php new file mode 100644 index 0000000..3bd7996 --- /dev/null +++ b/src/Controller/Admin/AccessCode/NotifyController.php @@ -0,0 +1,64 @@ +isMethod(Request::METHOD_POST)) { + return $this->render('admin/_modal_access_code_notify.html.twig', [ + 'recipients' => $this->users->countActive(), + 'csrf_token_id' => self::CSRF_TOKEN_ID, + ]); + } + + $this->denyUnlessCsrfValid($request, self::CSRF_TOKEN_ID); + + $report = $this->notifier->codesUpdated(); + $this->audit->accessCodesAnnounced($report->sent, $report->failed); + + if (0 === $report->failed) { + $this->addFlash('success', \sprintf('Benachrichtigung an %d Mitarbeitende versendet.', $report->sent)); + } else { + $this->addFlash('warning', \sprintf( + 'Benachrichtigung an %d Mitarbeitende versendet, %d konnten nicht erreicht werden.', + $report->sent, + $report->failed, + )); + } + + return new HxRedirectResponse($this->generateUrl('app_admin_access_code')); + } +} diff --git a/src/Email/Mailer.php b/src/Email/Mailer.php new file mode 100644 index 0000000..01b56e1 --- /dev/null +++ b/src/Email/Mailer.php @@ -0,0 +1,80 @@ + $recipients + * @param string $template the template pair's basename; both .html.twig + * and .txt.twig of it are rendered + * @param array $context + */ + public function send(iterable $recipients, string $subject, string $template, array $context = []): SendReport + { + $sent = 0; + $failed = 0; + + foreach ($recipients as $recipient) { + $email = (new TemplatedEmail()) + ->from(new Address($this->sender, 'E&P Platz')) + ->to($recipient) + ->subject($subject) + ->htmlTemplate($template.'.html.twig') + ->textTemplate($template.'.txt.twig') + ->context($context); + + try { + $this->mailer->send($email); + ++$sent; + } catch (\Throwable $e) { + // Caught per recipient, so one unreachable address does not cost the others theirs. + // \Throwable rather than TransportExceptionInterface because the templates are + // rendered inside send(): a typo in one of them would otherwise surface as an + // error in whatever action triggered the mail. + ++$failed; + $this->logger->error('Email could not be sent.', [ + 'subject' => $subject, + 'recipient' => $recipient instanceof Address ? $recipient->getAddress() : $recipient, + 'exception' => $e, + ]); + } + } + + return new SendReport($sent, $failed); + } +} diff --git a/src/Email/SendReport.php b/src/Email/SendReport.php new file mode 100644 index 0000000..41f361f --- /dev/null +++ b/src/Email/SendReport.php @@ -0,0 +1,18 @@ +findOneBy(['sub' => $sub]); } + + /** + * Everybody who can currently sign in -- the audience of a mail meant for the whole staff. + * A locked account is left out: its holder could not follow the link in it anyway. + * + * @return list + */ + public function findActive(): array + { + return $this->findBy(['disabledAt' => null], ['name' => 'ASC']); + } + + public function countActive(): int + { + return $this->count(['disabledAt' => null]); + } } diff --git a/src/Service/AccessCodeNotifier.php b/src/Service/AccessCodeNotifier.php new file mode 100644 index 0000000..bbe8ccc --- /dev/null +++ b/src/Service/AccessCodeNotifier.php @@ -0,0 +1,46 @@ +mailer->send( + array_map( + static fn (User $user): Address => new Address($user->getEmail(), $user->getName()), + $this->users->findActive(), + ), + 'Neue Zugangscodes bei E&P', + 'email/access_codes_updated', + ); + } +} diff --git a/src/Service/AuditLogger.php b/src/Service/AuditLogger.php index 409f0e4..5730bf8 100644 --- a/src/Service/AuditLogger.php +++ b/src/Service/AuditLogger.php @@ -295,6 +295,18 @@ final readonly class AuditLogger ]); } + /** + * The staff told that the codes changed. Counts only: the recipients are everybody, and + * which addresses failed is in the framework log, where a failure belongs. + */ + public function accessCodesAnnounced(int $sent, int $failed): void + { + $this->logger->info('Access code update announced.', [ + 'sent' => $sent, + 'failed' => $failed, + ]); + } + /** * @return array */ diff --git a/src/Service/BookingNotifier.php b/src/Service/BookingNotifier.php index aa782f2..264781f 100644 --- a/src/Service/BookingNotifier.php +++ b/src/Service/BookingNotifier.php @@ -4,14 +4,11 @@ declare(strict_types=1); namespace App\Service; +use App\Email\Mailer; use App\Entity\Booking; use App\Enum\ResourceType; use App\Model\SeriesResult; use App\Repository\ResourceTypeSettingsRepository; -use Psr\Log\LoggerInterface; -use Symfony\Bridge\Twig\Mime\TemplatedEmail; -use Symfony\Component\Mailer\MailerInterface; -use Symfony\Component\Mime\Address; /** * Tells whoever an administrator named that something was booked. @@ -22,25 +19,14 @@ use Symfony\Component\Mime\Address; * would otherwise never notify anybody. * * **Nothing here may break a booking.** It runs after the transaction has committed, so a - * failure cannot roll anything back, and a transport that is down is logged rather than thrown: - * the booking is made, and the person who made it should not be shown an error about somebody - * else's mail server. - * - * Every recipient gets their own mail, addressed only to them. Nobody sees who else is on the - * list, and a single bad address loses its own mail rather than everybody's. - * - * Each mail is multipart: the MyE&P layout for clients that render HTML, and a hand-written text - * part for those that do not. The text part is written rather than generated because there is no - * html-to-markdown converter installed, and three facts and a name read better as plain lines - * than as a flattened table anyway. + * failure cannot roll anything back -- and Mailer never throws, so the person who booked is not + * shown an error about somebody else's mail server. */ final readonly class BookingNotifier { public function __construct( - private MailerInterface $mailer, + private Mailer $mailer, private ResourceTypeSettingsRepository $settings, - private LoggerInterface $logger, - private string $sender, ) { } @@ -80,42 +66,10 @@ final readonly class BookingNotifier } /** - * @param string $template the template pair's basename; both .html.twig and - * .txt.twig of it are rendered * @param array $context */ private function send(ResourceType $type, string $subject, string $template, array $context): void { - $recipients = $this->settings->recipientsFor($type); - - if ([] === $recipients) { - return; - } - - foreach ($recipients as $recipient) { - // A fresh message per recipient: ->to() on a reused one appends rather than replaces, - // which would put the whole list back on everybody's copy. - $email = (new TemplatedEmail()) - ->from(new Address($this->sender, 'E&P Platz')) - ->to($recipient) - ->subject($subject) - ->htmlTemplate($template.'.html.twig') - ->textTemplate($template.'.txt.twig') - ->context($context); - - try { - $this->mailer->send($email); - } catch (\Throwable $e) { - // Caught per recipient, so one unreachable address does not cost the others theirs. - // \Throwable rather than TransportExceptionInterface because the templates are - // rendered inside send(): a typo in one of them would otherwise surface as a failed - // booking, which is exactly what this class promises never to do. - $this->logger->error('Booking notification could not be sent.', [ - 'subject' => $subject, - 'recipient' => $recipient, - 'exception' => $e, - ]); - } - } + $this->mailer->send($this->settings->recipientsFor($type), $subject, $template, $context); } } diff --git a/templates/admin/_modal_access_code_notify.html.twig b/templates/admin/_modal_access_code_notify.html.twig new file mode 100644 index 0000000..22dfe89 --- /dev/null +++ b/templates/admin/_modal_access_code_notify.html.twig @@ -0,0 +1,15 @@ +{% extends 'htmx_confirmation_modal.html.twig' %} + +{% block title %}Mitarbeitende benachrichtigen?{% endblock %} + +{% block content %} +

+ {{ recipients }} Mitarbeitende erhalten eine E-Mail, dass sich die Zugangscodes geändert + haben, mit einem Link zu dieser Anwendung. +

+

+ Die Codes selbst stehen aus Sicherheitsgründen nicht in der E-Mail. +

+{% endblock %} + +{% block button_confirm %}Senden{% endblock %} diff --git a/templates/admin/access_codes.html.twig b/templates/admin/access_codes.html.twig index 9ed73f2..a53d719 100644 --- a/templates/admin/access_codes.html.twig +++ b/templates/admin/access_codes.html.twig @@ -20,10 +20,16 @@

Zugangscodes verwalten

- +
+ + +
{% if rows is empty %} diff --git a/templates/email/access_codes_updated.html.twig b/templates/email/access_codes_updated.html.twig new file mode 100644 index 0000000..bcd0458 --- /dev/null +++ b/templates/email/access_codes_updated.html.twig @@ -0,0 +1,14 @@ +{# No codes and no door names -- see AccessCodeNotifier for why. #} +{% extends 'email/layout.html.twig' %} + +{% block body %} +

Neue Zugangscodes

+ +

+ Die Zugangscodes wurden aktualisiert. Die aktuell gültigen Codes findest du in E&P Platz. +

+ +

+ Zugangscodes ansehen +

+{% endblock %} diff --git a/templates/email/access_codes_updated.txt.twig b/templates/email/access_codes_updated.txt.twig new file mode 100644 index 0000000..af74bf8 --- /dev/null +++ b/templates/email/access_codes_updated.txt.twig @@ -0,0 +1,7 @@ +{# No codes and no door names -- see AccessCodeNotifier for why. #} +Die Zugangscodes wurden aktualisiert. Die aktuell gültigen Codes findest du in E&P Platz: + +{{ url('app_access_code') }} + +-- +E&P Platz diff --git a/tests/Controller/Admin/AccessCodeAdminControllerTest.php b/tests/Controller/Admin/AccessCodeAdminControllerTest.php index 88b3bde..01bcb0e 100644 --- a/tests/Controller/Admin/AccessCodeAdminControllerTest.php +++ b/tests/Controller/Admin/AccessCodeAdminControllerTest.php @@ -56,6 +56,10 @@ class AccessCodeAdminControllerTest extends WebTestCase self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN); $this->em->clear(); self::assertNotNull($this->em->find(AccessCode::class, $code->getId())); + + $this->client->request('POST', '/admin/access-codes/notify', ['_token' => 'whatever'], server: ['HTTP_HX-Request' => 'true']); + self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN); + self::assertEmailCount(0); } /** @@ -219,6 +223,59 @@ class AccessCodeAdminControllerTest extends WebTestCase self::assertNotNull($this->em->find(AccessCode::class, $code->getId())); } + /** + * Everybody who can sign in gets a mail of their own, and none of them contains a code -- not + * even one only the recipient's own role could read. + */ + public function testNotifyingTheStaffMailsEveryActiveUserWithoutTheCodes(): void + { + $this->code('Aufzug', '4711'); + $this->code('Tresor', '9876'); + $admin = $this->admin(); + $this->user('anna@ep-reisen.de'); + $locked = $this->user('weg@ep-reisen.de'); + $locked->disable('ausgeschieden'); + $this->em->flush(); + $expected = $this->em->getRepository(User::class)->count(['disabledAt' => null]); + $this->client->loginUser($admin); + $uri = '/admin/access-codes/notify'; + + $modal = $this->client->request('GET', $uri, server: ['HTTP_HX-Request' => 'true']); + self::assertStringContainsString(\sprintf('%d Mitarbeitende', $expected), $modal->text()); + + $this->client->request('POST', $uri, ['_token' => $this->confirmationToken($uri)], server: ['HTTP_HX-Request' => 'true']); + + self::assertSame('/admin/access-codes', $this->client->getResponse()->headers->get('HX-Redirect')); + self::assertEmailCount($expected); + + $recipients = []; + foreach (self::getMailerMessages() as $email) { + self::assertCount(1, $email->getTo()); + $recipients[] = $email->getTo()[0]->getAddress(); + + foreach ([$email->getTextBody(), $email->getHtmlBody()] as $body) { + self::assertStringContainsString('https://localhost/access-codes', $body); + self::assertStringNotContainsString('4711', $body); + self::assertStringNotContainsString('9876', $body); + self::assertStringNotContainsString('Aufzug', $body); + } + } + + self::assertContains('anna@ep-reisen.de', $recipients); + self::assertContains('chef@ep-reisen.de', $recipients); + self::assertNotContains('weg@ep-reisen.de', $recipients); + } + + public function testNotifyingTheStaffNeedsAValidToken(): void + { + $this->client->loginUser($this->admin()); + + $this->client->request('POST', '/admin/access-codes/notify', ['_token' => 'whatever'], server: ['HTTP_HX-Request' => 'true']); + + self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN); + self::assertEmailCount(0); + } + /** * Opens the modal, then posts the given fields with its CSRF token -- the round trip a * browser makes. diff --git a/tests/Email/MailerTest.php b/tests/Email/MailerTest.php new file mode 100644 index 0000000..5e4240e --- /dev/null +++ b/tests/Email/MailerTest.php @@ -0,0 +1,103 @@ +transport(); + $mailer = new Mailer($transport, new Logger('test'), 'platz@ep-reisen.de'); + + $report = $mailer->send( + ['a@ep-reisen.de', new Address('b@ep-reisen.de', 'B')], + 'Betreff', + 'email/some', + ['key' => 'value'], + ); + + self::assertSame(2, $report->sent); + self::assertSame(0, $report->failed); + self::assertCount(2, $transport->sent); + + foreach ($transport->sent as $i => $email) { + self::assertCount(1, $email->getTo()); + self::assertSame(['a@ep-reisen.de', 'b@ep-reisen.de'][$i], $email->getTo()[0]->getAddress()); + self::assertSame('platz@ep-reisen.de', $email->getFrom()[0]->getAddress()); + self::assertSame('E&P Platz', $email->getFrom()[0]->getName()); + self::assertSame('Betreff', $email->getSubject()); + self::assertSame('email/some.html.twig', $email->getHtmlTemplate()); + self::assertSame('email/some.txt.twig', $email->getTextTemplate()); + self::assertSame(['key' => 'value'], $email->getContext()); + } + } + + public function testAFailedRecipientIsLoggedAndTheOthersStillGetTheirs(): void + { + $transport = $this->transport(failFor: 'kaputt@ep-reisen.de'); + $log = new TestHandler(); + $mailer = new Mailer($transport, new Logger('test', [$log]), 'platz@ep-reisen.de'); + + $report = $mailer->send(['a@ep-reisen.de', 'kaputt@ep-reisen.de', 'b@ep-reisen.de'], 'Betreff', 'email/some'); + + self::assertSame(2, $report->sent); + self::assertSame(1, $report->failed); + self::assertCount(2, $transport->sent); + self::assertTrue($log->hasErrorThatContains('Email could not be sent.')); + self::assertSame('kaputt@ep-reisen.de', $log->getRecords()[0]->context['recipient']); + } + + public function testNoRecipientsSendsNothing(): void + { + $transport = $this->transport(); + $mailer = new Mailer($transport, new Logger('test'), 'platz@ep-reisen.de'); + + $report = $mailer->send([], 'Betreff', 'email/some'); + + self::assertSame(0, $report->sent); + self::assertSame([], $transport->sent); + } + + /** + * @return MailerInterface&object{sent: list} + */ + private function transport(?string $failFor = null): MailerInterface + { + return new class($failFor) implements MailerInterface { + /** @var list */ + public array $sent = []; + + public function __construct(private readonly ?string $failFor) + { + } + + public function send(RawMessage $message, ?Envelope $envelope = null): void + { + \assert($message instanceof TemplatedEmail); + + if ($message->getTo()[0]->getAddress() === $this->failFor) { + throw new TransportException('550 mailbox unavailable'); + } + + $this->sent[] = $message; + } + }; + } +} diff --git a/tests/Service/AuditLoggerTest.php b/tests/Service/AuditLoggerTest.php index 500e940..449442e 100644 --- a/tests/Service/AuditLoggerTest.php +++ b/tests/Service/AuditLoggerTest.php @@ -180,6 +180,14 @@ class AuditLoggerTest extends TestCase self::assertSame(7, $records[2]->context['access_code_id']); } + public function testAnAnnouncementIsRecordedWithItsCounts(): void + { + $this->audit->accessCodesAnnounced(50, 1); + + self::assertSame('Access code update announced.', $this->record()->message); + self::assertSame(['sent' => 50, 'failed' => 1], $this->record()->context); + } + private function record(): LogRecord { return $this->handler->getRecords()[0];