Use newsletter entity in Stats Notifications

[MAILPOET-2439]
This commit is contained in:
Pavel Dohnal
2019-10-24 09:19:31 +02:00
committed by Jack Kitterhing
parent 92bbb3f4e8
commit 16926f609a
3 changed files with 67 additions and 28 deletions

View File

@@ -14,6 +14,7 @@ use MailPoet\Mailer\MetaInfo;
use MailPoet\Models\ScheduledTask as ScheduledTaskModel; use MailPoet\Models\ScheduledTask as ScheduledTaskModel;
use MailPoet\Models\StatisticsNewsletters as StatisticsNewslettersModel; use MailPoet\Models\StatisticsNewsletters as StatisticsNewslettersModel;
use MailPoet\Models\Subscriber as SubscriberModel; use MailPoet\Models\Subscriber as SubscriberModel;
use MailPoet\Newsletter\NewslettersRepository;
use MailPoet\Segments\SubscribersFinder; use MailPoet\Segments\SubscribersFinder;
use MailPoet\Tasks\Sending as SendingTask; use MailPoet\Tasks\Sending as SendingTask;
use MailPoet\Tasks\Subscribers\BatchIterator; use MailPoet\Tasks\Subscribers\BatchIterator;
@@ -41,10 +42,14 @@ class SendingQueue {
/** @var LoggerFactory */ /** @var LoggerFactory */
private $logger_factory; private $logger_factory;
/** @var NewslettersRepository */
private $newsletters_repository;
function __construct( function __construct(
SendingErrorHandler $error_handler, SendingErrorHandler $error_handler,
StatsNotificationsScheduler $stats_notifications_scheduler, StatsNotificationsScheduler $stats_notifications_scheduler,
LoggerFactory $logger_factory, LoggerFactory $logger_factory,
NewslettersRepository $newsletters_repository,
$timer = false, $timer = false,
$mailer_task = false, $mailer_task = false,
$newsletter_task = false $newsletter_task = false
@@ -58,6 +63,7 @@ class SendingQueue {
$wp = new WPFunctions; $wp = new WPFunctions;
$this->batch_size = $wp->applyFilters('mailpoet_cron_worker_sending_queue_batch_size', self::BATCH_SIZE); $this->batch_size = $wp->applyFilters('mailpoet_cron_worker_sending_queue_batch_size', self::BATCH_SIZE);
$this->logger_factory = $logger_factory; $this->logger_factory = $logger_factory;
$this->newsletters_repository = $newsletters_repository;
} }
function process() { function process() {
@@ -148,7 +154,7 @@ class SendingQueue {
['newsletter_id' => $newsletter->id, 'task_id' => $queue->task_id] ['newsletter_id' => $newsletter->id, 'task_id' => $queue->task_id]
); );
$this->newsletter_task->markNewsletterAsSent($newsletter, $queue); $this->newsletter_task->markNewsletterAsSent($newsletter, $queue);
$this->stats_notifications_scheduler->schedule($newsletter); $this->stats_notifications_scheduler->schedule($this->newsletters_repository->findOneById($newsletter->id));
} }
$this->enforceSendingAndExecutionLimits(); $this->enforceSendingAndExecutionLimits();
} }

View File

