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]
This commit is contained in:
Rostislav Wolny
2022-11-29 15:41:23 +01:00
committed by Aschepikov
parent 970db8f0d8
commit ee83e4d748
6 changed files with 23 additions and 16 deletions

View File

@ -700,6 +700,7 @@ class Subscriber extends Model {
public static function subscribe($subscriberData = [], $segmentIds = []) { 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); 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); $service = ContainerWrapper::getInstance()->get(\MailPoet\Subscribers\SubscriberActions::class);
return $service->subscribe($subscriberData, $segmentIds); [$subscriber] = $service->subscribe($subscriberData, $segmentIds);
return $subscriber;
} }
} }

View File

@ -54,7 +54,11 @@ class SubscriberActions {
$this->segmentsRepository = $segmentsRepository; $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 // filter out keys from the subscriber_data array
// that should not be editable when subscribing // that should not be editable when subscribing
$subscriberData = $this->subscriberSaveController->filterOutReservedColumns($subscriberData); $subscriberData = $this->subscriberSaveController->filterOutReservedColumns($subscriberData);
@ -98,14 +102,16 @@ class SubscriberActions {
} }
$this->subscribersRepository->flush(); $this->subscribersRepository->flush();
$metaData = ['confirmationEmailResult' => false];
// link subscriber to segments // link subscriber to segments
$segments = $this->segmentsRepository->findBy(['id' => $segmentIds]); $segments = $this->segmentsRepository->findBy(['id' => $segmentIds]);
$this->subscriberSegmentRepository->subscribeToSegments($subscriber, $segments); $this->subscriberSegmentRepository->subscribeToSegments($subscriber, $segments);
try { try {
$this->confirmationEmailMailer->sendConfirmationEmailOnce($subscriber); $metaData['confirmationEmailResult'] = $this->confirmationEmailMailer->sendConfirmationEmailOnce($subscriber);
} catch (\Exception $e) { } catch (\Exception $e) {
// ignore errors $metaData['confirmationEmailResult'] = $e;
} }
// We want to send the notification on subscribe only when signupConfirmation is disabled // We want to send the notification on subscribe only when signupConfirmation is disabled
@ -118,6 +124,6 @@ class SubscriberActions {
); );
} }
return $subscriber; return [$subscriber, $metaData];
} }
} }

View File

