diff --git a/lib/Cron/Workers/StatsNotifications/Scheduler.php b/lib/Cron/Workers/StatsNotifications/Scheduler.php index 2608b7ec9a..a802687b07 100644 --- a/lib/Cron/Workers/StatsNotifications/Scheduler.php +++ b/lib/Cron/Workers/StatsNotifications/Scheduler.php @@ -6,7 +6,6 @@ use Carbon\Carbon; use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Entities\StatsNotificationEntity; -use MailPoet\Models\ScheduledTask; use MailPoet\Settings\SettingsController; use MailPoetVendor\Doctrine\ORM\EntityManager; @@ -29,9 +28,17 @@ class Scheduler { /** @var EntityManager */ private $entity_manager; - function __construct(SettingsController $settings, EntityManager $entity_manager) { + /** @var StatsNotificationsRepository */ + private $repository; + + function __construct( + SettingsController $settings, + EntityManager $entity_manager, + StatsNotificationsRepository $repository + ) { $this->settings = $settings; $this->entity_manager = $entity_manager; + $this->repository = $repository; } function schedule(NewsletterEntity $newsletter) { @@ -46,7 +53,7 @@ class Scheduler { $this->entity_manager->persist($task); $this->entity_manager->flush(); - $stats_notifications = new StatsNotificationEntity($newsletter->getId(), $task->getId()); + $stats_notifications = new StatsNotificationEntity($newsletter, $task); $this->entity_manager->persist($stats_notifications); $this->entity_manager->flush(); } @@ -55,10 +62,10 @@ class Scheduler { if ($this->isDisabled()) { return false; } - if ($this->isTaskScheduled($newsletter->getId())) { + if (!in_array($newsletter->getType(), $this->supported_types)) { return false; } - if (!in_array($newsletter->getType(), $this->supported_types)) { + if ($this->isTaskScheduled($newsletter->getId())) { return false; } return true; @@ -85,13 +92,8 @@ class Scheduler { } private function isTaskScheduled($newsletter_id) { -// $existing = ScheduledTask::tableAlias('tasks') -// ->join(StatsNotification::$_table, 'tasks.id = notification.task_id', 'notification') -// ->where('tasks.type', Worker::TASK_TYPE) -// ->where('notification.newsletter_id', $newsletter_id) -// ->findMany(); -// return (bool)$existing; - return false; + $existing = $this->repository->findAllForNewsletter($newsletter_id); + return count($existing) > 0; } private function getNextRunDate() { diff --git a/lib/Doctrine/EntityTraits/AutoincrementedIdTrait.php b/lib/Doctrine/EntityTraits/AutoincrementedIdTrait.php index 057429ffb0..fb7c1f60b2 100644 --- a/lib/Doctrine/EntityTraits/AutoincrementedIdTrait.php +++ b/lib/Doctrine/EntityTraits/AutoincrementedIdTrait.php @@ -17,4 +17,10 @@ trait AutoincrementedIdTrait { public function getId() { return $this->id; } + + /** @param int|null $id */ + public function setId($id) { + $this->id = $id; + } + } diff --git a/tests/integration/Cron/Workers/StatsNotifications/SchedulerTest.php b/tests/integration/Cron/Workers/StatsNotifications/SchedulerTest.php deleted file mode 100644 index a9f9b0cbd1..0000000000 --- a/tests/integration/Cron/Workers/StatsNotifications/SchedulerTest.php +++ /dev/null @@ -1,131 +0,0 @@ -settings = new SettingsController(); - $this->stats_notifications = new Scheduler($this->settings); - $this->settings->set(Worker::SETTINGS_KEY, [ - 'enabled' => true, - 'address' => 'email@example.com', - ]); - $this->settings->set('tracking.enabled', true); - } - - function testShouldSchedule() { - $newsletter_id = 5; - $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]); - $this->stats_notifications->schedule($newsletter); - $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne(); - expect($notification)->isInstanceOf(StatsNotification::class); - $task = ScheduledTask::where('id', $notification->task_id)->findOne(); - expect($task)->isInstanceOf(ScheduledTask::class); - } - - function testShouldScheduleForNotificationHistory() { - $newsletter_id = 4; - $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_NOTIFICATION_HISTORY]); - $this->stats_notifications->schedule($newsletter); - $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne(); - expect($notification)->isInstanceOf(StatsNotification::class); - $task = ScheduledTask::where('id', $notification->task_id)->findOne(); - expect($task)->isInstanceOf(ScheduledTask::class); - } - - function testShouldNotScheduleIfTrackingIsDisabled() { - $this->settings->set('tracking.enabled', false); - $newsletter_id = 13; - $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]); - $this->stats_notifications->schedule($newsletter); - $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne(); - expect($notification)->isEmpty(); - } - - function testShouldNotScheduleIfDisabled() { - $newsletter_id = 6; - $this->settings->set(Worker::SETTINGS_KEY, [ - 'enabled' => false, - 'address' => 'email@example.com', - ]); - $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]); - $this->stats_notifications->schedule($newsletter); - $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne(); - expect($notification)->isEmpty(); - } - - function testShouldNotScheduleIfSettingsMissing() { - $newsletter_id = 7; - $this->settings->set(Worker::SETTINGS_KEY, []); - $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]); - $this->stats_notifications->schedule($newsletter); - $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne(); - expect($notification)->isEmpty(); - } - - function testShouldNotScheduleIfEmailIsMissing() { - $newsletter_id = 8; - $this->settings->set(Worker::SETTINGS_KEY, [ - 'enabled' => true, - ]); - $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]); - $this->stats_notifications->schedule($newsletter); - $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne(); - expect($notification)->isEmpty(); - } - - function testShouldNotScheduleIfEmailIsEmpty() { - $newsletter_id = 9; - $this->settings->set(Worker::SETTINGS_KEY, [ - 'enabled' => true, - 'address' => ' ', - ]); - $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]); - $this->stats_notifications->schedule($newsletter); - $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne(); - expect($notification)->isEmpty(); - } - - function testShouldNotScheduleIfAlreadyScheduled() { - $newsletter_id = 10; - $existing_task = ScheduledTask::createOrUpdate([ - 'type' => Worker::TASK_TYPE, - 'status' => ScheduledTask::STATUS_SCHEDULED, - 'scheduled_at' => '2017-01-02 12:13:14', - ]); - $existing_notification = StatsNotification::createOrUpdate([ - 'newsletter_id' => $newsletter_id, - 'task_id' => $existing_task->id, - ]); - $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]); - $this->stats_notifications->schedule($newsletter); - $notifications = StatsNotification::where('newsletter_id', $newsletter_id)->findMany(); - expect($notifications)->count(1); - $tasks = ScheduledTask::where('id', $notifications[0]->task_id)->findMany(); - expect($tasks)->count(1); - expect($existing_notification->id)->equals($notifications[0]->id); - expect($existing_task->id)->equals($tasks[0]->id); - } - - function testShouldNotScheduleIfInvalidType() { - $newsletter_id = 11; - $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, Newsletter::TYPE_WELCOME]); - $this->stats_notifications->schedule($newsletter); - $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne(); - expect($notification)->isEmpty(); - } - -} diff --git a/tests/unit/Cron/Workers/StatsNotifications/SchedulerTest.php b/tests/unit/Cron/Workers/StatsNotifications/SchedulerTest.php new file mode 100644 index 0000000000..0ff5a8a859 --- /dev/null +++ b/tests/unit/Cron/Workers/StatsNotifications/SchedulerTest.php @@ -0,0 +1,261 @@ +settings = $this->createMock(SettingsController::class); + $this->entityManager = $this->createMock(EntityManager::class); + $this->entityManager->method('flush'); + $this->repository = $this->createMock(StatsNotificationsRepository::class); + $this->stats_notifications = new Scheduler( + $this->settings, + $this->entityManager, + $this->repository + ); + } + + function testShouldSchedule() { + $this->settings + ->method('get') + ->will($this->returnValueMap([ + [Worker::SETTINGS_KEY, null, ['enabled' => true, 'address' => 'email@example.com']], + ['tracking.enabled', null, true], + ])); + + $newsletter_id = 5; + $newsletter = new NewsletterEntity(); + $newsletter->setId($newsletter_id); + $newsletter->setType(NewsletterEntity::TYPE_STANDARD); + + $this->entityManager + ->expects($this->exactly(2)) + ->method('persist'); + $this->entityManager + ->expects($this->at(0)) + ->method('persist') + ->with($this->isInstanceOf(ScheduledTaskEntity::class)); + $this->entityManager + ->expects($this->at(1)) + ->method('flush'); + $this->entityManager + ->expects($this->at(2)) + ->method('persist') + ->with($this->isInstanceOf(StatsNotificationEntity::class)); + $this->entityManager + ->expects($this->at(3)) + ->method('flush'); + + $this->repository + ->expects($this->once()) + ->method('findAllForNewsletter') + ->with($newsletter_id) + ->willReturn([]); + + $this->stats_notifications->schedule($newsletter); + } + + function testShouldScheduleForNotificationHistory() { + $this->settings + ->method('get') + ->will($this->returnValueMap([ + [Worker::SETTINGS_KEY, null, ['enabled' => true, 'address' => 'email@example.com']], + ['tracking.enabled', null, true], + ])); + + $newsletter_id = 4; + $newsletter = new NewsletterEntity(); + $newsletter->setId($newsletter_id); + $newsletter->setType(NewsletterEntity::TYPE_NOTIFICATION_HISTORY); + + $this->entityManager + ->expects($this->exactly(2)) + ->method('persist'); + $this->entityManager + ->expects($this->at(0)) + ->method('persist') + ->with($this->isInstanceOf(ScheduledTaskEntity::class)); + $this->entityManager + ->expects($this->at(1)) + ->method('flush'); + $this->entityManager + ->expects($this->at(2)) + ->method('persist') + ->with($this->isInstanceOf(StatsNotificationEntity::class)); + $this->entityManager + ->expects($this->at(3)) + ->method('flush'); + + $this->repository + ->expects($this->once()) + ->method('findAllForNewsletter') + ->with($newsletter_id) + ->willReturn([]); + + $this->stats_notifications->schedule($newsletter); + } + + function testShouldNotScheduleIfTrackingIsDisabled() { + $this->settings + ->method('get') + ->will($this->returnValueMap([ + [Worker::SETTINGS_KEY, null, ['enabled' => true, 'address' => 'email@example.com']], + ['tracking.enabled', null, false], + ])); + + $this->entityManager + ->expects($this->never()) + ->method('persist'); + + $newsletter_id = 13; + $newsletter = new NewsletterEntity(); + $newsletter->setId($newsletter_id); + $newsletter->setType(NewsletterEntity::TYPE_STANDARD); + + $this->stats_notifications->schedule($newsletter); + } + + function testShouldNotScheduleIfDisabled() { + $this->settings + ->method('get') + ->will($this->returnValueMap([ + [Worker::SETTINGS_KEY, null, ['enabled' => false, 'address' => 'email@example.com']], + ['tracking.enabled', null, true], + ])); + + $this->entityManager + ->expects($this->never()) + ->method('persist'); + + $newsletter_id = 6; + $newsletter = new NewsletterEntity(); + $newsletter->setId($newsletter_id); + $newsletter->setType(NewsletterEntity::TYPE_STANDARD); + + $this->stats_notifications->schedule($newsletter); + } + + function testShouldNotScheduleIfSettingsMissing() { + $this->settings + ->method('get') + ->will($this->returnValueMap([ + [Worker::SETTINGS_KEY, null, []], + ['tracking.enabled', null, true], + ])); + + $this->entityManager + ->expects($this->never()) + ->method('persist'); + + $newsletter_id = 7; + + $newsletter = new NewsletterEntity(); + $newsletter->setId($newsletter_id); + $newsletter->setType(NewsletterEntity::TYPE_STANDARD); + + $this->stats_notifications->schedule($newsletter); + } + + function testShouldNotScheduleIfEmailIsMissing() { + $this->settings + ->method('get') + ->will($this->returnValueMap([ + [Worker::SETTINGS_KEY, null, ['enabled' => true]], + ['tracking.enabled', null, true], + ])); + + $this->entityManager + ->expects($this->never()) + ->method('persist'); + + $newsletter_id = 8; + $newsletter = new NewsletterEntity(); + $newsletter->setId($newsletter_id); + $newsletter->setType(NewsletterEntity::TYPE_STANDARD); + + $this->stats_notifications->schedule($newsletter); + } + + function testShouldNotScheduleIfEmailIsEmpty() { + $this->settings + ->method('get') + ->will($this->returnValueMap([ + [Worker::SETTINGS_KEY, null, ['enabled' => true, 'address' => '']], + ['tracking.enabled', null, true], + ])); + + $this->entityManager + ->expects($this->never()) + ->method('persist'); + + $newsletter_id = 9; + $newsletter = new NewsletterEntity(); + $newsletter->setId($newsletter_id); + $newsletter->setType(NewsletterEntity::TYPE_STANDARD); + $this->stats_notifications->schedule($newsletter); + } + + function testShouldNotScheduleIfAlreadyScheduled() { + $this->settings + ->method('get') + ->will($this->returnValueMap([ + [Worker::SETTINGS_KEY, null, ['enabled' => true, 'address' => 'email@example.com']], + ['tracking.enabled', null, true], + ])); + + $newsletter_id = 10; + $newsletter = new NewsletterEntity(); + $newsletter->setId($newsletter_id); + $newsletter->setType(NewsletterEntity::TYPE_STANDARD); + + $this->repository + ->expects($this->once()) + ->method('findAllForNewsletter') + ->with($newsletter_id) + ->willReturn([new ScheduledTaskEntity()]); + $this->entityManager + ->expects($this->never()) + ->method('persist'); + + $this->stats_notifications->schedule($newsletter); + } + + function testShouldNotScheduleIfInvalidType() { + $this->settings + ->method('get') + ->will($this->returnValueMap([ + [Worker::SETTINGS_KEY, null, ['enabled' => true, 'address' => 'email@example.com']], + ['tracking.enabled', null, true], + ])); + $this->entityManager + ->expects($this->never()) + ->method('persist'); + + $newsletter_id = 11; + $newsletter = new NewsletterEntity(); + $newsletter->setId($newsletter_id); + $newsletter->setType(NewsletterEntity::TYPE_WELCOME); + $this->stats_notifications->schedule($newsletter); + } + +}