diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php index e66390f396..62b0863ba4 100644 --- a/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/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(); } } } diff --git a/lib/Models/ScheduledTask.php b/lib/Models/ScheduledTask.php index 8d72dbf08f..5ceebcc90c 100644 --- a/lib/Models/ScheduledTask.php +++ b/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(); - } } diff --git a/lib/Newsletter/Sending/ScheduledTasksRepository.php b/lib/Newsletter/Sending/ScheduledTasksRepository.php index 06597e16ce..42ba735b97 100644 --- a/lib/Newsletter/Sending/ScheduledTasksRepository.php +++ b/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') diff --git a/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php b/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php index 50d13ccff6..2b900b3be6 100644 --- a/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php +++ b/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 ); } diff --git a/tests/integration/Models/ScheduledTaskTest.php b/tests/integration/Models/ScheduledTaskTest.php index 4b224f494d..38fe0bb916 100644 --- a/tests/integration/Models/ScheduledTaskTest.php +++ b/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); diff --git a/tests/integration/Newsletter/Sending/ScheduledTasksRepositoryTest.php b/tests/integration/Newsletter/Sending/ScheduledTasksRepositoryTest.php index 865e55595e..968965244f 100644 --- a/tests/integration/Newsletter/Sending/ScheduledTasksRepositoryTest.php +++ b/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); }