From 8617bf6c863e82657d57876215517f58454ddb28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Wed, 19 Aug 2026 13:00:43 +0200 Subject: [PATCH] fix: prevent generating offer links while booking is in draft --- .../GenerateAccessLinkController.php | 5 + .../SendAccessLinkController.php | 2 +- src/Controller/Groups/OfferController.php | 2 +- src/Entity/Groups/AccommodationBooking.php | 11 ++ src/Service/AccommodationBookingService.php | 22 ++- .../accommodation_booking/show.html.twig | 62 +++--- .../GenerateAccessLinkControllerTest.php | 146 ++++++++++++++ .../SendAccessLinkControllerTest.php | 178 ++++++++++++++++++ .../Controller/Groups/OfferControllerTest.php | 10 +- .../Groups/AccommodationBookingTest.php | 17 ++ .../AccommodationBookingServiceTest.php | 23 +++ 11 files changed, 442 insertions(+), 36 deletions(-) create mode 100644 tests/Controller/Admin/AccommodationBooking/GenerateAccessLinkControllerTest.php create mode 100644 tests/Controller/Admin/AccommodationBooking/SendAccessLinkControllerTest.php diff --git a/src/Controller/Admin/AccommodationBooking/GenerateAccessLinkController.php b/src/Controller/Admin/AccommodationBooking/GenerateAccessLinkController.php index 22ff510..786870b 100644 --- a/src/Controller/Admin/AccommodationBooking/GenerateAccessLinkController.php +++ b/src/Controller/Admin/AccommodationBooking/GenerateAccessLinkController.php @@ -26,6 +26,11 @@ class GenerateAccessLinkController extends AbstractController #[Route('/admin/accommodation-booking/{id}/generate-access-link', name: 'app_admin_accommodationbooking_generate_access_link')] public function index(AccommodationBooking $booking, Request $request): Response { + // A draft has not been offered to anyone yet, so there is nothing a link could show. + if (!$booking->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.'); diff --git a/src/Controller/Admin/AccommodationBooking/SendAccessLinkController.php b/src/Controller/Admin/AccommodationBooking/SendAccessLinkController.php index c8fcefa..65b6d59 100644 --- a/src/Controller/Admin/AccommodationBooking/SendAccessLinkController.php +++ b/src/Controller/Admin/AccommodationBooking/SendAccessLinkController.php @@ -26,7 +26,7 @@ class SendAccessLinkController extends AbstractController #[Route('/admin/accommodation-booking/{id}/send-access-link', name: 'app_admin_accommodationbooking_send_access_link')] public function index(AccommodationBooking $booking, Request $request): Response { - if (null === $booking->getAccessLinkIssuedAt()) { + if (!$booking->isCustomerAccessible() || null === $booking->getAccessLinkIssuedAt()) { return $this->redirectToRoute('app_admin_accommodationbooking_show', ['id' => $booking->getId()]); } diff --git a/src/Controller/Groups/OfferController.php b/src/Controller/Groups/OfferController.php index 96d47c5..3ec954a 100644 --- a/src/Controller/Groups/OfferController.php +++ b/src/Controller/Groups/OfferController.php @@ -128,7 +128,7 @@ class OfferController extends AbstractController */ private function isCustomerVisible(AccommodationBooking $booking): bool { - return !$booking->isDraft() && !$booking->isRequested() && !$booking->isDiscarded(); + return $booking->isCustomerAccessible() && !$booking->isRequested() && !$booking->isDiscarded(); } /** diff --git a/src/Entity/Groups/AccommodationBooking.php b/src/Entity/Groups/AccommodationBooking.php index 2c93be4..c034221 100644 --- a/src/Entity/Groups/AccommodationBooking.php +++ b/src/Entity/Groups/AccommodationBooking.php @@ -402,6 +402,17 @@ class AccommodationBooking implements BlameableEntityInterface, TimestampableEnt return AccommodationBookingStatus::Discarded === $this->status; } + /** + * Whether the record may be put in front of the customer at all. A draft is the office's + * own workbench — nothing has been offered yet, so an access link would point the customer + * at a half-prepared record. Every access link action (issuing, regenerating, sending) + * hangs off this, so the rule lives in one place rather than in each caller. + */ + public function isCustomerAccessible(): bool + { + return !$this->isDraft(); + } + public function getOrigin(): AccommodationBookingOrigin { return $this->origin; diff --git a/src/Service/AccommodationBookingService.php b/src/Service/AccommodationBookingService.php index c10d061..c77f324 100644 --- a/src/Service/AccommodationBookingService.php +++ b/src/Service/AccommodationBookingService.php @@ -436,12 +436,18 @@ class AccommodationBookingService } /** - * Sets accessLinkIssuedAt if the booking doesn't have one yet, whatever its status. - * No email side effect — the confirmation email is always sent separately via - * sendCustomerConfirmationEmail(), regardless of whether a link exists. + * Sets accessLinkIssuedAt if the booking doesn't have one yet. No email side effect — the + * confirmation email is always sent separately via sendCustomerConfirmationEmail(), + * regardless of whether a link exists. + * A no-op while the record is not customer accessible: a draft has nothing to show yet, so + * publishing the offer (sendOffer()) moves it out of Entwurf before asking for a link. */ public function issueAccessLink(AccommodationBooking $booking): void { + if (!$booking->isCustomerAccessible()) { + return; + } + if (null !== $booking->getAccessLinkIssuedAt()) { return; } @@ -489,9 +495,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(). + * A no-op for a record that is not customer accessible yet, for the same reason as + * issueAccessLink(). */ public function regenerateAccessLink(AccommodationBooking $booking): void { + if (!$booking->isCustomerAccessible()) { + return; + } + $booking->setAccessLinkIssuedAt(new \DateTimeImmutable()); $this->entityManager->flush(); } @@ -509,8 +521,10 @@ class AccommodationBookingService return; } - $this->issueAccessLink($booking); + // Offen first: the status is what makes the record customer accessible, and only then + // will issueAccessLink() hand out a link (it flushes both changes together). $booking->setStatus(AccommodationBookingStatus::Open); + $this->issueAccessLink($booking); $this->entityManager->flush(); $this->sendCustomerConfirmationEmail($booking); diff --git a/templates/admin/accommodation_booking/show.html.twig b/templates/admin/accommodation_booking/show.html.twig index a0f8a5f..bb6e0ff 100644 --- a/templates/admin/accommodation_booking/show.html.twig +++ b/templates/admin/accommodation_booking/show.html.twig @@ -212,42 +212,48 @@

