From ad6e6009d249f69aaef5ca64ad71ce75a259b4f0 Mon Sep 17 00:00:00 2001
From: Pavel Dohnal <pavel@mailpoet.com>
Date: Mon, 28 Oct 2019 09:37:01 +0100
Subject: [PATCH] Use Newsletter Link Entity in Stats Notifications

[MAILPOET-2439]
---
 .../NewsletterLinkRepository.php              | 33 +++++++
 .../Workers/StatsNotifications/Worker.php     | 20 +++-
 lib/DI/ContainerConfigurator.php              |  1 +
 lib/Entities/NewsletterLinkEntity.php         | 97 +++++++++++++++++++
 lib/Entities/StatisticsClicksEntity.php       | 51 ++++++++++
 lib/Models/NewsletterLink.php                 | 23 -----
 .../Workers/StatsNotifications/WorkerTest.php |  6 +-
 7 files changed, 202 insertions(+), 29 deletions(-)
 create mode 100644 lib/Cron/Workers/StatsNotifications/NewsletterLinkRepository.php
 create mode 100644 lib/Entities/NewsletterLinkEntity.php
 create mode 100644 lib/Entities/StatisticsClicksEntity.php

diff --git a/lib/Cron/Workers/StatsNotifications/NewsletterLinkRepository.php b/lib/Cron/Workers/StatsNotifications/NewsletterLinkRepository.php
new file mode 100644
index 0000000000..cc9170fc53
--- /dev/null
+++ b/lib/Cron/Workers/StatsNotifications/NewsletterLinkRepository.php
@@ -0,0 +1,33 @@
+<?php
+
+namespace MailPoet\Cron\Workers\StatsNotifications;
+
+use MailPoet\Doctrine\Repository;
+use MailPoet\Entities\NewsletterLinkEntity;
+
+class NewsletterLinkRepository extends Repository {
+  protected function getEntityClassName() {
+    return NewsletterLinkEntity::class;
+  }
+
+  /**
+   * @param int $newsletter_id
+   * @return NewsletterLinkEntity
+   * @throws \MailPoetVendor\Doctrine\ORM\NoResultException
+   * @throws \MailPoetVendor\Doctrine\ORM\NonUniqueResultException
+   */
+  public function findTopLinkForNewsletter($newsletter_id) {
+    return $this->doctrine_repository
+      ->createQueryBuilder('nlr')
+      ->join('nlr.clicks', 'c')
+      ->addSelect('COUNT(c.id) AS HIDDEN counter')
+      ->where('nlr.newsletter_id = :newsletterId')
+      ->setParameter('newsletterId', $newsletter_id)
+      ->groupBy('nlr.id')
+      ->orderBy('counter', 'desc')
+      ->setMaxResults(1)
+      ->getQuery()
+      ->getSingleResult();
+  }
+
+}
diff --git a/lib/Cron/Workers/StatsNotifications/Worker.php b/lib/Cron/Workers/StatsNotifications/Worker.php
index 044f24e31a..986fb1d507 100644
--- a/lib/Cron/Workers/StatsNotifications/Worker.php
+++ b/lib/Cron/Workers/StatsNotifications/Worker.php
@@ -5,6 +5,7 @@ namespace MailPoet\Cron\Workers\StatsNotifications;
 use Carbon\Carbon;
 use MailPoet\Config\Renderer;
 use MailPoet\Cron\CronHelper;
+use MailPoet\Entities\NewsletterLinkEntity;
 use MailPoet\Entities\ScheduledTaskEntity;
 use MailPoet\Entities\StatsNotificationEntity;
 use MailPoet\Mailer\Mailer;
