From c3251f4092b2e22dd51a23c2654679a8483b273f Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Tue, 9 Aug 2022 10:38:22 +0200 Subject: [PATCH] Refactor MP API unsubscribe from lists to doctrine [MAILPOET-4291] --- mailpoet/lib/API/MP/v1/API.php | 47 +------------------ mailpoet/lib/API/MP/v1/Subscribers.php | 45 ++++++++++++++---- mailpoet/tests/integration/API/MP/APITest.php | 11 ++++- 3 files changed, 48 insertions(+), 55 deletions(-) diff --git a/mailpoet/lib/API/MP/v1/API.php b/mailpoet/lib/API/MP/v1/API.php index 89e8cc04c4..f6b71d2804 100644 --- a/mailpoet/lib/API/MP/v1/API.php +++ b/mailpoet/lib/API/MP/v1/API.php @@ -3,9 +3,7 @@ namespace MailPoet\API\MP\v1; use MailPoet\Config\Changelog; -use MailPoet\Models\Segment; use MailPoet\Models\Subscriber; -use MailPoet\Models\SubscriberSegment; use MailPoet\Subscribers\RequiredCustomFieldValidator; use MailPoet\Subscribers\Source; use MailPoet\Util\Helpers; @@ -77,50 +75,7 @@ class API { } public function unsubscribeFromLists($subscriberId, array $listIds) { - if (empty($listIds)) { - throw new APIException(__('At least one segment ID is required.', 'mailpoet'), APIException::SEGMENT_REQUIRED); - } - - // throw exception when subscriber does not exist - $subscriber = Subscriber::findOne($subscriberId); - if (!$subscriber) { - throw new APIException(__('This subscriber does not exist.', 'mailpoet'), APIException::SUBSCRIBER_NOT_EXISTS); - } - - // throw exception when none of the segments exist - $foundSegments = Segment::whereIn('id', $listIds)->findMany(); - if (!$foundSegments) { - $exception = _n('This list does not exist.', 'These lists do not exist.', count($listIds), 'mailpoet'); - throw new APIException($exception, APIException::LIST_NOT_EXISTS); - } - - // throw exception when trying to subscribe to WP Users or WooCommerce Customers segments - $foundSegmentsIds = []; - foreach ($foundSegments as $segment) { - if ($segment->type === Segment::TYPE_WP_USERS) { - // translators: %d is the ID of the segment - throw new APIException(sprintf(__("Can't unsubscribe from a WordPress Users list with ID %d.", 'mailpoet'), $segment->id), APIException::SUBSCRIBING_TO_WP_LIST_NOT_ALLOWED); - } - if ($segment->type === Segment::TYPE_WC_USERS) { - // translators: %d is the ID of the segment - throw new APIException(sprintf(__("Can't unsubscribe from a WooCommerce Customers list with ID %d.", 'mailpoet'), $segment->id), APIException::SUBSCRIBING_TO_WC_LIST_NOT_ALLOWED); - } - $foundSegmentsIds[] = $segment->id; - } - - // throw an exception when one or more segments do not exist - if (count($foundSegmentsIds) !== count($listIds)) { - $missingIds = array_values(array_diff($listIds, $foundSegmentsIds)); - $exception = sprintf( - // translators: %s is a comma-separated list of list IDs. - _n('List with ID %s does not exist.', 'Lists with IDs %s do not exist.', count($missingIds), 'mailpoet'), - implode(', ', $missingIds) - ); - throw new APIException($exception, APIException::LIST_NOT_EXISTS); - } - - SubscriberSegment::unsubscribeFromSegments($subscriber, $foundSegmentsIds); - return $subscriber->withCustomFields()->withSubscriptions()->asArray(); + return $this->subscribers->unsubscribeFromLists($subscriberId, $listIds); } public function getLists(): array { diff --git a/mailpoet/lib/API/MP/v1/Subscribers.php b/mailpoet/lib/API/MP/v1/Subscribers.php index 0278f4a9f2..17ebaef3ee 100644 --- a/mailpoet/lib/API/MP/v1/Subscribers.php +++ b/mailpoet/lib/API/MP/v1/Subscribers.php @@ -17,6 +17,9 @@ use MailPoet\Tasks\Sending; use MailPoet\WP\Functions as WPFunctions; class Subscribers { + const CONTEXT_SUBSCRIBE = 'subscribe'; + const CONTEXT_UNSUBSCRIBE = 'unsubscribe'; + /** @var SettingsController */ private $settings; @@ -85,7 +88,7 @@ class Subscribers { $signupConfirmationEnabled = (bool)$this->settings->get('signup_confirmation.enabled'); $subscriber = $this->findSubscriber($subscriberId); - $foundSegments = $this->getAndValidateSegments($listIds); + $foundSegments = $this->getAndValidateSegments($listIds, self::CONTEXT_SUBSCRIBE); $this->subscribersSegmentRepository->subscribeToSegments($subscriber, $foundSegments); @@ -143,6 +146,14 @@ class Subscribers { return $this->subscribersResponseBuilder->build($subscriber); } + public function unsubscribeFromLists($subscriberIdOrEmail, array $listIds): array { + $subscriber = $this->findSubscriber($subscriberIdOrEmail); + $foundSegments = $this->getAndValidateSegments($listIds, self::CONTEXT_UNSUBSCRIBE); + $this->subscribersSegmentRepository->unsubscribeFromSegments($subscriber, $foundSegments); + + return $this->subscribersResponseBuilder->build($subscriber); + } + /** * @throws APIException */ @@ -202,7 +213,7 @@ class Subscribers { * @return SegmentEntity[] * @throws APIException */ - private function getAndValidateSegments(array $listIds): array { + private function getAndValidateSegments(array $listIds, string $context = self::CONTEXT_SUBSCRIBE): array { if (empty($listIds)) { throw new APIException(__('At least one segment ID is required.', 'mailpoet'), APIException::SEGMENT_REQUIRED); } @@ -218,16 +229,34 @@ class Subscribers { $foundSegmentsIds = []; foreach ($foundSegments as $foundSegment) { if ($foundSegment->getType() === SegmentEntity::TYPE_WP_USERS) { - // translators: %d is the ID of the segment - throw new APIException(sprintf(__("Can't subscribe to a WordPress Users list with ID %d.", 'mailpoet'), $foundSegment->getId()), APIException::SUBSCRIBING_TO_WP_LIST_NOT_ALLOWED); + if ($context === self::CONTEXT_SUBSCRIBE) { + // translators: %d is the ID of the segment + $message = __("Can't subscribe to a WordPress Users list with ID %d.", 'mailpoet'); + } else { + // translators: %d is the ID of the segment + $message = __("Can't unsubscribe from a WordPress Users list with ID %d.", 'mailpoet'); + } + throw new APIException(sprintf($message, $foundSegment->getId()), APIException::SUBSCRIBING_TO_WP_LIST_NOT_ALLOWED); } if ($foundSegment->getType() === SegmentEntity::TYPE_WC_USERS) { - // translators: %d is the ID of the segment - throw new APIException(sprintf(__("Can't subscribe to a WooCommerce Customers list with ID %d.", 'mailpoet'), $foundSegment->getId()), APIException::SUBSCRIBING_TO_WC_LIST_NOT_ALLOWED); + if ($context === self::CONTEXT_SUBSCRIBE) { + // translators: %d is the ID of the segment + $message = __("Can't subscribe to a WooCommerce Customers list with ID %d.", 'mailpoet'); + } else { + // translators: %d is the ID of the segment + $message = __("Can't unsubscribe from a WooCommerce Customers list with ID %d.", 'mailpoet'); + } + throw new APIException(sprintf($message, $foundSegment->getId()), APIException::SUBSCRIBING_TO_WC_LIST_NOT_ALLOWED); } if ($foundSegment->getType() !== SegmentEntity::TYPE_DEFAULT) { - // translators: %d is the ID of the segment - throw new APIException(sprintf(__("Can't subscribe to a list with ID %d.", 'mailpoet'), $foundSegment->getId()), APIException::SUBSCRIBING_TO_LIST_NOT_ALLOWED); + if ($context === self::CONTEXT_SUBSCRIBE) { + // translators: %d is the ID of the segment + $message = __("Can't subscribe to a list with ID %d.", 'mailpoet'); + } else { + // translators: %d is the ID of the segment + $message = __("Can't unsubscribe from a list with ID %d.", 'mailpoet'); + } + throw new APIException(sprintf($message, $foundSegment->getId()), APIException::SUBSCRIBING_TO_LIST_NOT_ALLOWED); } $foundSegmentsIds[] = $foundSegment->getId(); } diff --git a/mailpoet/tests/integration/API/MP/APITest.php b/mailpoet/tests/integration/API/MP/APITest.php index f5353e7e7f..7762f6b2a3 100644 --- a/mailpoet/tests/integration/API/MP/APITest.php +++ b/mailpoet/tests/integration/API/MP/APITest.php @@ -392,10 +392,19 @@ class APITest extends \MailPoetTest { $API->addSubscriber($subscriber, $segments, $options); } - public function testItDoesNotUnsubscribeMissingSubscriberFromLists() { + public function testItDoesNotUnsubscribeWhenSubscriberIdNotPasssedFromLists() { try { $this->getApi()->unsubscribeFromLists(false, [1,2,3]); $this->fail('Subscriber does not exist exception should have been thrown.'); + } catch (\Exception $e) { + expect($e->getMessage())->equals('A subscriber is required.'); + } + } + + public function testItDoesNotUnsubscribeMissingSubscriberFromLists() { + try { + $this->getApi()->unsubscribeFromLists('asdf', [1,2,3]); + $this->fail('Subscriber does not exist exception should have been thrown.'); } catch (\Exception $e) { expect($e->getMessage())->equals('This subscriber does not exist.'); }