feat: re-generate access links for bookings when sent via email

src/Controller/Admin/AccommodationBooking/GenerateAccessLinkController.php

@@ -1,55 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace App\Controller\Admin\AccommodationBooking;
-
-use App\Entity\Groups\AccommodationBooking;
-use App\Htmx\HxRedirectResponse;
-use App\Service\AccommodationBookingService;
-use Psr\Log\LoggerInterface;
-use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
-use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpFoundation\Response;
-use Symfony\Component\Routing\Attribute\Route;
-use Symfony\Component\Security\Http\Attribute\IsGranted;
-
-#[IsGranted('ROLE_GROUPS_MANAGER')]
-class GenerateAccessLinkController extends AbstractController
-{
-    public function __construct(
-        private readonly AccommodationBookingService $bookingService,
-        private readonly LoggerInterface $logger,
-    ) {
-    }
-
-    #[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.');
-            }
-
-            $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(),
-        ]);
-    }
-}

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(),

src/Service/AccommodationBookingService.php

@@ -492,13 +492,15 @@ class AccommodationBookingService
     }
 
     /**
-     * Explicit admin action: (re)issues the access link, invalidating any previously issued
+     * Explicit admin action: mails the access link again, refreshing its validity first so the
-     * link for this booking. No email side effect — sending is a separate, explicit admin
+     * customer always receives a link that is good for another full TTL — the mail is the only
-     * action via sendCustomerConfirmationEmail().
+     * 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);
     }
 
     /**

templates/admin/accommodation_booking/modal_generate_access_link.html.twig

@@ -1,8 +0,0 @@
-{% extends 'htmx_confirmation_modal.html.twig' %}
-
-{% block content %}
-    <div>
-        Möchtest du für <em>{{ booking.groupName }}</em> einen neuen Zugangslink generieren?
-        Ein zuvor generierter Link wird dadurch ungültig.
-    </div>
-{% endblock %}

templates/admin/accommodation_booking/modal_send_access_link.html.twig

@@ -2,7 +2,7 @@
 
 {% block content %}
     <div>
-        Möchtest du den aktuellen Zugangslink für <em>{{ booking.groupName }}</em> an
+        Möchtest du einen neuen Zugangslink für <em>{{ booking.groupName }}</em> an
-        <em>{{ booking.email }}</em> senden?
+        <em>{{ booking.email }}</em> senden? Der zuvor gesendete Link wird dadurch ungültig.
     </div>
 {% endblock %}

templates/admin/accommodation_booking/show.html.twig

@@ -214,10 +214,10 @@
         <h2 class="text-lg font-bold mb-2">Zugangslink</h2>
         {% 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.
+                Ein Zugangslink wird beim Versand des Angebots erzeugt.
             </p>
-        {% else %}
+        {% elseif accessLink %}
-            {% if accessLink %}
+            <div class="flex items-center gap-2">
                 <button type="button"
                         class="button button--secondary button--small"
                         data-controller="clipboard"
@@ -226,33 +226,24 @@
                         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-get="{{ path('app_admin_accommodationbooking_send_access_link', { id: booking.id }) }}"
                         hx-target="body"
                         hx-swap="beforeend">
-                    {{ accessLink ? 'Link erneut generieren' : 'Link generieren' }}
+                    Link senden
                 </button>
-                {% 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>
+            <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">
+                Der Zugangslink wird beim Versand des Angebots erzeugt.
+            </p>
         {% endif %}
     </div>
 

tests/Controller/Admin/AccommodationBooking/GenerateAccessLinkControllerTest.php

@@ -1,146 +0,0 @@
-<?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);
-    }
-}

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));
 

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('[email protected]');
         $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('[email protected]');
+        $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());
     }