Only schedule notification for standard newsletters

[MAILPOET-1571]

lib/Config/Migrator.php

@@ -132,10 +132,10 @@ class Migrator {
       'id int(11) unsigned NOT NULL AUTO_INCREMENT,',
       'newsletter_id int(11) unsigned NOT NULL,',
       'task_id int(11) unsigned NOT NULL,',
-      'created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,',
+      'created_at TIMESTAMP NULL,',
       'updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,',
       'PRIMARY KEY (id),',
-      'KEY newsletter_id (newsletter_id),',
+      'UNIQUE KEY newsletter_id_task_id (newsletter_id, task_id),',
       'KEY task_id (task_id)',
     );
     return $this->sqlify(__FUNCTION__, $attributes);

lib/Cron/Workers/StatsNotifications.php

@@ -9,10 +9,8 @@ use MailPoet\Models\ScheduledTask;
 use MailPoet\Models\Setting;
 use MailPoet\Models\StatsNotification;
 
-
 /**
  * TODO:
- * - only schedule for post notification and standard, need to do check here
  * - add processing of this task to Daemon
  * - check JIRA what to do next and how to send the newsletter
  * - see \MailPoet\Subscribers\NewSubscriberNotificationMailer how to send an email, now with DI everything should be easy
@@ -57,6 +55,9 @@ class StatsNotifications {
     if($this->isTaskScheduled($newsletter->id)) {
       return false;
     }
+    if(($newsletter->type !== Newsletter::TYPE_NOTIFICATION) && ($newsletter->type !== Newsletter::TYPE_STANDARD)) {
+      return false;
+    }
     return true;
   }
 
@@ -83,7 +84,7 @@ class StatsNotifications {
       ->where('tasks.type', self::TASK_TYPE)
       ->where('notification.newsletter_id', $newsletter_id)
       ->findMany();
-    return (bool) $existing;
+    return (bool)$existing;
   }
 
   private function getNextRunDate() {

lib/Cron/Workers/WorkersFactory.php

@@ -26,7 +26,11 @@ class WorkersFactory {
 
   /** @return SendingQueueWorker */
   function createQueueWorker($timer) {
-    return new SendingQueueWorker($this->sending_error_handler, $timer);
+    return new SendingQueueWorker($this->sending_error_handler, $this->createStatsNotificationsWorker(), $timer);
+  }
+
+  function createStatsNotificationsWorker() {
+    return new StatsNotifications();
   }
 
   /** @return SendingServiceKeyCheckWorker */

lib/Models/StatsNotification.php

@@ -5,5 +5,29 @@ namespace MailPoet\Models;
 class StatsNotification extends Model {
   public static $_table = MP_STATS_NOTIFICATIONS_TABLE;
 
+  /** @return StatsNotification */
+  static function createOrUpdate($data = array()) {
+    $model = null;
+
+    if(isset($data['id']) && (int)$data['id'] > 0) {
+      $model = static::findOne((int)$data['id']);
+    }
+
+    if(!$model && isset($data['task_id']) && $data['newsletter_id']) {
+      $model = self::where('newsletter_id', $data['newsletter_id'])
+        ->where('task_id', $data['task_id'])
+        ->findOne();
+    }
+
+    if(!$model) {
+      $model = static::create();
+      $model->hydrate($data);
+    } else {
+      unset($data['id']);
+      $model->set($data);
+    }
+
+    return $model->save();
+  }
 
 }

tests/integration/Cron/Workers/StatsNotificationsTest.php

@@ -5,7 +5,6 @@ namespace MailPoet\Test\Cron\Workers;
 use MailPoet\Cron\Workers\StatsNotifications;
 use MailPoet\Models\Newsletter;
 use MailPoet\Models\ScheduledTask;
-use MailPoet\Models\SendingQueue;
 use MailPoet\Models\Setting;
 use MailPoet\Models\StatsNotification;
 
@@ -24,7 +23,7 @@ class StatsNotificationsTest extends \MailPoetTest {
 
   function testShouldSchedule() {
     $newsletter_id = 5;
-    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id]);
+    $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);
@@ -38,19 +37,19 @@ class StatsNotificationsTest extends \MailPoetTest {
       'enabled' => false,
       'address' => 'email@example.com'
     ]);
-    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id]);
+    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]);
     $this->stats_notifications->schedule($newsletter);
-    $queue = SendingQueue::where('newsletter_id', $newsletter_id)->findOne();
-    expect($queue)->isEmpty();
+    $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne();
+    expect($notification)->isEmpty();
   }
 
   function testShouldNotScheduleIfSettingsMissing() {
     $newsletter_id = 7;
     Setting::setValue(StatsNotifications::SETTINGS_KEY, []);
-    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id]);
+    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]);
     $this->stats_notifications->schedule($newsletter);
-    $queue = SendingQueue::where('newsletter_id', $newsletter_id)->findOne();
-    expect($queue)->isEmpty();
+    $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne();
+    expect($notification)->isEmpty();
   }
 
   function testShouldNotScheduleIfEmailIsMissing() {
@@ -58,10 +57,10 @@ class StatsNotificationsTest extends \MailPoetTest {
     Setting::setValue(StatsNotifications::SETTINGS_KEY, [
       'enabled' => true,
     ]);
-    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id]);
+    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]);
     $this->stats_notifications->schedule($newsletter);
-    $queue = SendingQueue::where('newsletter_id', $newsletter_id)->findOne();
-    expect($queue)->isEmpty();
+    $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne();
+    expect($notification)->isEmpty();
   }
 
   function testShouldNotScheduleIfEmailIsEmpty() {
@@ -70,10 +69,10 @@ class StatsNotificationsTest extends \MailPoetTest {
       'enabled' => true,
       'address' => ' '
     ]);
-    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id]);
+    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]);
     $this->stats_notifications->schedule($newsletter);
-    $queue = SendingQueue::where('newsletter_id', $newsletter_id)->findOne();
-    expect($queue)->isEmpty();
+    $notification = StatsNotification::where('newsletter_id', $newsletter_id)->findOne();
+    expect($notification)->isEmpty();
   }
 
   function testShouldNotScheduleIfAlreadyScheduled() {
@@ -83,18 +82,26 @@ class StatsNotificationsTest extends \MailPoetTest {
       'status' => ScheduledTask::STATUS_SCHEDULED,
       'scheduled_at' => '2017-01-02 12:13:14',
     ]);
-    $existing_queue = SendingQueue::createOrUpdate([
+    $existing_notification = StatsNotification::createOrUpdate([
       'newsletter_id' => $newsletter_id,
       'task_id' => $existing_task->id,
     ]);
-    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id]);
+    $newsletter = Newsletter::createOrUpdate(['id' => $newsletter_id, 'type' => Newsletter::TYPE_STANDARD]);
     $this->stats_notifications->schedule($newsletter);
-    $queues = SendingQueue::where('newsletter_id', $newsletter_id)->findMany();
-    expect($queues)->count(1);
-    $tasks = ScheduledTask::where('id', $queues[0]->task_id)->findMany();
+    $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_queue->id)->equals($queues[0]->id);
+    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();
+  }
+
 }