@ -151,7 +151,7 @@ class SubscriberSubscribeController {
*/ */
$this->wp->doAction('mailpoet_subscription_before_subscribe', $data, $segmentIds, $form); $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) { if (!empty($captchaSettings['type']) && $captchaSettings['type'] === CaptchaConstants::TYPE_BUILTIN) {
// Captcha has been verified, invalidate the session vars // Captcha has been verified, invalidate the session vars

View File

@ -106,7 +106,7 @@ class Comment {
if (!empty($segmentIds)) { if (!empty($segmentIds)) {
$comment = WPFunctions::get()->getComment($commentId); $comment = WPFunctions::get()->getComment($commentId);
$result = $this->subscriberActions->subscribe( $this->subscriberActions->subscribe(
[ [
'email' => $comment->comment_author_email, // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps 'email' => $comment->comment_author_email, // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps
'first_name' => $comment->comment_author, // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps 'first_name' => $comment->comment_author, // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps

View File

@ -55,7 +55,7 @@ class SubscriberActionsTest extends \MailPoetTest {
$segment = $this->segmentsRepository->createOrUpdate('List #1'); $segment = $this->segmentsRepository->createOrUpdate('List #1');
$segment2 = $this->segmentsRepository->createOrUpdate('List #2'); $segment2 = $this->segmentsRepository->createOrUpdate('List #2');
$subscriber = $this->subscriberActions->subscribe( [$subscriber] = $this->subscriberActions->subscribe(
$this->testData, $this->testData,
[$segment->getId(), $segment2->getId()] [$segment->getId(), $segment2->getId()]
); );
@ -86,7 +86,7 @@ class SubscriberActionsTest extends \MailPoetTest {
(new NewsletterOption())->createMultipleOptions($newsletter, $newsletterOptions); (new NewsletterOption())->createMultipleOptions($newsletter, $newsletterOptions);
$this->settings->set('signup_confirmation.enabled', false); $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->getId() > 0)->equals(true);
expect($subscriber->getSegments())->count(1); expect($subscriber->getSegments())->count(1);
@ -113,7 +113,7 @@ class SubscriberActionsTest extends \MailPoetTest {
(new NewsletterOption())->createMultipleOptions($newsletter, $newsletterOptions); (new NewsletterOption())->createMultipleOptions($newsletter, $newsletterOptions);
$this->settings->set('signup_confirmation.enabled', true); $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->getId() > 0)->equals(true);
expect($subscriber->getSegments())->count(1); expect($subscriber->getSegments())->count(1);
$scheduledNotification = $this->sendingQueuesRepository->findOneByNewsletterAndTaskStatus( $scheduledNotification = $this->sendingQueuesRepository->findOneByNewsletterAndTaskStatus(
@ -126,7 +126,7 @@ class SubscriberActionsTest extends \MailPoetTest {
public function testItCannotSubscribeWithReservedColumns() { public function testItCannotSubscribeWithReservedColumns() {
$segment = $this->segmentsRepository->createOrUpdate('List #1'); $segment = $this->segmentsRepository->createOrUpdate('List #1');
$subscriber = $this->subscriberActions->subscribe( [$subscriber] = $this->subscriberActions->subscribe(
[ [
'email' => 'donald@mailpoet.com', 'email' => 'donald@mailpoet.com',
'first_name' => 'Donald', 'first_name' => 'Donald',
@ -174,7 +174,7 @@ class SubscriberActionsTest extends \MailPoetTest {
'last_name' => 'Example', 'last_name' => 'Example',
]; ];
$subscriber = $this->subscriberActions->subscribe( [$subscriber] = $this->subscriberActions->subscribe(
$data, $data,
[$segment->getId()] [$segment->getId()]
); );
@ -189,7 +189,7 @@ class SubscriberActionsTest extends \MailPoetTest {
$data2['first_name'] = 'Aaa'; $data2['first_name'] = 'Aaa';
$data2['last_name'] = 'Bbb'; $data2['last_name'] = 'Bbb';
$subscriber = $this->subscriberActions->subscribe( [$subscriber] = $this->subscriberActions->subscribe(
$data2, $data2,
[$segment->getId(), $segment2->getId()] [$segment->getId(), $segment2->getId()]
); );
@ -216,7 +216,7 @@ class SubscriberActionsTest extends \MailPoetTest {
'last_name' => 'Example', 'last_name' => 'Example',
]; ];
$subscriber = $this->subscriberActions->subscribe( [$subscriber] = $this->subscriberActions->subscribe(
$data, $data,
[$segment->getId()] [$segment->getId()]
); );
@ -233,7 +233,7 @@ class SubscriberActionsTest extends \MailPoetTest {
$data2['first_name'] = 'Aaa'; $data2['first_name'] = 'Aaa';
$data2['last_name'] = 'Bbb'; $data2['last_name'] = 'Bbb';
$subscriber = $this->subscriberActions->subscribe( [$subscriber] = $this->subscriberActions->subscribe(
$data2, $data2,
[$segment->getId(), $segment2->getId()] [$segment->getId(), $segment2->getId()]
); );

View File

@ -659,7 +659,7 @@ class SubscriberSubscribeControllerUnitTest extends \MailPoetUnitTest {
expect($receivedData)->equals($formFields); expect($receivedData)->equals($formFields);
expect($receivedSegmentIds)->equals($segmentIds); expect($receivedSegmentIds)->equals($segmentIds);
return $subscriber; return [$subscriber, ['confirmationEmailResult' => true]];
}, },
], ],
$this $this