Zugangslink

- {% if accessLink %} - -

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

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

{% else %} -

Es wurde noch kein Zugangslink generiert.

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

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

+ {% else %} +

Es wurde noch kein Zugangslink generiert.

+ {% endif %} +
+ - {% endif %} -
+ {% if accessLink %} + + {% endif %} +
+ {% endif %}
diff --git a/tests/Controller/Admin/AccommodationBooking/GenerateAccessLinkControllerTest.php b/tests/Controller/Admin/AccommodationBooking/GenerateAccessLinkControllerTest.php new file mode 100644 index 0000000..b2e9b67 --- /dev/null +++ b/tests/Controller/Admin/AccommodationBooking/GenerateAccessLinkControllerTest.php @@ -0,0 +1,146 @@ +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 new file mode 100644 index 0000000..19a26d2 --- /dev/null +++ b/tests/Controller/Admin/AccommodationBooking/SendAccessLinkControllerTest.php @@ -0,0 +1,178 @@ +createMock(AccommodationBookingService::class); + $bookingService->expects(self::never())->method('sendCustomerConfirmationEmail'); + + $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); + + $response = $controller->index($this->openBooking(), Request::create('/admin/accommodation-booking/1/send-access-link')); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + self::assertSame('admin/accommodation_booking/modal_send_access_link.html.twig', $controller->renderedView); + } + + public function testPostSendsTheLinkAndRedirectsTheBrowser(): void + { + $booking = $this->openBooking(); + + $bookingService = $this->createMock(AccommodationBookingService::class); + $bookingService->expects(self::once())->method('sendCustomerConfirmationEmail')->with($booking); + + $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); + + $response = $controller->index($booking, Request::create('/admin/accommodation-booking/1/send-access-link', 'POST')); + + self::assertTrue($response->headers->has('HX-Redirect')); + } + + public function testABookingWithoutALinkHasNothingToSend(): void + { + $booking = $this->openBooking(); + $booking->setAccessLinkIssuedAt(null); + + $bookingService = $this->createMock(AccommodationBookingService::class); + $bookingService->expects(self::never())->method('sendCustomerConfirmationEmail'); + + $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); + + $response = $controller->index($booking, Request::create('/admin/accommodation-booking/1/send-access-link', 'POST')); + + self::assertSame(Response::HTTP_FOUND, $response->getStatusCode()); + } + + /** + * Drafts are held back even when a link is on the record — a booking pushed back into + * Entwurf keeps its accessLinkIssuedAt, and it must not be handed out again. + * + * @dataProvider requestMethods + */ + public function testADraftLinkIsNeverSentEvenIfOneWasIssuedEarlier(string $method): void + { + $booking = $this->openBooking(); + $booking->setStatus(AccommodationBookingStatus::Draft); + + $bookingService = $this->createMock(AccommodationBookingService::class); + $bookingService->expects(self::never())->method('sendCustomerConfirmationEmail'); + + $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); + + $response = $controller->index($booking, Request::create('/admin/accommodation-booking/1/send-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('sendCustomerConfirmationEmail'); + + $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class), tokenValid: false); + + $this->expectException(AccessDeniedException::class); + + $controller->index($this->openBooking(), Request::create('/admin/accommodation-booking/1/send-access-link', 'POST')); + } + + public function testABookingWithoutAnEmailAddressGetsNoLink(): void + { + $booking = $this->openBooking(); + $booking->setEmail(null); + + $bookingService = $this->createMock(AccommodationBookingService::class); + $bookingService->expects(self::never())->method('sendCustomerConfirmationEmail'); + + $controller = new TestableSendAccessLinkController($bookingService, $this->createMock(LoggerInterface::class)); + + $response = $controller->index($booking, Request::create('/admin/accommodation-booking/1/send-access-link', 'POST')); + + self::assertSame(['error'], array_column($controller->flashes, 'type')); + self::assertStringContainsString('app_admin_accommodationbooking_edit', (string) $response->headers->get('HX-Redirect')); + } + + private function openBooking(): AccommodationBooking + { + $booking = new AccommodationBooking(); + $booking->setStatus(AccommodationBookingStatus::Open); + $booking->setGroupName('Schulklasse 7b'); + $booking->setEmail('customer@example.com'); + $booking->setAccessLinkIssuedAt(new \DateTimeImmutable()); + + return $booking; + } +} + +final class TestableSendAccessLinkController extends SendAccessLinkController +{ + 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/Groups/OfferControllerTest.php b/tests/Controller/Groups/OfferControllerTest.php index 1fc1cfa..9ab97e5 100644 --- a/tests/Controller/Groups/OfferControllerTest.php +++ b/tests/Controller/Groups/OfferControllerTest.php @@ -134,10 +134,16 @@ class OfferControllerTest extends TestCase yield 'discarded' => [AccommodationBookingStatus::Discarded]; } - public function testViewRendersUnavailableForDiscardedBooking(): void + /** + * The status is re-read on every view, so an authorized session is no free pass: a + * booking pushed back into Entwurf stops showing the offer from that moment on. + * + * @dataProvider statusesTheCustomerMustNotSee + */ + public function testViewRendersUnavailableForABookingTheCustomerMustNotSee(AccommodationBookingStatus $status): void { $booking = new AccommodationBooking(); - $booking->setStatus(AccommodationBookingStatus::Discarded); + $booking->setStatus($status); $booking->setAccommodation(new Accommodation()); $bookingRepository = $this->createMock(AccommodationBookingRepository::class); diff --git a/tests/Entity/Groups/AccommodationBookingTest.php b/tests/Entity/Groups/AccommodationBookingTest.php index 7c6af59..deba69e 100644 --- a/tests/Entity/Groups/AccommodationBookingTest.php +++ b/tests/Entity/Groups/AccommodationBookingTest.php @@ -72,6 +72,23 @@ class AccommodationBookingTest extends TestCase self::assertSame('Buchung', $accepted->recordLabel()); } + public function testADraftIsTheOnlyStatusTheCustomerMustNotReach(): void + { + // Entwurf is the office's own workbench — nothing has been offered yet, so there is + // nothing an access link could show. Whether a link that exists still opens anything + // is a separate question the offer page answers (a discarded booking does not). + foreach (AccommodationBookingStatus::cases() as $status) { + $booking = new AccommodationBooking(); + $booking->setStatus($status); + + self::assertSame( + AccommodationBookingStatus::Draft !== $status, + $booking->isCustomerAccessible(), + $status->value, + ); + } + } + public function testTheLabelIsIndependentOfWhereTheBookingCameFrom(): void { // Origin answers "how did this come about" and is deliberately not the same diff --git a/tests/Service/AccommodationBookingServiceTest.php b/tests/Service/AccommodationBookingServiceTest.php index 6591ad5..813286c 100644 --- a/tests/Service/AccommodationBookingServiceTest.php +++ b/tests/Service/AccommodationBookingServiceTest.php @@ -69,8 +69,11 @@ class AccommodationBookingServiceTest extends TestCase $service = $this->createServiceWithAccommodation(entityManager: $entityManager); + // Eingegangen is what persist() gives a direct booking — it is never a draft, which + // is the one status that would hold the link back. $booking = new AccommodationBooking(); $booking->setOrigin(AccommodationBookingOrigin::Direct); + $booking->setStatus(AccommodationBookingStatus::Received); $service->issueAccessLinkForDirectBooking($booking); @@ -101,6 +104,7 @@ class AccommodationBookingServiceTest extends TestCase $booking = new AccommodationBooking(); $booking->setOrigin(AccommodationBookingOrigin::Direct); + $booking->setStatus(AccommodationBookingStatus::Received); $issuedAt = new \DateTimeImmutable('2026-01-01'); $booking->setAccessLinkIssuedAt($issuedAt); @@ -268,6 +272,25 @@ class AccommodationBookingServiceTest extends TestCase self::assertNotSame($previousIssuedAt, $booking->getAccessLinkIssuedAt()); } + public function testADraftGetsNoAccessLinkGenerated(): void + { + // A draft has not been offered to anyone, so there is nothing a link could show — + // the link is issued by sendOffer(), together with the mail that carries it. + $booking = new AccommodationBooking(); + $booking->setEmail('customer@example.com'); + $booking->setStatus(AccommodationBookingStatus::Draft); + + $entityManager = $this->createMock(EntityManagerInterface::class); + $entityManager->expects(self::never())->method('flush'); + + $service = $this->createServiceWithAccommodation(entityManager: $entityManager); + + $service->issueAccessLink($booking); + $service->regenerateAccessLink($booking); + + self::assertNull($booking->getAccessLinkIssuedAt()); + } + public function testAnInquiryIsPersistedAsRequestedSoTheOfficeStillOwesAnOffer(): void { // Angefragt, not Offen: the customer has asked, nobody has offered anything yet.