Check if bounce sync is scheduled within 42 hours for all emails when sending

[MAILPOET-2705]

lib/Cron/Workers/Scheduler.php

@@ -14,7 +14,6 @@ use MailPoet\Newsletter\Scheduler\Scheduler as NewsletterScheduler;
 use MailPoet\Newsletter\Scheduler\WelcomeScheduler;
 use MailPoet\Segments\SubscribersFinder;
 use MailPoet\Tasks\Sending as SendingTask;
-use MailPoetVendor\Carbon\Carbon;
 
 class Scheduler {
   const TASK_BATCH_SIZE = 5;
@@ -130,7 +129,6 @@ class Scheduler {
       'post notification set status to sending',
       ['newsletter_id' => $newsletter->id, 'task_id' => $queue->taskId]
     );
-    $this->reScheduleBounceTask();
     return true;
   }
 
@@ -167,7 +165,6 @@ class Scheduler {
     $task->save();
     // update newsletter status
     $newsletter->setStatus(Newsletter::STATUS_SENDING);
-    $this->reScheduleBounceTask();
     return true;
   }
 
@@ -245,18 +242,6 @@ class Scheduler {
     ScheduledTask::touchAllByIds($ids);
   }
 
-  private function reScheduleBounceTask() {
-    $bounceTasks = ScheduledTask::findFutureScheduledByType(Bounce::TASK_TYPE);
-    if (count($bounceTasks)) {
-      $bounceTask = reset($bounceTasks);
-      if (Carbon::createFromTimestamp((int)current_time('timestamp'))->addHour(42)->lessThan($bounceTask->scheduledAt)) {
-        $randomOffset = rand(-6 * 60 * 60, 6 * 60 * 60);
-        $bounceTask->scheduledAt = Carbon::createFromTimestamp((int)current_time('timestamp'))->addSecond((36 * 60 * 60) + $randomOffset);
-        $bounceTask->save();
-      }
-    }
-  }
-
   public static function getScheduledQueues() {
     return SendingTask::getScheduledQueues(self::TASK_BATCH_SIZE);
   }

lib/Cron/Workers/SendingQueue/SendingQueue.php

@@ -3,6 +3,7 @@
 namespace MailPoet\Cron\Workers\SendingQueue;
 
 use MailPoet\Cron\CronHelper;
+use MailPoet\Cron\Workers\Bounce;
 use MailPoet\Cron\Workers\SendingQueue\Tasks\Links;
 use MailPoet\Cron\Workers\SendingQueue\Tasks\Mailer as MailerTask;
 use MailPoet\Cron\Workers\SendingQueue\Tasks\Newsletter as NewsletterTask;
@@ -11,6 +12,7 @@ use MailPoet\Logging\LoggerFactory;
 use MailPoet\Mailer\MailerError;
 use MailPoet\Mailer\MailerLog;
 use MailPoet\Mailer\MetaInfo;
+use MailPoet\Models\ScheduledTask;
 use MailPoet\Models\ScheduledTask as ScheduledTaskModel;
 use MailPoet\Models\StatisticsNewsletters as StatisticsNewslettersModel;
 use MailPoet\Models\Subscriber as SubscriberModel;
@@ -19,6 +21,7 @@ use MailPoet\Segments\SubscribersFinder;
 use MailPoet\Tasks\Sending as SendingTask;
 use MailPoet\Tasks\Subscribers\BatchIterator;
 use MailPoet\WP\Functions as WPFunctions;
+use MailPoetVendor\Carbon\Carbon;
 
 use function MailPoetVendor\array_column;
 
@@ -142,6 +145,10 @@ class SendingQueue {
           'before queue chunk processing',
           ['newsletter_id' => $newsletter->id, 'task_id' => $queue->taskId, 'found_subscribers_count' => count($foundSubscribers)]
         );