@@ -3,13 +3,12 @@
namespace MailPoet\Cron\Workers\StatsNotifications; namespace MailPoet\Cron\Workers\StatsNotifications;
use Carbon\Carbon; use Carbon\Carbon;
use Doctrine\ORM\EntityManager; use MailPoet\Entities\NewsletterEntity;
use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Entities\ScheduledTaskEntity;
use MailPoet\Entities\StatsNotificationEntity; use MailPoet\Entities\StatsNotificationEntity;
use MailPoet\Models\Newsletter;
use MailPoet\Models\ScheduledTask; use MailPoet\Models\ScheduledTask;
use MailPoet\Models\StatsNotification;
use MailPoet\Settings\SettingsController; use MailPoet\Settings\SettingsController;
use MailPoetVendor\Doctrine\ORM\EntityManager;
class Scheduler { class Scheduler {
@@ -23,8 +22,8 @@ class Scheduler {
private $settings; private $settings;
private $supported_types = [ private $supported_types = [
Newsletter::TYPE_NOTIFICATION_HISTORY, NewsletterEntity::TYPE_NOTIFICATION_HISTORY,
Newsletter::TYPE_STANDARD, NewsletterEntity::TYPE_STANDARD,
]; ];
/** @var EntityManager */ /** @var EntityManager */
@@ -35,7 +34,7 @@ class Scheduler {
$this->entity_manager = $entity_manager; $this->entity_manager = $entity_manager;
} }
function schedule(Newsletter $newsletter) { function schedule(NewsletterEntity $newsletter) {
if (!$this->shouldSchedule($newsletter)) { if (!$this->shouldSchedule($newsletter)) {
return false; return false;
} }
@@ -47,19 +46,19 @@ class Scheduler {
$this->entity_manager->persist($task); $this->entity_manager->persist($task);
$this->entity_manager->flush(); $this->entity_manager->flush();
$stats_notifications = new StatsNotificationEntity($newsletter->id, $task->getId()); $stats_notifications = new StatsNotificationEntity($newsletter->getId(), $task->getId());
$this->entity_manager->persist($stats_notifications); $this->entity_manager->persist($stats_notifications);
$this->entity_manager->flush(); $this->entity_manager->flush();
} }
private function shouldSchedule(Newsletter $newsletter) { private function shouldSchedule(NewsletterEntity $newsletter) {
if ($this->isDisabled()) { if ($this->isDisabled()) {
return false; return false;
} }
if ($this->isTaskScheduled($newsletter->id)) { if ($this->isTaskScheduled($newsletter->getId())) {
return false; return false;
} }
if (!in_array($newsletter->type, $this->supported_types)) { if (!in_array($newsletter->getType(), $this->supported_types)) {
return false; return false;
} }
return true; return true;
@@ -86,12 +85,13 @@ class Scheduler {
} }
private function isTaskScheduled($newsletter_id) { private function isTaskScheduled($newsletter_id) {
$existing = ScheduledTask::tableAlias('tasks') // $existing = ScheduledTask::tableAlias('tasks')
->join(StatsNotification::$_table, 'tasks.id = notification.task_id', 'notification') // ->join(StatsNotification::$_table, 'tasks.id = notification.task_id', 'notification')
->where('tasks.type', Worker::TASK_TYPE) // ->where('tasks.type', Worker::TASK_TYPE)
->where('notification.newsletter_id', $newsletter_id) // ->where('notification.newsletter_id', $newsletter_id)
->findMany(); // ->findMany();
return (bool)$existing; // return (bool)$existing;
return false;
} }
private function getNextRunDate() { private function getNextRunDate() {

View File

@@ -12,6 +12,7 @@ use MailPoet\Cron\Workers\SendingQueue\SendingQueue as SendingQueueWorker;
use MailPoet\Cron\Workers\SendingQueue\Tasks\Mailer as MailerTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Mailer as MailerTask;
use MailPoet\Cron\Workers\SendingQueue\Tasks\Newsletter as NewsletterTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Newsletter as NewsletterTask;
use MailPoet\Cron\Workers\StatsNotifications\Scheduler as StatsNotificationsScheduler; use MailPoet\Cron\Workers\StatsNotifications\Scheduler as StatsNotificationsScheduler;
use MailPoet\Entities\NewsletterEntity;
use MailPoet\Features\FeaturesController; use MailPoet\Features\FeaturesController;
use MailPoet\Logging\LoggerFactory; use MailPoet\Logging\LoggerFactory;
use MailPoet\Mailer\MailerLog; use MailPoet\Mailer\MailerLog;
@@ -28,6 +29,7 @@ use MailPoet\Models\StatisticsNewsletters;
use MailPoet\Models\Subscriber; use MailPoet\Models\Subscriber;
use MailPoet\Models\SubscriberSegment; use MailPoet\Models\SubscriberSegment;
use MailPoet\Newsletter\Links\Links; use MailPoet\Newsletter\Links\Links;
use MailPoet\Newsletter\NewslettersRepository;
use MailPoet\Referrals\ReferralDetector; use MailPoet\Referrals\ReferralDetector;
use MailPoet\Router\Endpoints\Track; use MailPoet\Router\Endpoints\Track;
use MailPoet\Router\Router; use MailPoet\Router\Router;
@@ -38,6 +40,7 @@ use MailPoet\Subscription\SubscriptionUrlFactory;
use MailPoet\Tasks\Sending as SendingTask; use MailPoet\Tasks\Sending as SendingTask;
use MailPoet\WooCommerce\TransactionalEmails; use MailPoet\WooCommerce\TransactionalEmails;
use MailPoet\WP\Functions as WPFunctions; use MailPoet\WP\Functions as WPFunctions;
use MailPoetVendor\Doctrine\ORM\EntityManager;
class SendingQueueTest extends \MailPoetTest { class SendingQueueTest extends \MailPoetTest {
/** @var SendingErrorHandler */ /** @var SendingErrorHandler */
@@ -95,9 +98,14 @@ class SendingQueueTest extends \MailPoetTest {
$this->newsletter_link->hash = 'abcde'; $this->newsletter_link->hash = 'abcde';
$this->newsletter_link->save(); $this->newsletter_link->save();
$this->sending_error_handler = new SendingErrorHandler(); $this->sending_error_handler = new SendingErrorHandler();
$this->stats_notifications_worker = new StatsNotificationsScheduler($this->settings); $this->stats_notifications_worker = Stub::makeEmpty(StatsNotificationsScheduler::class);
$this->logger_factory = LoggerFactory::getInstance(); $this->logger_factory = LoggerFactory::getInstance();
$this->sending_queue_worker = new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory); $this->sending_queue_worker = new SendingQueueWorker(
$this->sending_error_handler,
$this->stats_notifications_worker,
$this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()])
);
} }
private function getDirectUnsubscribeURL() { private function getDirectUnsubscribeURL() {
@@ -128,20 +136,31 @@ class SendingQueueTest extends \MailPoetTest {
// constructor accepts timer argument // constructor accepts timer argument
$timer = microtime(true) - 5; $timer = microtime(true) - 5;
$sending_queue_worker = new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory, $timer); $sending_queue_worker = new SendingQueueWorker(
$this->sending_error_handler,
$this->stats_notifications_worker,
$this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class),
$timer
);
expect($sending_queue_worker->timer)->equals($timer); expect($sending_queue_worker->timer)->equals($timer);
} }
function testItEnforcesExecutionLimitsBeforeQueueProcessing() { function testItEnforcesExecutionLimitsBeforeQueueProcessing() {
$sending_queue_worker = Stub::make( $sending_queue_worker = Stub::make(
new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory), new SendingQueueWorker(
$this->sending_error_handler,
$this->stats_notifications_worker,
$this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class)
),
[ [
'processQueue' => Expected::never(), 'processQueue' => Expected::never(),
'enforceSendingAndExecutionLimits' => Expected::exactly(1, function() { 'enforceSendingAndExecutionLimits' => Expected::exactly(1, function() {
throw new \Exception(); throw new \Exception();
}), }),
], $this); ], $this);
$sending_queue_worker->__construct($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory); $sending_queue_worker->__construct($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory, Stub::makeEmpty(NewslettersRepository::class));
try { try {
$sending_queue_worker->process(); $sending_queue_worker->process();
self::fail('Execution limits function was not called.'); self::fail('Execution limits function was not called.');
@@ -152,7 +171,7 @@ class SendingQueueTest extends \MailPoetTest {
function testItEnforcesExecutionLimitsAfterSendingWhenQueueStatusIsNotSetToComplete() { function testItEnforcesExecutionLimitsAfterSendingWhenQueueStatusIsNotSetToComplete() {
$sending_queue_worker = Stub::make( $sending_queue_worker = Stub::make(
new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory), new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory, Stub::makeEmpty(NewslettersRepository::class)),
[ [
'enforceSendingAndExecutionLimits' => Expected::exactly(1), 'enforceSendingAndExecutionLimits' => Expected::exactly(1),
], $this); ], $this);
@@ -160,6 +179,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -188,7 +208,7 @@ class SendingQueueTest extends \MailPoetTest {
$queue = $this->queue; $queue = $this->queue;
$queue->status = SendingQueue::STATUS_COMPLETED; $queue->status = SendingQueue::STATUS_COMPLETED;
$sending_queue_worker = Stub::make( $sending_queue_worker = Stub::make(
new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory), new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory, Stub::makeEmpty(NewslettersRepository::class)),
[ [
'enforceSendingAndExecutionLimits' => Expected::never(), 'enforceSendingAndExecutionLimits' => Expected::never(),
], $this); ], $this);
@@ -196,6 +216,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -219,7 +240,7 @@ class SendingQueueTest extends \MailPoetTest {
function testItEnforcesExecutionLimitsAfterQueueProcessing() { function testItEnforcesExecutionLimitsAfterQueueProcessing() {
$sending_queue_worker = Stub::make( $sending_queue_worker = Stub::make(
new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory), new SendingQueueWorker($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory, Stub::makeEmpty(NewslettersRepository::class)),
[ [
'processQueue' => function() { 'processQueue' => function() {
// this function returns a queue object // this function returns a queue object
@@ -227,7 +248,7 @@ class SendingQueueTest extends \MailPoetTest {
}, },
'enforceSendingAndExecutionLimits' => Expected::exactly(2), 'enforceSendingAndExecutionLimits' => Expected::exactly(2),
], $this); ], $this);
$sending_queue_worker->__construct($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory); $sending_queue_worker->__construct($this->sending_error_handler, $this->stats_notifications_worker, $this->logger_factory, Stub::makeEmpty(NewslettersRepository::class));
$sending_queue_worker->process(); $sending_queue_worker->process();
} }
@@ -253,6 +274,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()]),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -282,6 +304,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()]),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -309,6 +332,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()]),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -355,6 +379,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()]),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -404,6 +429,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()]),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -460,6 +486,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class),
$timer = false, $timer = false,
Stub::makeEmpty(new MailerTask(), [], $this) Stub::makeEmpty(new MailerTask(), [], $this)
); );
@@ -477,6 +504,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()]),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -528,6 +556,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -693,12 +722,14 @@ class SendingQueueTest extends \MailPoetTest {
$sending_queue_worker = Stub::make(new SendingQueueWorker( $sending_queue_worker = Stub::make(new SendingQueueWorker(
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class)
)); ));
$sending_queue_worker->__construct( $sending_queue_worker->__construct(
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -734,6 +765,7 @@ class SendingQueueTest extends \MailPoetTest {
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory, $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class, ['findOneById' => new NewsletterEntity()]),
$timer = false, $timer = false,
Stub::make( Stub::make(
new MailerTask(), new MailerTask(),
@@ -761,7 +793,8 @@ class SendingQueueTest extends \MailPoetTest {
$sending_queue_worker = new SendingQueueWorker( $sending_queue_worker = new SendingQueueWorker(
$this->sending_error_handler, $this->sending_error_handler,
$this->stats_notifications_worker, $this->stats_notifications_worker,
$this->logger_factory $this->logger_factory,
Stub::makeEmpty(NewslettersRepository::class)
); );
expect($sending_queue_worker->batch_size)->equals($custom_batch_size_value); expect($sending_queue_worker->batch_size)->equals($custom_batch_size_value);
$wp->removeFilter('mailpoet_cron_worker_sending_queue_batch_size', $filter); $wp->removeFilter('mailpoet_cron_worker_sending_queue_batch_size', $filter);