From e32c46a755d31421ed2cc1a3f75156aa9f5d1f21 Mon Sep 17 00:00:00 2001 From: Vlad Date: Tue, 25 Oct 2016 22:37:34 -0400 Subject: [PATCH 1/5] - Detaches posts_where action after posts are pulled from the database --- lib/Newsletter/AutomatedLatestContent.php | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/Newsletter/AutomatedLatestContent.php b/lib/Newsletter/AutomatedLatestContent.php index 9925d51c80..1802038d2b 100644 --- a/lib/Newsletter/AutomatedLatestContent.php +++ b/lib/Newsletter/AutomatedLatestContent.php @@ -15,12 +15,6 @@ class AutomatedLatestContent { function __construct($newsletter_id = false, $newer_than_timestamp = false) { $this->newsletter_id = $newsletter_id; $this->newer_than_timestamp = $newer_than_timestamp; - - $this->_attachSentPostsFilter(); - } - - function __destruct() { - $this->_detachSentPostsFilter(); } function filterOutSentPosts($where) { @@ -72,7 +66,10 @@ class AutomatedLatestContent { ); } - return get_posts($parameters); + $this->_attachSentPostsFilter(); + $posts = get_posts($parameters); + $this->_detachSentPostsFilter(); + return $posts; } function transformPosts($args, $posts) { @@ -129,4 +126,4 @@ class AutomatedLatestContent { remove_action('posts_where', array($this, 'filterOutSentPosts')); } } -} +} \ No newline at end of file From ef21a8cca705bad84ff711aa69211b3c92b82e03 Mon Sep 17 00:00:00 2001 From: Vlad Date: Thu, 20 Oct 2016 21:03:03 -0400 Subject: [PATCH 2/5] - Enables post notification schedule update upon newsletter saving during step 3 --- lib/API/Endpoints/Newsletters.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/API/Endpoints/Newsletters.php b/lib/API/Endpoints/Newsletters.php index a7d05611a0..358e21f4f9 100644 --- a/lib/API/Endpoints/Newsletters.php +++ b/lib/API/Endpoints/Newsletters.php @@ -91,9 +91,14 @@ class Newsletters extends APIEndpoint { } } - return $this->successResponse( - Newsletter::findOne($newsletter->id)->asArray() - ); + $newsletter = Newsletter::filter('filterWithOptions')->findOne($newsletter->id); + + // if this is a post notification, process options and update its schedule + if($newsletter->type === Newsletter::TYPE_NOTIFICATION) { + Scheduler::processPostNotificationSchedule($newsletter); + } + + return $this->successResponse($newsletter->asArray()); } } From 8330bfc8848b2868453db72cef413b3f7bb2a526 Mon Sep 17 00:00:00 2001 From: Vlad Date: Thu, 20 Oct 2016 21:05:07 -0400 Subject: [PATCH 3/5] - Fixes "completed" status update of notification history newsletters - Fixes detection of post notification newsletters that do not contain any posts (i.e., blank ALC blocks) - Updates unit test --- lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php | 11 +++++++---- .../Workers/SendingQueue/Tasks/NewsletterTest.php | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php index 01b081944a..4024e90096 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php @@ -8,7 +8,6 @@ use MailPoet\Models\Newsletter as NewsletterModel; use MailPoet\Models\Setting; use MailPoet\Newsletter\Links\Links as NewsletterLinks; use MailPoet\Newsletter\Renderer\PostProcess\OpenTracking; -use MailPoet\Newsletter\Renderer\Renderer; use MailPoet\Util\Helpers; if(!defined('ABSPATH')) exit; @@ -45,7 +44,9 @@ class Newsletter { } // check if this is a post notification and if it contains posts $newsletter_contains_posts = strpos($rendered_newsletter['html'], 'data-post-id'); - if($newsletter->type === 'notification' && !$newsletter_contains_posts) { + if($newsletter->type === NewsletterModel::TYPE_NOTIFICATION_HISTORY && + !$newsletter_contains_posts + ) { return false; } // extract and save newsletter posts @@ -91,8 +92,10 @@ class Newsletter { } function markNewsletterAsSent($newsletter) { - // if it's a standard newsletter, update its status - if($newsletter->type === NewsletterModel::TYPE_STANDARD) { + // if it's a standard or notification history newsletter, update its status + if($newsletter->type === NewsletterModel::TYPE_STANDARD || + $newsletter->type === NewsletterModel::TYPE_NOTIFICATION_HISTORY + ) { $newsletter->setStatus(NewsletterModel::STATUS_SENT); } } diff --git a/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php b/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php index 73a6a3130f..108b0bf5fb 100644 --- a/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php +++ b/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php @@ -76,7 +76,7 @@ class NewsletterTaskTest extends MailPoetTest { function testReturnsFalseWhenNewsletterIsANotificationWithoutPosts() { $newsletter = $this->newsletter; - $newsletter->type = Newsletter::TYPE_NOTIFICATION; + $newsletter->type = Newsletter::TYPE_NOTIFICATION_HISTORY; // replace post id data tag with something else $newsletter->body = str_replace('data-post-id', 'id', $newsletter->body); $newsletter->save(); From d08d5a3b6c00d51dc53b90e7e0d81ba6f475dd8f Mon Sep 17 00:00:00 2001 From: Vlad Date: Fri, 21 Oct 2016 11:19:09 -0400 Subject: [PATCH 4/5] - Updates unit tests --- tests/unit/API/Endpoints/NewslettersTest.php | 73 ++++++++++++++++--- .../SendingQueue/Tasks/NewsletterTest.php | 23 ++++-- 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/tests/unit/API/Endpoints/NewslettersTest.php b/tests/unit/API/Endpoints/NewslettersTest.php index dd2ddae7e8..78939a3b4b 100644 --- a/tests/unit/API/Endpoints/NewslettersTest.php +++ b/tests/unit/API/Endpoints/NewslettersTest.php @@ -2,10 +2,14 @@ use \MailPoet\API\Response as APIResponse; use \MailPoet\API\Error as APIError; use \MailPoet\API\Endpoints\Newsletters; +use MailPoet\Config\Populator; use \MailPoet\Models\Newsletter; +use MailPoet\Models\NewsletterOption; +use MailPoet\Models\NewsletterOptionField; use \MailPoet\Models\NewsletterSegment; use \MailPoet\Models\NewsletterTemplate; use \MailPoet\Models\Segment; +use MailPoet\Newsletter\Scheduler\Scheduler; class NewslettersTest extends MailPoetTest { function _before() { @@ -44,17 +48,26 @@ class NewslettersTest extends MailPoetTest { } function testItCanSaveANewNewsletter() { + $newsletter_option_field = NewsletterOptionField::create(); + $newsletter_option_field->name = 'some_option'; + $newsletter_option_field->newsletter_type = Newsletter::TYPE_STANDARD; + $newsletter_option_field->save(); + $valid_data = array( 'subject' => 'My First Newsletter', - 'type' => Newsletter::TYPE_STANDARD + 'type' => Newsletter::TYPE_STANDARD, + 'options' => array( + $newsletter_option_field->name => 'some_option_value', + ) ); $router = new Newsletters(); $response = $router->save($valid_data); + $saved_newsletter = Newsletter::filter('filterWithOptions')->findOne($response->data['id']); expect($response->status)->equals(APIResponse::STATUS_OK); - expect($response->data)->equals( - Newsletter::findOne($response->data['id'])->asArray() - ); + expect($response->data)->equals($saved_newsletter->asArray()); + // newsletter option should be saved + expect($saved_newsletter->some_option)->equals('some_option_value'); $invalid_data = array( 'subject' => 'Missing newsletter type' @@ -73,15 +86,56 @@ class NewslettersTest extends MailPoetTest { ); $response = $router->save($newsletter_data); - expect($response->status)->equals(APIResponse::STATUS_OK); - expect($response->data)->equals( - Newsletter::findOne($this->newsletter->id)->asArray() - ); - $updated_newsletter = Newsletter::findOne($this->newsletter->id); + expect($response->status)->equals(APIResponse::STATUS_OK); + expect($response->data)->equals($updated_newsletter->asArray()); expect($updated_newsletter->subject)->equals('My Updated Newsletter'); } + function testItCanUpdatePostNotificationScheduleUponSave() { + $newsletter_options = array( + 'intervalType', + 'timeOfDay', + 'weekDay', + 'monthDay', + 'nthWeekDay', + 'schedule' + ); + foreach($newsletter_options as $option) { + $newsletter_option_field = NewsletterOptionField::create(); + $newsletter_option_field->name = $option; + $newsletter_option_field->newsletter_type = Newsletter::TYPE_NOTIFICATION; + $newsletter_option_field->save(); + } + + $router = new Newsletters(); + $newsletter_data = array( + 'id' => $this->newsletter->id, + 'type' => Newsletter::TYPE_NOTIFICATION, + 'subject' => 'Newsletter', + 'options' => array( + 'intervalType' => Scheduler::INTERVAL_WEEKLY, + 'timeOfDay' => '50400', + 'weekDay' => '1', + 'monthDay' => '0', + 'nthWeekDay' => '1', + 'schedule' => '0 14 * * 1' + ) + ); + $response = $router->save($newsletter_data); + $saved_newsletter = Newsletter::filter('filterWithOptions') + ->findOne($response->data['id']); + expect($response->status)->equals(APIResponse::STATUS_OK); + expect($response->data)->equals($saved_newsletter->asArray()); + + // schedule should be recalculated when options change + $newsletter_data['options']['intervalType'] = Scheduler::INTERVAL_IMMEDIATELY; + $response = $router->save($newsletter_data); + $saved_newsletter = Newsletter::filter('filterWithOptions')->findOne($response->data['id']); + expect($response->status)->equals(APIResponse::STATUS_OK); + expect($saved_newsletter->schedule)->equals('* * * * *'); + } + function testItCanModifySegmentsOfExistingNewsletter() { $segment_1 = Segment::createOrUpdate(array('name' => 'Segment 1')); $fake_segment_id = 1; @@ -398,6 +452,7 @@ class NewslettersTest extends MailPoetTest { function _after() { Newsletter::deleteMany(); NewsletterSegment::deleteMany(); + NewsletterOptionField::deleteMany(); Segment::deleteMany(); } } diff --git a/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php b/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php index 108b0bf5fb..3d7641062d 100644 --- a/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php +++ b/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php @@ -76,6 +76,7 @@ class NewsletterTaskTest extends MailPoetTest { function testReturnsFalseWhenNewsletterIsANotificationWithoutPosts() { $newsletter = $this->newsletter; + $newsletter->type = Newsletter::TYPE_NOTIFICATION_HISTORY; // replace post id data tag with something else $newsletter->body = str_replace('data-post-id', 'id', $newsletter->body); @@ -92,17 +93,27 @@ class NewsletterTaskTest extends MailPoetTest { expect($newsletter_post->post_id)->equals('10'); } - function testItUpdatesStatusToSentOnlyForStandardNewsletters() { - // newsletter type is 'standard' + function testItUpdatesStatusToSentOnlyForStandardAndPostNotificationNewsletters() { $newsletter = $this->newsletter; - expect($newsletter->type)->equals(Newsletter::TYPE_STANDARD); - expect($newsletter->status)->notEquals(Newsletter::STATUS_SENT); + + // newsletter type is 'standard' + $newsletter->type = Newsletter::TYPE_STANDARD; + $newsletter->status = 'not_sent'; + $newsletter->save(); $this->newsletter_task->markNewsletterAsSent($newsletter); $updated_newsletter = Newsletter::findOne($newsletter->id); expect($updated_newsletter->status)->equals(Newsletter::STATUS_SENT); - // newsletter type is NOT 'standard' - $newsletter->type = Newsletter::TYPE_NOTIFICATION; + // newsletter type is 'notification history' + $newsletter->type = Newsletter::TYPE_NOTIFICATION_HISTORY; + $newsletter->status = 'not_sent'; + $newsletter->save(); + $this->newsletter_task->markNewsletterAsSent($newsletter); + $updated_newsletter = Newsletter::findOne($newsletter->id); + expect($updated_newsletter->status)->equals(Newsletter::STATUS_SENT); + + // all other newsletter types + $newsletter->type = Newsletter::TYPE_WELCOME; $newsletter->status = 'not_sent'; $newsletter->save(); $this->newsletter_task->markNewsletterAsSent($newsletter); From 83114a8be4a4996680306efe057cf63b627d805b Mon Sep 17 00:00:00 2001 From: Vlad Date: Mon, 24 Oct 2016 08:54:32 -0400 Subject: [PATCH 5/5] - Removes unused class declarations --- tests/unit/API/Endpoints/NewslettersTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/unit/API/Endpoints/NewslettersTest.php b/tests/unit/API/Endpoints/NewslettersTest.php index 78939a3b4b..20b8d1ba9c 100644 --- a/tests/unit/API/Endpoints/NewslettersTest.php +++ b/tests/unit/API/Endpoints/NewslettersTest.php @@ -1,13 +1,9 @@