diff --git a/mailpoet/lib/API/MP/v1/Subscribers.php b/mailpoet/lib/API/MP/v1/Subscribers.php index 17ebaef3ee..45862bf46a 100644 --- a/mailpoet/lib/API/MP/v1/Subscribers.php +++ b/mailpoet/lib/API/MP/v1/Subscribers.php @@ -87,6 +87,7 @@ class Subscribers { $skipSubscriberNotification = isset($options['skip_subscriber_notification']) && $options['skip_subscriber_notification'] === true; $signupConfirmationEnabled = (bool)$this->settings->get('signup_confirmation.enabled'); + $this->checkSubscriberAndListParams($subscriberId, $listIds); $subscriber = $this->findSubscriber($subscriberId); $foundSegments = $this->getAndValidateSegments($listIds, self::CONTEXT_SUBSCRIBE); @@ -147,6 +148,7 @@ class Subscribers { } public function unsubscribeFromLists($subscriberIdOrEmail, array $listIds): array { + $this->checkSubscriberAndListParams($subscriberIdOrEmail, $listIds); $subscriber = $this->findSubscriber($subscriberIdOrEmail); $foundSegments = $this->getAndValidateSegments($listIds, self::CONTEXT_UNSUBSCRIBE); $this->subscribersSegmentRepository->unsubscribeFromSegments($subscriber, $foundSegments); @@ -190,10 +192,19 @@ class Subscribers { /** * @throws APIException */ - private function findSubscriber($subscriberIdOrEmail): SubscriberEntity { + private function checkSubscriberAndListParams($subscriberIdOrEmail, array $listIds): void { + if (empty($listIds)) { + throw new APIException(__('At least one segment ID is required.', 'mailpoet'), APIException::SEGMENT_REQUIRED); + } if (empty($subscriberIdOrEmail)) { throw new APIException(__('A subscriber is required.', 'mailpoet'), APIException::SUBSCRIBER_NOT_EXISTS); } + } + + /** + * @throws APIException + */ + private function findSubscriber($subscriberIdOrEmail): SubscriberEntity { // throw exception when subscriber does not exist $subscriber = null; if (is_int($subscriberIdOrEmail) || (string)(int)$subscriberIdOrEmail === $subscriberIdOrEmail) { @@ -214,10 +225,6 @@ class Subscribers { * @throws APIException */ 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); - } - // throw exception when none of the segments exist $foundSegments = $this->segmentsRepository->findBy(['id' => $listIds]); if (!$foundSegments) { diff --git a/mailpoet/tests/integration/API/MP/SubscribersTest.php b/mailpoet/tests/integration/API/MP/SubscribersTest.php index 8eb5ba7bfd..ea6a47325c 100644 --- a/mailpoet/tests/integration/API/MP/SubscribersTest.php +++ b/mailpoet/tests/integration/API/MP/SubscribersTest.php @@ -374,6 +374,24 @@ class SubscribersTest extends \MailPoetTest { expect($result['subscriptions'][0]['status'])->equals(SubscriberEntity::STATUS_UNSUBSCRIBED); } + public function testItChecksEmptyParamsInCorrectOrder() { + // test if segments are specified + try { + $this->getApi()->unsubscribeFromLists(null, []); + $this->fail('Segments are required exception should have been thrown.'); + } catch (\Exception $e) { + expect($e->getMessage())->equals('At least one segment ID is required.'); + } + + // test if segments are specified + try { + $this->getApi()->unsubscribeFromLists(null, [1]); + $this->fail('Subscriber is required exception should have been thrown.'); + } catch (\Exception $e) { + expect($e->getMessage())->equals('A subscriber is required.'); + } + } + public function _after() { $this->truncateEntity(SubscriberEntity::class); $this->truncateEntity(SegmentEntity::class);