From 70debcc828e8e4210c0c920214ca62ea6452e2d0 Mon Sep 17 00:00:00 2001 From: Pavel Dohnal Date: Thu, 11 Oct 2018 12:06:49 +0200 Subject: [PATCH] Refactor confirmation email sending Aspect mock stopped working for me so I had to create a separate service for sending confirmation emails. [MAILPOET-1522] --- lib/API/MP/v1/API.php | 4 +- lib/Models/Subscriber.php | 70 +------------- lib/Subscribers/SendConfirmationEmail.php | 91 +++++++++++++++++++ .../SendNewSubscriberNotification.php | 2 +- tests/_bootstrap.php | 5 +- tests/unit/Models/SubscriberTest.php | 43 --------- .../Subscribers/SendConfirmationEmailTest.php | 71 +++++++++++++++ 7 files changed, 172 insertions(+), 114 deletions(-) create mode 100644 lib/Subscribers/SendConfirmationEmail.php create mode 100644 tests/unit/Subscribers/SendConfirmationEmailTest.php diff --git a/lib/API/MP/v1/API.php b/lib/API/MP/v1/API.php index 1b12290ee9..6f59cf2129 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\RequiredCustomFieldValidator; +use MailPoet\Subscribers\SendConfirmationEmail; use MailPoet\Subscribers\SendNewSubscriberNotification; use MailPoet\Subscribers\Source; use MailPoet\Tasks\Sending; @@ -254,7 +255,8 @@ class API { } protected function _sendConfirmationEmail(Subscriber $subscriber) { - return $subscriber->sendConfirmationEmail(); + $sender = new SendConfirmationEmail(); + return $sender->sendConfirmationEmail($subscriber); } protected function _scheduleWelcomeNotification(Subscriber $subscriber, array $segments) { diff --git a/lib/Models/Subscriber.php b/lib/Models/Subscriber.php index a2cd24dd36..616d0ae904 100644 --- a/lib/Models/Subscriber.php +++ b/lib/Models/Subscriber.php @@ -2,6 +2,7 @@ namespace MailPoet\Models; use MailPoet\Mailer\Mailer; use MailPoet\Newsletter\Scheduler\Scheduler; +use MailPoet\Subscribers\SendConfirmationEmail; use MailPoet\Subscribers\SendNewSubscriberNotification; use MailPoet\Subscribers\Source; use MailPoet\Util\Helpers; @@ -83,71 +84,6 @@ class Subscriber extends Model { return self::where('wp_user_id', $wp_user->ID)->findOne(); } - function sendConfirmationEmail() { - $signup_confirmation = Setting::getValue('signup_confirmation'); - - if((bool)$signup_confirmation['enabled'] === false) { - return false; - } - - $segments = $this->segments()->findMany(); - $segment_names = array_map(function($segment) { - return $segment->name; - }, $segments); - - $body = nl2br($signup_confirmation['body']); - - // replace list of segments shortcode - $body = str_replace( - '[lists_to_confirm]', - ''.join(', ', $segment_names).'', - $body - ); - - // replace activation link - $body = Helpers::replaceLinkTags( - $body, - Subscription\Url::getConfirmationUrl($this), - array('target' => '_blank'), - 'activation_link' - ); - - // build email data - $email = array( - 'subject' => $signup_confirmation['subject'], - 'body' => array( - 'html' => $body, - 'text' => $body - ) - ); - - // convert subscriber to array - $subscriber = $this->asArray(); - - // set from - $from = ( - !empty($signup_confirmation['from']) - && !empty($signup_confirmation['from']['address']) - ) ? $signup_confirmation['from'] - : false; - - // set reply to - $reply_to = ( - !empty($signup_confirmation['reply_to']) - && !empty($signup_confirmation['reply_to']['address']) - ) ? $signup_confirmation['reply_to'] - : false; - - // send email - try { - $mailer = new Mailer(false, $from, $reply_to); - return $mailer->send($email, $subscriber); - } catch(\Exception $e) { - $this->setError($e->getMessage()); - return false; - } - } - static function generateToken($email = null) { if($email !== null) { $auth_key = ''; @@ -216,8 +152,8 @@ class Subscriber extends Model { // link subscriber to segments SubscriberSegment::subscribeToSegments($subscriber, $segment_ids); - // signup confirmation - $subscriber->sendConfirmationEmail(); + $sender = new SendConfirmationEmail(); + $sender->sendConfirmationEmail($subscriber); if($subscriber->status === self::STATUS_SUBSCRIBED) { $sender = new SendNewSubscriberNotification(); diff --git a/lib/Subscribers/SendConfirmationEmail.php b/lib/Subscribers/SendConfirmationEmail.php new file mode 100644 index 0000000000..821faa0685 --- /dev/null +++ b/lib/Subscribers/SendConfirmationEmail.php @@ -0,0 +1,91 @@ +mailer = $mailer; + } + } + + function sendConfirmationEmail(Subscriber $subscriber) { + $signup_confirmation = Setting::getValue('signup_confirmation'); + + if((bool)$signup_confirmation['enabled'] === false) { + return false; + } + + $segments = $subscriber->segments()->findMany(); + $segment_names = array_map(function($segment) { + return $segment->name; + }, $segments); + + $body = nl2br($signup_confirmation['body']); + + // replace list of segments shortcode + $body = str_replace( + '[lists_to_confirm]', + ''.join(', ', $segment_names).'', + $body + ); + + // replace activation link + $body = Helpers::replaceLinkTags( + $body, + Url::getConfirmationUrl($subscriber), + array('target' => '_blank'), + 'activation_link' + ); + + // build email data + $email = array( + 'subject' => $signup_confirmation['subject'], + 'body' => array( + 'html' => $body, + 'text' => $body + ) + ); + + // set from + $from = ( + !empty($signup_confirmation['from']) + && !empty($signup_confirmation['from']['address']) + ) ? $signup_confirmation['from'] + : false; + + // set reply to + $reply_to = ( + !empty($signup_confirmation['reply_to']) + && !empty($signup_confirmation['reply_to']['address']) + ) ? $signup_confirmation['reply_to'] + : false; + + // send email + try { + if(!$this->mailer) { + $this->mailer = new Mailer(false, $from, $reply_to); + } + $this->mailer->getSenderNameAndAddress($from); + $this->mailer->getReplyToNameAndAddress($reply_to); + return $this->mailer->send($email, $subscriber); + } catch(\Exception $e) { + $subscriber->setError($e->getMessage()); + return false; + } + } + +} diff --git a/lib/Subscribers/SendNewSubscriberNotification.php b/lib/Subscribers/SendNewSubscriberNotification.php index 1894b1e7c0..03d6e4cce6 100644 --- a/lib/Subscribers/SendNewSubscriberNotification.php +++ b/lib/Subscribers/SendNewSubscriberNotification.php @@ -33,7 +33,7 @@ class SendNewSubscriberNotification { if($mailer) { $this->mailer = $mailer; } else { - $this->mailer = new \MailPoet\Mailer\Mailer(); + $this->mailer = new \MailPoet\Mailer\Mailer(false, $this->constructSenderEmail()); } } diff --git a/tests/_bootstrap.php b/tests/_bootstrap.php index 35786f0571..805849f47a 100644 --- a/tests/_bootstrap.php +++ b/tests/_bootstrap.php @@ -59,7 +59,8 @@ $kernel->init( array( 'debug' => true, 'cacheDir' => $cacheDir, - 'includePaths' => [__DIR__ . '/../lib'] + 'includePaths' => [__DIR__ . '/../lib'], + 'appDir' => __DIR__ . '/../' ) ); @@ -139,4 +140,4 @@ abstract class MailPoetTest extends \Codeception\TestCase\Test { return $method->invokeArgs($object, $parameters); } -} \ No newline at end of file +} diff --git a/tests/unit/Models/SubscriberTest.php b/tests/unit/Models/SubscriberTest.php index cf67cdc5a0..7332a3b325 100644 --- a/tests/unit/Models/SubscriberTest.php +++ b/tests/unit/Models/SubscriberTest.php @@ -1,7 +1,6 @@ null, - 'send' => function($email) { - return $email; - } - ]); - Mock::double('MailPoet\Subscription\Url', [ - 'getConfirmationUrl' => 'http://example.com' - ]); - - $segment = Segment::createOrUpdate( - array( - 'name' => 'Test segment' - ) - ); - SubscriberSegment::subscribeToSegments( - $this->subscriber, - array($segment->id) - ); - - $result = $this->subscriber->sendConfirmationEmail(); - - // email contains subscriber's lists - expect($result['body']['html'])->contains('Test segment'); - // email contains activation link - expect($result['body']['html'])->contains('Click here to confirm your subscription.'); - } - - function testItSetsErrorsWhenConfirmationEmailCannotBeSent() { - Mock::double('MailPoet\Mailer\Mailer', [ - '__construct' => null, - 'send' => function() { - throw new \Exception('send error'); - } - ]); - - $this->subscriber->sendConfirmationEmail(); - // error is set on the subscriber model object - expect($this->subscriber->getErrors()[0])->equals('send error'); - } - function _after() { \ORM::raw_execute('TRUNCATE ' . Subscriber::$_table); \ORM::raw_execute('TRUNCATE ' . Segment::$_table); diff --git a/tests/unit/Subscribers/SendConfirmationEmailTest.php b/tests/unit/Subscribers/SendConfirmationEmailTest.php new file mode 100644 index 0000000000..ba8787662c --- /dev/null +++ b/tests/unit/Subscribers/SendConfirmationEmailTest.php @@ -0,0 +1,71 @@ + 'http://example.com' + ]); + $subscriber = Subscriber::create(); + $subscriber->hydrate([ + 'first_name' => 'John', + 'last_name' => 'Mailer', + 'email' => 'john@mailpoet.com' + ]); + + $mailer = Stub::makeEmpty(Mailer::class, [ + 'send' => + Stub\Expected::once(function($email) { + expect($email['body']['html'])->contains('Test segment'); + expect($email['body']['html'])->contains('Click here to confirm your subscription.'); + }), + ], $this); + + $sender = new SendConfirmationEmail($mailer); + + + $segment = Segment::createOrUpdate( + array( + 'name' => 'Test segment' + ) + ); + SubscriberSegment::subscribeToSegments( + $subscriber, + array($segment->id) + ); + + $sender->sendConfirmationEmail($subscriber); + } + + function testItSetsErrorsWhenConfirmationEmailCannotBeSent() { + $subscriber = Subscriber::create(); + $subscriber->hydrate([ + 'first_name' => 'John', + 'last_name' => 'Mailer', + 'email' => 'john@mailpoet.com' + ]); + + $mailer = Stub::makeEmpty(Mailer::class, [ + 'send' => + Stub\Expected::once(function () { + throw new \Exception('send error'); + }), + ], $this); + + $sender = new SendConfirmationEmail($mailer); + + $sender->sendConfirmationEmail($subscriber); + // error is set on the subscriber model object + expect($subscriber->getErrors()[0])->equals('send error'); + } + +}