@@ -42,12 +43,16 @@ class Worker {
   /** @var EntityManager */
   private $entity_manager;
 
+  /** @var NewsletterLinkRepository */
+  private $newsletter_link_repository;
+
   function __construct(
     Mailer $mailer,
     Renderer $renderer,
     SettingsController $settings,
     MetaInfo $mailerMetaInfo,
     StatsNotificationsRepository $repository,
+    NewsletterLinkRepository $newsletter_link_repository,
     EntityManager $entity_manager,
     $timer = false
   ) {
@@ -58,6 +63,7 @@ class Worker {
     $this->mailerMetaInfo = $mailerMetaInfo;
     $this->repository = $repository;
     $this->entity_manager = $entity_manager;
+    $this->newsletter_link_repository = $newsletter_link_repository;
   }
 
   /** @throws \Exception */
@@ -89,7 +95,11 @@ class Worker {
 
   private function constructNewsletter(StatsNotificationEntity $stats_notification_entity) {
     $newsletter = $this->getNewsletter($stats_notification_entity);
-    $link = NewsletterLink::findTopLinkForNewsletter($newsletter);
+    try {
+      $link = $this->newsletter_link_repository->findTopLinkForNewsletter($newsletter->id);
+    } catch (\MailPoetVendor\Doctrine\ORM\UnexpectedResultException $e) {
+      $link = null;
+    }
     $context = $this->prepareContext($newsletter, $link);
     $subject = $newsletter->queue['newsletter_rendered_subject'];
     return [
@@ -112,10 +122,10 @@ class Worker {
 
   /**
    * @param Newsletter $newsletter
-   * @param \stdClass|NewsletterLink $link
+   * @param NewsletterLinkEntity $link
    * @return array
    */
-  private function prepareContext(Newsletter $newsletter, $link = null) {
+  private function prepareContext(Newsletter $newsletter, NewsletterLinkEntity $link = null) {
     $clicked = ($newsletter->statistics['clicked'] * 100) / $newsletter->total_sent;
     $opened = ($newsletter->statistics['opened'] * 100) / $newsletter->total_sent;
     $unsubscribed = ($newsletter->statistics['unsubscribed'] * 100) / $newsletter->total_sent;
@@ -135,9 +145,9 @@ class Worker {
       'opened' => $opened,
     ];
     if ($link) {
-      $context['topLinkClicks'] = (int)$link->clicksCount;
+      $context['topLinkClicks'] = $link->getTotalClicksCount();
       $mappings = self::getShortcodeLinksMapping();
-      $context['topLink'] = isset($mappings[$link->url]) ? $mappings[$link->url] : $link->url;
+      $context['topLink'] = isset($mappings[$link->getUrl()]) ? $mappings[$link->getUrl()] : $link->getUrl();
     }
     return $context;
   }
diff --git a/lib/DI/ContainerConfigurator.php b/lib/DI/ContainerConfigurator.php
index 44a6c023ee..e2b2b8d36d 100644
--- a/lib/DI/ContainerConfigurator.php
+++ b/lib/DI/ContainerConfigurator.php
@@ -131,6 +131,7 @@ class ContainerConfigurator implements IContainerConfigurator {
     $container->autowire(\MailPoet\Cron\Workers\SendingQueue\SendingErrorHandler::class)->setPublic(true);
     $container->autowire(\MailPoet\Cron\Workers\StatsNotifications\Scheduler::class);
     $container->autowire(\MailPoet\Cron\Workers\StatsNotifications\StatsNotificationsRepository::class);
+    $container->autowire(\MailPoet\Cron\Workers\StatsNotifications\NewsletterLinkRepository::class);
     $container->autowire(\MailPoet\Cron\CronTrigger::class)->setPublic(true);
     // Custom field
     $container->autowire(\MailPoet\CustomFields\ApiDataSanitizer::class);
diff --git a/lib/Entities/NewsletterLinkEntity.php b/lib/Entities/NewsletterLinkEntity.php
new file mode 100644
index 0000000000..556a24aa42
--- /dev/null
+++ b/lib/Entities/NewsletterLinkEntity.php
@@ -0,0 +1,97 @@
+<?php
+
+namespace MailPoet\Entities;
+
+use MailPoet\Doctrine\EntityTraits\AutoincrementedIdTrait;
+use MailPoet\Doctrine\EntityTraits\CreatedAtTrait;
+use MailPoet\Doctrine\EntityTraits\UpdatedAtTrait;
+use MailPoetVendor\Doctrine\Common\Collections\ArrayCollection;
+use MailPoetVendor\Doctrine\Common\Collections\Criteria;
+use MailPoetVendor\Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity()
+ * @ORM\Table(name="newsletter_links")
+ */
+class NewsletterLinkEntity {
+  use AutoincrementedIdTrait;
+  use CreatedAtTrait;
+  use UpdatedAtTrait;
+
+  /**
+   * @ORM\Column(type="integer")
+   * @var integer
+   */
+  private $newsletter_id;
+
+  /**
+   * @ORM\OneToOne(targetEntity="MailPoet\Entities\NewsletterEntity")
+   * @ORM\JoinColumn(name="newsletter_id", referencedColumnName="id")
+   * @var NewsletterEntity|null
+   */
+  private $newsletter;
+
+  /**
+   * @ORM\OneToOne(targetEntity="MailPoet\Entities\SendingQueueEntity")
+   * @ORM\JoinColumn(name="queue_id", referencedColumnName="id")
+   * @var SendingQueueEntity|null
+   */
+  private $queue;
+
+  /**
+   * @ORM\Column(type="string")
+   * @var string|null
+   */
+  private $url;
+
+  /**
+   * @ORM\Column(type="string")
+   * @var string|null
+   */
+  private $hash;
+
+  /**
+   * Extra lazy is here for `getTotalClicksCount`.
+   * If we didn't specify extra lazy the function would load all clicks and count them. This way it uses a single count query.
+   * @ORM\OneToMany(targetEntity="MailPoet\Entities\StatisticsClicksEntity", mappedBy="link", fetch="EXTRA_LAZY")
+   * @var StatisticsClicksEntity[]|ArrayCollection
+   */
+  private $clicks;
+
+  /**
+   * @return NewsletterEntity|null
+   */
+  public function getNewsletter() {
+    return $this->newsletter;
+  }
+
+  /**
+   * @return SendingQueueEntity|null
+   */
+  public function getQueue() {
+    return $this->queue;
+  }
+
+  /**
+   * @return string|null
+   */
+  public function getUrl() {
+    return $this->url;
+  }
+
+  /**
+   * @return string|null
+   */
+  public function getHash() {
+    return $this->hash;
+  }
+
+  /**
+   * @return int
+   */
+  function getTotalClicksCount() {
+    $criteria = Criteria::create()
+      ->where(Criteria::expr()->eq("link", $this));
+    return $this->clicks->matching($criteria)->count();
+  }
+}
diff --git a/lib/Entities/StatisticsClicksEntity.php b/lib/Entities/StatisticsClicksEntity.php
new file mode 100644
index 0000000000..64e32de911
--- /dev/null
+++ b/lib/Entities/StatisticsClicksEntity.php
@@ -0,0 +1,51 @@
+<?php
+
+namespace MailPoet\Entities;
+
+use MailPoet\Doctrine\EntityTraits\AutoincrementedIdTrait;
+use MailPoet\Doctrine\EntityTraits\CreatedAtTrait;
+use MailPoet\Doctrine\EntityTraits\UpdatedAtTrait;
+use MailPoetVendor\Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity()
+ * @ORM\Table(name="statistics_clicks")
+ */
+class StatisticsClicksEntity {
+  use AutoincrementedIdTrait;
+  use CreatedAtTrait;
+  use UpdatedAtTrait;
+
+  /**
+   * @ORM\OneToOne(targetEntity="MailPoet\Entities\NewsletterEntity")
+   * @ORM\JoinColumn(name="newsletter_id", referencedColumnName="id")
+   * @var NewsletterEntity|null
+   */
+  private $newsletter;
+
+  /**
+   * @ORM\OneToOne(targetEntity="MailPoet\Entities\SendingQueueEntity")
+   * @ORM\JoinColumn(name="queue_id", referencedColumnName="id")
+   * @var SendingQueueEntity|null
+   */
+  private $queue;
+
+  /**
+   * @ORM\Column(type="integer")
+   * @var int|null
+   */
+  private $subscriber_id;
+
+  /**
+   * @ORM\ManyToOne(targetEntity="MailPoet\Entities\NewsletterLinkEntity", inversedBy="clicks")
+   * @var NewsletterLinkEntity
+   */
+  private $link;
+
+  /**
+   * @ORM\Column(type="integer")
+   * @var int|null
+   */
+  private $count;
+
+}
diff --git a/lib/Models/NewsletterLink.php b/lib/Models/NewsletterLink.php
index 34d55b71b2..a26d6eaf27 100644
--- a/lib/Models/NewsletterLink.php
+++ b/lib/Models/NewsletterLink.php
@@ -12,27 +12,4 @@ namespace MailPoet\Models;
 class NewsletterLink extends Model {
   public static $_table = MP_NEWSLETTER_LINKS_TABLE;
   const UNSUBSCRIBE_LINK_SHORT_CODE = '[link:subscription_unsubscribe_url]';
-
-  /**
-   * @param Newsletter $newsletter
-   * @return NewsletterLink|null
-   */
-  static function findTopLinkForNewsletter(Newsletter $newsletter) {
-    $link = self::selectExpr('links.*')
-      ->selectExpr('count(*)', 'clicksCount')
-      ->tableAlias('links')
-      ->innerJoin(StatisticsClicks::$_table,
-        ['clicks.link_id', '=', 'links.id'],
-        'clicks')
-      ->where('newsletter_id', $newsletter->id())
-      ->groupBy('links.id')
-      ->orderByDesc('clicksCount')
-      ->limit(1)
-      ->findOne();
-    if (!$link) {
-      return null;
-    }
-    return $link;
-  }
-
 }
diff --git a/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php b/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php
index ac271bec07..d7a82e049e 100644
--- a/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php
+++ b/tests/integration/Cron/Workers/StatsNotifications/WorkerTest.php
@@ -17,7 +17,6 @@ use MailPoet\Models\StatisticsUnsubscribes;
 use MailPoet\Models\StatsNotification;
 use MailPoet\Settings\SettingsController;
 use MailPoet\WooCommerce\Helper as WCHelper;
-use MailPoetVendor\Doctrine\ORM\EntityManager;
 use PHPUnit\Framework\MockObject\MockObject;
 
 class WorkerTest extends \MailPoetTest {
@@ -43,12 +42,16 @@ class WorkerTest extends \MailPoetTest {
   /** @var StatsNotificationsRepository */
   private $repository;
 
+  /** @var NewsletterLinkRepository */
+  private $newsletter_link_repository;
+
   function _before() {
     parent::_before();
     \ORM::raw_execute('TRUNCATE ' . Newsletter::$_table);
     \ORM::raw_execute('TRUNCATE ' . ScheduledTask::$_table);
     \ORM::raw_execute('TRUNCATE ' . SendingQueue::$_table);
     $this->repository = ContainerWrapper::getInstance()->get(StatsNotificationsRepository::class);
+    $this->newsletter_link_repository = ContainerWrapper::getInstance()->get(NewsletterLinkRepository::class);
     $this->repository->truncate();
     $this->mailer = $this->createMock(Mailer::class);
     $this->renderer = $this->createMock(Renderer::class);
@@ -59,6 +62,7 @@ class WorkerTest extends \MailPoetTest {
       $this->settings,
       new MetaInfo,
       $this->repository,
+      $this->newsletter_link_repository,
       $this->entity_manager
     );
     $this->settings->set(Worker::SETTINGS_KEY, [