fix: prevent generating offer links while booking is in draft

2026-08-19 13:00:43 +02:00
parent 2601e46ec4
commit 8617bf6c86
11 changed files with 442 additions and 36 deletions
@@ -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.');
@@ -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()]);
        }

@@ -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();
    }

    /**
@@ -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;
@@ -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);
@@ -212,42 +212,48 @@

    <div class="mt-6 border-t border-gray-200 pt-6">
        <h2 class="text-lg font-bold mb-2">Zugangslink</h2>
-        {% if accessLink %}
-            <button type="button"
-                    class="button button--secondary button--small"
-                    data-controller="clipboard"
-                    data-clipboard-content-value="{{ accessLink }}"
-                    data-clipboard-label-value="Kopiert!"
-                    data-action="clipboard#copy">
-                {{ icon('copy', 'w-4 h-4') }} Link kopieren
-            </button>
-            <p class="text-xs text-gray-500 mt-1">
-                Gültig bis {{ accessLinkExpiresAt | date('d.m.Y') }}
-                {% if accessLinkExpiresAt < date() %}
-                    <span class="text-red-600 font-medium">(abgelaufen)</span>
-                {% endif %}
+        {% if not booking.customerAccessible %}
+            <p class="text-sm text-gray-500">
+                Ein Zugangslink wird beim Versand des Angebots erzeugt und kann erst danach erneuert werden.
            </p>
        {% else %}
-            <p class="text-sm text-gray-500">Es wurde noch kein Zugangslink generiert.</p>
-        {% endif %}
-        <div class="mt-2 flex items-center gap-2">
-            <button type="button"
-                    class="button button--secondary button--small"
-                    hx-get="{{ path('app_admin_accommodationbooking_generate_access_link', { id: booking.id }) }}"
-                    hx-target="body"
-                    hx-swap="beforeend">
-                {{ accessLink ? 'Link erneut generieren' : 'Link generieren' }}
-            </button>
            {% if accessLink %}
                <button type="button"
                        class="button button--secondary button--small"
-                        hx-get="{{ path('app_admin_accommodationbooking_send_access_link', { id: booking.id }) }}"
+                        data-controller="clipboard"
+                        data-clipboard-content-value="{{ accessLink }}"
+                        data-clipboard-label-value="Kopiert!"
+                        data-action="clipboard#copy">
+                    {{ icon('copy', 'w-4 h-4') }} Link kopieren
+                </button>
+                <p class="text-xs text-gray-500 mt-1">
+                    Gültig bis {{ accessLinkExpiresAt | date('d.m.Y') }}
+                    {% if accessLinkExpiresAt < date() %}
+                        <span class="text-red-600 font-medium">(abgelaufen)</span>
+                    {% endif %}
+                </p>
+            {% else %}
+                <p class="text-sm text-gray-500">Es wurde noch kein Zugangslink generiert.</p>
+            {% endif %}
+            <div class="mt-2 flex items-center gap-2">
+                <button type="button"
+                        class="button button--secondary button--small"
+                        hx-get="{{ path('app_admin_accommodationbooking_generate_access_link', { id: booking.id }) }}"
                        hx-target="body"
                        hx-swap="beforeend">
-                    Link senden
+                    {{ accessLink ? 'Link erneut generieren' : 'Link generieren' }}
                </button>
-            {% endif %}
-        </div>
+                {% if accessLink %}
+                    <button type="button"
+                            class="button button--secondary button--small"
+                            hx-get="{{ path('app_admin_accommodationbooking_send_access_link', { id: booking.id }) }}"
+                            hx-target="body"
+                            hx-swap="beforeend">
+                        Link senden
+                    </button>
+                {% endif %}
+            </div>
+        {% endif %}
    </div>

    <div class="flex justify-between pt-8">
@@ -0,0 +1,146 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Controller\Admin\AccommodationBooking;
+
+use App\Controller\Admin\AccommodationBooking\GenerateAccessLinkController;
+use App\Entity\Groups\AccommodationBooking;
+use App\Enum\Groups\AccommodationBookingStatus;
+use App\Service\AccommodationBookingService;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Security\Core\Exception\AccessDeniedException;
+
+/**
+ * Covers the guards around issuing an access link — the issuing itself is tested in
+ * AccommodationBookingServiceTest.
+ */
+class GenerateAccessLinkControllerTest extends TestCase
+{
+    public function testGetRendersTheConfirmationModal(): void
+    {
+        $bookingService = $this->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<string, array{string}>
+     */
+    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('[email protected]');
+
+        return $booking;
+    }
+}
+
+final class TestableGenerateAccessLinkController extends GenerateAccessLinkController
+{
+    public ?string $renderedView = null;
+
+    /** @var list<array{type: string, message: mixed}> */
+    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<string, mixed> $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<string, mixed> $parameters
+     */
+    protected function generateUrl(string $route, array $parameters = [], int $referenceType = 1): string
+    {
+        return '/'.$route.'?'.http_build_query($parameters);
+    }
+}
@@ -0,0 +1,178 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Controller\Admin\AccommodationBooking;
+
+use App\Controller\Admin\AccommodationBooking\SendAccessLinkController;
+use App\Entity\Groups\AccommodationBooking;
+use App\Enum\Groups\AccommodationBookingStatus;
+use App\Service\AccommodationBookingService;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Security\Core\Exception\AccessDeniedException;
+
+/**
+ * Covers the guards around sending the access link — the mail itself is tested in
+ * AccommodationBookingServiceTest.
+ */
+class SendAccessLinkControllerTest extends TestCase
+{
+    public function testGetRendersTheConfirmationModal(): void
+    {
+        $bookingService = $this->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<string, array{string}>
+     */
+    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('[email protected]');
+        $booking->setAccessLinkIssuedAt(new \DateTimeImmutable());
+
+        return $booking;
+    }
+}
+
+final class TestableSendAccessLinkController extends SendAccessLinkController
+{
+    public ?string $renderedView = null;
+
+    /** @var list<array{type: string, message: mixed}> */
+    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<string, mixed> $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<string, mixed> $parameters
+     */
+    protected function generateUrl(string $route, array $parameters = [], int $referenceType = 1): string
+    {
+        return '/'.$route.'?'.http_build_query($parameters);
+    }
+}
@@ -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);
@@ -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
@@ -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('[email protected]');
+        $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.