From ef439f6faf98937ede38e95dc686dac1a4228e93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Thu, 20 Aug 2026 14:21:02 +0200 Subject: [PATCH] feat: re-generate access links for bookings when sent via email --- .../GenerateAccessLinkController.php | 55 ------- .../SendAccessLinkController.php | 4 +- src/Service/AccommodationBookingService.php | 12 +- .../modal_generate_access_link.html.twig | 8 - .../modal_send_access_link.html.twig | 4 +- .../accommodation_booking/show.html.twig | 39 ++--- .../GenerateAccessLinkControllerTest.php | 146 ------------------ .../SendAccessLinkControllerTest.php | 12 +- .../AccommodationBookingServiceTest.php | 30 +++- 9 files changed, 58 insertions(+), 252 deletions(-) delete mode 100644 src/Controller/Admin/AccommodationBooking/GenerateAccessLinkController.php delete mode 100644 templates/admin/accommodation_booking/modal_generate_access_link.html.twig delete mode 100644 tests/Controller/Admin/AccommodationBooking/GenerateAccessLinkControllerTest.php diff --git a/src/Controller/Admin/AccommodationBooking/GenerateAccessLinkController.php b/src/Controller/Admin/AccommodationBooking/GenerateAccessLinkController.php deleted file mode 100644 index 786870b..0000000 --- a/src/Controller/Admin/AccommodationBooking/GenerateAccessLinkController.php +++ /dev/null @@ -1,55 +0,0 @@ -isCustomerAccessible()) { - return $this->redirectToRoute('app_admin_accommodationbooking_show', ['id' => $booking->getId()]); - } - - if ($request->isMethod(Request::METHOD_POST)) { - if (!$this->isCsrfTokenValid('generate_accommodation_booking_access_link_'.$booking->getId(), $request->request->getString('_token'))) { - throw $this->createAccessDeniedException('Invalid CSRF token.'); - } - - $this->bookingService->regenerateAccessLink($booking); - - $this->addFlash('success', 'Der Zugangslink wurde neu generiert.'); - - $this->logger->info('Generated accommodation booking access link', [ - 'id' => $booking->getId(), - ]); - - return new HxRedirectResponse($this->generateUrl('app_admin_accommodationbooking_show', ['id' => $booking->getId()])); - } - - return $this->render('admin/accommodation_booking/modal_generate_access_link.html.twig', [ - 'booking' => $booking, - 'csrf_token_id' => 'generate_accommodation_booking_access_link_'.$booking->getId(), - ]); - } -} diff --git a/src/Controller/Admin/AccommodationBooking/SendAccessLinkController.php b/src/Controller/Admin/AccommodationBooking/SendAccessLinkController.php index 65b6d59..6b2aa2a 100644 --- a/src/Controller/Admin/AccommodationBooking/SendAccessLinkController.php +++ b/src/Controller/Admin/AccommodationBooking/SendAccessLinkController.php @@ -43,9 +43,9 @@ class SendAccessLinkController extends AbstractController return new HxRedirectResponse($this->generateUrl('app_admin_accommodationbooking_edit', ['id' => $booking->getId()])); } - $this->bookingService->sendCustomerConfirmationEmail($booking); + $this->bookingService->sendAccessLink($booking); - $this->addFlash('success', 'Der Zugangslink wurde dem Kunden per E-Mail zugestellt.'); + $this->addFlash('success', 'Ein neuer Zugangslink wurde dem Kunden per E-Mail zugestellt.'); $this->logger->info('Sent accommodation booking access link', [ 'id' => $booking->getId(), diff --git a/src/Service/AccommodationBookingService.php b/src/Service/AccommodationBookingService.php index 104f4ce..efab090 100644 --- a/src/Service/AccommodationBookingService.php +++ b/src/Service/AccommodationBookingService.php @@ -492,13 +492,15 @@ class AccommodationBookingService } /** - * Explicit admin action: (re)issues the access link, invalidating any previously issued - * link for this booking. No email side effect — sending is a separate, explicit admin - * action via sendCustomerConfirmationEmail(). + * Explicit admin action: mails the access link again, refreshing its validity first so the + * customer always receives a link that is good for another full TTL — the mail is the only + * way a link reaches the customer, so sending and issuing belong together. + * The previously issued link stops working, which is the point: one booking has one valid + * link at a time. * A no-op for a record that is not customer accessible yet, for the same reason as * issueAccessLink(). */ - public function regenerateAccessLink(AccommodationBooking $booking): void + public function sendAccessLink(AccommodationBooking $booking): void { if (!$booking->isCustomerAccessible()) { return; @@ -506,6 +508,8 @@ class AccommodationBookingService $booking->setAccessLinkIssuedAt(new \DateTimeImmutable()); $this->entityManager->flush(); + + $this->sendCustomerConfirmationEmail($booking); } /** diff --git a/templates/admin/accommodation_booking/modal_generate_access_link.html.twig b/templates/admin/accommodation_booking/modal_generate_access_link.html.twig deleted file mode 100644 index edd3e83..0000000 --- a/templates/admin/accommodation_booking/modal_generate_access_link.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends 'htmx_confirmation_modal.html.twig' %} - -{% block content %} -
- Möchtest du für {{ booking.groupName }} einen neuen Zugangslink generieren? - Ein zuvor generierter Link wird dadurch ungültig. -
-{% endblock %} diff --git a/templates/admin/accommodation_booking/modal_send_access_link.html.twig b/templates/admin/accommodation_booking/modal_send_access_link.html.twig index 927091f..954d6a9 100644 --- a/templates/admin/accommodation_booking/modal_send_access_link.html.twig +++ b/templates/admin/accommodation_booking/modal_send_access_link.html.twig @@ -2,7 +2,7 @@ {% block content %}
- Möchtest du den aktuellen Zugangslink für {{ booking.groupName }} an - {{ booking.email }} senden? + Möchtest du einen neuen Zugangslink für {{ booking.groupName }} an + {{ booking.email }} senden? Der zuvor gesendete Link wird dadurch ungültig.
{% endblock %} diff --git a/templates/admin/accommodation_booking/show.html.twig b/templates/admin/accommodation_booking/show.html.twig index 4731e2d..4e2ea17 100644 --- a/templates/admin/accommodation_booking/show.html.twig +++ b/templates/admin/accommodation_booking/show.html.twig @@ -214,10 +214,10 @@

Zugangslink

{% if not booking.customerAccessible %}

- Ein Zugangslink wird beim Versand des Angebots erzeugt und kann erst danach erneuert werden. + Ein Zugangslink wird beim Versand des Angebots erzeugt.

- {% else %} - {% if accessLink %} + {% elseif accessLink %} +
-

- Gültig bis {{ accessLinkExpiresAt | date('d.m.Y') }} - {% if accessLinkExpiresAt < date() %} - (abgelaufen) - {% endif %} -

- {% else %} -

Es wurde noch kein Zugangslink generiert.

- {% endif %} -
- {% if accessLink %} - - {% endif %}
+

+ Gültig bis {{ accessLinkExpiresAt | date('d.m.Y') }} + {% if accessLinkExpiresAt < date() %} + (abgelaufen) + {% endif %} +

+ {% else %} +

+ Der Zugangslink wird beim Versand des Angebots erzeugt. +

{% endif %}
diff --git a/tests/Controller/Admin/AccommodationBooking/GenerateAccessLinkControllerTest.php b/tests/Controller/Admin/AccommodationBooking/GenerateAccessLinkControllerTest.php deleted file mode 100644 index b2e9b67..0000000 --- a/tests/Controller/Admin/AccommodationBooking/GenerateAccessLinkControllerTest.php +++ /dev/null @@ -1,146 +0,0 @@ -createMock(AccommodationBookingService::class); - $bookingService->expects(self::never())->method('regenerateAccessLink'); - - $controller = new TestableGenerateAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); - - $response = $controller->index($this->openBooking(), Request::create('/admin/accommodation-booking/1/generate-access-link')); - - self::assertSame(Response::HTTP_OK, $response->getStatusCode()); - self::assertSame('admin/accommodation_booking/modal_generate_access_link.html.twig', $controller->renderedView); - } - - public function testPostGeneratesTheLinkAndRedirectsTheBrowser(): void - { - $booking = $this->openBooking(); - - $bookingService = $this->createMock(AccommodationBookingService::class); - $bookingService->expects(self::once())->method('regenerateAccessLink')->with($booking); - - $controller = new TestableGenerateAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); - - $response = $controller->index($booking, Request::create('/admin/accommodation-booking/1/generate-access-link', 'POST')); - - self::assertTrue($response->headers->has('HX-Redirect')); - } - - /** - * A draft is the office's own workbench: nothing has been offered, so a link would - * point the customer at a half-prepared record. - * - * @dataProvider requestMethods - */ - public function testADraftGetsNoAccessLinkAtAll(string $method): void - { - $booking = $this->openBooking(); - $booking->setStatus(AccommodationBookingStatus::Draft); - - $bookingService = $this->createMock(AccommodationBookingService::class); - $bookingService->expects(self::never())->method('regenerateAccessLink'); - - $controller = new TestableGenerateAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); - - $response = $controller->index($booking, Request::create('/admin/accommodation-booking/1/generate-access-link', $method)); - - self::assertSame(Response::HTTP_FOUND, $response->getStatusCode()); - self::assertNull($controller->renderedView, 'not even the modal offering the action'); - } - - /** - * @return iterable - */ - public static function requestMethods(): iterable - { - yield 'GET' => [Request::METHOD_GET]; - yield 'POST' => [Request::METHOD_POST]; - } - - public function testPostWithAnInvalidTokenIsDenied(): void - { - $bookingService = $this->createMock(AccommodationBookingService::class); - $bookingService->expects(self::never())->method('regenerateAccessLink'); - - $controller = new TestableGenerateAccessLinkController($bookingService, $this->createMock(LoggerInterface::class), tokenValid: false); - - $this->expectException(AccessDeniedException::class); - - $controller->index($this->openBooking(), Request::create('/admin/accommodation-booking/1/generate-access-link', 'POST')); - } - - private function openBooking(): AccommodationBooking - { - $booking = new AccommodationBooking(); - $booking->setStatus(AccommodationBookingStatus::Open); - $booking->setGroupName('Schulklasse 7b'); - $booking->setEmail('customer@example.com'); - - return $booking; - } -} - -final class TestableGenerateAccessLinkController extends GenerateAccessLinkController -{ - public ?string $renderedView = null; - - /** @var list */ - public array $flashes = []; - - public function __construct( - AccommodationBookingService $bookingService, - LoggerInterface $logger, - private readonly bool $tokenValid = true, - ) { - parent::__construct($bookingService, $logger); - } - - protected function isCsrfTokenValid(string $id, #[\SensitiveParameter] ?string $token): bool - { - return $this->tokenValid; - } - - /** - * @param array $parameters - */ - protected function render(string $view, array $parameters = [], ?Response $response = null): Response - { - $this->renderedView = $view; - - return new Response(); - } - - protected function addFlash(string $type, mixed $message): void - { - $this->flashes[] = ['type' => $type, 'message' => $message]; - } - - /** - * @param array $parameters - */ - protected function generateUrl(string $route, array $parameters = [], int $referenceType = 1): string - { - return '/'.$route.'?'.http_build_query($parameters); - } -} diff --git a/tests/Controller/Admin/AccommodationBooking/SendAccessLinkControllerTest.php b/tests/Controller/Admin/AccommodationBooking/SendAccessLinkControllerTest.php index 19a26d2..55c2632 100644 --- a/tests/Controller/Admin/AccommodationBooking/SendAccessLinkControllerTest.php +++ b/tests/Controller/Admin/AccommodationBooking/SendAccessLinkControllerTest.php @@ -23,7 +23,7 @@ class SendAccessLinkControllerTest extends TestCase public function testGetRendersTheConfirmationModal(): void { $bookingService = $this->createMock(AccommodationBookingService::class); - $bookingService->expects(self::never())->method('sendCustomerConfirmationEmail'); + $bookingService->expects(self::never())->method('sendAccessLink'); $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); @@ -38,7 +38,7 @@ class SendAccessLinkControllerTest extends TestCase $booking = $this->openBooking(); $bookingService = $this->createMock(AccommodationBookingService::class); - $bookingService->expects(self::once())->method('sendCustomerConfirmationEmail')->with($booking); + $bookingService->expects(self::once())->method('sendAccessLink')->with($booking); $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); @@ -53,7 +53,7 @@ class SendAccessLinkControllerTest extends TestCase $booking->setAccessLinkIssuedAt(null); $bookingService = $this->createMock(AccommodationBookingService::class); - $bookingService->expects(self::never())->method('sendCustomerConfirmationEmail'); + $bookingService->expects(self::never())->method('sendAccessLink'); $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); @@ -74,7 +74,7 @@ class SendAccessLinkControllerTest extends TestCase $booking->setStatus(AccommodationBookingStatus::Draft); $bookingService = $this->createMock(AccommodationBookingService::class); - $bookingService->expects(self::never())->method('sendCustomerConfirmationEmail'); + $bookingService->expects(self::never())->method('sendAccessLink'); $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); @@ -96,7 +96,7 @@ class SendAccessLinkControllerTest extends TestCase public function testPostWithAnInvalidTokenIsDenied(): void { $bookingService = $this->createMock(AccommodationBookingService::class); - $bookingService->expects(self::never())->method('sendCustomerConfirmationEmail'); + $bookingService->expects(self::never())->method('sendAccessLink'); $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class), tokenValid: false); @@ -111,7 +111,7 @@ class SendAccessLinkControllerTest extends TestCase $booking->setEmail(null); $bookingService = $this->createMock(AccommodationBookingService::class); - $bookingService->expects(self::never())->method('sendCustomerConfirmationEmail'); + $bookingService->expects(self::never())->method('sendAccessLink'); $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); diff --git a/tests/Service/AccommodationBookingServiceTest.php b/tests/Service/AccommodationBookingServiceTest.php index 6cd9aa4..cb2511f 100644 --- a/tests/Service/AccommodationBookingServiceTest.php +++ b/tests/Service/AccommodationBookingServiceTest.php @@ -245,8 +245,10 @@ class AccommodationBookingServiceTest extends TestCase $service->sendCustomerConfirmationEmail($booking); } - public function testRegenerateAccessLinkOverwritesAccessLinkIssuedAt(): void + public function testSendAccessLinkRefreshesTheLinkBeforeSending(): void { + // The mail is the only way a link reaches the customer, so every send hands out a + // link that is good for another full TTL — the previous one stops working. $booking = new AccommodationBooking(); $booking->setEmail('customer@example.com'); $booking->setStatus(AccommodationBookingStatus::Open); @@ -260,7 +262,7 @@ class AccommodationBookingServiceTest extends TestCase $breakdownCalculator->method('compute')->willReturn(null); $mailer = $this->createMock(Mailer::class); - $mailer->expects(self::never())->method('createAndSendEmail'); + $mailer->expects(self::once())->method('createAndSendEmail'); $service = $this->createServiceWithAccommodation( entityManager: $entityManager, @@ -268,9 +270,28 @@ class AccommodationBookingServiceTest extends TestCase breakdownCalculator: $breakdownCalculator, ); - $service->regenerateAccessLink($booking); + $service->sendAccessLink($booking); - self::assertNotSame($previousIssuedAt, $booking->getAccessLinkIssuedAt()); + self::assertGreaterThan($previousIssuedAt, $booking->getAccessLinkIssuedAt()); + } + + public function testSendAccessLinkNoOpsForADraft(): void + { + $booking = new AccommodationBooking(); + $booking->setEmail('customer@example.com'); + $booking->setStatus(AccommodationBookingStatus::Draft); + + $entityManager = $this->createMock(EntityManagerInterface::class); + $entityManager->expects(self::never())->method('flush'); + + $mailer = $this->createMock(Mailer::class); + $mailer->expects(self::never())->method('createAndSendEmail'); + + $service = $this->createServiceWithAccommodation(entityManager: $entityManager, mailer: $mailer); + + $service->sendAccessLink($booking); + + self::assertNull($booking->getAccessLinkIssuedAt()); } public function testADraftGetsNoAccessLinkGenerated(): void @@ -287,7 +308,6 @@ class AccommodationBookingServiceTest extends TestCase $service = $this->createServiceWithAccommodation(entityManager: $entityManager); $service->issueAccessLink($booking); - $service->regenerateAccessLink($booking); self::assertNull($booking->getAccessLinkIssuedAt()); }