From 34d09ce0c97a7d6fc6e196d2d21cf96dfb829a3b Mon Sep 17 00:00:00 2001 From: Vlad Date: Fri, 14 Jul 2017 21:41:03 -0400 Subject: [PATCH] Sets mailer log error when queue cannot be saved during newsletter pre-processing --- lib/Cron/Workers/SendingQueue/SendingQueue.php | 2 +- .../Workers/SendingQueue/Tasks/Newsletter.php | 7 +++++++ lib/Mailer/MailerLog.php | 2 +- .../Workers/SendingQueue/Tasks/LinksTest.php | 6 +++--- .../SendingQueue/Tasks/NewsletterTest.php | 17 ++++++++++++++++- tests/unit/Mailer/MailerLogTest.php | 2 +- 6 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php index 9aafb7ecbd..bc6f201d08 100644 --- a/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -157,7 +157,7 @@ class SendingQueue { ); // log error message and schedule retry/pause sending if($send_result['response'] === false) { - MailerLog::processSendingError( + MailerLog::processError( $send_result['operation'], $send_result['error_message'] ); diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php index cbf1d17031..505cd64278 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php @@ -4,6 +4,7 @@ namespace MailPoet\Cron\Workers\SendingQueue\Tasks; use MailPoet\Cron\Workers\SendingQueue\Tasks\Links as LinksTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Posts as PostsTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Shortcodes as ShortcodesTask; +use MailPoet\Mailer\MailerLog; use MailPoet\Models\Newsletter as NewsletterModel; use MailPoet\Models\NewsletterSegment as NewsletterSegmentModel; use MailPoet\Models\Setting; @@ -88,6 +89,12 @@ class Newsletter { $queue->newsletter_rendered_subject = Shortcodes::process($newsletter->subject, $newsletter, null, $queue); $queue->newsletter_rendered_body = $rendered_newsletter; $queue->save(); + if($queue->getErrors()) { + return MailerLog::processError( + 'queue_save', + __('There was an error processing your newsletter during sending. If possible, please contact us and report this issue.') + ); + } return $newsletter; } diff --git a/lib/Mailer/MailerLog.php b/lib/Mailer/MailerLog.php index 4becde7853..52e384ff6a 100644 --- a/lib/Mailer/MailerLog.php +++ b/lib/Mailer/MailerLog.php @@ -75,7 +75,7 @@ class MailerLog { return self::resetMailerLog(); } - static function processSendingError($operation, $error_message) { + static function processError($operation, $error_message) { $mailer_log = self::getMailerLog(); (int)$mailer_log['retry_attempt']++; $mailer_log['retry_at'] = time() + self::RETRY_INTERVAL; diff --git a/tests/unit/Cron/Workers/SendingQueue/Tasks/LinksTest.php b/tests/unit/Cron/Workers/SendingQueue/Tasks/LinksTest.php index 4c058cb4d5..0b915e6b13 100644 --- a/tests/unit/Cron/Workers/SendingQueue/Tasks/LinksTest.php +++ b/tests/unit/Cron/Workers/SendingQueue/Tasks/LinksTest.php @@ -9,7 +9,7 @@ class LinkTaskTest extends MailPoetTest { function testItCanSaveLinks() { $links = array( array( - 'url' => 'http://example.com', + 'link' => 'http://example.com', 'hash' => 'some_hash' ) ); @@ -20,7 +20,7 @@ class LinkTaskTest extends MailPoetTest { ->findOne(); expect($newsletter_link->newsletter_id)->equals($newsletter->id); expect($newsletter_link->queue_id)->equals($queue->id); - expect($newsletter_link->url)->equals($links[0]['url']); + expect($newsletter_link->url)->equals($links[0]['link']); } function testItCanHashAndReplaceLinks() { @@ -35,7 +35,7 @@ class LinkTaskTest extends MailPoetTest { ->contains($processed_and_hashed_links[0]['hash']); expect($processed_rendered_newsletter_body['text']) ->contains($processed_and_hashed_links[0]['hash']); - expect($processed_and_hashed_links[0]['url'])->equals('http://example.com'); + expect($processed_and_hashed_links[0]['link'])->equals('http://example.com'); } function testItCanProcessRenderedBody() { diff --git a/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php b/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php index e8bfa04577..1ebabf2599 100644 --- a/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php +++ b/tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php @@ -1,7 +1,9 @@ newsletter; $queue = new stdClass(); - $queue->processed_at = date('Y-m-d H:i:s'); + $queue->processed_at = date('Y-m-d H:i:s'); // newsletter type is 'standard' $newsletter->type = Newsletter::TYPE_STANDARD; @@ -256,6 +258,19 @@ class NewsletterTaskTest extends MailPoetTest { ); } + function testItLogsErrorWhenQueueWithCannotBeSaved() { + $queue = $this->queue; + $queue->non_existent_column = true; // this will trigger save error + try { + $this->newsletter_task->preProcessNewsletter($this->newsletter, $queue); + self::fail('Sending error exception was not thrown.'); + } catch(Exception $e) { + $mailer_log = MailerLog::getMailerLog(); + expect($mailer_log['error']['operation'])->equals('queue_save'); + expect($mailer_log['error']['error_message'])->equals('There was an error processing your newsletter during sending. If possible, please contact us and report this issue.'); + } + } + function _after() { WPHooksHelper::releaseAllHooks(); ORM::raw_execute('TRUNCATE ' . Subscriber::$_table); diff --git a/tests/unit/Mailer/MailerLogTest.php b/tests/unit/Mailer/MailerLogTest.php index b089c1341b..9e4ef1b691 100644 --- a/tests/unit/Mailer/MailerLogTest.php +++ b/tests/unit/Mailer/MailerLogTest.php @@ -148,7 +148,7 @@ class MailerLogTest extends MailPoetTest { expect($mailer_log['retry_at'])->null(); expect($mailer_log['error'])->null(); // retry attempt should be incremented, error logged, retry attempt scheduled - MailerLog::processSendingError($operation = 'send', $error = 'email rejected'); + MailerLog::processError($operation = 'send', $error = 'email rejected'); $mailer_log = MailerLog::getMailerLog(); expect($mailer_log['retry_attempt'])->equals(1); expect($mailer_log['retry_at'])->greaterThan(time());