feat: improved performance by avoiding redundant MailJet API calls

api.http

@@ -116,7 +116,9 @@ Authorization: Bearer {{$auth.token("oauth2_newsletter")}}
 
 {
   "email": "{{$random.email}}",
-  "listIds": [10321569]
+  "firstName": "{{$random.address.firstName}}",
+  "lastName": "{{$random.address.lastName}}",
+  "listIds": [10321569,10321382,10321383]
 }
 
 ### API pickups planning webhook

src/Service/NewsletterManager.php

@@ -96,23 +96,26 @@ class NewsletterManager
             }
         }
 
-        $subscribedListIds = $this->subscribedMailjetListIds($normalizedEmail, $normalizedListIds);
-        $missingListIds = array_values(array_diff($normalizedListIds, $subscribedListIds));
         $hasConfirmedOptIn = null !== $this->consentRepository->findActiveByEmail($normalizedEmail);
 
-        if ([] === $missingListIds) {
+        // We trust our local consent state for DOI decisions. If Mailjet is ahead of the database,
-            $this->recordSubscription($normalizedEmail, $normalizedListIds, [], $normalizedFirstName, $normalizedLastName);
+        // we accept a redundant confirmation cycle and reconcile the contact on confirmation.
+        if (true === $hasConfirmedOptIn) {
+            $confirmedListIds = $this->confirmedMailjetListIds($normalizedEmail, $normalizedListIds);
+            $missingListIds = array_values(array_diff($normalizedListIds, $confirmedListIds));
 
-            return new NewsletterSubscriptionRequestResult(
+            if ([] === $missingListIds) {
-                $normalizedEmail,
+                $this->recordSubscription($normalizedEmail, $normalizedListIds, [], $normalizedFirstName, $normalizedLastName);
-                $normalizedListIds,
+
-                NewsletterSubscriptionRequestResult::STATE_SUBSCRIBED,
+                return new NewsletterSubscriptionRequestResult(
-                false,
+                    $normalizedEmail,
-                $this->createListStates($normalizedListIds, NewsletterSubscriptionRequestResult::LIST_STATE_ALREADY_REGISTERED),
+                    $normalizedListIds,
-            );
+                    NewsletterSubscriptionRequestResult::STATE_SUBSCRIBED,
-        }
+                    false,
+                    $this->createListStates($normalizedListIds, NewsletterSubscriptionRequestResult::LIST_STATE_ALREADY_REGISTERED),
+                );
+            }
 
-        if (true === $hasConfirmedOptIn || [] !== $subscribedListIds || true === $this->isSubscribedToKnownList($normalizedEmail, $knownNormalizedListIds, $normalizedListIds)) {
             $this->recordSubscription($normalizedEmail, $normalizedListIds, $missingListIds, $normalizedFirstName, $normalizedLastName);
 
             return new NewsletterSubscriptionRequestResult(
@@ -389,17 +392,18 @@ class NewsletterManager
      *
      * @return list<int>
      */
-    private function subscribedMailjetListIds(string $email, array $mailjetListIds): array
+    private function confirmedMailjetListIds(string $email, array $mailjetListIds): array
     {
-        $subscribedListIds = [];
+        $confirmedListIds = [];
 
         foreach ($mailjetListIds as $mailjetListId) {
-            if (true === $this->newsletterService->isSubscribed($email, $mailjetListId)) {
+            $consent = $this->consentRepository->findOneByEmailAndListId($email, $mailjetListId);
-                $subscribedListIds[] = $mailjetListId;
+            if (null !== $consent && true === $consent->isConfirmed()) {
+                $confirmedListIds[] = $mailjetListId;
             }
         }
 
-        return $subscribedListIds;
+        return $confirmedListIds;
     }
 
     /**
@@ -484,27 +488,6 @@ class NewsletterManager
         }
     }
 
-    /**
-     * @param list<int> $knownMailjetListIds
-     * @param list<int> $alreadyCheckedListIds
-     */
-    private function isSubscribedToKnownList(string $normalizedEmail, array $knownMailjetListIds, array $alreadyCheckedListIds): bool
-    {
-        $alreadyCheckedListIdMap = array_fill_keys($alreadyCheckedListIds, true);
-
-        foreach ($knownMailjetListIds as $mailjetListId) {
-            if (true === isset($alreadyCheckedListIdMap[$mailjetListId])) {
-                continue;
-            }
-
-            if (true === $this->newsletterService->isSubscribed($normalizedEmail, $mailjetListId)) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
     private function defaultMailjetListId(): int
     {
         if (null === $this->defaultMailjetListId || '' === trim($this->defaultMailjetListId)) {

tests/Service/NewsletterManagerTest.php

@@ -29,12 +29,10 @@ class NewsletterManagerTest extends TestCase
         $mailjet = $this->createMock(MailjetApiClient::class);
         $mailer = $this->createMock(Mailer::class);
 
-        $mailjet
+        $mailjet->expects(self::never())->method('isSubscribed');
-            ->expects(self::exactly(2))
+        $mailjet->expects(self::never())->method('ensureSubscribed');
-            ->method('isSubscribed')
-            ->withConsecutive(['[email protected]', 1], ['[email protected]', 2])
-            ->willReturn(false);
         $consents->expects(self::once())->method('findActiveByEmail')->with('[email protected]')->willReturn(null);
+        $consents->expects(self::never())->method('findOneByEmailAndListId');
         $repository->expects(self::once())->method('deleteExpiredPendingByEmail')->with('[email protected]')->willReturn(0);
         $repository->expects(self::once())->method('findPendingByEmail')->with('[email protected]')->willReturn(null);
 
@@ -88,8 +86,10 @@ class NewsletterManagerTest extends TestCase
         $mailjet = $this->createMock(MailjetApiClient::class);
         $mailer = $this->createMock(Mailer::class);
 
-        $mailjet->method('isSubscribed')->willReturn(false);
+        $mailjet->expects(self::never())->method('isSubscribed');
+        $mailjet->expects(self::never())->method('ensureSubscribed');
         $consents->expects(self::once())->method('findActiveByEmail')->with('[email protected]')->willReturn(null);
+        $consents->expects(self::never())->method('findOneByEmailAndListId');
         $repository->method('deleteExpiredPendingByEmail')->willReturn(0);
         $repository->expects(self::once())->method('findPendingByEmail')->with('[email protected]')->willReturn($pending);
 
@@ -117,8 +117,10 @@ class NewsletterManagerTest extends TestCase
         $mailjet = $this->createMock(MailjetApiClient::class);
         $mailer = $this->createMock(Mailer::class);
 
-        $mailjet->method('isSubscribed')->willReturn(false);
+        $mailjet->expects(self::never())->method('isSubscribed');
+        $mailjet->expects(self::never())->method('ensureSubscribed');
         $consents->expects(self::once())->method('findActiveByEmail')->with('[email protected]')->willReturn(null);
+        $consents->expects(self::never())->method('findOneByEmailAndListId');
         $repository->method('deleteExpiredPendingByEmail')->willReturn(0);
         $repository->expects(self::exactly(2))->method('findPendingByEmail')->with('[email protected]')->willReturn($pending);
 
@@ -136,20 +138,33 @@ class NewsletterManagerTest extends TestCase
         self::assertSame('Person', $pending->getLastName());
     }
 
-    public function testApiRequestRecordsConsentWhenAllListsAreAlreadySubscribed(): void
+    public function testApiRequestUsesLocalConsentStateForAlreadyRegisteredLists(): void
     {
+        $existingConsent = new NewsletterConsent('[email protected]', 1, 'Old', 'Name');
+        $existingConsent->markConfirmed();
+        $existingOtherConsent = new NewsletterConsent('[email protected]', 2, 'Old', 'Name');
+        $existingOtherConsent->markConfirmed();
         $repository = $this->createMock(NewsletterOptInRequestRepository::class);
         $consents = $this->createMock(NewsletterConsentRepository::class);
         $entityManager = $this->createMock(EntityManagerInterface::class);
         $mailjet = $this->createMock(MailjetApiClient::class);
         $mailer = $this->createMock(Mailer::class);
 
-        $mailjet->method('isSubscribed')->willReturn(true);
+        $mailjet->expects(self::never())->method('isSubscribed');
+        $mailjet->expects(self::never())->method('ensureSubscribed');
         $mailjet->expects(self::once())->method('upsertContact')->with('[email protected]', 'Mia', 'Muster');
-        $consents->method('findActiveByEmail')->with('[email protected]')->willReturn(null);
+        $consents->method('findActiveByEmail')->with('[email protected]')->willReturn($existingConsent);
-        $consents->method('findOneByEmailAndListId')->willReturn(null);
+        $consents
+            ->method('findOneByEmailAndListId')
+            ->willReturnCallback(static function (string $email, int $mailjetListId) use ($existingConsent, $existingOtherConsent): ?NewsletterConsent {
+                return match ($mailjetListId) {
+                    1 => $existingConsent,
+                    2 => $existingOtherConsent,
+                    default => null,
+                };
+            });
         $repository->expects(self::once())->method('deletePendingByEmail')->with('[email protected]')->willReturn(0);
-        $entityManager->expects(self::exactly(2))->method('persist')->with(self::isInstanceOf(NewsletterConsent::class));
+        $entityManager->expects(self::never())->method('persist');
         $entityManager->expects(self::once())->method('flush');
         $mailer->expects(self::never())->method('createAndSendEmail');
 
@@ -192,20 +207,18 @@ class NewsletterManagerTest extends TestCase
 
     public function testApiRequestDirectlySubscribesMissingListWhenEmailIsSubscribedToKnownList(): void
     {
+        $existingConsent = new NewsletterConsent('[email protected]', 1, 'Old', 'Name');
+        $existingConsent->markConfirmed();
         $repository = $this->createMock(NewsletterOptInRequestRepository::class);
         $consents = $this->createMock(NewsletterConsentRepository::class);
         $entityManager = $this->createMock(EntityManagerInterface::class);
         $mailjet = $this->createMock(MailjetApiClient::class);
         $mailer = $this->createMock(Mailer::class);
 
-        $mailjet
+        $mailjet->expects(self::never())->method('isSubscribed');
-            ->expects(self::exactly(2))
-            ->method('isSubscribed')
-            ->withConsecutive(['[email protected]', 2], ['[email protected]', 1])
-            ->willReturnOnConsecutiveCalls(false, true);
         $mailjet->expects(self::once())->method('upsertContact')->with('[email protected]', 'Mia', 'Muster');
         $mailjet->expects(self::once())->method('ensureSubscribed')->with('[email protected]', 2);
-        $consents->method('findActiveByEmail')->with('[email protected]')->willReturn(null);
+        $consents->method('findActiveByEmail')->with('[email protected]')->willReturn($existingConsent);
         $consents->method('findOneByEmailAndListId')->willReturn(null);
         $repository->expects(self::once())->method('deletePendingByEmail')->with('[email protected]')->willReturn(0);
         $repository->expects(self::never())->method('findPendingByEmail');
@@ -233,7 +246,7 @@ class NewsletterManagerTest extends TestCase
         $mailjet = $this->createMock(MailjetApiClient::class);
         $mailer = $this->createMock(Mailer::class);
 
-        $mailjet->method('isSubscribed')->with('[email protected]', 2)->willReturn(false);
+        $mailjet->expects(self::never())->method('isSubscribed');
         $mailjet->expects(self::once())->method('upsertContact')->with('[email protected]', 'Mia', 'Muster');
         $mailjet->expects(self::once())->method('ensureSubscribed')->with('[email protected]', 2);
         $consents->method('findActiveByEmail')->with('[email protected]')->willReturn($existingConsent);