diff --git a/lib/API/MP/v1/API.php b/lib/API/MP/v1/API.php index 8724206f0b..8c7d5a512c 100644 --- a/lib/API/MP/v1/API.php +++ b/lib/API/MP/v1/API.php @@ -7,6 +7,7 @@ use MailPoet\Models\Subscriber; use MailPoet\Models\SubscriberSegment; use MailPoet\Newsletter\Scheduler\Scheduler; use MailPoet\Subscribers\Source; +use MailPoet\Tasks\Sending; if(!defined('ABSPATH')) exit; @@ -186,7 +187,12 @@ class API { // send confirmation email if($send_confirmation_email && $new_subscriber->status === Subscriber::STATUS_UNCONFIRMED) { - $this->_sendConfirmationEmail($new_subscriber); + $result = $this->_sendConfirmationEmail($new_subscriber); + if(!$result && $new_subscriber->getErrors()) { + throw new \Exception( + __(sprintf('Subscriber added, but confirmation email failed to send: %s', strtolower(implode(', ', $new_subscriber->getErrors()))), 'mailpoet') + ); + } } // schedule welcome email(s) @@ -242,6 +248,16 @@ class API { } protected function _scheduleWelcomeNotification(Subscriber $subscriber, array $segments) { - return Scheduler::scheduleSubscriberWelcomeNotification($subscriber->id, $segments); + $result = Scheduler::scheduleSubscriberWelcomeNotification($subscriber->id, $segments); + if(is_array($result)) { + foreach($result as $queue) { + if($queue instanceof Sending && $queue->getErrors()) { + throw new \Exception( + __(sprintf('Subscriber added, but welcome email failed to send: %s', strtolower(implode(', ', $queue->getErrors()))), 'mailpoet') + ); + } + } + } + return $result; } } diff --git a/lib/Models/Subscriber.php b/lib/Models/Subscriber.php index 632596b9ac..fd96ab71ac 100644 --- a/lib/Models/Subscriber.php +++ b/lib/Models/Subscriber.php @@ -904,4 +904,4 @@ class Subscriber extends Model { ->orderByAsc('name') ->findArray(); } -} \ No newline at end of file +} diff --git a/lib/Newsletter/Scheduler/Scheduler.php b/lib/Newsletter/Scheduler/Scheduler.php index 57b51faa62..4edfcf3064 100644 --- a/lib/Newsletter/Scheduler/Scheduler.php +++ b/lib/Newsletter/Scheduler/Scheduler.php @@ -38,13 +38,15 @@ class Scheduler { static function scheduleSubscriberWelcomeNotification($subscriber_id, $segments) { $newsletters = self::getNewsletters(Newsletter::TYPE_WELCOME); if(empty($newsletters)) return false; + $result = array(); foreach($newsletters as $newsletter) { if($newsletter->event === 'segment' && in_array($newsletter->segment, $segments) ) { - self::createWelcomeNotificationSendingTask($newsletter, $subscriber_id); + $result[] = self::createWelcomeNotificationSendingTask($newsletter, $subscriber_id); } } + return $result; } static function scheduleAutomaticEmail($group, $event, $scheduling_condition = false, $subscriber_id = false, $meta = false) { @@ -233,4 +235,4 @@ class Scheduler { static function formatDatetimeString($datetime_string) { return Carbon::parse($datetime_string)->format('Y-m-d H:i:s'); } -} \ No newline at end of file +} diff --git a/lib/Router/Endpoints/CronDaemon.php b/lib/Router/Endpoints/CronDaemon.php index 626e82c010..f99f872af8 100644 --- a/lib/Router/Endpoints/CronDaemon.php +++ b/lib/Router/Endpoints/CronDaemon.php @@ -40,4 +40,4 @@ class CronDaemon { $queue = new Daemon(); $queue->ping(); } -} \ No newline at end of file +} diff --git a/tests/unit/API/MP/APITest.php b/tests/unit/API/MP/APITest.php index 95fb312198..1966e3d393 100644 --- a/tests/unit/API/MP/APITest.php +++ b/tests/unit/API/MP/APITest.php @@ -2,13 +2,16 @@ namespace MailPoet\Test\API\MP; +use AspectMock\Test as Mock; use Codeception\Util\Fixtures; use Codeception\Util\Stub; use MailPoet\API\API; use MailPoet\Models\CustomField; +use MailPoet\Models\ScheduledTask; use MailPoet\Models\Segment; use MailPoet\Models\SendingQueue; use MailPoet\Models\Subscriber; +use MailPoet\Tasks\Sending; class APITest extends \MailPoetTest { const VERSION = 'v1'; @@ -298,6 +301,12 @@ class APITest extends \MailPoetTest { } function testItSubscribesToSegmentsWhenAddingSubscriber() { + $API = Stub::makeEmptyExcept( + new \MailPoet\API\MP\v1\API(), + 'addSubscriber', + array( + '_sendConfirmationEmail' => Stub::once() + ), $this); $segment = Segment::createOrUpdate( array( 'name' => 'Default', @@ -308,7 +317,7 @@ class APITest extends \MailPoetTest { 'email' => 'test@example.com' ); - $result = API::MP(self::VERSION)->addSubscriber($subscriber, array($segment->id)); + $result = $API->addSubscriber($subscriber, array($segment->id)); expect($result['id'])->greaterThan(0); expect($result['email'])->equals($subscriber['email']); expect($result['subscriptions'][0]['segment_id'])->equals($segment->id); @@ -329,6 +338,30 @@ class APITest extends \MailPoetTest { $API->addSubscriber($subscriber, $segments); } + function testItThrowsIfWelcomeEmailFails() { + $task = ScheduledTask::create(); + $task->type = 'sending'; + $task->setError("Big Error"); + $sendingStub = Sending::create($task, SendingQueue::create()); + Mock::double('MailPoet\Newsletter\Scheduler\Scheduler', array( + 'scheduleSubscriberWelcomeNotification' => array($sendingStub), + )); + $segment = Segment::createOrUpdate( + array( + 'name' => 'Default', + 'type' => Segment::TYPE_DEFAULT + ) + ); + $API = new \MailPoet\API\MP\v1\API(); + $subscriber = array( + 'email' => 'test@example.com', + 'status' => Subscriber::STATUS_SUBSCRIBED + ); + $segments = array($segment->id()); + $this->setExpectedException('\Exception'); + $API->addSubscriber($subscriber, $segments, array('schedule_welcome_email' => true, 'send_confirmation_email' => false)); + } + function testItDoesNotScheduleWelcomeNotificationAfterAddingSubscriberIfStatusIsNotSubscribed() { $API = Stub::makeEmptyExcept( new \MailPoet\API\MP\v1\API(), @@ -373,6 +406,29 @@ class APITest extends \MailPoetTest { $API->addSubscriber($subscriber, $segments); } + function testItThrowsWhenConfirmationEmailFailsToSend() { + $API = Stub::makeEmptyExcept( + new \MailPoet\API\MP\v1\API(), + 'addSubscriber', + array( + '_sendConfirmationEmail' => function($subscriber) { + $subscriber->setError('Big Error'); + return false; + }, + ), $this); + $segment = Segment::createOrUpdate( + array( + 'name' => 'Default', + 'type' => Segment::TYPE_DEFAULT + ) + ); + $subscriber = array( + 'email' => 'test@example.com' + ); + $this->setExpectedException('\Exception', 'Subscriber added, but confirmation email failed to send: big error'); + $API->addSubscriber($subscriber, array($segment->id), array('send_confirmation_email' => true)); + } + function testItDoesNotSendConfirmationEmailAfterAddingSubscriberWhenOptionIsSet() { $API = Stub::makeEmptyExcept( new \MailPoet\API\MP\v1\API(), @@ -562,6 +618,7 @@ class APITest extends \MailPoetTest { } function _after() { + Mock::clean(); \ORM::raw_execute('TRUNCATE ' . Subscriber::$_table); \ORM::raw_execute('TRUNCATE ' . CustomField::$_table); \ORM::raw_execute('TRUNCATE ' . Segment::$_table); diff --git a/tests/unit/Newsletter/Scheduler/SchedulerTest.php b/tests/unit/Newsletter/Scheduler/SchedulerTest.php index c278f43bb8..da02fd679d 100644 --- a/tests/unit/Newsletter/Scheduler/SchedulerTest.php +++ b/tests/unit/Newsletter/Scheduler/SchedulerTest.php @@ -250,7 +250,7 @@ class SchedulerTest extends \MailPoetTest { ); // queue is created and scheduled for delivery one day later - Scheduler::scheduleSubscriberWelcomeNotification( + $result = Scheduler::scheduleSubscriberWelcomeNotification( $subscriber_id = 10, $segments = array( 3, @@ -263,6 +263,7 @@ class SchedulerTest extends \MailPoetTest { ->findOne(); expect(Carbon::parse($queue->scheduled_at)->format('Y-m-d H:i')) ->equals($current_time->addDay()->format('Y-m-d H:i')); + expect($result[0]->id())->equals($queue->id()); } function itDoesNotScheduleAnythingWhenNewsletterDoesNotExist() { @@ -795,4 +796,4 @@ class SchedulerTest extends \MailPoetTest { \ORM::raw_execute('TRUNCATE ' . SendingQueue::$_table); \ORM::raw_execute('TRUNCATE ' . Subscriber::$_table); } -} \ No newline at end of file +}