- Removes requirement of passing newsletter id when tracking clicks

- Extracts common tracking data processing/validation code into the Track
  class
- Refactors Clicks, Opens and View in Browser classes to enforce
  subscriber id and token check
- Allows admin users to preview newsletters without tracking statistics

lib/API/Endpoints/Newsletters.php

@@ -5,6 +5,7 @@ use MailPoet\API\Error as APIError;
 
 use MailPoet\Listing;
 use MailPoet\Models\Newsletter;
+use MailPoet\Models\SendingQueue;
 use MailPoet\Models\Setting;
 use MailPoet\Models\NewsletterTemplate;
 use MailPoet\Models\NewsletterSegment;
@@ -252,8 +253,10 @@ class Newsletters extends APIEndpoint {
       $data
     );
     $listing_data = $listing->get();
+    $subscriber = Subscriber::getCurrentWPUser();
 
     foreach($listing_data['items'] as $key => $newsletter) {
+      $queue = false;
 
       if($newsletter->type === Newsletter::TYPE_STANDARD) {
         $newsletter
@@ -277,8 +280,15 @@ class Newsletters extends APIEndpoint {
           ->withStatistics();
       }
 
+      if($newsletter->status === Newsletter::STATUS_SENT ||
+         $newsletter->status === Newsletter::STATUS_SENDING
+      ) {
+        $queue = SendingQueue::where('newsletter_id', $newsletter->id)->findOne();
+      }
+
       // get preview url
-      $newsletter->preview_url = NewsletterUrl::getViewInBrowserUrl($newsletter);
+      $newsletter->preview_url = NewsletterUrl::getViewInBrowserUrl(
+        $newsletter, $subscriber, $queue, $preview = true);
 
       // convert object to array
       $listing_data['items'][$key] = $newsletter->asArray();

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

@@ -88,7 +88,6 @@ class Newsletter {
     );
     if($this->tracking_enabled) {
       $prepared_newsletter = NewsletterLinks::replaceSubscriberData(
-        $newsletter['id'],
         $subscriber['id'],
         $queue['id'],
         $prepared_newsletter

lib/Newsletter/Links/Links.php

@@ -1,6 +1,7 @@
 <?php
 namespace MailPoet\Newsletter\Links;
 
+use MailPoet\Models\Subscriber;
 use MailPoet\Router\Front as FrontRouter;
 use MailPoet\Router\Endpoints\Track as TrackEndpoint;
 use MailPoet\Models\NewsletterLink;
@@ -92,10 +93,10 @@ class Links {
   }
 
   static function replaceSubscriberData(
-    $newsletter_id,
     $subscriber_id,
     $queue_id,
-    $content
+    $content,
+    $preview = false
   ) {
     // match data tags
     $regex = sprintf(
@@ -103,6 +104,7 @@ class Links {
       preg_quote(self::DATA_TAG_CLICK),
       preg_quote(self::DATA_TAG_OPEN)
     );
+    $subscriber = Subscriber::findOne($subscriber_id);
     preg_match_all($regex, $content, $matches);
     foreach($matches[1] as $index => $match) {
       $hash = null;
@@ -110,10 +112,11 @@ class Links {
         list(, $hash) = explode('-', $match);
       }
       $data = array(
-        'newsletter' => $newsletter_id,
+        'subscriber_id' => $subscriber->id,
-        'subscriber' => $subscriber_id,
+        'subscriber_token' => Subscriber::generateToken($subscriber->email),
-        'queue' => $queue_id,
+        'queue_id' => $queue_id,
-        'hash' => $hash
+        'link_hash' => $hash,
+        'preview' => $preview
       );
       $router_action = ($matches[2][$index] === self::DATA_TAG_CLICK) ?
         TrackEndpoint::ACTION_CLICK :

lib/Newsletter/Url.php

@@ -30,18 +30,19 @@ class Url {
       $queue = ($queue) ? $queue->asArray() : false;
     }
     $data = array(
-      'newsletter' => (!empty($newsletter['id'])) ?
+      'newsletter_id' => (!empty($newsletter['id'])) ?
         $newsletter['id'] :
         $newsletter,
-      'subscriber' => (!empty($subscriber['id'])) ?
+      'subscriber_id' => (!empty($subscriber['id'])) ?
         $subscriber['id'] :
         $subscriber,
       'subscriber_token' => (!empty($subscriber['id'])) ?
         Subscriber::generateToken($subscriber['email']) :
         false,
-      'queue' => (!empty($queue['id'])) ?
+      'queue_id' => (!empty($queue['id'])) ?
         $queue['id'] :
-        $queue
+        $queue,
+      'preview' => $preview
     );
     return FrontRouter::buildRequest(
       ViewInBrowserEndpoint::ENDPOINT,

lib/Newsletter/ViewInBrowser.php

@@ -12,44 +12,78 @@ use MailPoet\Newsletter\Shortcodes\Shortcodes;
 class ViewInBrowser {
   public $data;
 
-  function __construct($data) {
+  static function view($data) {
-    $this->data = $data;
+    $data = self::processData($data);
-  }
+    if(!$data) return false;
-
-  function view($data = false) {
-    $data = ($data) ? $data : $this->data;
-    $newsletter = ($data['newsletter'] !== false) ?
-      Newsletter::findOne($data['newsletter']) :
-      false;
-    if(!$newsletter) $this->abort();
-    $subscriber = ($data['subscriber'] !== false) ?
-      $this->verifySubscriber($data['subscriber'], $data['subscriber_token']) :
-      false;
-    $queue = ($data['queue'] !== false) ?
-      SendingQueue::findOne($data['queue']) :
-      false;
     $rendered_newsletter =
-      $this->getAndRenderNewsletter($newsletter, $subscriber, $queue);
+      self::getAndRenderNewsletter(
+        $data['newsletter'],
+        $data['subscriber'],
+        $data['queue'],
+        $data['preview']
+      );
     header('Content-Type: text/html; charset=utf-8');
     echo $rendered_newsletter;
     exit;
   }
 
-  function verifySubscriber($subscriber_id, $subscriber_token) {
+  static function processData($data) {
-    $subscriber = Subscriber::findOne($subscriber_id);
+    if(empty($data['subscriber_id']) ||
-    if(!$subscriber ||
+      empty($data['subscriber_token']) ||
-      !Subscriber::verifyToken($subscriber->email, $subscriber_token)
+      empty($data['newsletter_id'])
     ) {
       return false;
     }
-    return $subscriber;
+    $data['newsletter'] = self::getNewsletter($data['newsletter_id']);
+    $data['subscriber'] = self::getSubscriber($data['subscriber_id']);
+    $data['queue'] = self::getQueue($data['queue_id']);
+    $data_processed_successfully =
+      ($data['subscriber'] && $data['newsletter']);
+    return ($data_processed_successfully) ?
+      self::validateData($data) :
+      false;
   }
 
-  function getAndRenderNewsletter($newsletter, $subscriber, $queue) {
+  static function validateData($data) {
-    if($queue) {
+    if(!$data['subscriber']) return false;
-      $newsletter_body = json_decode($queue->newsletter_rendered_body, true);
+    $subscriber_token_match =
+      Subscriber::verifyToken($data['subscriber']['email'], $data['subscriber_token']);
+    // return if this is an administrator user previewing the newsletter
+    if($data['subscriber']['wp_user_id'] && $subscriber_token_match && $data['preview']) {
+      return ($subscriber_token_match) ? $data : false;
+    }
+    // if queue exists, check if the newsletter was sent to the subscriber
+    if($data['queue'] && $data['subscriber']) {
+      $is_valid_subscriber =
+        (!empty($data['queue']['subscribers']['processed']) &&
+          in_array($data['subscriber']['id'], $data['queue']['subscribers']['processed']));
+      $data = ($is_valid_subscriber && $subscriber_token_match) ? $data : false;
     } else {
-      $renderer = new Renderer($newsletter->asArray(), $preview = true);
+      $data = ($subscriber_token_match) ? $data : false;
+    }
+    return $data;
+  }
+
+  static function getNewsletter($newsletter_id) {
+    $newsletter = Newsletter::findOne($newsletter_id);
+    return ($newsletter) ? $newsletter->asArray() : $newsletter;
+  }
+
+  static function getQueue($queue_id) {
+    $queue = SendingQueue::findOne($queue_id);
+    return ($queue) ? $queue->asArray() : $queue;
+  }
+
+  static function getSubscriber($subscriber_id) {
+    $subscriber = Subscriber::findOne($subscriber_id);
+    return ($subscriber) ? $subscriber->asArray() : $subscriber;
+  }
+
+  static function getAndRenderNewsletter($newsletter, $subscriber, $queue, $preview) {
+    if($queue && $queue['newsletter_rendered_body']) {
+      $newsletter_body = json_decode($queue['newsletter_rendered_body'], true);
+    } else {
+      $renderer = new Renderer($newsletter, $preview);
       $newsletter_body = $renderer->render();
     }
     $shortcodes = new Shortcodes(
@@ -60,16 +94,16 @@ class ViewInBrowser {
     $rendered_newsletter = $shortcodes->replace($newsletter_body['html']);
     if($queue && (boolean)Setting::getValue('tracking.enabled')) {
       $rendered_newsletter = Links::replaceSubscriberData(
-        $newsletter->id,
+        $subscriber['id'],
-        $subscriber->id,
+        $queue['id'],
-        $queue->id,
+        $rendered_newsletter,
-        $rendered_newsletter
+        $preview
       );
     }
     return $rendered_newsletter;
   }
 
-  private function abort() {
+  private static function abort() {
     status_header(404);
     exit;
   }

lib/Router/Endpoints/Track.php

@@ -1,6 +1,10 @@
 <?php
 namespace MailPoet\Router\Endpoints;
 
+use MailPoet\Models\Newsletter;
+use MailPoet\Models\NewsletterLink;
+use MailPoet\Models\SendingQueue;
+use MailPoet\Models\Subscriber;
 use MailPoet\Statistics\Track\Clicks;
 use MailPoet\Statistics\Track\Opens;
 
@@ -12,12 +16,68 @@ class Track {
   const ACTION_OPEN = 'open';
 
   static function click($data) {
-    $clicks = new Clicks($data);
+    Clicks::track(self::_processTrackData($data));
-    $clicks->track();
   }
 
   static function open($data) {
-    $opens = new Opens($data);
+    Opens::track(self::_processTrackData($data));
-    $opens->track();
+  }
+
+  static function _processTrackData($data) {
+    if(empty($data['queue_id']) ||
+       empty($data['subscriber_id']) ||
+       empty($data['subscriber_token'])
+    ) {
+      return false;
+    }
+    $data['queue'] = self::_getQueue($data['queue_id']);
+    $data['subscriber'] = self::_getSubscriber($data['subscriber_id']);
+    $data['newsletter'] = (!empty($data['queue']['newsletter_id'])) ?
+      self::_getNewsletter($data['queue']['newsletter_id']) :
+      false;
+    if(!empty($data['link_hash'])) {
+      $data['link'] = self::_getLink($data['link_hash']);
+    }
+    $data_processed_successfully =
+      ($data['queue'] && $data['subscriber'] && $data['newsletter']);
+    return ($data_processed_successfully) ?
+      self::_validateTrackData($data) :
+      false;
+  }
+
+  static function _validateTrackData($data) {
+    if(!$data['subscriber']) return false;
+    $subscriber_token_match =
+      Subscriber::verifyToken($data['subscriber']['email'], $data['subscriber_token']);
+    // return if this is an administrator user previewing the newsletter
+    if($data['subscriber']['wp_user_id'] && $data['preview']) {
+      return ($subscriber_token_match) ? $data : false;
+    }
+    // check if the newsletter was sent to the subscriber
+    $is_valid_subscriber =
+      (!empty($data['queue']['subscribers']['processed']) &&
+        in_array($data['subscriber']['id'], $data['queue']['subscribers']['processed']));
+    return ($is_valid_subscriber && $subscriber_token_match) ? $data : false;
+  }
+
+  static function _getNewsletter($newsletter_id) {
+    $newsletter = Newsletter::findOne($newsletter_id);
+    return ($newsletter) ? $newsletter->asArray() : $newsletter;
+  }
+
+  static function _getSubscriber($subscriber_id) {
+    $subscriber = Subscriber::findOne($subscriber_id);
+    return ($subscriber) ? $subscriber->asArray() : $subscriber;
+  }
+
+  static function _getQueue($queue_id) {
+    $queue = SendingQueue::findOne($queue_id);
+    return ($queue) ? $queue->asArray() : $queue;
+  }
+
+  static function _getLink($link_hash) {
+    $link = NewsletterLink::where('hash', $link_hash)
+      ->findOne();
+    return ($link) ? $link->asArray() : $link;
   }
 }

lib/Router/Endpoints/ViewInBrowser.php

@@ -10,7 +10,6 @@ class ViewInBrowser {
   const ACTION_VIEW = 'view';
 
   static function view($data) {
-    $viewer = new NewsletterViewInBrowser($data);
+    NewsletterViewInBrowser::view($data);
-    $viewer->view();
   }
 }

lib/Statistics/Track/Clicks.php

@@ -1,44 +1,21 @@
 <?php
 namespace MailPoet\Statistics\Track;
 
-use MailPoet\Models\Newsletter;
-use MailPoet\Models\NewsletterLink;
-use MailPoet\Models\SendingQueue;
 use MailPoet\Models\StatisticsClicks;
-use MailPoet\Models\Subscriber;
 use MailPoet\Newsletter\Shortcodes\Categories\Link;
 
 if(!defined('ABSPATH')) exit;
 
 class Clicks {
-  public $data;
+  static function track($data) {
-
+    if(!$data || empty($data['link'])) return false;
-  function __construct($data) {
+    $subscriber = $data['subscriber'];
-    $this->data = $data;
+    $queue = $data['queue'];
-  }
+    $newsletter = $data['newsletter'];
-
+    $link = $data['link'];
-  function track($data = false) {
+    // log statistics only if the action did not come from
-    $data = ($data) ? $data : $this->data;
+    // an admin user previewing the newsletter
-    $newsletter = $this->getNewsletter($data['newsletter']);
+    if(!$data['preview'] && !$subscriber['wp_user_id']) {
-    $queue = $this->getQueue($data['queue']);
-    // verify if queue belongs to the newsletter
-    if($newsletter && $queue) {
-      $queue = ($queue['newsletter_id'] === $newsletter['id']) ?
-        $queue :
-        false;
-    }
-    $subscriber = $this->getSubscriber($data['subscriber']);
-    // verify if subscriber belongs to the queue
-    if($queue && $subscriber) {
-      // check if this newsletter was sent to
-      $subscriber = (in_array($subscriber['id'], $queue['subscribers']['processed'])) ?
-        $subscriber :
-        false;
-    }
-    $link = $this->getLink($data['hash']);
-    if(!$subscriber || !$newsletter || !$link || !$queue) {
-      $this->abort();
-    }
       $statistics = StatisticsClicks::where('link_id', $link['id'])
         ->where('subscriber_id', $subscriber['id'])
         ->where('newsletter_id', $newsletter['id'])
@@ -46,7 +23,7 @@ class Clicks {
       ->findOne();
       if(!$statistics) {
         // track open event in case it did not register
-      $this->trackOpenEvent($data);
+        self::trackOpenEvent($data);
         $statistics = StatisticsClicks::create();
         $statistics->newsletter_id = $newsletter['id'];
         $statistics->link_id = $link['id'];
@@ -58,34 +35,14 @@ class Clicks {
         $statistics->count++;
         $statistics->save();
       }
-    $url = $this->processUrl($link['url'], $newsletter, $subscriber, $queue);
+    }
-    $this->redirectToUrl($url);
+    $url = self::processUrl($link['url'], $newsletter, $subscriber, $queue);
+    self::redirectToUrl($url);
   }
 
-  function getNewsletter($newsletter_id) {
+  static function processUrl($url, $newsletter, $subscriber, $queue) {
-    $newsletter = Newsletter::findOne($newsletter_id);
-    return ($newsletter) ? $newsletter->asArray() : $newsletter;
-  }
-
-  function getSubscriber($subscriber_id) {
-    $subscriber = Subscriber::findOne($subscriber_id);
-    return ($subscriber) ? $subscriber->asArray() : $subscriber;
-  }
-
-  function getQueue($queue_id) {
-    $queue = SendingQueue::findOne($queue_id);
-    return ($queue) ? $queue->asArray() : $queue;
-  }
-
-  function getLink($hash) {
-    $link = NewsletterLink::where('hash', $hash)
-      ->findOne();
-    return ($link) ? $link->asArray() : $link;
-  }
-
-  function processUrl($url, $newsletter, $subscriber, $queue) {
     if(preg_match('/\[link:(?P<action>.*?)\]/', $url, $shortcode)) {
-      if(!$shortcode['action']) $this->abort();
+      if(!$shortcode['action']) self::abort();
       $url = Link::processShortcodeAction(
         $shortcode['action'],
         $newsletter,
@@ -96,17 +53,16 @@ class Clicks {
     return $url;
   }
 
-  function trackOpenEvent($data) {
+  static function trackOpenEvent($data) {
-    $open = new Opens($data, $display_image = false);
+    return Opens::track($data, $display_image = false);
-    return $open->track();
   }
 
-  function abort() {
+  static function abort() {
     status_header(404);
     exit;
   }
 
-  function redirectToUrl($url) {
+  static function redirectToUrl($url) {
     header('Location: ' . $url, true, 302);
     exit;
   }

lib/Statistics/Track/Opens.php

@@ -1,76 +1,37 @@
 <?php
 namespace MailPoet\Statistics\Track;
 
-use MailPoet\Models\Newsletter;
-use MailPoet\Models\SendingQueue;
 use MailPoet\Models\StatisticsOpens;
-use MailPoet\Models\Subscriber;
 
 if(!defined('ABSPATH')) exit;
 
 class Opens {
-  public $data;
+  static function track($data, $display_image = true) {
-  public $return_image;
+    if(!$data) return self::displayImage();
-
+    $subscriber = $data['subscriber'];
-  function __construct($data, $return_image = true) {
+    $queue = $data['queue'];
-    $this->data = $data;
+    $newsletter = $data['newsletter'];
-    $this->return_image = $return_image;
+    // log statistics only if the action did not come from
-  }
+    // an admin user previewing the newsletter
-
+    if (!$data['preview'] && !$subscriber['wp_user_id']) {
-  function track($data = false) {
+      $statistics = StatisticsOpens::where('subscriber_id', $subscriber['id'])
-    $data = ($data) ? $data : $this->data;
+        ->where('newsletter_id', $newsletter['id'])
-    $newsletter = $this->getNewsletter($data['newsletter']);
+        ->where('queue_id', $queue['id'])
-    $queue = $this->getQueue($data['queue']);
-    // verify if queue belongs to the newsletter
-    if($newsletter && $queue) {
-      $queue = ($queue['newsletter_id'] === $newsletter['id']) ?
-        $queue :
-        false;
-    }
-    $subscriber = $this->getSubscriber($data['subscriber']);
-    // verify if subscriber belongs to the queue
-    if($queue && $subscriber) {
-      if(empty($queue['']))
-      $subscriber = (in_array($subscriber['id'], $queue['subscribers']['processed'])) ?
-        $subscriber :
-        false;
-    }
-    if(!$subscriber || !$newsletter || !$queue) {
-      return false;
-    }
-    $statistics = StatisticsOpens::where('subscriber_id', $data['subscriber'])
-      ->where('newsletter_id', $data['newsletter'])
-      ->where('queue_id', $data['queue'])
         ->findOne();
       if(!$statistics) {
         $statistics = StatisticsOpens::create();
-      $statistics->newsletter_id = $data['newsletter'];
+        $statistics->newsletter_id = $newsletter['id'];
-      $statistics->subscriber_id = $data['subscriber'];
+        $statistics->subscriber_id = $subscriber['id'];
-      $statistics->queue_id = $data['queue'];
+        $statistics->queue_id = $queue['id'];
         $statistics->save();
       }
-    if($this->return_image) {
-      $this->returnImage();
     }
-    return true;
+    return ($display_image) ?
+      self::displayImage() :
+      true;
   }
 
-  function getNewsletter($newsletter_id) {
+  static function displayImage() {
-    $newsletter = Newsletter::findOne($newsletter_id);
-    return ($newsletter) ? $newsletter->asArray() : $newsletter;
-  }
-
-  function getSubscriber($subscriber_id) {
-    $subscriber = Subscriber::findOne($subscriber_id);
-    return ($subscriber) ? $subscriber->asArray() : $subscriber;
-  }
-
-  function getQueue($queue_id) {
-    $queue = SendingQueue::findOne($queue_id);
-    return ($queue) ? $queue->asArray() : $queue;
-  }
-
-  function returnImage() {
     // return 1x1 pixel transparent gif image
     header('Content-Type: image/gif');
     echo "\x47\x49\x46\x38\x37\x61\x1\x0\x1\x0\x80\x0\x0\xfc\x6a\x6c\x0\x0\x0\x2c\x0\x0\x0\x0\x1\x0\x1\x0\x0\x2\x2\x44\x1\x0\x3b";