From 773cd4f8471882eaadb0e898299d77bf0e954dcd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= <bjoern.fromme@ep-reisen.de>
Date: Sat, 26 Sep 2026 13:57:50 +0200
Subject: [PATCH] fix: keep post-booking newsletter failures out of the booking
 flow

---
 src/Service/BookingNewsletterOptIn.php       | 28 ++++++++-------
 tests/Service/BookingNewsletterOptInTest.php | 38 ++++++++++++++++++++
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/src/Service/BookingNewsletterOptIn.php b/src/Service/BookingNewsletterOptIn.php
index af63f60b..c2d4d850 100644
--- a/src/Service/BookingNewsletterOptIn.php
+++ b/src/Service/BookingNewsletterOptIn.php
@@ -6,7 +6,6 @@ namespace App\Service;
 
 use App\Entity\User;
 use App\Enum\NewsletterOptInContext;
-use App\Exception\NewsletterProviderException;
 use App\Form\Model\BookingDto;
 use App\Form\Model\ParticipantDto;
 use Psr\Log\LoggerInterface;
@@ -102,13 +101,16 @@ class BookingNewsletterOptIn
 
     public function subscribeApplicant(string $email, ?ParticipantDto $applicant): void
     {
+        // Everything here runs after the booking API has accepted the booking, so nothing may
+        // escape: an exception would reach the controller's generic handler, show a technical
+        // error and leave the booking session intact, inviting a duplicate booking.
         try {
             $this->newsletterManager->requestConfirmation(
                 $email,
                 $applicant?->firstName,
                 $applicant?->lastName,
             );
-        } catch (NewsletterProviderException|\InvalidArgumentException $exception) {
+        } catch (\Throwable $exception) {
             $this->logger->warning('Newsletter confirmation request failed after booking', [
                 'email' => $email,
                 'error' => $exception->getMessage(),
@@ -124,16 +126,18 @@ class BookingNewsletterOptIn
         foreach ($companions as $companion) {
             $email = mb_strtolower(trim((string) $companion->email));
 
-            // Skipping a pending request avoids replacing a token the companion asked for
-            // themselves, which would silently break the link they already hold.
-            if (true === $this->newsletterManager->hasConfirmedOptIn($email)
-                || true === $this->newsletterManager->hasPendingConfirmation($email)) {
-                continue;
-            }
-
-            // The booking already succeeded, so one unusable address must not stop the others
-            // and must never surface as a booking failure.
+            // The booking already succeeded, so one unusable address must not stop the others and
+            // must never surface as a booking failure. The consent lookups are inside the block for
+            // the same reason: a repository failure there would otherwise reach the controller's
+            // generic handler and show a technical error for a booking that already exists.
             try {
+                // Skipping a pending request avoids replacing a token the companion asked for
+                // themselves, which would silently break the link they already hold.
+                if (true === $this->newsletterManager->hasConfirmedOptIn($email)
+                    || true === $this->newsletterManager->hasPendingConfirmation($email)) {
+                    continue;
+                }
+
                 $this->newsletterManager->requestConfirmation(
                     $email,
                     $companion->firstName,
@@ -141,7 +145,7 @@ class BookingNewsletterOptIn
                     NewsletterOptInContext::Companion,
                     $requestedByName,
                 );
-            } catch (NewsletterProviderException|\InvalidArgumentException $exception) {
+            } catch (\Throwable $exception) {
                 $this->logger->warning('Companion newsletter confirmation request failed after booking', [
                     'email' => $email,
                     'error' => $exception->getMessage(),
diff --git a/tests/Service/BookingNewsletterOptInTest.php b/tests/Service/BookingNewsletterOptInTest.php
index 392e662c..d562262d 100644
--- a/tests/Service/BookingNewsletterOptInTest.php
+++ b/tests/Service/BookingNewsletterOptInTest.php
@@ -110,6 +110,44 @@ class BookingNewsletterOptInTest extends TestCase
         self::assertSame(['broken@example.com', 'fine@example.com'], $requestedEmails);
     }
 
+    public function testAFailingConsentLookupDoesNotEscapeToTheCaller(): void
+    {
+        $companions = $this->createCompanions([
+            'lookup-fails@example.com',
+            'fine@example.com',
+        ]);
+
+        $newsletterManager = $this->createMock(NewsletterManager::class);
+        $newsletterManager
+            ->method('hasConfirmedOptIn')
+            ->willReturnCallback(static function (string $email): bool {
+                if ('lookup-fails@example.com' === $email) {
+                    throw new \RuntimeException('Database gone.');
+                }
+
+                return false;
+            });
+        $newsletterManager->method('hasPendingConfirmation')->willReturn(false);
+        $newsletterManager
+            ->expects(self::once())
+            ->method('requestConfirmation')
+            ->with('fine@example.com');
+
+        $this->createService($newsletterManager)->subscribeCompanions($companions, 'Mia Muster');
+    }
+
+    public function testApplicantOptInFailureDoesNotEscapeToTheCaller(): void
+    {
+        $newsletterManager = $this->createStub(NewsletterManager::class);
+        $newsletterManager
+            ->method('requestConfirmation')
+            ->willThrowException(new \RuntimeException('Database gone.'));
+
+        $this->createService($newsletterManager)->subscribeApplicant('applicant@example.com', null);
+
+        $this->expectNotToPerformAssertions();
+    }
+
     public function testApplicantOptInUsesTheOwnWording(): void
     {
         $bookingDto = $this->createBookingDto([['applicant@example.com', 30]]);