fix: keep post-booking newsletter failures out of the booking flow
This commit is contained in:
1 parent
876e69e62c
commit
773cd4f847
2 files changed
+48
-6
No files matched your search
@@ -6,7 +6,6 @@ namespace App\Service;
|
|||||||
|
|
||||||
use App\Entity\User;
|
use App\Entity\User;
|
||||||
use App\Enum\NewsletterOptInContext;
|
use App\Enum\NewsletterOptInContext;
|
||||||
use App\Exception\NewsletterProviderException;
|
|
||||||
use App\Form\Model\BookingDto;
|
use App\Form\Model\BookingDto;
|
||||||
use App\Form\Model\ParticipantDto;
|
use App\Form\Model\ParticipantDto;
|
||||||
use Psr\Log\LoggerInterface;
|
use Psr\Log\LoggerInterface;
|
||||||
@@ -102,13 +101,16 @@ class BookingNewsletterOptIn
|
|||||||
|
|
||||||
public function subscribeApplicant(string $email, ?ParticipantDto $applicant): void
|
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 {
|
try {
|
||||||
$this->newsletterManager->requestConfirmation(
|
$this->newsletterManager->requestConfirmation(
|
||||||
$email,
|
$email,
|
||||||
$applicant?->firstName,
|
$applicant?->firstName,
|
||||||
$applicant?->lastName,
|
$applicant?->lastName,
|
||||||
);
|
);
|
||||||
} catch (NewsletterProviderException|\InvalidArgumentException $exception) {
|
} catch (\Throwable $exception) {
|
||||||
$this->logger->warning('Newsletter confirmation request failed after booking', [
|
$this->logger->warning('Newsletter confirmation request failed after booking', [
|
||||||
'email' => $email,
|
'email' => $email,
|
||||||
'error' => $exception->getMessage(),
|
'error' => $exception->getMessage(),
|
||||||
@@ -124,6 +126,11 @@ class BookingNewsletterOptIn
|
|||||||
foreach ($companions as $companion) {
|
foreach ($companions as $companion) {
|
||||||
$email = mb_strtolower(trim((string) $companion->email));
|
$email = mb_strtolower(trim((string) $companion->email));
|
||||||
|
|
||||||
|
// 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
|
// Skipping a pending request avoids replacing a token the companion asked for
|
||||||
// themselves, which would silently break the link they already hold.
|
// themselves, which would silently break the link they already hold.
|
||||||
if (true === $this->newsletterManager->hasConfirmedOptIn($email)
|
if (true === $this->newsletterManager->hasConfirmedOptIn($email)
|
||||||
@@ -131,9 +138,6 @@ class BookingNewsletterOptIn
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
// The booking already succeeded, so one unusable address must not stop the others
|
|
||||||
// and must never surface as a booking failure.
|
|
||||||
try {
|
|
||||||
$this->newsletterManager->requestConfirmation(
|
$this->newsletterManager->requestConfirmation(
|
||||||
$email,
|
$email,
|
||||||
$companion->firstName,
|
$companion->firstName,
|
||||||
@@ -141,7 +145,7 @@ class BookingNewsletterOptIn
|
|||||||
NewsletterOptInContext::Companion,
|
NewsletterOptInContext::Companion,
|
||||||
$requestedByName,
|
$requestedByName,
|
||||||
);
|
);
|
||||||
} catch (NewsletterProviderException|\InvalidArgumentException $exception) {
|
} catch (\Throwable $exception) {
|
||||||
$this->logger->warning('Companion newsletter confirmation request failed after booking', [
|
$this->logger->warning('Companion newsletter confirmation request failed after booking', [
|
||||||
'email' => $email,
|
'email' => $email,
|
||||||
'error' => $exception->getMessage(),
|
'error' => $exception->getMessage(),
|
||||||
|
|||||||
@@ -110,6 +110,44 @@ class BookingNewsletterOptInTest extends TestCase
|
|||||||
self::assertSame(['[email protected]', '[email protected]'], $requestedEmails);
|
self::assertSame(['[email protected]', '[email protected]'], $requestedEmails);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testAFailingConsentLookupDoesNotEscapeToTheCaller(): void
|
||||||
|
{
|
||||||
|
$companions = $this->createCompanions([
|
||||||
|
'[email protected]',
|
||||||
|
'[email protected]',
|
||||||
|
]);
|
||||||
|
|
||||||
|
$newsletterManager = $this->createMock(NewsletterManager::class);
|
||||||
|
$newsletterManager
|
||||||
|
->method('hasConfirmedOptIn')
|
||||||
|
->willReturnCallback(static function (string $email): bool {
|
||||||
|
if ('[email protected]' === $email) {
|
||||||
|
throw new \RuntimeException('Database gone.');
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
});
|
||||||
|
$newsletterManager->method('hasPendingConfirmation')->willReturn(false);
|
||||||
|
$newsletterManager
|
||||||
|
->expects(self::once())
|
||||||
|
->method('requestConfirmation')
|
||||||
|
->with('[email protected]');
|
||||||
|
|
||||||
|
$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('[email protected]', null);
|
||||||
|
|
||||||
|
$this->expectNotToPerformAssertions();
|
||||||
|
}
|
||||||
|
|
||||||
public function testApplicantOptInUsesTheOwnWording(): void
|
public function testApplicantOptInUsesTheOwnWording(): void
|
||||||
{
|
{
|
||||||
$bookingDto = $this->createBookingDto([['[email protected]', 30]]);
|
$bookingDto = $this->createBookingDto([['[email protected]', 30]]);
|
||||||
|
|||||||
Reference in new issue
Block a user