Only processes queues when newsletter exists and is active/sending

lib/Cron/Workers/SendingQueue/SendingQueue.php

@@ -6,10 +6,10 @@ use MailPoet\Cron\Workers\SendingQueue\Tasks\Links;
 use MailPoet\Cron\Workers\SendingQueue\Tasks\Mailer as MailerTask;
 use MailPoet\Cron\Workers\SendingQueue\Tasks\Newsletter as NewsletterTask;
 use MailPoet\Mailer\MailerLog;
+use MailPoet\Models\Newsletter as NewsletterModel;
 use MailPoet\Models\SendingQueue as SendingQueueModel;
 use MailPoet\Models\StatisticsNewsletters as StatisticsNewslettersModel;
 use MailPoet\Models\Subscriber as SubscriberModel;
-use MailPoet\Router\Endpoints\Track;
 
 if(!defined('ABSPATH')) exit;
 
@@ -28,8 +28,12 @@ class SendingQueue {
   function process() {
     $this->enforceSendingAndExecutionLimits();
     foreach(self::getRunningQueues() as $queue) {
-      // get and pre-process newsletter (render, replace shortcodes/links, etc.)
+      $newsletter = $this->newsletter_task->getNewsletterFromQueue($queue);
-      $newsletter = $this->newsletter_task->getAndPreProcess($queue);
+      if(!$newsletter) {
+        continue;
+      }
+      // pre-process newsletter (render, replace shortcodes/links, etc.)
+      $newsletter = $this->newsletter_task->preProcessNewsletter($newsletter, $queue);
       if(!$newsletter) {
         $queue->delete();
         continue;
@@ -37,7 +41,7 @@ class SendingQueue {
       // configure mailer
       $this->mailer_task->configureMailer($newsletter);
       // get newsletter segments
-      $newsletter_segments_ids = $this->newsletter_task->getSegments($newsletter);
+      $newsletter_segments_ids = $this->newsletter_task->getNewsletterSegments($newsletter);
       // get subscribers
       $queue->subscribers = $queue->getSubscribers();
       $subscriber_batches = array_chunk(

lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php

@@ -22,11 +22,31 @@ class Newsletter {
     $this->tracking_enabled = (boolean)Setting::getValue('tracking.enabled');
   }
 
-  function getAndPreProcess($queue) {
+  function getNewsletterFromQueue($queue) {
-    $newsletter = $queue->newsletter()->whereNull('deleted_at')->findOne();
+    // get existing active or sending newsletter
-    if(!$newsletter) {
+    $newsletter = $queue->newsletter()
-      return false;
+      ->whereNull('deleted_at')
+      ->whereAnyIs(array(
+        array('status' => NewsletterModel::STATUS_ACTIVE),
+        array('status' => NewsletterModel::STATUS_SENDING)
+      ))
+      ->findOne();
+    if(!$newsletter) return false;
+    // if this is a notification history, get existing active or sending parent newsletter
+    if($newsletter->type == NewsletterModel::TYPE_NOTIFICATION_HISTORY) {
+      $parent_newsletter = $newsletter->parent()
+        ->whereNull('deleted_at')
+        ->whereAnyIs(array(
+          array('status' => NewsletterModel::STATUS_ACTIVE),
+          array('status' => NewsletterModel::STATUS_SENDING)
+        ))
+        ->findOne();
+      if(!$parent_newsletter) return false;
     }
+    return $newsletter;
+  }
+
+  function preProcessNewsletter($newsletter, $queue) {
     // return the newsletter if it was previously rendered
     if(!is_null($queue->getNewsletterRenderedBody())) {
       return $newsletter;
@@ -113,7 +133,7 @@ class Newsletter {
     }
   }
 
-  function getSegments($newsletter) {
+  function getNewsletterSegments($newsletter) {
     $segments = NewsletterSegmentModel::where('newsletter_id', $newsletter->id)
       ->select('segment_id')
       ->findArray();

lib/Models/Newsletter.php

@@ -39,6 +39,14 @@ class Newsletter extends Model {
     );
   }
 
+  function parent() {
+    return $this->hasOne(
+      __NAMESPACE__.'\Newsletter',
+      'id',
+      'parent_id'
+    );
+  }
+
   function segments() {
     return $this->hasManyThrough(
       __NAMESPACE__.'\Segment',

tests/unit/Cron/Workers/SendingQueue/SendingQueueTest.php

@@ -43,6 +43,7 @@ class SendingQueueTest extends MailPoetTest {
     $this->subscriber_segment->save();
     $this->newsletter = Newsletter::create();
     $this->newsletter->type = Newsletter::TYPE_STANDARD;
+    $this->newsletter->status = Newsletter::STATUS_ACTIVE;
     $this->newsletter->subject = Fixtures::get('newsletter_subject_template');
     $this->newsletter->body = Fixtures::get('newsletter_body_template');
     $this->newsletter->save();

tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php

@@ -22,9 +22,18 @@ class NewsletterTaskTest extends MailPoetTest {
     $this->subscriber->save();
     $this->newsletter = Newsletter::create();
     $this->newsletter->type = Newsletter::TYPE_STANDARD;
+    $this->newsletter->status = Newsletter::STATUS_ACTIVE;
     $this->newsletter->subject = Fixtures::get('newsletter_subject_template');
     $this->newsletter->body = Fixtures::get('newsletter_body_template');
+    $this->newsletter->preheader = '';
     $this->newsletter->save();
+    $this->parent_newsletter = Newsletter::create();
+    $this->parent_newsletter->type = Newsletter::TYPE_STANDARD;
+    $this->parent_newsletter->status = Newsletter::STATUS_ACTIVE;
+    $this->parent_newsletter->subject = 'parent newsletter';
+    $this->parent_newsletter->body = 'parent body';
+    $this->parent_newsletter->preheader = '';
+    $this->parent_newsletter->save();
     $this->queue = SendingQueue::create();
     $this->queue->newsletter_id = $this->newsletter->id;
     $this->queue->save();
@@ -34,16 +43,78 @@ class NewsletterTaskTest extends MailPoetTest {
     expect($this->newsletter_task->tracking_enabled)->true();
   }
 
-  function testItFailsToGetAndProcessNewsletterWhenNewsletterDoesNotExist() {
+  function testItDoesNotGetNewsletterWhenStatusIsNotActiveOrSending() {
-    $queue = $this->queue;
+    // draft or any other status return false
-    $queue->newsletter_id = 0;
+    $newsletter = $this->newsletter;
-    expect($this->newsletter_task->getAndPreProcess($queue))->false();
+    $newsletter->status = Newsletter::STATUS_DRAFT;
+    $newsletter->save();
+    expect($this->newsletter_task->getNewsletterFromQueue($this->queue))->false();
+
+    // active or sending statuses return newsletter
+    $newsletter = $this->newsletter;
+    $newsletter->status = Newsletter::STATUS_ACTIVE;
+    $newsletter->save();
+    expect($this->newsletter_task->getNewsletterFromQueue($this->queue))->isInstanceOf('Mailpoet\Models\Newsletter');
+
+    $newsletter = $this->newsletter;
+    $newsletter->status = Newsletter::STATUS_SENDING;
+    $newsletter->save();
+    expect($this->newsletter_task->getNewsletterFromQueue($this->queue))->isInstanceOf('Mailpoet\Models\Newsletter');
   }
 
-  function testItReturnsNewsletterObjectWhenRenderedNewssletterBodyExistsInTheQueue() {
+  function testItDoesNotGetDeletedNewsletter() {
+    $newsletter = $this->newsletter;
+    $newsletter->set_expr('deleted_at', 'NOW()');
+    $newsletter->save();
+    expect($this->newsletter_task->getNewsletterFromQueue($this->queue))->false();
+  }
+
+  function testItDoesNotGetNewsletterWhenParentNewsletterStatusIsNotActiveOrSending() {
+    // draft or any other status return false
+    $parent_newsletter = $this->parent_newsletter;
+    $parent_newsletter->status = Newsletter::STATUS_DRAFT;
+    $parent_newsletter->save();
+    $newsletter = $this->newsletter;
+    $newsletter->type = Newsletter::TYPE_NOTIFICATION_HISTORY;
+    $newsletter->parent_id = $parent_newsletter->id;
+    $newsletter->save();
+    expect($this->newsletter_task->getNewsletterFromQueue($this->queue))->false();
+
+    // active or sending statuses return newsletter
+    $parent_newsletter = $this->parent_newsletter;
+    $parent_newsletter->status = Newsletter::STATUS_ACTIVE;
+    $parent_newsletter->save();
+    $newsletter = $this->newsletter;
+    $newsletter->type = Newsletter::TYPE_NOTIFICATION_HISTORY;
+    $newsletter->parent_id = $parent_newsletter->id;
+    $newsletter->save();
+    expect($this->newsletter_task->getNewsletterFromQueue($this->queue))->isInstanceOf('Mailpoet\Models\Newsletter');
+
+    $parent_newsletter = $this->parent_newsletter;
+    $parent_newsletter->status = Newsletter::STATUS_SENDING;
+    $parent_newsletter->save();
+    $newsletter = $this->newsletter;
+    $newsletter->type = Newsletter::TYPE_NOTIFICATION_HISTORY;
+    $newsletter->parent_id = $parent_newsletter->id;
+    $newsletter->save();
+    expect($this->newsletter_task->getNewsletterFromQueue($this->queue))->isInstanceOf('Mailpoet\Models\Newsletter');
+  }
+
+  function testItDoesNotGetDeletedNewsletterWhenParentNewsletterIsDeleted() {
+    $parent_newsletter = $this->parent_newsletter;
+    $parent_newsletter->set_expr('deleted_at', 'NOW()');
+    $parent_newsletter->save();
+    $newsletter = $this->newsletter;
+    $newsletter->type = Newsletter::TYPE_NOTIFICATION_HISTORY;
+    $newsletter->parent_id = $parent_newsletter->id;
+    $newsletter->save();
+    expect($this->newsletter_task->getNewsletterFromQueue($this->queue))->false();
+  }
+
+  function testItReturnsNewsletterObjectWhenRenderedNewsletterBodyExistsInTheQueue() {
     $queue = $this->queue;
     $queue->newsletter_rendered_body = true;
-    $result = $this->newsletter_task->getAndPreProcess($queue);
+    $result = $this->newsletter_task->preProcessNewsletter($this->newsletter, $queue);
     expect($result instanceof \MailPoet\Models\Newsletter)->true();
   }
 
@@ -51,7 +122,7 @@ class NewsletterTaskTest extends MailPoetTest {
     WPHooksHelper::interceptApplyFilters();
     $newsletter_task = $this->newsletter_task;
     $newsletter_task->tracking_enabled = true;
-    $newsletter_task->getAndPreProcess($this->queue);
+    $newsletter_task->preProcessNewsletter($this->newsletter, $this->queue);
     $link = NewsletterLink::where('newsletter_id', $this->newsletter->id)
       ->findOne();
     $updated_queue = SendingQueue::findOne($this->queue->id);
@@ -71,7 +142,7 @@ class NewsletterTaskTest extends MailPoetTest {
     WPHooksHelper::interceptApplyFilters();
     $newsletter_task = $this->newsletter_task;
     $newsletter_task->tracking_enabled = false;
-    $newsletter_task->getAndPreProcess($this->queue);
+    $newsletter_task->preProcessNewsletter($this->newsletter, $this->queue);
     $link = NewsletterLink::where('newsletter_id', $this->newsletter->id)
       ->findOne();
     expect($link)->false();
@@ -88,15 +159,16 @@ class NewsletterTaskTest extends MailPoetTest {
     expect(WPHooksHelper::getFilterApplied($hook_name)[1] instanceof Newsletter)->true();
   }
 
-  function testItReturnsFalseAndDeletesNewsletterWhenPostNotificationContainsNoPostsn() {
+  function testItReturnsFalseAndDeletesNewsletterWhenPostNotificationContainsNoPosts() {
     $newsletter = $this->newsletter;
 
     $newsletter->type = Newsletter::TYPE_NOTIFICATION_HISTORY;
+    $newsletter->parent_id = $newsletter->id;
     // replace post id data tag with something else
     $newsletter->body = str_replace('data-post-id', 'id', $newsletter->body);
     $newsletter->save();
     // returned result is false
-    $result = $this->newsletter_task->getAndPreProcess($this->queue);
+    $result = $this->newsletter_task->preProcessNewsletter($this->newsletter, $this->queue);
     expect($result)->false();
     // newsletter is deleted
     $newsletter = Newsletter::findOne($newsletter->id);
@@ -104,7 +176,7 @@ class NewsletterTaskTest extends MailPoetTest {
   }
 
   function testItSavesNewsletterPosts() {
-    $result = $this->newsletter_task->getAndPreProcess($this->queue);
+    $result = $this->newsletter_task->preProcessNewsletter($this->newsletter, $this->queue);
     $newsletter_post = NewsletterPost::where('newsletter_id', $this->newsletter->id)
       ->findOne();
     expect($result)->notEquals(false);
@@ -140,7 +212,7 @@ class NewsletterTaskTest extends MailPoetTest {
   }
 
   function testItRendersShortcodesAndReplacesSubscriberDataInLinks() {
-    $newsletter = $this->newsletter_task->getAndPreProcess($this->queue);
+    $newsletter = $this->newsletter_task->preProcessNewsletter($this->newsletter, $this->queue);
     $result = $this->newsletter_task->prepareNewsletterForSending(
       $newsletter,
       $this->subscriber,
@@ -156,7 +228,7 @@ class NewsletterTaskTest extends MailPoetTest {
   function testItDoesNotReplaceSubscriberDataInLinksWhenTrackingIsNotEnabled() {
     $newsletter_task = $this->newsletter_task;
     $newsletter_task->tracking_enabled = false;
-    $newsletter = $newsletter_task->getAndPreProcess($this->queue);
+    $newsletter = $newsletter_task->preProcessNewsletter($this->newsletter, $this->queue);
     $result = $newsletter_task->prepareNewsletterForSending(
       $newsletter,
       $this->subscriber,
@@ -175,7 +247,7 @@ class NewsletterTaskTest extends MailPoetTest {
       $newsletter_segment->segment_id = $i;
       $newsletter_segment->save();
     }
-    expect($this->newsletter_task->getSegments($this->newsletter))->equals(
+    expect($this->newsletter_task->getNewsletterSegments($this->newsletter))->equals(
       array(1,2,3)
     );
   }

tests/unit/Models/NewsletterTest.php

@@ -109,6 +109,18 @@ class NewsletterTest extends MailPoetTest {
     expect($newsletter_segments[1]['name'])->equals('Segment 2');
   }
 
+  function itCanHaveParentNewsletter() {
+    $parent_newsletter = Newsletter::create();
+    $parent_newsletter->type = Newsletter::TYPE_STANDARD;
+    $parent_newsletter->save();
+    $newsletter = $this->newsletter;
+    $newsletter->parent_id = $parent_newsletter->id;
+    $newsletter->save();
+    $parent = $newsletter->parent()-findOne();
+    expect($parent)->isInstanceOf('MailPoet\Models\Newsletter');
+    expect($parent->id)->equals($parent_newsletter->id);
+  }
+
   function testItCanHaveDeletedSegments() {
     $this->segment_2->delete();
     $this->newsletter->withSegments(true);