Remove calls to Subscriber::setError() from ConfirmationEmailMailer

This commit is part of a task to replace Paris with Doctrine in the
class ConfirmationEmailMailer. Specifically, it removes two calls to
Subscriber::setError() inside ConfirmationEmailMailer::sendConfirmationEmail().

setError() was used to define an error message that was used only in one
of the instances where sendConfirmationEmail() is called
(API::subscribeToLists()). setError() was replaced with code that throws
an exception when there is an error. Thus it was necessary to change all
the places where ConfirmationEmailMailer::sendConfirmationEmail() is
called to handle the exception.

In some cases, there are some oddities as sendConfirmationEmail() can
return false or throw an exception in case of an error and calling code
must account for both. I decided to settle with this approach as
refactoring the rest of this method to use exceptions instead of
returning false seemed outside of the scope of this task.

[MAILPOET-3815]

lib/API/JSON/v1/Subscribers.php

@@ -199,12 +199,19 @@ class Subscribers extends APIEndpoint {
     $id = (isset($data['id']) ? (int)$data['id'] : false);
     $subscriber = $this->subscribersRepository->findOneById($id);
     if ($subscriber instanceof SubscriberEntity) {
-      if ($this->confirmationEmailMailer->sendConfirmationEmail($subscriber)) {
+      try {
-        return $this->successResponse();
+        if ($this->confirmationEmailMailer->sendConfirmationEmail($subscriber)) {
+          return $this->successResponse();
+        } else {
+          return $this->errorResponse([
+            APIError::UNKNOWN => __('There was a problem with your sending method. Please check if your sending method is properly configured.', 'mailpoet'),
+          ]);
+        }
+      } catch (\Exception $e) {
+        return $this->errorResponse([
+          APIError::UNKNOWN => __('There was a problem with your sending method. Please check if your sending method is properly configured.', 'mailpoet'),
+        ]);
       }
-      return $this->errorResponse([
-        APIError::UNKNOWN => __('There was a problem with your sending method. Please check if your sending method is properly configured.', 'mailpoet'),
-      ]);
     } else {
       return $this->errorResponse([
         APIError::NOT_FOUND => WPFunctions::get()->__('This subscriber does not exist.', 'mailpoet'),

lib/API/MP/v1/API.php

@@ -148,11 +148,13 @@ class API {
 
     // send confirmation email
     if ($sendConfirmationEmail) {
-      $result = $this->_sendConfirmationEmail($subscriber);
+      try {
-      if (!$result && $subscriber->getErrors()) {
+        $this->_sendConfirmationEmail($subscriber);
+      } catch (\Exception $e) {
         throw new APIException(
-          __(sprintf('Subscriber added to lists, but confirmation email failed to send: %s', strtolower(implode(', ', $subscriber->getErrors()))), 'mailpoet'),
+          __(sprintf('Subscriber added to lists, but confirmation email failed to send: %s', strtolower($e->getMessage())), 'mailpoet'),
-        APIException::CONFIRMATION_FAILED_TO_SEND);
+          APIException::CONFIRMATION_FAILED_TO_SEND
+        );
       }
     }
 

lib/Segments/WP.php

@@ -151,7 +151,11 @@ class WP {
         $confirmationEmailMailer = ContainerWrapper::getInstance()->get(ConfirmationEmailMailer::class);
         $subscriberEntity = $this->subscribersRepository->findOneById($subscriber->id);
         if ($subscriberEntity instanceof SubscriberEntity) {
-          $confirmationEmailMailer->sendConfirmationEmailOnce($subscriberEntity);
+          try {
+            $confirmationEmailMailer->sendConfirmationEmailOnce($subscriberEntity);
+          } catch (\Exception $e) {
+            // ignore errors
+          }
         }
       }
 

lib/Subscribers/ConfirmationEmailMailer.php

@@ -7,7 +7,6 @@ use MailPoet\Entities\SegmentEntity;
 use MailPoet\Entities\SubscriberEntity;
 use MailPoet\Mailer\Mailer;
 use MailPoet\Mailer\MetaInfo;
-use MailPoet\Models\Subscriber;
 use MailPoet\Services\AuthorizedEmailsController;
 use MailPoet\Services\Bridge;
 use MailPoet\Settings\SettingsController;
@@ -59,6 +58,7 @@ class ConfirmationEmailMailer {
    * Use this method if you want to make sure the confirmation email
    * is not sent multiple times within a single request
    * e.g. if sending confirmation emails from hooks
+   * @throws \Exception if unable to send the email.
    */
   public function sendConfirmationEmailOnce(SubscriberEntity $subscriber): bool {
     if (isset($this->sentEmails[$subscriber->getId()])) {
@@ -67,6 +67,9 @@ class ConfirmationEmailMailer {
     return $this->sendConfirmationEmail($subscriber);
   }
 
+  /**
+   * @throws \Exception if unable to send the email.
+   */
   public function sendConfirmationEmail(SubscriberEntity $subscriber) {
     $signupConfirmation = $this->settings->get('signup_confirmation');
     if ((bool)$signupConfirmation['enabled'] === false) {
@@ -119,29 +122,27 @@ class ConfirmationEmailMailer {
       ],
     ];
 
-    $subscriberModel = Subscriber::findOne($subscriber->getId());
-
     // send email
+    $extraParams = [
+      'meta' => $this->mailerMetaInfo->getConfirmationMetaInfo($subscriber),
+    ];
     try {
-      $extraParams = [
-        'meta' => $this->mailerMetaInfo->getConfirmationMetaInfo($subscriber),
-      ];
       $result = $this->mailer->send($email, $subscriber, $extraParams);
-      if ($result['response'] === false) {
-        $subscriberModel->setError(__('Something went wrong with your subscription. Please contact the website owner.', 'mailpoet'));
-        return false;
-      };
-
-      if (!$this->wp->isUserLoggedIn()) {
-        $subscriber->setConfirmationsCount($subscriber->getConfirmationsCount() + 1);
-        $this->subscribersRepository->persist($subscriber);
-        $this->subscribersRepository->flush();
-      }
-      $this->sentEmails[$subscriber->getId()] = true;
-      return true;
     } catch (\Exception $e) {
-      $subscriberModel->setError(__('Something went wrong with your subscription. Please contact the website owner.', 'mailpoet'));
+      throw new \Exception(__('Something went wrong with your subscription. Please contact the website owner.', 'mailpoet'));
-      return false;
     }
+
+    if ($result['response'] === false) {
+      throw new \Exception(__('Something went wrong with your subscription. Please contact the website owner.', 'mailpoet'));
+    };
+
+    if (!$this->wp->isUserLoggedIn()) {
+      $subscriber->setConfirmationsCount($subscriber->getConfirmationsCount() + 1);
+      $this->subscribersRepository->persist($subscriber);
+      $this->subscribersRepository->flush();
+    }
+    $this->sentEmails[$subscriber->getId()] = true;
+
+    return true;
   }
 }

lib/Subscribers/SubscriberActions.php

@@ -103,7 +103,12 @@ class SubscriberActions {
     // link subscriber to segments
     $segments = $this->segmentsRepository->findBy(['id' => $segmentIds]);
     $this->subscriberSegmentRepository->subscribeToSegments($subscriber, $segments);
-    $this->confirmationEmailMailer->sendConfirmationEmailOnce($subscriber);
+
+    try {
+      $this->confirmationEmailMailer->sendConfirmationEmailOnce($subscriber);
+    } catch (\Exception $e) {
+      // ignore errors
+    }
 
     $subscriberModel = Subscriber::findOne($subscriber->getId());
 

lib/WooCommerce/Subscription.php

@@ -225,7 +225,11 @@ class Subscription {
       $this->subscribersRepository->persist($subscriberEntity);
       $this->subscribersRepository->flush();
 
-      $this->confirmationEmailMailer->sendConfirmationEmailOnce($subscriberEntity);
+      try {
+        $this->confirmationEmailMailer->sendConfirmationEmailOnce($subscriberEntity);
+      } catch (\Exception $e) {
+        // ignore errors
+      }
     }
   }
 

tests/integration/API/MP/APITest.php

@@ -576,9 +576,8 @@ class APITest extends \MailPoetTest {
     $confirmationMailer = $this->createMock(ConfirmationEmailMailer::class);
     $confirmationMailer->expects($this->once())
       ->method('sendConfirmationEmailOnce')
-      ->willReturnCallback(function (Subscriber $subscriber) {
+      ->willReturnCallback(function () {
-        $subscriber->setError('Big Error');
+        throw new \Exception('Big Error');
-        return false;
       });
 
     $API = Stub::copy($this->getApi(), [

tests/integration/Subscribers/ConfirmationEmailMailerTest.php

@@ -95,7 +95,7 @@ class ConfirmationEmailMailerTest extends \MailPoetTest {
     expect($this->subscriber->getConfirmationsCount())->equals(1);
   }
 
-  public function testItReturnsFalseWhenConfirmationEmailCannotBeSent() {
+  public function testItThrowsExceptionWhenConfirmationEmailCannotBeSent() {
     $mailer = Stub::makeEmpty(Mailer::class, [
       'send' =>
         Stub\Expected::once(function () {
@@ -111,7 +111,27 @@ class ConfirmationEmailMailerTest extends \MailPoetTest {
       $this->diContainer->get(SubscriptionUrlFactory::class)
     );
 
-    $this->assertFalse($sender->sendConfirmationEmail($this->subscriber));
+    $this->expectException(\Exception::class);
+    $this->expectExceptionMessage(__('Something went wrong with your subscription. Please contact the website owner.', 'mailpoet'));
+    $sender->sendConfirmationEmail($this->subscriber);
+  }
+
+  public function testSendConfirmationEmailThrowsWhenSendReturnsFalse() {
+    $mailer = Stub::makeEmpty(Mailer::class, [
+      'send' => ['response' => false],
+    ], $this);
+
+    $sender = new ConfirmationEmailMailer(
+      $mailer,
+      $this->diContainer->get(WPFunctions::class),
+      $this->diContainer->get(SettingsController::class),
+      $this->diContainer->get(SubscribersRepository::class),
+      $this->diContainer->get(SubscriptionUrlFactory::class)
+    );
+
+    $this->expectException(\Exception::class);
+    $this->expectExceptionMessage(__('Something went wrong with your subscription. Please contact the website owner.', 'mailpoet'));
+    $sender->sendConfirmationEmail($this->subscriber);
   }
 
   public function testItDoesntSendWhenMSSIsActiveAndConfirmationEmailIsNotAuthorized() {