Merge pull request #1347 from mailpoet/queue_rendered_subject_improvement

Uses queue's rendered subject when sending newsletter [MAILPOET-1373]

lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php

@@ -122,7 +122,7 @@ class Newsletter {
     $rendered_newsletter = $queue->getNewsletterRenderedBody();
     $prepared_newsletter = Helpers::joinObject(
       array(
-        $newsletter->subject,
+        $queue->newsletter_rendered_subject,
         $rendered_newsletter['html'],
         $rendered_newsletter['text']
       )

lib/Newsletter/Shortcodes/Categories/Date.php

@@ -6,9 +6,7 @@ use MailPoet\WP\Functions as WPFunctions;
 
 class Date {
   static function process(
-    $action,
+    $shortcode_details
-    $action_argument = false,
-    $action_argument_value = false
   ) {
     $action_mapping = array(
       'd' => 'd',
@@ -18,11 +16,11 @@ class Date {
       'mtext' => 'F',
       'y' => 'Y'
     );
-    if(!empty($action_mapping[$action])) {
+    if(!empty($action_mapping[$shortcode_details['action']])) {
-      return date_i18n($action_mapping[$action], WPFunctions::currentTime('timestamp'));
+      return date_i18n($action_mapping[$shortcode_details['action']], WPFunctions::currentTime('timestamp'));
     }
-    return ($action === 'custom' && $action_argument === 'format') ?
+    return ($shortcode_details['action'] === 'custom' && $shortcode_details['action_argument'] === 'format') ?
-      date_i18n($action_argument_value, WPFunctions::currentTime('timestamp')) :
+      date_i18n($shortcode_details['action_argument_value'], WPFunctions::currentTime('timestamp')) :
       false;
   }
 }

lib/Newsletter/Shortcodes/Categories/Link.php

@@ -1,4 +1,5 @@
 <?php
+
 namespace MailPoet\Newsletter\Shortcodes\Categories;
 
 use MailPoet\Models\Setting;
@@ -10,19 +11,17 @@ class Link {
   const CATEGORY_NAME = 'link';
 
   static function process(
-    $action,
+    $shortcode_details,
-    $action_argument,
-    $action_argument_value,
     $newsletter,
     $subscriber,
     $queue,
     $content,
     $wp_user_preview
   ) {
-    switch($action) {
+    switch($shortcode_details['action']) {
       case 'subscription_unsubscribe_url':
         return self::processUrl(
-          $action,
+          $shortcode_details['action'],
           SubscriptionUrl::getUnsubscribeUrl($wp_user_preview ? false : $subscriber),
           $queue,
           $wp_user_preview
@@ -30,7 +29,7 @@ class Link {
 
       case 'subscription_manage_url':
         return self::processUrl(
-          $action,
+          $shortcode_details['action'],
           SubscriptionUrl::getManageUrl($wp_user_preview ? false : $subscriber),
           $queue,
           $wp_user_preview
@@ -44,10 +43,10 @@ class Link {
           $queue,
           $wp_user_preview
         );
-        return self::processUrl($action, $url, $queue, $wp_user_preview);
+        return self::processUrl($shortcode_details['action'], $url, $queue, $wp_user_preview);
 
       default:
-        $shortcode = self::getFullShortcode($action);
+        $shortcode = self::getFullShortcode($shortcode_details['action']);
         $url = apply_filters(
           'mailpoet_newsletter_shortcode_link',
           $shortcode,
@@ -57,14 +56,13 @@ class Link {
           $wp_user_preview
         );
         return ($url !== $shortcode) ?
-          self::processUrl($action, $url, $queue, $wp_user_preview) :
+          self::processUrl($shortcode_details['action'], $url, $queue, $wp_user_preview) :
           false;
     }
   }
 
   static function processUrl($action, $url, $queue, $wp_user_preview = false) {
-    if ($wp_user_preview)
+    if($wp_user_preview) return $url;
-      return $url;
     return ($queue !== false && (boolean)Setting::getValue('tracking.enabled')) ?
       self::getFullShortcode($action) :
       $url;

lib/Newsletter/Shortcodes/Categories/Newsletter.php

@@ -9,15 +9,14 @@ if(!defined('ABSPATH')) exit;
 require_once(ABSPATH . "wp-includes/pluggable.php");
 
 class Newsletter {
-  static function process($action,
+  static function process(
-    $action_argument,
+    $shortcode_details,
-    $action_argument_value,
     $newsletter,
     $subscriber,
     $queue,
     $content
   ) {
-    switch($action) {
+    switch($shortcode_details['action']) {
       case 'subject':
         return ($newsletter) ? $newsletter->subject : false;
 

lib/Newsletter/Shortcodes/Categories/Subscriber.php

@@ -9,17 +9,15 @@ require_once(ABSPATH . 'wp-includes/pluggable.php');
 
 class Subscriber {
   static function process(
-    $action,
+    $shortcode_details,
-    $action_argument,
-    $action_argument_value,
     $newsletter,
     $subscriber
   ) {
-    if($subscriber !== false && !is_object($subscriber)) return false;
+    if($subscriber !== false && !is_object($subscriber)) return $shortcode_details['shortcode'];
-    $default_value = ($action_argument === 'default') ?
+    $default_value = ($shortcode_details['action_argument'] === 'default') ?
-      $action_argument_value :
+      $shortcode_details['action_argument_value'] :
       '';
-    switch($action) {
+    switch($shortcode_details['action']) {
       case 'firstname':
         return (!empty($subscriber->first_name)) ? $subscriber->first_name : $default_value;
       case 'lastname':
@@ -36,7 +34,7 @@ class Subscriber {
         return SubscriberModel::filter('subscribed')
           ->count();
       default:
-        if(preg_match('/cf_(\d+)/', $action, $custom_field) &&
+        if(preg_match('/cf_(\d+)/', $shortcode_details['action'], $custom_field) &&
           !empty($subscriber->id)
         ) {
           $custom_field = SubscriberCustomField

lib/Newsletter/Shortcodes/Shortcodes.php

@@ -49,20 +49,21 @@ class Shortcodes {
     $processed_shortcodes = array_map(
       function($shortcode) use ($content, $_this) {
         $shortcode_details = $_this->match($shortcode);
-        $shortcode_category = !empty($shortcode_details['category']) ?
+        $shortcode_details['shortcode'] = $shortcode;
-          ucfirst($shortcode_details['category']) :
+        $shortcode_details['category'] = !empty($shortcode_details['category']) ?
+          $shortcode_details['category'] :
           false;
-        $shortcode_action = !empty($shortcode_details['action']) ?
+        $shortcode_details['action'] = !empty($shortcode_details['action']) ?
           $shortcode_details['action'] :
           false;
-        $shortcode_class =
+        $shortcode_details['action_argument'] = !empty($shortcode_details['argument']) ?
-          Shortcodes::SHORTCODE_CATEGORY_NAMESPACE . $shortcode_category;
-        $shortcode_argument = !empty($shortcode_details['argument']) ?
           $shortcode_details['argument'] :
           false;
-        $shortcode_argument_value = !empty($shortcode_details['argument_value']) ?
+        $shortcode_details['action_argument_value'] = !empty($shortcode_details['argument_value']) ?
           $shortcode_details['argument_value'] :
           false;
+        $shortcode_class =
+          Shortcodes::SHORTCODE_CATEGORY_NAMESPACE . ucfirst($shortcode_details['category']);
         if(!class_exists($shortcode_class)) {
           $custom_shortcode = apply_filters(
             'mailpoet_newsletter_shortcode',
@@ -78,9 +79,7 @@ class Shortcodes {
             $custom_shortcode;
         }
         return $shortcode_class::process(
-          $shortcode_action,
+          $shortcode_details,
-          $shortcode_argument,
-          $shortcode_argument_value,
           $_this->newsletter,
           $_this->subscriber,
           $_this->queue,

tests/unit/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php

@@ -16,6 +16,7 @@ use MailPoet\Models\Setting;
 use MailPoet\Models\Subscriber;
 use MailPoet\Router\Router;
 use MailPoet\Tasks\Sending as SendingTask;
+use MailPoet\WP\Functions;
 
 if(!defined('ABSPATH')) exit;
 
@@ -224,6 +225,34 @@ class NewsletterTest extends \MailPoetTest {
     expect($updated_newsletter->status)->notEquals(Newsletter::STATUS_SENT);
   }
 
+  function testItDoesNotRenderSubscriberShortcodeInSubjectWhenPreprocessingNewsletter() {
+    $newsletter = $this->newsletter;
+    $newsletter->subject = 'Newsletter for [subscriber:firstname] [date:dordinal]';
+    $queue = $this->queue;
+    $newsletter = $this->newsletter_task->preProcessNewsletter($newsletter, $queue);
+    $queue = SendingTask::getByNewsletterId($newsletter->id);
+    expect($queue->newsletter_rendered_subject)
+      ->contains(date_i18n('dS', Functions::currentTime('timestamp')));
+  }
+
+
+  function testItUsesRenderedNewsletterBodyAndSubjectFromQueueObjectWhenPreparingNewsletterForSending() {
+    $queue = $this->queue;
+    $queue->newsletter_rendered_body = array(
+      'html' => 'queue HTML body',
+      'text' => 'queue TEXT body'
+    );
+    $queue->newsletter_rendered_subject = 'queue subject';
+    $result = $this->newsletter_task->prepareNewsletterForSending(
+      $this->newsletter,
+      $this->subscriber,
+      $queue
+    );
+    expect($result['subject'])->equals('queue subject');
+    expect($result['body']['html'])->equals('queue HTML body');
+    expect($result['body']['text'])->equals('queue TEXT body');
+  }
+
   function testItRendersShortcodesAndReplacesSubscriberDataInLinks() {
     $newsletter = $this->newsletter_task->preProcessNewsletter($this->newsletter, $this->queue);
     $result = $this->newsletter_task->prepareNewsletterForSending(

tests/unit/Newsletter/ShortcodesTest.php

@@ -82,15 +82,21 @@ class ShortcodesTest extends \MailPoetTest {
   }
 
   function testItCanProcessDateShortcodes() {
-    $date = new \DateTime(current_time('mysql'));
+    $shortcode_details = array('action' => 'd');
-    expect(Date::process('d'))->equals(date_i18n('d', current_time('timestamp')));
+    expect(Date::process($shortcode_details))->equals(date_i18n('d', current_time('timestamp')));
-    expect(Date::process('dordinal'))->equals(date_i18n('dS', current_time('timestamp')));
+    $shortcode_details = array('action' => 'dordinal');
-    expect(Date::process('dtext'))->equals(date_i18n('l', current_time('timestamp')));
+    expect(Date::process($shortcode_details))->equals(date_i18n('dS', current_time('timestamp')));
-    expect(Date::process('m'))->equals(date_i18n('m', current_time('timestamp')));
+    $shortcode_details = array('action' => 'dtext');
-    expect(Date::process('mtext'))->equals(date_i18n('F', current_time('timestamp')));
+    expect(Date::process($shortcode_details))->equals(date_i18n('l', current_time('timestamp')));
-    expect(Date::process('y'))->equals(date_i18n('Y', current_time('timestamp')));
+    $shortcode_details = array('action' => 'm');
+    expect(Date::process($shortcode_details))->equals(date_i18n('m', current_time('timestamp')));
+    $shortcode_details = array('action' => 'mtext');
+    expect(Date::process($shortcode_details))->equals(date_i18n('F', current_time('timestamp')));
+    $shortcode_details = array('action' => 'y');
+    expect(Date::process($shortcode_details))->equals(date_i18n('Y', current_time('timestamp')));
     // allow custom date formats (http://php.net/manual/en/function.date.php)
-    expect(Date::process('custom', 'format', 'U F'))->equals(date_i18n('U F', current_time('timestamp')));
+    $shortcode_details = array('action' => 'custom', 'action_argument' => 'format', 'action_argument_value' => 'U F');
+    expect(Date::process($shortcode_details))->equals(date_i18n('U F', current_time('timestamp')));
   }
 
   function testItCanProcessNewsletterShortcodes() {
@@ -152,13 +158,13 @@ class ShortcodesTest extends \MailPoetTest {
     );
     $result = $shortcodes_object->process(array('[subscriber:firstname | default:test]'));
     expect($result[0])->equals($this->subscriber->first_name);
-    // when subscriber is not empty and not an object, false is returned
+    // when subscriber is not empty and not an object, shortcode is returned
     $shortcodes_object = new \MailPoet\Newsletter\Shortcodes\Shortcodes(
       $this->newsletter,
       $subscriber = array()
     );
     $result = $shortcodes_object->process(array('[subscriber:firstname | default:test]'));
-    expect($result[0])->false();
+    expect($result[0])->equals('[subscriber:firstname | default:test]');
   }
 
   function testSubscriberFirstAndLastNameShortcodesReturnDefaultValueWhenDataIsEmpty() {