+
+        // reschedule bounce task to run sooner, if needed
+        $this->reScheduleBounceTask();
+
         $queue = $this->processQueue(
           $queue,
           $_newsletter,
@@ -311,4 +318,16 @@ class SendingQueue {
   public static function getRunningQueues() {
     return SendingTask::getRunningQueues(self::TASK_BATCH_SIZE);
   }
+
+  private function reScheduleBounceTask() {
+    $bounceTasks = ScheduledTask::findFutureScheduledByType(Bounce::TASK_TYPE);
+    if (count($bounceTasks)) {
+      $bounceTask = reset($bounceTasks);
+      if (Carbon::createFromTimestamp((int)current_time('timestamp'))->addHour(42)->lessThan($bounceTask->scheduledAt)) {
+        $randomOffset = rand(-6 * 60 * 60, 6 * 60 * 60);
+        $bounceTask->scheduledAt = Carbon::createFromTimestamp((int)current_time('timestamp'))->addSecond((36 * 60 * 60) + $randomOffset);
+        $bounceTask->save();
+      }
+    }
+  }
 }

tasks/phpstan/phpstan-baseline-integration-tests.neon

@@ -267,7 +267,7 @@ parameters:
 
 		-
 			message: "#^Cannot access property \\$scheduledAt on MailPoet\\\\Models\\\\ScheduledTask\\|false\\.$#"
-			count: 2
+			count: 1
 			path: ../../tests/integration/Cron/Workers/SchedulerTest.php
 
 		-

tests/integration/Cron/Workers/SchedulerTest.php

@@ -596,22 +596,6 @@ class SchedulerTest extends \MailPoetTest {
     $scheduler->process();
   }
 
-  public function testItReSchedulesBounceTask() {
-    $task = ScheduledTask::createOrUpdate([
-      'type' => 'bounce',
-      'status' => ScheduledTask::STATUS_SCHEDULED,
-      'scheduled_at' => Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp'))->addMonths(1),
-    ]);
-    $newsletter = $this->_createNewsletter(Newsletter::TYPE_STANDARD, Newsletter::STATUS_DRAFT);
-    $queue = $this->_createQueue($newsletter->id);
-    $finder = $this->makeEmpty(SubscribersFinder::class);
-    $scheduler = new Scheduler($finder, $this->loggerFactory, $this->cronHelper);
-
-    $scheduler->processScheduledStandardNewsletter($newsletter, $queue);
-    $refetchedTask = ScheduledTask::where('id', $task->id)->findOne();
-    expect($refetchedTask->scheduledAt)->lessThan(Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp'))->addHours(42));
-  }
-
   public function testItDoesNotReSchedulesBounceTaskWhenSoon() {
     $task = ScheduledTask::createOrUpdate([
       'type' => 'bounce',

tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php

@@ -830,6 +830,55 @@ class SendingQueueTest extends \MailPoetTest {
     $wp->removeFilter('mailpoet_cron_worker_sending_queue_batch_size', $filter);
   }
 
+  public function testItReschedulesBounceTaskWhenPlannedInFarFuture() {
+    $task = ScheduledTask::createOrUpdate([
+      'type' => 'bounce',
+      'status' => ScheduledTask::STATUS_SCHEDULED,
+      'scheduled_at' => Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp'))->addMonths(1),
+    ]);
+
+    $sendingQueueWorker = new SendingQueueWorker(
+      $this->sendingErrorHandler,
+      $this->statsNotificationsWorker,
+      $this->loggerFactory,
+      Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()]),
+      $this->cronHelper,
+      $this->make(new MailerTask(), [
+        'send' => $this->mailerTaskDummyResponse,
+      ])
+    );
+    $sendingQueueWorker->process();
+
+    $refetchedTask = ScheduledTask::where('id', $task->id)->findOne();
+    assert($refetchedTask instanceof ScheduledTask); // PHPStan
+    expect($refetchedTask->scheduledAt)->lessThan(Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp'))->addHours(42));
+  }
+
+  public function testDoesNoRescheduleBounceTaskWhenPlannedInNearFuture() {
+    $inOneHour = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp'))->addHours(1);
+    $task = ScheduledTask::createOrUpdate([
+      'type' => 'bounce',
+      'status' => ScheduledTask::STATUS_SCHEDULED,
+      'scheduled_at' => $inOneHour,
+    ]);
+
+    $sendingQueueWorker = new SendingQueueWorker(
+      $this->sendingErrorHandler,
+      $this->statsNotificationsWorker,
+      $this->loggerFactory,
+      Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()]),
+      $this->cronHelper,
+      $this->make(new MailerTask(), [
+        'send' => $this->mailerTaskDummyResponse,
+      ])
+    );
+    $sendingQueueWorker->process();
+
+    $refetchedTask = ScheduledTask::where('id', $task->id)->findOne();
+    assert($refetchedTask instanceof ScheduledTask); // PHPStan
+    expect($refetchedTask->scheduledAt)->equals($inOneHour);
+  }
+
   public function _after() {
     ORM::raw_execute('TRUNCATE ' . Subscriber::$_table);
     ORM::raw_execute('TRUNCATE ' . SubscriberSegment::$_table);