diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php b/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php index 1617bd2623..ac71cfc30a 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Shortcodes.php @@ -32,18 +32,24 @@ class Shortcodes { } if ($queue instanceof SendingQueueEntity) { $shortcodes->setQueue($queue); + } else { + $shortcodes->setQueue(null); } if ($newsletter instanceof \MailPoet\Models\Newsletter && $newsletter->id) { $newsletter = $newsletterRepository->findOneById($newsletter->id); } if ($newsletter instanceof NewsletterEntity) { $shortcodes->setNewsletter($newsletter); + } else { + $shortcodes->setNewsletter(null); } if ($subscriber instanceof Subscriber && $subscriber->id) { $subscriber = $subscribersRepository->findOneById($subscriber->id); } if ($subscriber instanceof SubscriberEntity) { $shortcodes->setSubscriber($subscriber); + } else { + $shortcodes->setSubscriber(null); } return $shortcodes->replace($content, $contentSource); } diff --git a/lib/Newsletter/Shortcodes/Categories/Subscriber.php b/lib/Newsletter/Shortcodes/Categories/Subscriber.php index a1fe487be7..be0f3bdcda 100644 --- a/lib/Newsletter/Shortcodes/Categories/Subscriber.php +++ b/lib/Newsletter/Shortcodes/Categories/Subscriber.php @@ -34,18 +34,21 @@ class Subscriber implements CategoryInterface { string $content = '', bool $wpUserPreview = false ): ?string { + if (!($subscriber instanceof SubscriberEntity)) { + return $shortcodeDetails['shortcode']; + } $defaultValue = ($shortcodeDetails['action_argument'] === 'default') ? $shortcodeDetails['action_argument_value'] : ''; switch ($shortcodeDetails['action']) { case 'firstname': - return (($subscriber instanceof SubscriberEntity) && !empty($subscriber->getFirstName())) ? $subscriber->getFirstName() : $defaultValue; + return (!empty($subscriber->getFirstName())) ? $subscriber->getFirstName() : $defaultValue; case 'lastname': - return (($subscriber instanceof SubscriberEntity) && !empty($subscriber->getLastName())) ? $subscriber->getLastName() : $defaultValue; + return !empty($subscriber->getLastName()) ? $subscriber->getLastName() : $defaultValue; case 'email': - return ($subscriber instanceof SubscriberEntity) ? $subscriber->getEmail() : $defaultValue; + return $subscriber->getEmail(); case 'displayname': - if (($subscriber instanceof SubscriberEntity) && $subscriber->getWpUserId()) { + if ($subscriber->getWpUserId()) { $wpUser = WPFunctions::get()->getUserdata($subscriber->getWpUserId()); return $wpUser->user_login; // phpcs:ignore Squiz.NamingConventions.ValidVariableName.NotCamelCaps } diff --git a/lib/Newsletter/Shortcodes/Shortcodes.php b/lib/Newsletter/Shortcodes/Shortcodes.php index 56fc49d072..c6dd46d6e6 100644 --- a/lib/Newsletter/Shortcodes/Shortcodes.php +++ b/lib/Newsletter/Shortcodes/Shortcodes.php @@ -49,7 +49,7 @@ class Shortcodes { $this->subscriberCategory = $subscriberCategory; } - public function setNewsletter(NewsletterEntity $newsletter): void { + public function setNewsletter(NewsletterEntity $newsletter = null): void { $this->newsletter = $newsletter; } @@ -57,7 +57,7 @@ class Shortcodes { $this->subscriber = $subscriber; } - public function setQueue(SendingQueueEntity $queue): void { + public function setQueue(SendingQueueEntity $queue = null): void { $this->queue = $queue; } diff --git a/tests/integration/Newsletter/ShortcodesTest.php b/tests/integration/Newsletter/ShortcodesTest.php index 418adb69a7..ca04953b4c 100644 --- a/tests/integration/Newsletter/ShortcodesTest.php +++ b/tests/integration/Newsletter/ShortcodesTest.php @@ -160,14 +160,19 @@ class ShortcodesTest extends \MailPoetTest { } public function testSubscriberShortcodesRequireSubscriberObjectOrFalseValue() { - // when subscriber is empty, default value is returned + // when subscriber is empty, original value is returned $this->shortcodesObject->setSubscriber(null); $result = $this->shortcodesObject->process(['[subscriber:firstname | default:test]']); - expect($result[0])->equals('test'); + expect($result[0])->equals('[subscriber:firstname | default:test]'); // when subscriber is an object, proper value is returned $this->shortcodesObject->setSubscriber($this->subscriber); $result = $this->shortcodesObject->process(['[subscriber:firstname | default:test]']); expect($result[0])->equals($this->subscriber->getFirstName()); + // when subscriber hasn't name, the default value is returned + $this->subscriber->setFirstName(''); + $this->shortcodesObject->setSubscriber($this->subscriber); + $result = $this->shortcodesObject->process(['[subscriber:firstname | default:test]']); + expect($result[0])->equals('test'); } public function testSubscriberFirstAndLastNameShortcodesReturnDefaultValueWhenDataIsEmpty() {