Replace old Segment model with Doctrine in SubscribersFinder

[MAILPOET-3925]

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([

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;

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<int>    $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<int> $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<int> $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);
     }

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) {

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);

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);