From ee83e4d7487c082843f1271df17d01b100aa036a Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Tue, 29 Nov 2022 15:41:23 +0100 Subject: [PATCH] Refactor SubscriberActions:subscribe to return status of confirmation email We need to pass the error or success info about the confirmation email in order to be able to be able to display the error message. [MAILPOET-4736] --- mailpoet/lib/Models/Subscriber.php | 3 ++- mailpoet/lib/Subscribers/SubscriberActions.php | 14 ++++++++++---- .../SubscriberSubscribeController.php | 2 +- mailpoet/lib/Subscription/Comment.php | 2 +- .../Subscribers/SubscriberActionsTest.php | 16 ++++++++-------- .../SubscriberSubscribeControllerUnitTest.php | 2 +- 6 files changed, 23 insertions(+), 16 deletions(-) diff --git a/mailpoet/lib/Models/Subscriber.php b/mailpoet/lib/Models/Subscriber.php index 6a50d1d1e6..b19ab67d95 100644 --- a/mailpoet/lib/Models/Subscriber.php +++ b/mailpoet/lib/Models/Subscriber.php @@ -700,6 +700,7 @@ class Subscriber extends Model { public static function subscribe($subscriberData = [], $segmentIds = []) { trigger_error('Calling Subscriber::subscribe() is deprecated and will be removed. Use MailPoet\API\MP\v1\API instead. ', E_USER_DEPRECATED); $service = ContainerWrapper::getInstance()->get(\MailPoet\Subscribers\SubscriberActions::class); - return $service->subscribe($subscriberData, $segmentIds); + [$subscriber] = $service->subscribe($subscriberData, $segmentIds); + return $subscriber; } } diff --git a/mailpoet/lib/Subscribers/SubscriberActions.php b/mailpoet/lib/Subscribers/SubscriberActions.php index e2370f4580..6887f033c6 100644 --- a/mailpoet/lib/Subscribers/SubscriberActions.php +++ b/mailpoet/lib/Subscribers/SubscriberActions.php @@ -54,7 +54,11 @@ class SubscriberActions { $this->segmentsRepository = $segmentsRepository; } - public function subscribe($subscriberData = [], $segmentIds = []): SubscriberEntity { + /** + * Returns SubscriberEntity and associative array with some metadata related to the subscription (e.g. ['confirmationEmailResult' => $exception]) + * @return array{0: SubscriberEntity, 1: array{confirmationEmailResult: bool|\Exception}} + */ + public function subscribe($subscriberData = [], $segmentIds = []): array { // filter out keys from the subscriber_data array // that should not be editable when subscribing $subscriberData = $this->subscriberSaveController->filterOutReservedColumns($subscriberData); @@ -98,14 +102,16 @@ class SubscriberActions { } $this->subscribersRepository->flush(); + + $metaData = ['confirmationEmailResult' => false]; // link subscriber to segments $segments = $this->segmentsRepository->findBy(['id' => $segmentIds]); $this->subscriberSegmentRepository->subscribeToSegments($subscriber, $segments); try { - $this->confirmationEmailMailer->sendConfirmationEmailOnce($subscriber); + $metaData['confirmationEmailResult'] = $this->confirmationEmailMailer->sendConfirmationEmailOnce($subscriber); } catch (\Exception $e) { - // ignore errors + $metaData['confirmationEmailResult'] = $e; } // We want to send the notification on subscribe only when signupConfirmation is disabled @@ -118,6 +124,6 @@ class SubscriberActions { ); } - return $subscriber; + return [$subscriber, $metaData]; } } diff --git a/mailpoet/lib/Subscribers/SubscriberSubscribeController.php b/mailpoet/lib/Subscribers/SubscriberSubscribeController.php index 7ea02bfade..406c2856e2 100644 --- a/mailpoet/lib/Subscribers/SubscriberSubscribeController.php +++ b/mailpoet/lib/Subscribers/SubscriberSubscribeController.php @@ -151,7 +151,7 @@ class SubscriberSubscribeController { */ $this->wp->doAction('mailpoet_subscription_before_subscribe', $data, $segmentIds, $form); - $subscriber = $this->subscriberActions->subscribe($data, $segmentIds); + [$subscriber] = $this->subscriberActions->subscribe($data, $segmentIds); if (!empty($captchaSettings['type']) && $captchaSettings['type'] === CaptchaConstants::TYPE_BUILTIN) { // Captcha has been verified, invalidate the session vars diff --git a/mailpoet/lib/Subscription/Comment.php b/mailpoet/lib/Subscription/Comment.php index 1246930396..1555f1e4d7 100644 --- a/mailpoet/lib/Subscription/Comment.php +++ b/mailpoet/lib/Subscription/Comment.php @@ -106,7 +106,7 @@ class Comment { if (!empty($segmentIds)) { $comment = WPFunctions::get()->getComment($commentId); - $result = $this->subscriberActions->subscribe( + $this->subscriberActions->subscribe( [ 'email' => $comment->comment_author_email, // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps 'first_name' => $comment->comment_author, // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps diff --git a/mailpoet/tests/integration/Subscribers/SubscriberActionsTest.php b/mailpoet/tests/integration/Subscribers/SubscriberActionsTest.php index bfb167a13c..75570b64f7 100644 --- a/mailpoet/tests/integration/Subscribers/SubscriberActionsTest.php +++ b/mailpoet/tests/integration/Subscribers/SubscriberActionsTest.php @@ -55,7 +55,7 @@ class SubscriberActionsTest extends \MailPoetTest { $segment = $this->segmentsRepository->createOrUpdate('List #1'); $segment2 = $this->segmentsRepository->createOrUpdate('List #2'); - $subscriber = $this->subscriberActions->subscribe( + [$subscriber] = $this->subscriberActions->subscribe( $this->testData, [$segment->getId(), $segment2->getId()] ); @@ -86,7 +86,7 @@ class SubscriberActionsTest extends \MailPoetTest { (new NewsletterOption())->createMultipleOptions($newsletter, $newsletterOptions); $this->settings->set('signup_confirmation.enabled', false); - $subscriber = $this->subscriberActions->subscribe($this->testData, [$segment->getId()]); + [$subscriber] = $this->subscriberActions->subscribe($this->testData, [$segment->getId()]); expect($subscriber->getId() > 0)->equals(true); expect($subscriber->getSegments())->count(1); @@ -113,7 +113,7 @@ class SubscriberActionsTest extends \MailPoetTest { (new NewsletterOption())->createMultipleOptions($newsletter, $newsletterOptions); $this->settings->set('signup_confirmation.enabled', true); - $subscriber = $this->subscriberActions->subscribe($this->testData, [$segment->getId()]); + [$subscriber] = $this->subscriberActions->subscribe($this->testData, [$segment->getId()]); expect($subscriber->getId() > 0)->equals(true); expect($subscriber->getSegments())->count(1); $scheduledNotification = $this->sendingQueuesRepository->findOneByNewsletterAndTaskStatus( @@ -126,7 +126,7 @@ class SubscriberActionsTest extends \MailPoetTest { public function testItCannotSubscribeWithReservedColumns() { $segment = $this->segmentsRepository->createOrUpdate('List #1'); - $subscriber = $this->subscriberActions->subscribe( + [$subscriber] = $this->subscriberActions->subscribe( [ 'email' => 'donald@mailpoet.com', 'first_name' => 'Donald', @@ -174,7 +174,7 @@ class SubscriberActionsTest extends \MailPoetTest { 'last_name' => 'Example', ]; - $subscriber = $this->subscriberActions->subscribe( + [$subscriber] = $this->subscriberActions->subscribe( $data, [$segment->getId()] ); @@ -189,7 +189,7 @@ class SubscriberActionsTest extends \MailPoetTest { $data2['first_name'] = 'Aaa'; $data2['last_name'] = 'Bbb'; - $subscriber = $this->subscriberActions->subscribe( + [$subscriber] = $this->subscriberActions->subscribe( $data2, [$segment->getId(), $segment2->getId()] ); @@ -216,7 +216,7 @@ class SubscriberActionsTest extends \MailPoetTest { 'last_name' => 'Example', ]; - $subscriber = $this->subscriberActions->subscribe( + [$subscriber] = $this->subscriberActions->subscribe( $data, [$segment->getId()] ); @@ -233,7 +233,7 @@ class SubscriberActionsTest extends \MailPoetTest { $data2['first_name'] = 'Aaa'; $data2['last_name'] = 'Bbb'; - $subscriber = $this->subscriberActions->subscribe( + [$subscriber] = $this->subscriberActions->subscribe( $data2, [$segment->getId(), $segment2->getId()] ); diff --git a/mailpoet/tests/unit/Subscribers/SubscriberSubscribeControllerUnitTest.php b/mailpoet/tests/unit/Subscribers/SubscriberSubscribeControllerUnitTest.php index cbded2d8ea..9cc783d20e 100644 --- a/mailpoet/tests/unit/Subscribers/SubscriberSubscribeControllerUnitTest.php +++ b/mailpoet/tests/unit/Subscribers/SubscriberSubscribeControllerUnitTest.php @@ -659,7 +659,7 @@ class SubscriberSubscribeControllerUnitTest extends \MailPoetUnitTest { expect($receivedData)->equals($formFields); expect($receivedSegmentIds)->equals($segmentIds); - return $subscriber; + return [$subscriber, ['confirmationEmailResult' => true]]; }, ], $this