From cd3652eaa6066adf31dbca18b7db8d23c376b482 Mon Sep 17 00:00:00 2001
From: Rostislav Wolny <rosta.wolny@automattic.com>
Date: Fri, 21 Oct 2022 13:01:56 +0200
Subject: [PATCH] Fix canceling multiple automatic emails

When we deleted sending queue using SQL it remained in the entity manager
and subsequent flush (not the first one) triggered the error, because it didn't know the ScheduledTask entity attached
to the orphaned SendingQueue entity.

This commit fixes this by refactoring deletion of sending queue using standard repository method.
After fixing the issue for sending queue there was another issue with SchedulesTaskSubscriberEntity that remained in memory.
I fixed that by detaching those. Theoretically there might be many SchedulesTaskSubscriberEntities for an Automatic email so
I consider still safer to delete using SQL and if there are some loaded (in this case there is one) detach them.
[MAILPOET-4741]
---
 .../Scheduler/AutomaticEmailScheduler.php     |  9 +++-
 .../WooCommerce/Events/AbandonedCartTest.php  |  1 +
 .../Scheduler/AutomaticEmailTest.php          | 49 +++++++++++++++----
 3 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/mailpoet/lib/Newsletter/Scheduler/AutomaticEmailScheduler.php b/mailpoet/lib/Newsletter/Scheduler/AutomaticEmailScheduler.php
index 4b81110d08..af5b307136 100644
--- a/mailpoet/lib/Newsletter/Scheduler/AutomaticEmailScheduler.php
+++ b/mailpoet/lib/Newsletter/Scheduler/AutomaticEmailScheduler.php
@@ -122,8 +122,15 @@ class AutomaticEmailScheduler {
       // try to find existing scheduled task for given subscriber
       $task = $this->scheduledTasksRepository->findOneScheduledByNewsletterAndSubscriber($newsletter, $subscriber);
       if ($task) {
-        $this->sendingQueuesRepository->deleteByTask($task);
+        $queue = $task->getSendingQueue();
+        if ($queue instanceof SendingQueueEntity) {
+          $this->sendingQueuesRepository->remove($queue);
+        }
         $this->scheduledTaskSubscribersRepository->deleteByTask($task);
+        // In case any of task associated SchedulesTaskSubscriberEntity was loaded we need to detach them
+        foreach ($task->getSubscribers() as $taskSubscriber) {
+          $this->scheduledTaskSubscribersRepository->detach($taskSubscriber);
+        }
         $this->scheduledTasksRepository->remove($task);
         $this->scheduledTasksRepository->flush();
       }
diff --git a/mailpoet/tests/integration/AutomaticEmails/WooCommerce/Events/AbandonedCartTest.php b/mailpoet/tests/integration/AutomaticEmails/WooCommerce/Events/AbandonedCartTest.php
index 7a19cc3f52..812a66ac06 100644
--- a/mailpoet/tests/integration/AutomaticEmails/WooCommerce/Events/AbandonedCartTest.php
+++ b/mailpoet/tests/integration/AutomaticEmails/WooCommerce/Events/AbandonedCartTest.php
@@ -347,6 +347,7 @@ class AbandonedCartTest extends \MailPoetTest {
     $this->entityManager->flush();
 
     $scheduledTask->setScheduledAt($scheduleAt);
+    $scheduledTask->setSendingQueue($sendingQueue);
     $scheduledTask->setStatus(($this->currentTime < $scheduleAt) ? ScheduledTaskEntity::STATUS_SCHEDULED : ScheduledTaskEntity::STATUS_COMPLETED);
     $this->entityManager->flush();
 
diff --git a/mailpoet/tests/integration/Newsletter/Scheduler/AutomaticEmailTest.php b/mailpoet/tests/integration/Newsletter/Scheduler/AutomaticEmailTest.php
index 36557cd319..6728d68950 100644
--- a/mailpoet/tests/integration/Newsletter/Scheduler/AutomaticEmailTest.php
+++ b/mailpoet/tests/integration/Newsletter/Scheduler/AutomaticEmailTest.php
@@ -50,16 +50,7 @@ class AutomaticEmailTest extends \MailPoetTest {
     $this->scheduledTasksRepository = $this->diContainer->get(ScheduledTasksRepository::class);
 
     $this->newsletterFactory = new NewsletterFactory();
-    $this->newsletter = $this->newsletterFactory->withActiveStatus()->withAutomaticType()->create();
-    $this->newsletterOptionFactory = new NewsletterOptionFactory();
-    $this->newsletterOptionFactory->createMultipleOptions(
-      $this->newsletter,
-      [
-        'sendTo' => 'user',
-        'afterTimeType' => 'hours',
-        'afterTimeNumber' => 2,
-      ]
-    );
+    $this->newsletter = $this->createAutomaticNewsletter();
   }
 
   public function testItCreatesScheduledAutomaticEmailSendingTaskForUser() {
@@ -149,6 +140,30 @@ class AutomaticEmailTest extends \MailPoetTest {
     $this->assertCount(0, $this->sendingQueuesRepository->findAll());
   }
 
+  public function testItCanCancelMultipleAutomaticEmails() {
+    $newsletter = $this->newslettersRepository->findOneById($this->newsletter->getId());
+    $this->newsletterOptionFactory->createMultipleOptions(
+      $this->newsletter,
+      [
+        'group' => 'some_group',
+        'event' => 'some_event',
+      ]
+    );
+    $newsletter2 = $this->createAutomaticNewsletter();
+    $this->newsletterOptionFactory->createMultipleOptions(
+      $newsletter2,
+      [
+        'group' => 'some_group',
+        'event' => 'some_event',
+      ]
+    );
+    $this->assertInstanceOf(NewsletterEntity::class, $newsletter);
+    $subscriber = (new SubscriberFactory())->create();
+    $this->automaticEmailScheduler->createAutomaticEmailScheduledTask($newsletter, $subscriber);
+    $this->automaticEmailScheduler->createAutomaticEmailScheduledTask($newsletter2, $subscriber);
+    $this->automaticEmailScheduler->cancelAutomaticEmail('some_group', 'some_event', $subscriber);
+  }
+
   public function testItSchedulesAutomaticEmailWhenConditionMatches() {
     $currentTime = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp'));
     $this->newsletterOptionFactory->createMultipleOptions(
@@ -200,6 +215,20 @@ class AutomaticEmailTest extends \MailPoetTest {
       ->equals($currentTime->addHours(2)->format('Y-m-d H:i'));
   }
 
+  private function createAutomaticNewsletter(): NewsletterEntity {
+    $newsletter = $this->newsletterFactory->withActiveStatus()->withAutomaticType()->create();
+    $this->newsletterOptionFactory = new NewsletterOptionFactory();
+    $this->newsletterOptionFactory->createMultipleOptions(
+      $newsletter,
+      [
+        'sendTo' => 'user',
+        'afterTimeType' => 'hours',
+        'afterTimeNumber' => 2,
+      ]
+    );
+    return $newsletter;
+  }
+
   public function _after() {
     Carbon::setTestNow();
     $this->truncateEntity(NewsletterEntity::class);