diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Links.php b/lib/Cron/Workers/SendingQueue/Tasks/Links.php index a41bf02f90..8258c6fac7 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Links.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Links.php @@ -63,7 +63,7 @@ class Links { ); } else { $subscriptionUrlFactory = SubscriptionUrlFactory::getInstance(); - $url = $subscriptionUrlFactory->getUnsubscribeUrl($subscriber); + $url = $subscriptionUrlFactory->getUnsubscribeUrl($subscriber, $queue->id); } return $url; } diff --git a/lib/Newsletter/Shortcodes/Categories/Link.php b/lib/Newsletter/Shortcodes/Categories/Link.php index c8dd03a7d6..2d8f3baef5 100644 --- a/lib/Newsletter/Shortcodes/Categories/Link.php +++ b/lib/Newsletter/Shortcodes/Categories/Link.php @@ -4,8 +4,8 @@ namespace MailPoet\Newsletter\Shortcodes\Categories; use MailPoet\Newsletter\Url as NewsletterUrl; use MailPoet\Settings\SettingsController; -use MailPoet\Statistics\Track\Unsubscribes; use MailPoet\Subscription\SubscriptionUrlFactory; +use MailPoet\Tasks\Sending; use MailPoet\WP\Functions as WPFunctions; class Link { @@ -20,11 +20,12 @@ class Link { $wpUserPreview ) { $subscriptionUrlFactory = SubscriptionUrlFactory::getInstance(); + switch ($shortcodeDetails['action']) { case 'subscription_unsubscribe_url': return self::processUrl( $shortcodeDetails['action'], - $subscriptionUrlFactory->getConfirmUnsubscribeUrl($wpUserPreview ? null : $subscriber), + $subscriptionUrlFactory->getConfirmUnsubscribeUrl($wpUserPreview ? null : $subscriber, self::getSendingQueueId($queue)), $queue, $wpUserPreview ); @@ -32,7 +33,7 @@ class Link { case 'subscription_instant_unsubscribe_url': return self::processUrl( $shortcodeDetails['action'], - $subscriptionUrlFactory->getUnsubscribeUrl($wpUserPreview ? null : $subscriber), + $subscriptionUrlFactory->getUnsubscribeUrl($wpUserPreview ? null : $subscriber, self::getSendingQueueId($queue)), $queue, $wpUserPreview ); @@ -84,10 +85,10 @@ class Link { $subscriptionUrlFactory = SubscriptionUrlFactory::getInstance(); switch ($shortcodeAction) { case 'subscription_unsubscribe_url': - $url = $subscriptionUrlFactory->getConfirmUnsubscribeUrl($subscriber); + $url = $subscriptionUrlFactory->getConfirmUnsubscribeUrl($subscriber, self::getSendingQueueId($queue)); break; case 'subscription_instant_unsubscribe_url': - $url = $subscriptionUrlFactory->getUnsubscribeUrl($subscriber); + $url = $subscriptionUrlFactory->getUnsubscribeUrl($subscriber, self::getSendingQueueId($queue)); break; case 'subscription_manage_url': $url = $subscriptionUrlFactory->getManageUrl($subscriber); @@ -120,6 +121,13 @@ class Link { return sprintf('[link:%s]', $action); } + /** + * @return int|null + */ + private static function getSendingQueueId($queue) { + if ($queue instanceof Sending) { + return (int)$queue->id; } + return null; } } diff --git a/lib/Subscription/SubscriptionUrlFactory.php b/lib/Subscription/SubscriptionUrlFactory.php index 154393d2b4..5393065773 100644 --- a/lib/Subscription/SubscriptionUrlFactory.php +++ b/lib/Subscription/SubscriptionUrlFactory.php @@ -50,9 +50,10 @@ class SubscriptionUrlFactory { return $this->getSubscriptionUrl($post, 'confirm', $subscriber); } - public function getConfirmUnsubscribeUrl(Subscriber $subscriber = null) { + public function getConfirmUnsubscribeUrl(Subscriber $subscriber = null, int $queueId = null) { $post = $this->getPost($this->settings->get('subscription.pages.confirm_unsubscribe')); - return $this->getSubscriptionUrl($post, 'confirm_unsubscribe', $subscriber); + $data = $queueId && $subscriber ? ['queueId' => $queueId] : null; + return $this->getSubscriptionUrl($post, 'confirm_unsubscribe', $subscriber, $data); } public function getManageUrl(Subscriber $subscriber = null) { @@ -60,9 +61,10 @@ class SubscriptionUrlFactory { return $this->getSubscriptionUrl($post, 'manage', $subscriber); } - public function getUnsubscribeUrl(Subscriber $subscriber = null) { + public function getUnsubscribeUrl(Subscriber $subscriber = null, int $queueId = null) { $post = $this->getPost($this->settings->get('subscription.pages.unsubscribe')); - return $this->getSubscriptionUrl($post, 'unsubscribe', $subscriber); + $data = $queueId && $subscriber ? ['queueId' => $queueId] : null; + return $this->getSubscriptionUrl($post, 'unsubscribe', $subscriber, $data); } public function getSubscriptionUrl( @@ -75,10 +77,11 @@ class SubscriptionUrlFactory { $url = $this->wp->getPermalink($post); if ($subscriber !== null) { - $data = [ + $subscriberData = [ 'token' => $this->linkTokens->getToken($subscriber), 'email' => $subscriber->email, ]; + $data = array_merge($data ?? [], $subscriberData); } elseif (is_null($data)) { $data = [ 'preview' => 1, diff --git a/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php b/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php index 83308425a8..3007278b61 100644 --- a/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php +++ b/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php @@ -121,7 +121,7 @@ class SendingQueueTest extends \MailPoetTest { } private function getDirectUnsubscribeURL() { - return SubscriptionUrlFactory::getInstance()->getUnsubscribeUrl($this->subscriber); + return SubscriptionUrlFactory::getInstance()->getUnsubscribeUrl($this->subscriber, $this->queue->id); } private function getTrackedUnsubscribeURL() { diff --git a/tests/integration/Subscription/UrlTest.php b/tests/integration/Subscription/UrlTest.php index 3d08700f83..d0fa0dcd95 100644 --- a/tests/integration/Subscription/UrlTest.php +++ b/tests/integration/Subscription/UrlTest.php @@ -76,7 +76,7 @@ class UrlTest extends \MailPoetTest { expect($url)->contains('action=confirm'); expect($url)->contains('endpoint=subscription'); - $this->checkData($url); + $this->checkSubscriberData($url); } public function testItReturnsTheManageSubscriptionUrl() { @@ -94,7 +94,7 @@ class UrlTest extends \MailPoetTest { expect($url)->contains('action=manage'); expect($url)->contains('endpoint=subscription'); - $this->checkData($url); + $this->checkSubscriberData($url); } public function testItReturnsTheUnsubscribeUrl() { @@ -103,6 +103,8 @@ class UrlTest extends \MailPoetTest { expect($url)->notNull(); expect($url)->contains('action=unsubscribe'); expect($url)->contains('endpoint=subscription'); + $data = $this->getUrlData($url); + expect($data['preview'])->equals(1); // actual subscriber $subscriber = Subscriber::createOrUpdate([ @@ -112,17 +114,75 @@ class UrlTest extends \MailPoetTest { expect($url)->contains('action=unsubscribe'); expect($url)->contains('endpoint=subscription'); - $this->checkData($url); + $this->checkSubscriberData($url); + + // subscriber and query id + $url = $this->url->getUnsubscribeUrl($subscriber, 10); + expect($url)->contains('action=unsubscribe'); + expect($url)->contains('endpoint=subscription'); + + $data = $this->checkSubscriberData($url); + expect($data['queueId'])->equals(10); + + // no subscriber but query id + $url = $this->url->getUnsubscribeUrl(null, 10); + expect($url)->contains('action=unsubscribe'); + expect($url)->contains('endpoint=subscription'); + + $data = $this->getUrlData($url); + expect(isset($data['data']['queueId']))->false(); + expect($data['preview'])->equals(1); } - private function checkData($url) { + public function testItReturnsConfirmUnsubscribeUrl() { + // preview + $url = $this->url->getConfirmUnsubscribeUrl(null); + expect($url)->notNull(); + expect($url)->contains('action=confirm_unsubscribe'); + expect($url)->contains('endpoint=subscription'); + $data = $this->getUrlData($url); + expect($data['preview'])->equals(1); + + // actual subscriber + $subscriber = Subscriber::createOrUpdate([ + 'email' => 'john@mailpoet.com', + ]); + $url = $this->url->getConfirmUnsubscribeUrl($subscriber); + expect($url)->contains('action=confirm_unsubscribe'); + expect($url)->contains('endpoint=subscription'); + + $this->checkSubscriberData($url); + + // subscriber and query id + $url = $this->url->getConfirmUnsubscribeUrl($subscriber, 10); + expect($url)->contains('action=confirm_unsubscribe'); + expect($url)->contains('endpoint=subscription'); + + $data = $this->checkSubscriberData($url); + expect($data['queueId'])->equals(10); + + // no subscriber but query id + $url = $this->url->getConfirmUnsubscribeUrl(null, 10); + expect($url)->contains('action=confirm_unsubscribe'); + expect($url)->contains('endpoint=subscription'); + + $data = $this->getUrlData($url); + expect(isset($data['data']['queueId']))->false(); + expect($data['preview'])->equals(1); + } + + private function checkSubscriberData(string $url) { + $data = $this->getUrlData($url); + expect($data['email'])->contains('john@mailpoet.com'); + expect($data['token'])->notEmpty(); + return $data; + } + + private function getUrlData(string $url) { // extract & decode data from url $urlParamsQuery = parse_url($url, PHP_URL_QUERY); parse_str((string)$urlParamsQuery, $params); - $data = Router::decodeRequestData($params['data']); - - expect($data['email'])->contains('john@mailpoet.com'); - expect($data['token'])->notEmpty(); + return Router::decodeRequestData($params['data']); } public function _after() {