From 4fe642916ebfc0b79e91c9848d9127b4dbcabcad Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 23 Nov 2023 14:58:12 -0300 Subject: [PATCH] Replace Newsletter model with Doctrine in SendingQueue [MAILPOET-5737] --- .../Workers/SendingQueue/SendingQueue.php | 66 +++++++------------ .../Workers/SendingQueue/Tasks/Mailer.php | 19 +++--- .../Workers/SendingQueue/SendingQueueTest.php | 7 -- .../Workers/SendingQueue/Tasks/MailerTest.php | 12 ++-- 4 files changed, 40 insertions(+), 64 deletions(-) diff --git a/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php b/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php index a6d85ba6f6..25afac04ed 100644 --- a/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -15,9 +15,7 @@ use MailPoet\InvalidStateException; use MailPoet\Logging\LoggerFactory; use MailPoet\Mailer\MailerLog; use MailPoet\Mailer\MetaInfo; -use MailPoet\Models\Newsletter; use MailPoet\Models\StatisticsNewsletters as StatisticsNewslettersModel; -use MailPoet\Newsletter\NewslettersRepository; use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Newsletter\Sending\ScheduledTaskSubscribersRepository; use MailPoet\Newsletter\Sending\SendingQueuesRepository; @@ -56,9 +54,6 @@ class SendingQueue { /** @var LoggerFactory */ private $loggerFactory; - /** @var NewslettersRepository */ - private $newslettersRepository; - /** @var CronHelper */ private $cronHelper; @@ -94,7 +89,6 @@ class SendingQueue { SendingThrottlingHandler $throttlingHandler, StatsNotificationsScheduler $statsNotificationsScheduler, LoggerFactory $loggerFactory, - NewslettersRepository $newslettersRepository, CronHelper $cronHelper, SubscribersFinder $subscriberFinder, SegmentsRepository $segmentsRepository, @@ -118,7 +112,6 @@ class SendingQueue { $this->mailerMetaInfo = new MetaInfo; $this->wp = $wp; $this->loggerFactory = $loggerFactory; - $this->newslettersRepository = $newslettersRepository; $this->cronHelper = $cronHelper; $this->links = $links; $this->scheduledTasksRepository = $scheduledTasksRepository; @@ -169,38 +162,30 @@ class SendingQueue { $this->deleteTaskIfNewsletterDoesNotExist($task); $queue = $task->getSendingQueue(); - $newsletterEntity = $this->newsletterTask->getNewsletterFromQueue($task); - if (!$queue || !$newsletterEntity) { + $newsletter = $this->newsletterTask->getNewsletterFromQueue($task); + if (!$queue || !$newsletter) { return; } // pre-process newsletter (render, replace shortcodes/links, etc.) - $newsletterEntity = $this->newsletterTask->preProcessNewsletter($newsletterEntity, $task); + $newsletter = $this->newsletterTask->preProcessNewsletter($newsletter, $task); - if (!$newsletterEntity) { + if (!$newsletter) { $this->deleteTask($task); return; } - $newsletter = Newsletter::findOne($newsletterEntity->getId()); - if (!$newsletter) { - return; - } - - $isTransactional = in_array($newsletter->type, [ + $isTransactional = in_array($newsletter->getType(), [ NewsletterEntity::TYPE_AUTOMATION_TRANSACTIONAL, NewsletterEntity::TYPE_WC_TRANSACTIONAL_EMAIL, ]); - // clone the original object to be used for processing - $_newsletter = (object)$newsletter->asArray(); - $_newsletter->options = $newsletterEntity->getOptionsAsArray(); // configure mailer $this->mailerTask->configureMailer($newsletter); // get newsletter segments - $newsletterSegmentsIds = $newsletterEntity->getSegmentIds(); + $newsletterSegmentsIds = $newsletter->getSegmentIds(); $segmentIdsToCheck = $newsletterSegmentsIds; - $filterSegmentId = $newsletterEntity->getFilterSegmentId(); + $filterSegmentId = $newsletter->getFilterSegmentId(); if (is_int($filterSegmentId)) { $segmentIdsToCheck[] = $filterSegmentId; @@ -214,7 +199,7 @@ class SendingQueue { ); $task->setStatus(ScheduledTaskEntity::STATUS_PAUSED); $this->scheduledTasksRepository->flush(); - $this->wp->setTransient(self::EMAIL_WITH_INVALID_SEGMENT_OPTION, $newsletter->subject); + $this->wp->setTransient(self::EMAIL_WITH_INVALID_SEGMENT_OPTION, $newsletter->getSubject()); return; } @@ -232,7 +217,7 @@ class SendingQueue { foreach ($subscriberBatches as $subscribersToProcessIds) { $this->loggerFactory->getLogger(LoggerFactory::TOPIC_NEWSLETTERS)->info( 'subscriber batch processing', - ['newsletter_id' => $newsletter->id, 'task_id' => $task->getId(), 'subscriber_batch_count' => count($subscribersToProcessIds)] + ['newsletter_id' => $newsletter->getId(), 'task_id' => $task->getId(), 'subscriber_batch_count' => count($subscribersToProcessIds)] ); if (!empty($newsletterSegmentsIds[0])) { // Check that subscribers are in segments @@ -259,7 +244,7 @@ class SendingQueue { ->setParameter('subscriberIds', $subscribersToProcessIds) ->andWhere('s.deletedAt IS NULL'); - if ($newsletterEntity->getType() === NewsletterEntity::TYPE_AUTOMATION_TRANSACTIONAL) { + if ($newsletter->getType() === NewsletterEntity::TYPE_AUTOMATION_TRANSACTIONAL) { $queryBuilder->andWhere('s.status != :bouncedStatus') ->setParameter('bouncedStatus', SubscriberEntity::STATUS_BOUNCED); } else { @@ -285,7 +270,7 @@ class SendingQueue { $this->sendingQueuesRepository->updateCounts($queue); if (!$queue->getCountToProcess()) { - $this->newsletterTask->markNewsletterAsSent($newsletterEntity); + $this->newsletterTask->markNewsletterAsSent($newsletter); continue; } // if there aren't any subscribers to process in batch (e.g. all unsubscribed or were deleted) continue with next batch @@ -295,16 +280,16 @@ class SendingQueue { } $this->loggerFactory->getLogger(LoggerFactory::TOPIC_NEWSLETTERS)->info( 'before queue chunk processing', - ['newsletter_id' => $newsletter->id, 'task_id' => $task->getId(), 'found_subscribers_count' => count($foundSubscribers)] + ['newsletter_id' => $newsletter->getId(), 'task_id' => $task->getId(), 'found_subscribers_count' => count($foundSubscribers)] ); // reschedule bounce task to run sooner, if needed $this->reScheduleBounceTask(); - if ($newsletterEntity->getStatus() !== NewsletterEntity::STATUS_CORRUPT) { + if ($newsletter->getStatus() !== NewsletterEntity::STATUS_CORRUPT) { $this->processQueue( $task, - $_newsletter, + $newsletter, $foundSubscribers, $timer ); @@ -318,22 +303,22 @@ class SendingQueue { } $this->loggerFactory->getLogger(LoggerFactory::TOPIC_NEWSLETTERS)->info( 'after queue chunk processing', - ['newsletter_id' => $newsletter->id, 'task_id' => $task->getId()] + ['newsletter_id' => $newsletter->getId(), 'task_id' => $task->getId()] ); if ($task->getStatus() === ScheduledTaskEntity::STATUS_COMPLETED) { $this->loggerFactory->getLogger(LoggerFactory::TOPIC_NEWSLETTERS)->info( 'completed newsletter sending', - ['newsletter_id' => $newsletter->id, 'task_id' => $task->getId()] + ['newsletter_id' => $newsletter->getId(), 'task_id' => $task->getId()] ); - $this->newsletterTask->markNewsletterAsSent($newsletterEntity); - $this->statsNotificationsScheduler->schedule($newsletterEntity); + $this->newsletterTask->markNewsletterAsSent($newsletter); + $this->statsNotificationsScheduler->schedule($newsletter); } $this->enforceSendingAndExecutionLimits($timer); } else { $this->sendingQueuesRepository->pause($queue); $this->loggerFactory->getLogger(LoggerFactory::TOPIC_NEWSLETTERS)->error( 'Can\'t send corrupt newsletter', - ['newsletter_id' => $newsletter->id, 'task_id' => $task->getId()] + ['newsletter_id' => $newsletter->getId(), 'task_id' => $task->getId()] ); } } @@ -346,7 +331,7 @@ class SendingQueue { /** * @param SubscriberEntity[] $subscribers */ - public function processQueue(ScheduledTaskEntity $task, $newsletter, array $subscribers, $timer) { + public function processQueue(ScheduledTaskEntity $task, NewsletterEntity $newsletter, array $subscribers, $timer) { // determine if processing is done in bulk or individually $processingMethod = $this->mailerTask->getProcessingMethod(); $preparedNewsletters = []; @@ -364,16 +349,11 @@ class SendingQueue { $sendingQueueMeta = $sendingQueueEntity->getMeta() ?? []; $campaignId = $sendingQueueMeta['campaignId'] ?? null; - $newsletterEntity = $this->newslettersRepository->findOneById($newsletter->id); - if (!$newsletterEntity) { - return; - } - foreach ($subscribers as $subscriber) { // render shortcodes and replace subscriber data in tracked links $preparedNewsletters[] = $this->newsletterTask->prepareNewsletterForSending( - $newsletterEntity, + $newsletter, $subscriber, $sendingQueueEntity ); @@ -386,7 +366,7 @@ class SendingQueue { $unsubscribeUrls[] = $this->links->getUnsubscribeUrl($sendingQueueEntity->getId(), $subscriber); $oneClickUnsubscribeUrls[] = $this->links->getOneClickUnsubscribeUrl($sendingQueueEntity->getId(), $subscriber); - $metasForSubscriber = $this->mailerMetaInfo->getNewsletterMetaInfo($newsletterEntity, $subscriber); + $metasForSubscriber = $this->mailerMetaInfo->getNewsletterMetaInfo($newsletter, $subscriber); if ($campaignId) { $metasForSubscriber['campaign_id'] = $campaignId; } @@ -394,7 +374,7 @@ class SendingQueue { // keep track of values for statistics purposes $statistics[] = [ - 'newsletter_id' => $newsletter->id, + 'newsletter_id' => $newsletter->getId(), 'subscriber_id' => $subscriber->getId(), 'queue_id' => $sendingQueueEntity->getId(), ]; diff --git a/mailpoet/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php b/mailpoet/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php index a66207d12b..c09d8c9c31 100644 --- a/mailpoet/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php +++ b/mailpoet/lib/Cron/Workers/SendingQueue/Tasks/Mailer.php @@ -2,6 +2,7 @@ namespace MailPoet\Cron\Workers\SendingQueue\Tasks; +use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\SubscriberEntity; use MailPoet\Mailer\Mailer as MailerInstance; use MailPoet\Mailer\MailerFactory; @@ -23,18 +24,18 @@ class Mailer { $this->mailer = $this->configureMailer(); } - public function configureMailer($newsletter = null) { - $sender['address'] = (!empty($newsletter->senderAddress)) ? - $newsletter->senderAddress : + public function configureMailer(NewsletterEntity $newsletter = null) { + $sender['address'] = ($newsletter && !empty($newsletter->getSenderAddress())) ? + $newsletter->getSenderAddress() : null; - $sender['name'] = (!empty($newsletter->senderName)) ? - $newsletter->senderName : + $sender['name'] = ($newsletter && !empty($newsletter->getSenderName())) ? + $newsletter->getSenderName() : null; - $replyTo['address'] = (!empty($newsletter->replyToAddress)) ? - $newsletter->replyToAddress : + $replyTo['address'] = ($newsletter && !empty($newsletter->getReplyToAddress())) ? + $newsletter->getReplyToAddress() : null; - $replyTo['name'] = (!empty($newsletter->replyToName)) ? - $newsletter->replyToName : + $replyTo['name'] = ($newsletter && !empty($newsletter->getReplyToName())) ? + $newsletter->getReplyToName() : null; if (!$sender['address']) { $sender = null; diff --git a/mailpoet/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php b/mailpoet/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php index 46077e6d32..aca7537217 100644 --- a/mailpoet/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php +++ b/mailpoet/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php @@ -206,7 +206,6 @@ class SendingQueueTest extends \MailPoetTest { $this->sendingThrottlingHandler, $this->statsNotificationsWorker, $this->loggerFactory, - $this->newslettersRepository, $this->cronHelper, $this->subscribersFinder, $this->segmentsRepository, @@ -238,7 +237,6 @@ class SendingQueueTest extends \MailPoetTest { $this->sendingThrottlingHandler, $this->statsNotificationsWorker, $this->loggerFactory, - $this->makeEmpty(NewslettersRepository::class), $this->cronHelper, $this->subscribersFinder, $this->segmentsRepository, @@ -287,7 +285,6 @@ class SendingQueueTest extends \MailPoetTest { $this->sendingThrottlingHandler, $this->statsNotificationsWorker, $this->loggerFactory, - $this->makeEmpty(NewslettersRepository::class), $this->cronHelper, $this->subscribersFinder, $this->segmentsRepository, @@ -334,7 +331,6 @@ class SendingQueueTest extends \MailPoetTest { $this->sendingThrottlingHandler, $this->statsNotificationsWorker, $this->loggerFactory, - $this->newslettersRepository, $this->cronHelper, $this->subscribersFinder, $this->segmentsRepository, @@ -651,7 +647,6 @@ class SendingQueueTest extends \MailPoetTest { $this->sendingThrottlingHandler, $this->statsNotificationsWorker, $this->loggerFactory, - $this->makeEmpty(NewslettersRepository::class), $this->cronHelper, $this->subscribersFinder, $this->segmentsRepository, @@ -1112,7 +1107,6 @@ class SendingQueueTest extends \MailPoetTest { $this->sendingThrottlingHandler, $this->statsNotificationsWorker, $this->loggerFactory, - $this->makeEmpty(NewslettersRepository::class), $this->cronHelper, $this->subscribersFinder, $this->segmentsRepository, @@ -1447,7 +1441,6 @@ class SendingQueueTest extends \MailPoetTest { $this->sendingThrottlingHandler, $this->statsNotificationsWorker, $this->loggerFactory, - $this->newslettersRepository, $this->cronHelper, $this->subscribersFinder, $this->segmentsRepository, diff --git a/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/MailerTest.php b/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/MailerTest.php index 698cc37ab6..3471379e1d 100644 --- a/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/MailerTest.php +++ b/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/MailerTest.php @@ -9,6 +9,7 @@ use MailPoet\Cron\Workers\SendingQueue\Tasks\Mailer as MailerTask; use MailPoet\Mailer\Mailer; use MailPoet\Mailer\MailerFactory; use MailPoet\Settings\SettingsController; +use MailPoet\Test\DataFactories\Newsletter as NewsletterFactory; use MailPoet\Test\DataFactories\Subscriber as SubscriberFactory; class MailerTest extends \MailPoetTest { @@ -33,11 +34,12 @@ class MailerTest extends \MailPoetTest { } public function testItCanConfigureMailerWithSenderAndReplyToAddressesFromEmail() { - $newsletter = new \stdClass(); - $newsletter->senderName = 'Sender'; - $newsletter->senderAddress = 'from@example.com'; - $newsletter->replyToName = 'Reply-to'; - $newsletter->replyToAddress = 'reply-to@example.com'; + $newsletter = (new NewsletterFactory()) + ->withSenderName('Sender') + ->withSenderAddress('from@example.com') + ->withReplyToName('Reply-to') + ->withReplyToAddress('reply-to@example.com') + ->create(); $mailerFactoryMock = $this->createMock(MailerFactory::class); // First call in constructor