Merge pull request #512 from mailpoet/preview_link_refactoring

Extracts browser preview URl logic into a separate class

lib/Config/PublicAPI.php

@@ -2,7 +2,7 @@
 namespace MailPoet\Config;
 
 use MailPoet\Cron\Daemon;
-use MailPoet\Newsletter\Viewer\ViewInBrowser;
+use MailPoet\Newsletter\ViewInBrowser;
 use MailPoet\Statistics\Track\Clicks;
 use MailPoet\Statistics\Track\Opens;
 use MailPoet\Subscription;

lib/Newsletter/Shortcodes/Categories/Link.php

@@ -2,7 +2,7 @@
 namespace MailPoet\Newsletter\Shortcodes\Categories;
 
 use MailPoet\Models\Setting;
-use MailPoet\Models\Subscriber as SubscriberModel;
+use MailPoet\Newsletter\Url as NewsletterUrl;
 use MailPoet\Statistics\Track\Unsubscribes;
 use MailPoet\Subscription\Url as SubscriptionUrl;
 
@@ -58,8 +58,8 @@ class Link {
       break;
 
       case 'newsletter_view_in_browser':
-        $action = 'view_in_browser_url';
+        $action = 'newsletter_view_in_browser_url';
-        $url = esc_attr(self::getViewInBrowserUrl($newsletter, $subscriber, $queue));
+        $url = esc_attr(NewsletterUrl::getViewInBrowserUrl($newsletter, $subscriber, $queue));
         $url = self::processUrl($action, $url, $queue);
         return sprintf(
           '<a target="_blank" href="%s">%s</a>',
@@ -69,7 +69,7 @@ class Link {
       break;
 
       case 'newsletter_view_in_browser_url':
-        $url = self::getViewInBrowserUrl($newsletter, $subscriber, $queue);
+        $url = NewsletterUrl::getViewInBrowserUrl($newsletter, $subscriber, $queue);
         return self::processUrl($action, $url, $queue);
       break;
 
@@ -89,29 +89,6 @@ class Link {
     }
   }
 
-  static function getViewInBrowserUrl(
-    $newsletter,
-    $subscriber = false,
-    $queue = false
-  ) {
-    $data = array(
-      'newsletter' => (isset($newsletter['id'])) ?
-        $newsletter['id'] :
-        $newsletter,
-      'subscriber' => (isset($subscriber['id'])) ?
-        $subscriber['id'] :
-        $subscriber,
-      'subscriber_token' => (isset($subscriber['id'])) ?
-        SubscriberModel::generateToken($subscriber['email']) :
-        false,
-      'queue' => (isset($queue['id'])) ?
-        $queue['id'] :
-        $queue
-    );
-    $data = rtrim(base64_encode(serialize($data)), '=');
-    return home_url() . '/?mailpoet&endpoint=view_in_browser&data=' . $data;
-  }
-
   static function processUrl($action, $url, $queue) {
     return ($queue !== false && (boolean) Setting::getValue('tracking.enabled')) ?
       self::getShortcode($action) :
@@ -134,7 +111,7 @@ class Link {
         $url = SubscriptionUrl::getManageUrl($subscriber);
       break;
       case 'newsletter_view_in_browser_url':
-        $url = Link::getViewInBrowserUrl($newsletter, $subscriber, $queue);
+        $url = NewsletterUrl::getViewInBrowserUrl($newsletter, $subscriber, $queue);
       break;
       default:
         $shortcode = self::getShortcode($shortcode_action);

lib/Newsletter/Url.php

@@ -0,0 +1,42 @@
+<?php
+namespace MailPoet\Newsletter;
+
+use MailPoet\Models\Subscriber;
+
+class Url {
+  static function getViewInBrowserUrl(
+    $newsletter,
+    $subscriber = false,
+    $queue = false
+  ) {
+    if(is_object($newsletter)) {
+      $newsletter = $newsletter->asArray();
+    }
+    if(is_object($subscriber)) {
+      $subscriber = $subscriber->asArray();
+    }
+    if(is_object($queue)) {
+      $queue = $queue->asArray();
+    }
+    $data = array(
+      'newsletter' => (!empty($newsletter['id'])) ?
+        $newsletter['id'] :
+        $newsletter,
+      'subscriber' => (!empty($subscriber['id'])) ?
+        $subscriber['id'] :
+        $subscriber,
+      'subscriber_token' => (!empty($subscriber['id'])) ?
+        Subscriber::generateToken($subscriber['email']) :
+        false,
+      'queue' => (!empty($queue['id'])) ?
+        $queue['id'] :
+        $queue
+    );
+    $params = array(
+      'mailpoet' => '',
+      'endpoint' => 'view_in_browser',
+      'data' => base64_encode(serialize($data))
+    );
+    return add_query_arg($params, home_url());
+  }
+}

lib/Newsletter/ViewInBrowser.php

@@ -1,5 +1,5 @@
 <?php
-namespace MailPoet\Newsletter\Viewer;
+namespace MailPoet\Newsletter;
 
 use MailPoet\Models\Newsletter;
 use MailPoet\Models\SendingQueue;

lib/Router/Newsletters.php

@@ -11,7 +11,7 @@ use MailPoet\Models\NewsletterOption;
 use MailPoet\Models\Subscriber;
 use MailPoet\Newsletter\Renderer\Renderer;
 use MailPoet\Newsletter\Scheduler\Scheduler;
-use MailPoet\Newsletter\Shortcodes\Categories\Link;
+use MailPoet\Newsletter\Url as NewsletterUrl;
 use MailPoet\Util\Helpers;
 
 if(!defined('ABSPATH')) exit;
@@ -154,7 +154,7 @@ class Newsletters {
     $subscriber = Subscriber::where('email', $wp_user->data->user_email)
       ->findOne();
     $subscriber = ($subscriber) ? $subscriber->asArray() : $subscriber;
-    $preview_url = Link::getViewInBrowserUrl($data, $subscriber);
+    $preview_url = NewsletterUrl::getViewInBrowserUrl($data, $subscriber);
     return array(
       'result' => true,
       'data' => array('url' => $preview_url)

tests/unit/Newsletter/ShortcodesTest.php

@@ -165,25 +165,25 @@ class ShortcodesTest extends MailPoetTest {
     $shortcodes_object = $this->shortcodes_object;
     $result =
       $shortcodes_object->process(array('[link:subscription_unsubscribe]'));
-    expect(preg_match('/^<a.*?\/a>$/', $result['0']))->equals(1);
+    expect($result['0'])->regExp('/^<a.*?\/a>$/');
-    expect(preg_match('/action=unsubscribe/', $result['0']))->equals(1);
+    expect($result['0'])->regExp('/action=unsubscribe/');
     $result =
       $shortcodes_object->process(array('[link:subscription_unsubscribe_url]'));
-    expect(preg_match('/^http.*?action=unsubscribe/', $result['0']))->equals(1);
+    expect($result['0'])->regExp('/^http.*?action=unsubscribe/');
     $result =
       $shortcodes_object->process(array('[link:subscription_manage]'));
-    expect(preg_match('/^<a.*?\/a>$/', $result['0']))->equals(1);
+    expect($result['0'])->regExp('/^<a.*?\/a>$/');
-    expect(preg_match('/action=manage/', $result['0']))->equals(1);
+    expect($result['0'])->regExp('/action=manage/');
     $result =
       $shortcodes_object->process(array('[link:subscription_manage_url]'));
-    expect(preg_match('/^http.*?action=manage/', $result['0']))->equals(1);
+    expect($result['0'])->regExp('/^http.*?action=manage/');
     $result =
       $shortcodes_object->process(array('[link:newsletter_view_in_browser]'));
-    expect(preg_match('/^<a.*?\/a>$/', $result['0']))->equals(1);
+    expect($result['0'])->regExp('/^<a.*?\/a>$/');
-    expect(preg_match('/endpoint=view_in_browser/', $result['0']))->equals(1);
+    expect($result['0'])->regExp('/endpoint=view_in_browser/');
     $result =
       $shortcodes_object->process(array('[link:newsletter_view_in_browser_url]'));
-    expect(preg_match('/^http.*?endpoint=view_in_browser/', $result['0']))->equals(1);
+    expect($result['0'])->regExp('/^http.*?endpoint=view_in_browser/');
   }
 
   function testItReturnsShortcodeWhenTrackingEnabled() {
@@ -191,9 +191,9 @@ class ShortcodesTest extends MailPoetTest {
     $shortcode = '[link:subscription_unsubscribe_url]';
     $result =
       $shortcodes_object->process(array($shortcode));
-    expect(preg_match('/^http.*?action=unsubscribe/', $result['0']))->equals(1);
+    expect($result['0'])->regExp('/^http.*?action=unsubscribe/');
     Setting::setValue('tracking.enabled', true);
-    $shortcodes = array(
+    $initial_shortcodes = array(
       '[link:subscription_unsubscribe]',
       '[link:subscription_unsubscribe_url]',
       '[link:subscription_manage]',
@@ -201,13 +201,26 @@ class ShortcodesTest extends MailPoetTest {
       '[link:newsletter_view_in_browser]',
       '[link:newsletter_view_in_browser_url]'
     );
+    $expected_transformed_shortcodes = array(
+      '[link:subscription_unsubscribe_url]',
+      '[link:subscription_unsubscribe_url]',
+      '[link:subscription_manage_url]',
+      '[link:subscription_manage_url]',
+      '[link:newsletter_view_in_browser_url]',
+      '[link:newsletter_view_in_browser_url]'
+    );
     // tracking function only works during sending, so queue object must not be false
     $shortcodes_object->queue = true;
-    $result =
+    $result = $shortcodes_object->process($initial_shortcodes);
-      $shortcodes_object->process($shortcodes);
+    foreach($result as $index => $transformed_shortcode) {
-    // all returned shortcodes must end with url
+      // 1. result must not contain a link
-    $result = join(',', $result);
+      expect($transformed_shortcode)->regExp('/^((?!href="http).)*$/');
-    expect(substr_count($result, '_url'))->equals(count($shortcodes));
+      // 2. result must include a URL shortcode. for example:
+      // [link:subscription_unsubscribe] should become
+      // [link:subscription_unsubscribe_url]
+      expect($transformed_shortcode)
+        ->regExp('/' . preg_quote($expected_transformed_shortcodes[$index]) . '/');
+    }
   }
 
   function testItCanProcessCustomLinkShortcodes() {