Move ScheduledTask::findFutureScheduledByType() to ScheduledTasksRepository

Now that all other methods that use ScheduledTask::findByTypeAndStatus()
were moved to ScheduledTasksRepository, we can move
findFutureScheduledByType() as well and delete
ScheduledTask::findByTypeAndStatus().

[MAILPOET-3845]

lib/Cron/Workers/SendingQueue/SendingQueue.php

@@ -19,6 +19,7 @@ use MailPoet\Models\ScheduledTask as ScheduledTaskModel;
 use MailPoet\Models\StatisticsNewsletters as StatisticsNewslettersModel;
 use MailPoet\Models\Subscriber as SubscriberModel;
 use MailPoet\Newsletter\NewslettersRepository;
+use MailPoet\Newsletter\Sending\ScheduledTasksRepository;
 use MailPoet\Segments\SegmentsRepository;
 use MailPoet\Segments\SubscribersFinder;
 use MailPoet\Tasks\Sending as SendingTask;
@@ -65,6 +66,9 @@ class SendingQueue {
   /** @var Links */
   private $links;
 
+  /** @var ScheduledTasksRepository */
+  private $scheduledTasksRepository;
+
   public function __construct(
     SendingErrorHandler $errorHandler,
     SendingThrottlingHandler $throttlingHandler,
@@ -76,6 +80,7 @@ class SendingQueue {
     SegmentsRepository $segmentsRepository,
     WPFunctions $wp,
     Links $links,
+    ScheduledTasksRepository $scheduledTasksRepository,
     $mailerTask = false,
     $newsletterTask = false
   ) {
@@ -92,6 +97,7 @@ class SendingQueue {
     $this->newslettersRepository = $newslettersRepository;
     $this->cronHelper = $cronHelper;
     $this->links = $links;
+    $this->scheduledTasksRepository = $scheduledTasksRepository;
   }
 
   public function process($timer = false) {
@@ -413,13 +419,14 @@ class SendingQueue {
   }
 
   private function reScheduleBounceTask() {
-    $bounceTasks = ScheduledTask::findFutureScheduledByType(Bounce::TASK_TYPE);
+    $bounceTasks = $this->scheduledTasksRepository->findFutureScheduledByType(Bounce::TASK_TYPE);
     if (count($bounceTasks)) {
       $bounceTask = reset($bounceTasks);
-      if (Carbon::createFromTimestamp((int)current_time('timestamp'))->addHours(42)->lessThan($bounceTask->scheduledAt)) {
+      if (Carbon::createFromTimestamp((int)current_time('timestamp'))->addHours(42)->lessThan($bounceTask->getScheduledAt())) {
         $randomOffset = rand(-6 * 60 * 60, 6 * 60 * 60);
-        $bounceTask->scheduledAt = Carbon::createFromTimestamp((int)current_time('timestamp'))->addSeconds((36 * 60 * 60) + $randomOffset);
-        $bounceTask->save();
+        $bounceTask->setScheduledAt(Carbon::createFromTimestamp((int)current_time('timestamp'))->addSeconds((36 * 60 * 60) + $randomOffset));
+        $this->scheduledTasksRepository->persist($bounceTask);
+        $this->scheduledTasksRepository->flush();
       }
     }
   }

lib/Models/ScheduledTask.php

@@ -5,7 +5,6 @@ namespace MailPoet\Models;
 use MailPoet\Entities\ScheduledTaskEntity;
 use MailPoet\Util\Helpers;
 use MailPoet\WP\Functions as WPFunctions;
-use MailPoetVendor\Carbon\Carbon;
 use MailPoetVendor\Idiorm\ORM;
 
 /**
@@ -153,32 +152,4 @@ class ScheduledTask extends Model {
       ->whereNull('tasks.deleted_at')
       ->findOne() ?: null;
   }
-
-  public static function findFutureScheduledByType($type, $limit = null) {
-    return self::findByTypeAndStatus($type, ScheduledTask::STATUS_SCHEDULED, $limit, true);
-  }
-
-  private static function findByTypeAndStatus($type, $status, $limit = null, $future = false) {
-    $query = ScheduledTask::where('type', $type)
-      ->whereNull('deleted_at');
-
-    $now = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp'));
-    if ($future) {
-      $query->whereGt('scheduled_at', $now);
-    } else {
-      $query->whereLte('scheduled_at', $now);
-    }
-
-    if ($status === null) {
-      $query->whereNull('status');
-    } else {
-      $query->where('status', $status);
-    }
-
-    if ($limit !== null) {
-      $query->limit($limit);
-    }
-
-    return $query->findMany();
-  }
 }

lib/Newsletter/Sending/ScheduledTasksRepository.php

@@ -117,6 +117,10 @@ class ScheduledTasksRepository extends Repository {
     return $this->findByTypeAndStatus($type, ScheduledTaskEntity::STATUS_COMPLETED, $limit);
   }
 
+  public function findFutureScheduledByType($type, $limit = null) {
+    return $this->findByTypeAndStatus($type, ScheduledTaskEntity::STATUS_SCHEDULED, $limit, true);
+  }
+
   protected function findByTypeAndStatus($type, $status, $limit = null, $future = false) {
     $queryBuilder = $this->doctrineRepository->createQueryBuilder('st')
       ->select('st')

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

@@ -39,6 +39,7 @@ use MailPoet\Models\Subscriber;
 use MailPoet\Models\SubscriberSegment;
 use MailPoet\Newsletter\Links\Links;
 use MailPoet\Newsletter\NewslettersRepository;
+use MailPoet\Newsletter\Sending\ScheduledTasksRepository;
 use MailPoet\Router\Endpoints\Track;
 use MailPoet\Router\Router;
 use MailPoet\Segments\SegmentsRepository;
@@ -84,6 +85,8 @@ class SendingQueueTest extends \MailPoetTest {
   private $wp;
   /** @var TasksLinks */
   private $tasksLinks;
+  /** @var ScheduledTasksRepository */
+  private $scheduledTasksRepository;
 
   public function _before() {
     parent::_before();
@@ -138,6 +141,7 @@ class SendingQueueTest extends \MailPoetTest {
     $this->newslettersRepository = $this->diContainer->get(NewslettersRepository::class);
     $this->segmentsRepository = $this->diContainer->get(SegmentsRepository::class);
     $this->tasksLinks = $this->diContainer->get(TasksLinks::class);
+    $this->scheduledTasksRepository = $this->diContainer->get(ScheduledTasksRepository::class);
     $this->sendingQueueWorker = $this->getSendingQueueWorker(Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()]));
   }
 
@@ -190,7 +194,8 @@ class SendingQueueTest extends \MailPoetTest {
       $this->subscribersFinder,
       $this->segmentsRepository,
       $this->wp,
-      $this->tasksLinks
+      $this->tasksLinks,
+      $this->scheduledTasksRepository
     );
     try {
       $sendingQueueWorker->process();
@@ -217,6 +222,7 @@ class SendingQueueTest extends \MailPoetTest {
       $this->segmentsRepository,
       $this->wp,
       $this->tasksLinks,
+      $this->scheduledTasksRepository,
       Stub::make(
         new MailerTask(),
         [
@@ -260,6 +266,7 @@ class SendingQueueTest extends \MailPoetTest {
       $this->segmentsRepository,
       $this->wp,
       $this->tasksLinks,
+      $this->scheduledTasksRepository,
       Stub::make(
         new MailerTask(),
         [
@@ -301,7 +308,8 @@ class SendingQueueTest extends \MailPoetTest {
       $this->subscribersFinder,
       $this->segmentsRepository,
       $this->wp,
-      $this->tasksLinks
+      $this->tasksLinks,
+      $this->scheduledTasksRepository
     );
     $sendingQueueWorker->process();
   }
@@ -820,6 +828,7 @@ class SendingQueueTest extends \MailPoetTest {
       $this->segmentsRepository,
       $this->wp,
       $this->tasksLinks,
+      $this->scheduledTasksRepository,
       Stub::make(
         new MailerTask(),
         [
@@ -1036,6 +1045,7 @@ class SendingQueueTest extends \MailPoetTest {
       $this->segmentsRepository,
       $this->wp,
       $this->tasksLinks,
+      $this->scheduledTasksRepository,
       $mailerMock
     );
   }

tests/integration/Models/ScheduledTaskTest.php

@@ -163,40 +163,6 @@ class ScheduledTaskTest extends \MailPoetTest {
     expect($task->meta)->equals($meta);
   }
 
-  public function testItCanGetFutureScheduledTasks() {
-    // scheduled (in future)
-    ScheduledTask::createOrUpdate([
-      'type' => 'test',
-      'status' => ScheduledTask::STATUS_SCHEDULED,
-      'scheduled_at' => Carbon::now()->addDay(),
-    ]);
-
-    // deleted (should not be fetched)
-    ScheduledTask::createOrUpdate([
-      'type' => 'test',
-      'status' => ScheduledTask::STATUS_SCHEDULED,
-      'scheduled_at' => Carbon::now()->addDay(),
-      'deleted_at' => Carbon::now(),
-    ]);
-
-    // scheduled in past (should not be fetched)
-    ScheduledTask::createOrUpdate([
-      'type' => 'test',
-      'status' => ScheduledTask::STATUS_SCHEDULED,
-      'scheduled_at' => Carbon::now()->subDay(),
-    ]);
-
-    // wrong status (should not be fetched)
-    ScheduledTask::createOrUpdate([
-      'type' => 'test',
-      'status' => null,
-      'scheduled_at' => Carbon::now()->addDay(),
-    ]);
-
-    $tasks = ScheduledTask::findFutureScheduledByType('test', 10);
-    expect($tasks)->count(1);
-  }
-
   public function _after() {
     ORM::raw_execute('TRUNCATE ' . ScheduledTask::$_table);
     ORM::raw_execute('TRUNCATE ' . ScheduledTaskSubscriber::$_table);

tests/integration/Newsletter/Sending/ScheduledTasksRepositoryTest.php

@@ -53,6 +53,16 @@ class ScheduledTasksRepositoryTest extends \MailPoetTest {
     $this->assertSame($expectedResult, $tasks);
   }
 
+  public function testItCanGetFutureScheduledTasks() {
+    $expectedResult[] = $this->scheduledTaskFactory->create('test', ScheduledTaskEntity::STATUS_SCHEDULED, Carbon::now()->addDay()); // scheduled (in future)
+    $this->scheduledTaskFactory->create('test', ScheduledTaskEntity::STATUS_SCHEDULED, Carbon::now()->addDay(), Carbon::now()); // deleted (should not be fetched)
+    $this->scheduledTaskFactory->create('test', ScheduledTaskEntity::STATUS_SCHEDULED, Carbon::now()->subDay()); // scheduled in past (should not be fetched)
+    $this->scheduledTaskFactory->create('test', null, Carbon::now()->addDay()); // wrong status (should not be fetched)
+
+    $tasks = $this->repository->findFutureScheduledByType('test', 10);
+    $this->assertSame($expectedResult, $tasks);
+  }
+
   public function cleanup() {
     $this->truncateEntity(ScheduledTaskEntity::class);
   }