diff --git a/lib/API/JSON/v1/Subscribers.php b/lib/API/JSON/v1/Subscribers.php index e093070447..096bf20fdc 100644 --- a/lib/API/JSON/v1/Subscribers.php +++ b/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)) { - return $this->successResponse(); + try { + 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'), diff --git a/lib/API/MP/v1/API.php b/lib/API/MP/v1/API.php index 602a1e9da4..4a4f4d6442 100644 --- a/lib/API/MP/v1/API.php +++ b/lib/API/MP/v1/API.php @@ -148,11 +148,13 @@ class API { // send confirmation email if ($sendConfirmationEmail) { - $result = $this->_sendConfirmationEmail($subscriber); - if (!$result && $subscriber->getErrors()) { + try { + $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'), - APIException::CONFIRMATION_FAILED_TO_SEND); + __(sprintf('Subscriber added to lists, but confirmation email failed to send: %s', strtolower($e->getMessage())), 'mailpoet'), + APIException::CONFIRMATION_FAILED_TO_SEND + ); } } diff --git a/lib/Segments/WP.php b/lib/Segments/WP.php index 82b755089a..7c43edc181 100644 --- a/lib/Segments/WP.php +++ b/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 + } } } diff --git a/lib/Subscribers/ConfirmationEmailMailer.php b/lib/Subscribers/ConfirmationEmailMailer.php index ae43c5f864..41090b542f 100644 --- a/lib/Subscribers/ConfirmationEmailMailer.php +++ b/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')); - return false; + throw new \Exception(__('Something went wrong with your subscription. Please contact the website owner.', 'mailpoet')); } + + 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; } } diff --git a/lib/Subscribers/SubscriberActions.php b/lib/Subscribers/SubscriberActions.php index 7284cd6d6e..8c8c18ca5c 100644 --- a/lib/Subscribers/SubscriberActions.php +++ b/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()); diff --git a/lib/WooCommerce/Subscription.php b/lib/WooCommerce/Subscription.php index 6e5eccaf2e..ec0757cf11 100644 --- a/lib/WooCommerce/Subscription.php +++ b/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 + } } } diff --git a/tests/integration/API/MP/APITest.php b/tests/integration/API/MP/APITest.php index c00d11ebdf..14b5b12b4a 100644 --- a/tests/integration/API/MP/APITest.php +++ b/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) { - $subscriber->setError('Big Error'); - return false; + ->willReturnCallback(function () { + throw new \Exception('Big Error'); }); $API = Stub::copy($this->getApi(), [ diff --git a/tests/integration/Subscribers/ConfirmationEmailMailerTest.php b/tests/integration/Subscribers/ConfirmationEmailMailerTest.php index b7358e0de0..06ee329a77 100644 --- a/tests/integration/Subscribers/ConfirmationEmailMailerTest.php +++ b/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() {