diff --git a/mailpoet/lib/API/JSON/v1/SendingQueue.php b/mailpoet/lib/API/JSON/v1/SendingQueue.php index f8976dbc03..8442dbcf84 100644 --- a/mailpoet/lib/API/JSON/v1/SendingQueue.php +++ b/mailpoet/lib/API/JSON/v1/SendingQueue.php @@ -128,7 +128,7 @@ class SendingQueue extends APIEndpoint { $queue->status = SendingQueueModel::STATUS_SCHEDULED; $queue->scheduledAt = Scheduler::formatDatetimeString($newsletterEntity->getOptionValue('scheduledAt')); } else { - $segments = $newsletter->segments()->findMany(); + $segments = $newsletterEntity->getSegmentIds(); $subscribersCount = $this->subscribersFinder->addSubscribersToTaskFromSegments($queue->task(), $segments); if (!$subscribersCount) { return $this->errorResponse([ diff --git a/mailpoet/lib/Cron/Workers/Scheduler.php b/mailpoet/lib/Cron/Workers/Scheduler.php index 6d2203205b..f2f4e93529 100644 --- a/mailpoet/lib/Cron/Workers/Scheduler.php +++ b/mailpoet/lib/Cron/Workers/Scheduler.php @@ -6,16 +6,18 @@ use MailPoet\Cron\CronHelper; use MailPoet\Cron\CronWorkerScheduler; use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\ScheduledTaskEntity; +use MailPoet\Entities\SegmentEntity; use MailPoet\Logging\LoggerFactory; use MailPoet\Models\Newsletter; use MailPoet\Models\ScheduledTask; -use MailPoet\Models\Segment; use MailPoet\Models\Subscriber; use MailPoet\Models\SubscriberSegment; +use MailPoet\Newsletter\NewslettersRepository; use MailPoet\Newsletter\Scheduler\PostNotificationScheduler; use MailPoet\Newsletter\Scheduler\Scheduler as NewsletterScheduler; use MailPoet\Newsletter\Scheduler\WelcomeScheduler; use MailPoet\Newsletter\Sending\ScheduledTasksRepository; +use MailPoet\Segments\SegmentsRepository; use MailPoet\Segments\SubscribersFinder; use MailPoet\Tasks\Sending as SendingTask; @@ -37,18 +39,28 @@ class Scheduler { /** @var ScheduledTasksRepository */ private $scheduledTasksRepository; + /** @var NewslettersRepository */ + private $newslettersRepository; + + /** @var SegmentsRepository */ + private $segmentsRepository; + public function __construct( SubscribersFinder $subscribersFinder, LoggerFactory $loggerFactory, CronHelper $cronHelper, CronWorkerScheduler $cronWorkerScheduler, - ScheduledTasksRepository $scheduledTasksRepository + ScheduledTasksRepository $scheduledTasksRepository, + NewslettersRepository $newslettersRepository, + SegmentsRepository $segmentsRepository ) { $this->cronHelper = $cronHelper; $this->subscribersFinder = $subscribersFinder; $this->loggerFactory = $loggerFactory; $this->cronWorkerScheduler = $cronWorkerScheduler; $this->scheduledTasksRepository = $scheduledTasksRepository; + $this->newslettersRepository = $newslettersRepository; + $this->segmentsRepository = $segmentsRepository; } public function process($timer = false) { @@ -109,20 +121,23 @@ class Scheduler { 'process post notification in scheduler', ['newsletter_id' => $newsletter->id, 'task_id' => $queue->taskId] ); - // ensure that segments exist - $segments = $newsletter->segments()->findMany(); - if (empty($segments)) { - $this->loggerFactory->getLogger(LoggerFactory::TOPIC_POST_NOTIFICATIONS)->addInfo( - 'post notification no segments', - ['newsletter_id' => $newsletter->id, 'task_id' => $queue->taskId] - ); - return $this->deleteQueueOrUpdateNextRunDate($queue, $newsletter); + $newsletterEntity = $this->newslettersRepository->findOneById($newsletter->id); + + if ($newsletterEntity instanceof NewsletterEntity) { + // ensure that segments exist + $segments = $newsletterEntity->getSegmentIds(); + if (empty($segments)) { + $this->loggerFactory->getLogger(LoggerFactory::TOPIC_POST_NOTIFICATIONS)->addInfo( + 'post notification no segments', + ['newsletter_id' => $newsletter->id, 'task_id' => $queue->taskId] + ); + return $this->deleteQueueOrUpdateNextRunDate($queue, $newsletter); + } + + // ensure that subscribers are in segments + $subscribersCount = $this->subscribersFinder->addSubscribersToTaskFromSegments($queue->task(), $segments); } - // ensure that subscribers are in segments - - $subscribersCount = $this->subscribersFinder->addSubscribersToTaskFromSegments($queue->task(), $segments); - if (empty($subscribersCount)) { $this->loggerFactory->getLogger(LoggerFactory::TOPIC_POST_NOTIFICATIONS)->addInfo( 'post notification no subscribers', @@ -151,11 +166,13 @@ class Scheduler { public function processScheduledAutomaticEmail($newsletter, $queue) { if ($newsletter->sendTo === 'segment') { - $segment = Segment::findOne($newsletter->segment); - $result = $this->subscribersFinder->addSubscribersToTaskFromSegments($queue->task(), [$segment]); - if (empty($result)) { - $queue->delete(); - return false; + $segment = $this->segmentsRepository->findOneById($newsletter->segment); + if ($segment instanceof SegmentEntity) { + $result = $this->subscribersFinder->addSubscribersToTaskFromSegments($queue->task(), [(int)$segment->getId()]); + if (empty($result)) { + $queue->delete(); + return false; + } } } else { $subscribers = $queue->getSubscribers(); @@ -177,8 +194,13 @@ class Scheduler { } public function processScheduledStandardNewsletter($newsletter, SendingTask $task) { - $segments = $newsletter->segments()->findMany(); - $this->subscribersFinder->addSubscribersToTaskFromSegments($task->task(), $segments); + $newsletterEntity = $this->newslettersRepository->findOneById($newsletter->id); + + if ($newsletterEntity instanceof NewsletterEntity) { + $segments = $newsletterEntity->getSegmentIds(); + $this->subscribersFinder->addSubscribersToTaskFromSegments($task->task(), $segments); + } + // update current queue $task->updateCount(); $task->status = null; diff --git a/mailpoet/lib/Segments/SubscribersFinder.php b/mailpoet/lib/Segments/SubscribersFinder.php index 4c659d012c..4fcbf8cafc 100644 --- a/mailpoet/lib/Segments/SubscribersFinder.php +++ b/mailpoet/lib/Segments/SubscribersFinder.php @@ -6,7 +6,6 @@ use MailPoet\Entities\SegmentEntity; use MailPoet\InvalidStateException; use MailPoet\Models\ScheduledTask; use MailPoet\Models\ScheduledTaskSubscriber; -use MailPoet\Models\Segment; use MailPoet\Models\Subscriber; use MailPoetVendor\Idiorm\ORM; @@ -46,35 +45,43 @@ class SubscribersFinder { } } - private function isStaticSegment(Segment $segment) { - return in_array($segment->type, [Segment::TYPE_DEFAULT, Segment::TYPE_WP_USERS, Segment::TYPE_WC_USERS], true); - } - - public function addSubscribersToTaskFromSegments(ScheduledTask $task, array $segments) { + /** + * @param ScheduledTask $task + * @param array $segmentIds + * + * @return float|int + */ + public function addSubscribersToTaskFromSegments(ScheduledTask $task, array $segmentIds) { // Prepare subscribers on the DB side for performance reasons - $staticSegments = []; - $dynamicSegments = []; - foreach ($segments as $segment) { - if ($this->isStaticSegment($segment)) { - $staticSegments[] = $segment; - } elseif ($segment->type === SegmentEntity::TYPE_DYNAMIC) { - $dynamicSegments[] = $segment; + $staticSegmentIds = []; + $dynamicSegmentIds = []; + foreach ($segmentIds as $segment) { + $segment = $this->segmentsRepository->findOneById($segment); + if ($segment instanceof SegmentEntity) { + if ($segment->isStatic()) { + $staticSegmentIds[] = (int)$segment->getId(); + } elseif ($segment->getType() === SegmentEntity::TYPE_DYNAMIC) { + $dynamicSegmentIds[] = (int)$segment->getId(); + } } } $count = 0; - if (!empty($staticSegments)) { - $count += $this->addSubscribersToTaskFromStaticSegments($task, $staticSegments); + if (!empty($staticSegmentIds)) { + $count += $this->addSubscribersToTaskFromStaticSegments($task, $staticSegmentIds); } - if (!empty($dynamicSegments)) { - $count += $this->addSubscribersToTaskFromDynamicSegments($task, $dynamicSegments); + if (!empty($dynamicSegmentIds)) { + $count += $this->addSubscribersToTaskFromDynamicSegments($task, $dynamicSegmentIds); } return $count; } - private function addSubscribersToTaskFromStaticSegments(ScheduledTask $task, array $segments) { - $segmentIds = array_map(function($segment) { - return $segment->id; - }, $segments); + /** + * @param ScheduledTask $task + * @param array $segmentIds + * + * @return int + */ + private function addSubscribersToTaskFromStaticSegments(ScheduledTask $task, array $segmentIds) { Subscriber::rawExecute( 'INSERT IGNORE INTO ' . MP_SCHEDULED_TASK_SUBSCRIBERS_TABLE . ' (task_id, subscriber_id, processed) @@ -95,17 +102,23 @@ class SubscribersFinder { return ORM::getLastStatement()->rowCount(); } - private function addSubscribersToTaskFromDynamicSegments(ScheduledTask $task, array $segments) { + /** + * @param ScheduledTask $task + * @param array $segmentIds + * + * @return int + */ + private function addSubscribersToTaskFromDynamicSegments(ScheduledTask $task, array $segmentIds) { $count = 0; - foreach ($segments as $segment) { - $count += $this->addSubscribersToTaskFromDynamicSegment($task, $segment); + foreach ($segmentIds as $segmentId) { + $count += $this->addSubscribersToTaskFromDynamicSegment($task, (int)$segmentId); } return $count; } - private function addSubscribersToTaskFromDynamicSegment(ScheduledTask $task, Segment $segment) { + private function addSubscribersToTaskFromDynamicSegment(ScheduledTask $task, int $segmentId) { $count = 0; - $subscribers = $this->segmentSubscriberRepository->getSubscriberIdsInSegment((int)$segment->id); + $subscribers = $this->segmentSubscriberRepository->getSubscriberIdsInSegment($segmentId); if ($subscribers) { $count += $this->addSubscribersToTaskByIds($task, $subscribers); } diff --git a/mailpoet/tests/DataFactories/Segment.php b/mailpoet/tests/DataFactories/Segment.php index 02aed4b37b..26a32564c0 100644 --- a/mailpoet/tests/DataFactories/Segment.php +++ b/mailpoet/tests/DataFactories/Segment.php @@ -53,6 +53,11 @@ class Segment { return $this; } + public function withType($type) { + $this->data['type'] = $type; + return $this; + } + public function create(): SegmentEntity { $segment = $this->saveController->save($this->data); if (($this->data['deleted_at'] ?? null) instanceof \DateTimeInterface) { diff --git a/mailpoet/tests/integration/Cron/Workers/SchedulerTest.php b/mailpoet/tests/integration/Cron/Workers/SchedulerTest.php index e3daa43d3c..07df0855bb 100644 --- a/mailpoet/tests/integration/Cron/Workers/SchedulerTest.php +++ b/mailpoet/tests/integration/Cron/Workers/SchedulerTest.php @@ -18,8 +18,10 @@ use MailPoet\Models\Segment; use MailPoet\Models\SendingQueue; use MailPoet\Models\Subscriber; use MailPoet\Models\SubscriberSegment; +use MailPoet\Newsletter\NewslettersRepository; use MailPoet\Newsletter\Scheduler\WelcomeScheduler; use MailPoet\Newsletter\Sending\ScheduledTasksRepository; +use MailPoet\Segments\SegmentsRepository; use MailPoet\Segments\SubscribersFinder; use MailPoet\Settings\SettingsRepository; use MailPoet\Tasks\Sending as SendingTask; @@ -42,6 +44,12 @@ class SchedulerTest extends \MailPoetTest { /** @var ScheduledTasksRepository */ private $scheduledTasksRepository; + /** @var NewslettersRepository */ + private $newslettersRepository; + + /** @var SegmentsRepository */ + private $segmentsRepository; + public function _before() { parent::_before(); $this->loggerFactory = LoggerFactory::getInstance(); @@ -49,11 +57,13 @@ class SchedulerTest extends \MailPoetTest { $this->subscribersFinder = $this->diContainer->get(SubscribersFinder::class); $this->cronWorkerScheduler = $this->diContainer->get(CronWorkerScheduler::class); $this->scheduledTasksRepository = $this->diContainer->get(ScheduledTasksRepository::class); + $this->newslettersRepository = $this->diContainer->get(NewslettersRepository::class); + $this->segmentsRepository = $this->diContainer->get(SegmentsRepository::class); } public function testItThrowsExceptionWhenExecutionLimitIsReached() { try { - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $scheduler->process(microtime(true) - $this->cronHelper->getDaemonExecutionLimit()); self::fail('Maximum execution time limit exception was not thrown.'); } catch (\Exception $e) { @@ -83,7 +93,7 @@ class SchedulerTest extends \MailPoetTest { expect($notificationHistory)->isEmpty(); // create notification history and ensure that it exists - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $scheduler->createNotificationHistory($newsletter->id); $notificationHistory = Newsletter::where('type', Newsletter::TYPE_NOTIFICATION_HISTORY) ->where('parent_id', $newsletter->id) @@ -100,7 +110,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // queue and associated newsletter should be deleted when interval type is set to "immediately" expect(SendingQueue::findMany())->notEmpty(); @@ -118,7 +128,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // queue's next run date should change when interval type is set to anything // other than "immediately" @@ -152,7 +162,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // return false and delete queue when subscriber is not a WP user $result = $scheduler->verifyWPSubscriber($subscriber->id, $newsletter, $queue); @@ -176,7 +186,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // return false and delete queue when subscriber role is different from the one // specified for the welcome email @@ -199,7 +209,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // return true when user exists and WP role matches the one specified for the welcome email $result = $scheduler->verifyWPSubscriber($subscriber->id, $newsletter, $queue); @@ -221,7 +231,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // true when user exists and has any role $result = $scheduler->verifyWPSubscriber($subscriber->id, $newsletter, $queue); @@ -235,7 +245,7 @@ class SchedulerTest extends \MailPoetTest { $queue->setSubscribers([]); // delete queue when the list of subscribers to process is blank - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $result = $scheduler->processWelcomeNewsletter($newsletter, $queue); expect($result)->false(); expect(SendingQueue::findMany())->count(0); @@ -309,7 +319,7 @@ class SchedulerTest extends \MailPoetTest { } public function testItFailsMailpoetSubscriberVerificationWhenSubscriberDoesNotExist() { - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $newsletter = $this->_createNewsletter(); $queue = $this->_createQueue($newsletter->id); @@ -330,7 +340,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // return false $result = $scheduler->verifyMailpoetSubscriber($subscriber->id, $newsletter, $queue); @@ -362,7 +372,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // return false $result = $scheduler->verifyMailpoetSubscriber($subscriber->id, $newsletter, $queue); @@ -394,7 +404,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // return false $result = $scheduler->verifyMailpoetSubscriber($subscriber->id, $newsletter, $queue); @@ -418,7 +428,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // return true after successful verification $result = $scheduler->verifyMailpoetSubscriber($subscriber->id, $newsletter, $queue); @@ -441,7 +451,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); $queue = $this->_createQueue($newsletter->id); - $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // return true expect($scheduler->processScheduledStandardNewsletter($newsletter, $queue))->true(); @@ -470,6 +480,7 @@ class SchedulerTest extends \MailPoetTest { }), 'loggerFactory' => $this->loggerFactory, 'cronHelper' => $this->cronHelper, + 'newslettersRepository' => $this->newslettersRepository, ], $this); expect($scheduler->processPostNotificationNewsletter($newsletter, $queue))->false(); } @@ -481,7 +492,7 @@ class SchedulerTest extends \MailPoetTest { $newsletterSegment = $this->_createNewsletterSegment($newsletter->id, $segment->id); // delete or reschedule queue when there are no subscribers in segments - $scheduler = $this->construct(Scheduler::class, [$this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository], [ + $scheduler = $this->construct(Scheduler::class, [$this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository], [ 'deleteQueueOrUpdateNextRunDate' => Expected::exactly(1, function() { return false; }), @@ -505,7 +516,7 @@ class SchedulerTest extends \MailPoetTest { $newsletter = Newsletter::filter('filterWithOptions', Newsletter::TYPE_NOTIFICATION) ->findOne($newsletter->id); assert($newsletter instanceof Newsletter); - $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); // return true expect($scheduler->processPostNotificationNewsletter($newsletter, $queue))->true(); @@ -532,7 +543,7 @@ class SchedulerTest extends \MailPoetTest { } public function testItFailsToProcessWhenScheduledQueuesNotFound() { - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); expect($scheduler->process())->false(); } @@ -540,7 +551,7 @@ class SchedulerTest extends \MailPoetTest { $queue = $this->_createQueue(1); $queue->scheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); $queue->save(); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $scheduler->process(); expect(SendingQueue::findMany())->count(0); } @@ -552,7 +563,7 @@ class SchedulerTest extends \MailPoetTest { $queue = $this->_createQueue($newsletter->id); $queue->scheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); $queue->save(); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $scheduler->process(); expect(SendingQueue::findMany())->count(0); } @@ -644,7 +655,7 @@ class SchedulerTest extends \MailPoetTest { $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, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($finder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $scheduler->processScheduledStandardNewsletter($newsletter, $queue); $refetchedTask = ScheduledTask::where('id', $task->id)->findOne(); @@ -683,7 +694,7 @@ class SchedulerTest extends \MailPoetTest { expect($task->getSubscribers())->equals([$subscriber->id]); // task should have its status set to null (i.e., sending) - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $scheduler->process(); $task = SendingTask::getByNewsletterId($newsletter->id); expect($task->status)->null(); @@ -707,7 +718,7 @@ class SchedulerTest extends \MailPoetTest { expect($task->getSubscribers())->equals([$subscriber->id]); // task should be deleted - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $scheduler->process(); $task = SendingTask::getByNewsletterId($newsletter->id); expect($task)->false(); @@ -735,7 +746,7 @@ class SchedulerTest extends \MailPoetTest { expect($task->newsletterId)->equals($newsletter->id); // task should have its status set to null (i.e., sending) - $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->subscribersFinder, $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $scheduler->process(); $task = SendingTask::getByNewsletterId($newsletter->id); expect($task->status)->null(); @@ -752,7 +763,7 @@ class SchedulerTest extends \MailPoetTest { $queue->scheduledAt = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); $queue->updatedAt = $originalUpdated; $queue->save(); - $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository); + $scheduler = new Scheduler($this->makeEmpty(SubscribersFinder::class), $this->loggerFactory, $this->cronHelper, $this->cronWorkerScheduler, $this->scheduledTasksRepository, $this->newslettersRepository, $this->segmentsRepository); $scheduler->process(); $newQueue = ScheduledTask::findOne($queue->taskId); assert($newQueue instanceof ScheduledTask); diff --git a/mailpoet/tests/integration/Segments/SubscribersFinderTest.php b/mailpoet/tests/integration/Segments/SubscribersFinderTest.php index 2104a90429..37a1009c9e 100644 --- a/mailpoet/tests/integration/Segments/SubscribersFinderTest.php +++ b/mailpoet/tests/integration/Segments/SubscribersFinderTest.php @@ -9,10 +9,10 @@ use MailPoet\Entities\ScheduledTaskSubscriberEntity; use MailPoet\Entities\SegmentEntity; use MailPoet\Entities\SubscriberEntity; use MailPoet\Entities\SubscriberSegmentEntity; -use MailPoet\Models\Segment; use MailPoet\Models\Subscriber; use MailPoet\Models\SubscriberSegment; use MailPoet\Tasks\Sending as SendingTask; +use MailPoet\Test\DataFactories\Segment as SegmentFactory; use PHPUnit\Framework\MockObject\MockObject; class SubscribersFinderTest extends \MailPoetTest { @@ -32,9 +32,10 @@ class SubscribersFinderTest extends \MailPoetTest { public function _before() { parent::_before(); - $this->segment1 = Segment::createOrUpdate(['name' => 'Segment 1', 'type' => 'default']); - $this->segment2 = Segment::createOrUpdate(['name' => 'Segment 2', 'type' => 'default']); - $this->segment3 = Segment::createOrUpdate(['name' => 'Segment 3', 'type' => 'not default']); + $segmentFactory = new SegmentFactory(); + $this->segment1 = $segmentFactory->withName('Segment 1')->withType(SegmentEntity::TYPE_DEFAULT)->create(); + $this->segment2 = $segmentFactory->withName('Segment 2')->withType(SegmentEntity::TYPE_DEFAULT)->create(); + $this->segment3 = $segmentFactory->withName('Segment 3')->withType(SegmentEntity::TYPE_DYNAMIC)->create(); $this->subscriber1 = Subscriber::createOrUpdate([ 'email' => 'john@mailpoet.com', 'first_name' => 'John', @@ -47,7 +48,7 @@ class SubscribersFinderTest extends \MailPoetTest { 'last_name' => 'Doe', 'status' => Subscriber::STATUS_SUBSCRIBED, 'segments' => [ - $this->segment1->id, + $this->segment1->getId(), ], ]); $this->subscriber3 = Subscriber::createOrUpdate([ @@ -56,7 +57,7 @@ class SubscribersFinderTest extends \MailPoetTest { 'last_name' => 'Doe', 'status' => Subscriber::STATUS_SUBSCRIBED, 'segments' => [ - $this->segment3->id, + $this->segment3->getId(), ], ]); SubscriberSegment::resubscribeToAllSegments($this->subscriber2); @@ -68,7 +69,7 @@ class SubscribersFinderTest extends \MailPoetTest { public function testFindSubscribersInSegmentInSegmentDefaultSegment() { $deletedSegmentId = 1000; // non-existent segment - $subscribers = $this->subscribersFinder->findSubscribersInSegments([$this->subscriber2->id], [$this->segment1->id, $deletedSegmentId]); + $subscribers = $this->subscribersFinder->findSubscribersInSegments([$this->subscriber2->id], [$this->segment1->getId(), $deletedSegmentId]); expect($subscribers)->count(1); expect($subscribers[$this->subscriber2->id])->equals($this->subscriber2->id); } @@ -82,7 +83,7 @@ class SubscribersFinderTest extends \MailPoetTest { ->will($this->returnValue([$this->subscriber3->id])); $finder = new SubscribersFinder($mock, $this->segmentsRepository); - $subscribers = $finder->findSubscribersInSegments([$this->subscriber3->id], [$this->segment3->id]); + $subscribers = $finder->findSubscribersInSegments([$this->subscriber3->id], [$this->segment3->getId()]); expect($subscribers)->count(1); expect($subscribers)->contains($this->subscriber3->id); } @@ -96,7 +97,7 @@ class SubscribersFinderTest extends \MailPoetTest { ->will($this->returnValue([$this->subscriber3->id])); $finder = new SubscribersFinder($mock, $this->segmentsRepository); - $subscribers = $finder->findSubscribersInSegments([$this->subscriber3->id], [$this->segment3->id, $this->segment3->id]); + $subscribers = $finder->findSubscribersInSegments([$this->subscriber3->id], [$this->segment3->getId(), $this->segment3->getId()]); expect($subscribers)->count(1); } @@ -104,8 +105,8 @@ class SubscribersFinderTest extends \MailPoetTest { $subscribersCount = $this->subscribersFinder->addSubscribersToTaskFromSegments( $this->sending->task(), [ - $this->getDummySegment($this->segment1->id, Segment::TYPE_DEFAULT), - $this->getDummySegment($this->segment2->id, Segment::TYPE_DEFAULT), + $this->segment1->getId(), + $this->segment2->getId(), ] ); expect($subscribersCount)->equals(1); @@ -113,10 +114,11 @@ class SubscribersFinderTest extends \MailPoetTest { } public function testItDoesNotAddSubscribersToTaskFromNoSegment() { + $this->segment3->setType('Invalid type'); $subscribersCount = $this->subscribersFinder->addSubscribersToTaskFromSegments( $this->sending->task(), [ - $this->getDummySegment($this->segment1->id, 'UNKNOWN SEGMENT'), + $this->segment3->getId(), ] ); expect($subscribersCount)->equals(0); @@ -129,12 +131,13 @@ class SubscribersFinderTest extends \MailPoetTest { ->expects($this->once()) ->method('getSubscriberIdsInSegment') ->will($this->returnValue([$this->subscriber1->id])); + $this->segment2->setType(SegmentEntity::TYPE_DYNAMIC); $finder = new SubscribersFinder($mock, $this->segmentsRepository); $subscribersCount = $finder->addSubscribersToTaskFromSegments( $this->sending->task(), [ - $this->getDummySegment($this->segment2->id, SegmentEntity::TYPE_DYNAMIC), + $this->segment2->getId(), ] ); expect($subscribersCount)->equals(1); @@ -148,14 +151,15 @@ class SubscribersFinderTest extends \MailPoetTest { ->expects($this->once()) ->method('getSubscriberIdsInSegment') ->will($this->returnValue([$this->subscriber2->id])); + $this->segment3->setType(SegmentEntity::TYPE_DYNAMIC); $finder = new SubscribersFinder($mock, $this->segmentsRepository); $subscribersCount = $finder->addSubscribersToTaskFromSegments( $this->sending->task(), [ - $this->getDummySegment($this->segment1->id, Segment::TYPE_DEFAULT), - $this->getDummySegment($this->segment2->id, Segment::TYPE_DEFAULT), - $this->getDummySegment($this->segment3->id, SegmentEntity::TYPE_DYNAMIC), + $this->segment1->getId(), + $this->segment2->getId(), + $this->segment3->getId(), ] ); @@ -163,13 +167,6 @@ class SubscribersFinderTest extends \MailPoetTest { expect($this->sending->getSubscribers())->equals([$this->subscriber2->id]); } - private function getDummySegment($id, $type) { - $segment = Segment::create(); - $segment->id = $id; - $segment->type = $type; - return $segment; - } - public function _after() { parent::_after(); $this->truncateEntity(ScheduledTaskEntity::class);