Change order of MP API subscribe/unsubscribe calls checks to be backward compatible
In the previous version we checked the lists parameter before the subscriber. [MAILPOET-4291]
This commit is contained in:
committed by
Veljko V
parent
877fd9d7da
commit
5d3c851d02
@ -87,6 +87,7 @@ class Subscribers {
|
|||||||
$skipSubscriberNotification = isset($options['skip_subscriber_notification']) && $options['skip_subscriber_notification'] === true;
|
$skipSubscriberNotification = isset($options['skip_subscriber_notification']) && $options['skip_subscriber_notification'] === true;
|
||||||
$signupConfirmationEnabled = (bool)$this->settings->get('signup_confirmation.enabled');
|
$signupConfirmationEnabled = (bool)$this->settings->get('signup_confirmation.enabled');
|
||||||
|
|
||||||
|
$this->checkSubscriberAndListParams($subscriberId, $listIds);
|
||||||
$subscriber = $this->findSubscriber($subscriberId);
|
$subscriber = $this->findSubscriber($subscriberId);
|
||||||
$foundSegments = $this->getAndValidateSegments($listIds, self::CONTEXT_SUBSCRIBE);
|
$foundSegments = $this->getAndValidateSegments($listIds, self::CONTEXT_SUBSCRIBE);
|
||||||
|
|
||||||
@ -147,6 +148,7 @@ class Subscribers {
|
|||||||
}
|
}
|
||||||
|
|
||||||
public function unsubscribeFromLists($subscriberIdOrEmail, array $listIds): array {
|
public function unsubscribeFromLists($subscriberIdOrEmail, array $listIds): array {
|
||||||
|
$this->checkSubscriberAndListParams($subscriberIdOrEmail, $listIds);
|
||||||
$subscriber = $this->findSubscriber($subscriberIdOrEmail);
|
$subscriber = $this->findSubscriber($subscriberIdOrEmail);
|
||||||
$foundSegments = $this->getAndValidateSegments($listIds, self::CONTEXT_UNSUBSCRIBE);
|
$foundSegments = $this->getAndValidateSegments($listIds, self::CONTEXT_UNSUBSCRIBE);
|
||||||
$this->subscribersSegmentRepository->unsubscribeFromSegments($subscriber, $foundSegments);
|
$this->subscribersSegmentRepository->unsubscribeFromSegments($subscriber, $foundSegments);
|
||||||
@ -190,10 +192,19 @@ class Subscribers {
|
|||||||
/**
|
/**
|
||||||
* @throws APIException
|
* @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)) {
|
if (empty($subscriberIdOrEmail)) {
|
||||||
throw new APIException(__('A subscriber is required.', 'mailpoet'), APIException::SUBSCRIBER_NOT_EXISTS);
|
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
|
// throw exception when subscriber does not exist
|
||||||
$subscriber = null;
|
$subscriber = null;
|
||||||
if (is_int($subscriberIdOrEmail) || (string)(int)$subscriberIdOrEmail === $subscriberIdOrEmail) {
|
if (is_int($subscriberIdOrEmail) || (string)(int)$subscriberIdOrEmail === $subscriberIdOrEmail) {
|
||||||
@ -214,10 +225,6 @@ class Subscribers {
|
|||||||
* @throws APIException
|
* @throws APIException
|
||||||
*/
|
*/
|
||||||
private function getAndValidateSegments(array $listIds, string $context = self::CONTEXT_SUBSCRIBE): 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);
|
|
||||||
}
|
|
||||||
|
|
||||||
// throw exception when none of the segments exist
|
// throw exception when none of the segments exist
|
||||||
$foundSegments = $this->segmentsRepository->findBy(['id' => $listIds]);
|
$foundSegments = $this->segmentsRepository->findBy(['id' => $listIds]);
|
||||||
if (!$foundSegments) {
|
if (!$foundSegments) {
|
||||||
|
@ -374,6 +374,24 @@ class SubscribersTest extends \MailPoetTest {
|
|||||||
expect($result['subscriptions'][0]['status'])->equals(SubscriberEntity::STATUS_UNSUBSCRIBED);
|
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() {
|
public function _after() {
|
||||||
$this->truncateEntity(SubscriberEntity::class);
|
$this->truncateEntity(SubscriberEntity::class);
|
||||||
$this->truncateEntity(SegmentEntity::class);
|
$this->truncateEntity(SegmentEntity::class);
|
||||||
|
Reference in New Issue
Block a user