diff --git a/lib/Cron/Workers/Bounce.php b/lib/Cron/Workers/Bounce.php index 0afab39c74..4d18bfaa93 100644 --- a/lib/Cron/Workers/Bounce.php +++ b/lib/Cron/Workers/Bounce.php @@ -9,7 +9,6 @@ use MailPoet\Entities\SubscriberEntity; use MailPoet\Mailer\Mailer; use MailPoet\Models\ScheduledTask; use MailPoet\Models\ScheduledTaskSubscriber; -use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Newsletter\Sending\SendingQueuesRepository; use MailPoet\Services\Bridge; use MailPoet\Services\Bridge\API; @@ -40,9 +39,6 @@ class Bounce extends SimpleWorker { /** @var SubscribersRepository */ private $subscribersRepository; - /** @var ScheduledTasksRepository */ - private $scheduledTasksRepository; - /** @var SendingQueuesRepository */ private $sendingQueuesRepository; @@ -52,7 +48,6 @@ class Bounce extends SimpleWorker { public function __construct( SettingsController $settings, SubscribersRepository $subscribersRepository, - ScheduledTasksRepository $scheduledTasksRepository, SendingQueuesRepository $sendingQueuesRepository, StatisticsBouncesRepository $statisticsBouncesRepository, Bridge $bridge @@ -61,7 +56,6 @@ class Bounce extends SimpleWorker { $this->bridge = $bridge; parent::__construct(); $this->subscribersRepository = $subscribersRepository; - $this->scheduledTasksRepository = $scheduledTasksRepository; $this->sendingQueuesRepository = $sendingQueuesRepository; $this->statisticsBouncesRepository = $statisticsBouncesRepository; } diff --git a/lib/Cron/Workers/SimpleWorker.php b/lib/Cron/Workers/SimpleWorker.php index 8ab948e747..069f930d41 100644 --- a/lib/Cron/Workers/SimpleWorker.php +++ b/lib/Cron/Workers/SimpleWorker.php @@ -8,6 +8,7 @@ use MailPoet\Cron\CronWorkerRunner; use MailPoet\Cron\CronWorkerScheduler; use MailPoet\DI\ContainerWrapper; use MailPoet\Models\ScheduledTask; +use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\WP\Functions as WPFunctions; use MailPoetVendor\Carbon\Carbon; @@ -27,6 +28,9 @@ abstract class SimpleWorker implements CronWorkerInterface { /** @var WPFunctions */ protected $wp; + /** @var ScheduledTasksRepository */ + protected $scheduledTasksRepository; + public function __construct( WPFunctions $wp = null ) { @@ -38,6 +42,7 @@ abstract class SimpleWorker implements CronWorkerInterface { $this->wp = $wp; $this->cronHelper = ContainerWrapper::getInstance()->get(CronHelper::class); $this->cronWorkerScheduler = ContainerWrapper::getInstance()->get(CronWorkerScheduler::class); + $this->scheduledTasksRepository = ContainerWrapper::getInstance()->get(ScheduledTasksRepository::class); } public function getTaskType() { @@ -88,6 +93,6 @@ abstract class SimpleWorker implements CronWorkerInterface { } protected function getCompletedTasks() { - return ScheduledTask::findCompletedByType(static::TASK_TYPE, CronWorkerRunner::TASK_BATCH_SIZE); + return $this->scheduledTasksRepository->findCompletedByType(static::TASK_TYPE, CronWorkerRunner::TASK_BATCH_SIZE); } } diff --git a/lib/Cron/Workers/SubscribersCountCacheRecalculation.php b/lib/Cron/Workers/SubscribersCountCacheRecalculation.php index d3e95ecc30..926f17be83 100644 --- a/lib/Cron/Workers/SubscribersCountCacheRecalculation.php +++ b/lib/Cron/Workers/SubscribersCountCacheRecalculation.php @@ -5,7 +5,6 @@ namespace MailPoet\Cron\Workers; use MailPoet\Cache\TransientCache; use MailPoet\Entities\SegmentEntity; use MailPoet\Models\ScheduledTask; -use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Segments\SegmentsRepository; use MailPoet\Subscribers\SubscribersCountsController; use MailPoet\WP\Functions as WPFunctions; @@ -26,21 +25,16 @@ class SubscribersCountCacheRecalculation extends SimpleWorker { /** @var SubscribersCountsController */ private $subscribersCountsController; - /** @var ScheduledTasksRepository */ - private $scheduledTasksRepository; - public function __construct( TransientCache $transientCache, SegmentsRepository $segmentsRepository, SubscribersCountsController $subscribersCountsController, - ScheduledTasksRepository $scheduledTasksRepository, WPFunctions $wp ) { parent::__construct($wp); $this->transientCache = $transientCache; $this->segmentsRepository = $segmentsRepository; $this->subscribersCountsController = $subscribersCountsController; - $this->scheduledTasksRepository = $scheduledTasksRepository; } public function processTaskStrategy(ScheduledTask $task, $timer) { diff --git a/lib/Models/ScheduledTask.php b/lib/Models/ScheduledTask.php index aec9d454f1..fb84f7c316 100644 --- a/lib/Models/ScheduledTask.php +++ b/lib/Models/ScheduledTask.php @@ -168,10 +168,6 @@ class ScheduledTask extends Model { return self::findByTypeAndStatus($type, ScheduledTask::STATUS_SCHEDULED, $limit, true); } - public static function findCompletedByType($type, $limit = null) { - return self::findByTypeAndStatus($type, ScheduledTask::STATUS_COMPLETED, $limit); - } - private static function findByTypeAndStatus($type, $status, $limit = null, $future = false) { $query = ScheduledTask::where('type', $type) ->whereNull('deleted_at'); diff --git a/lib/Newsletter/Sending/ScheduledTasksRepository.php b/lib/Newsletter/Sending/ScheduledTasksRepository.php index a6fe9f90b7..b4da53288d 100644 --- a/lib/Newsletter/Sending/ScheduledTasksRepository.php +++ b/lib/Newsletter/Sending/ScheduledTasksRepository.php @@ -97,6 +97,10 @@ class ScheduledTasksRepository extends Repository { return $this->findByTypeAndStatus($type, null, $limit); } + public function findCompletedByType($type, $limit = null) { + return $this->findByTypeAndStatus($type, ScheduledTaskEntity::STATUS_COMPLETED, $limit); + } + protected function findByTypeAndStatus($type, $status, $limit = null, $future = false) { $queryBuilder = $this->doctrineRepository->createQueryBuilder('st') ->select('st') diff --git a/tests/integration/Cron/Workers/BounceTest.php b/tests/integration/Cron/Workers/BounceTest.php index 43d22e428a..491fdf282f 100644 --- a/tests/integration/Cron/Workers/BounceTest.php +++ b/tests/integration/Cron/Workers/BounceTest.php @@ -13,7 +13,6 @@ use MailPoet\Entities\SubscriberEntity; use MailPoet\Mailer\Mailer; use MailPoet\Models\ScheduledTask; use MailPoet\Models\ScheduledTaskSubscriber; -use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Newsletter\Sending\SendingQueuesRepository; use MailPoet\Services\Bridge; use MailPoet\Services\Bridge\API; @@ -57,7 +56,6 @@ class BounceTest extends \MailPoetTest { $this->worker = new Bounce( $this->diContainer->get(SettingsController::class), $this->subscribersRepository, - $this->diContainer->get(ScheduledTasksRepository::class), $this->diContainer->get(SendingQueuesRepository::class), $this->diContainer->get(StatisticsBouncesRepository::class), $this->diContainer->get(Bridge::class) @@ -77,7 +75,6 @@ class BounceTest extends \MailPoetTest { $worker = new Bounce( $this->diContainer->get(SettingsController::class), $this->subscribersRepository, - $this->diContainer->get(ScheduledTasksRepository::class), $this->diContainer->get(SendingQueuesRepository::class), $this->diContainer->get(StatisticsBouncesRepository::class), $this->diContainer->get(Bridge::class) diff --git a/tests/integration/Models/ScheduledTaskTest.php b/tests/integration/Models/ScheduledTaskTest.php index 02ff350bab..302b77674c 100644 --- a/tests/integration/Models/ScheduledTaskTest.php +++ b/tests/integration/Models/ScheduledTaskTest.php @@ -181,40 +181,6 @@ class ScheduledTaskTest extends \MailPoetTest { expect($timeout)->equals(ScheduledTask::MAX_RESCHEDULE_TIMEOUT); } - public function testItCanGetCompletedTasks() { - // completed (scheduled in past) - ScheduledTask::createOrUpdate([ - 'type' => 'test', - 'status' => ScheduledTask::STATUS_COMPLETED, - 'scheduled_at' => Carbon::now()->subDay(), - ]); - - // deleted (should not be fetched) - ScheduledTask::createOrUpdate([ - 'type' => 'test', - 'status' => ScheduledTask::STATUS_COMPLETED, - 'scheduled_at' => Carbon::now()->subDay(), - 'deleted_at' => Carbon::now(), - ]); - - // scheduled in future (should not be fetched) - ScheduledTask::createOrUpdate([ - 'type' => 'test', - 'status' => ScheduledTask::STATUS_COMPLETED, - 'scheduled_at' => Carbon::now()->addDay(), - ]); - - // wrong status (should not be fetched) - ScheduledTask::createOrUpdate([ - 'type' => 'test', - 'status' => ScheduledTask::STATUS_SCHEDULED, - 'scheduled_at' => Carbon::now()->subDay(), - ]); - - $tasks = ScheduledTask::findCompletedByType('test', 10); - expect($tasks)->count(1); - } - public function testItCanGetFutureScheduledTasks() { // scheduled (in future) ScheduledTask::createOrUpdate([ diff --git a/tests/integration/Newsletter/Sending/ScheduledTasksRepositoryTest.php b/tests/integration/Newsletter/Sending/ScheduledTasksRepositoryTest.php index 044475b76b..70ba47e613 100644 --- a/tests/integration/Newsletter/Sending/ScheduledTasksRepositoryTest.php +++ b/tests/integration/Newsletter/Sending/ScheduledTasksRepositoryTest.php @@ -38,6 +38,16 @@ class ScheduledTasksRepositoryTest extends \MailPoetTest { $this->assertSame($expectedResult, $tasks); } + public function testItCanGetCompletedTasks() { + $expectedResult[] = $this->createScheduledTask('test', ScheduledTaskEntity::STATUS_COMPLETED, Carbon::now()->subDay()); // completed (scheduled in past) + $this->createScheduledTask('test', ScheduledTaskEntity::STATUS_COMPLETED, Carbon::now()->subDay(), Carbon::now()); // deleted (should not be fetched) + $this->createScheduledTask('test', ScheduledTaskEntity::STATUS_COMPLETED, Carbon::now()->addDay()); // scheduled in future (should not be fetched) + $this->createScheduledTask('test', ScheduledTaskEntity::STATUS_SCHEDULED, Carbon::now()->subDay()); // wrong status (should not be fetched) + + $tasks = $this->repository->findCompletedByType('test', 10); + $this->assertSame($expectedResult, $tasks); + } + public function cleanup() { $this->truncateEntity(ScheduledTaskEntity::class); }