From d39440f1dcddfa9b820be6db2f3a90665cae75b5 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Fri, 23 Sep 2022 16:21:10 -0300 Subject: [PATCH] Refactor Newsletter::preProcessNewsletter() to use Doctrine This commit also removes almost all the usages of the old Newsletter model from NewsletterTest. [MAILPOET-4680] --- .../Workers/SendingQueue/SendingQueue.php | 21 ++- .../Workers/SendingQueue/Tasks/Newsletter.php | 31 +++-- .../Newsletter/NewsletterSaveController.php | 2 +- .../SendingQueue/Tasks/NewsletterTest.php | 121 +++++++++--------- 4 files changed, 89 insertions(+), 86 deletions(-) diff --git a/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php b/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php index 19aa8d744b..d519b6b26e 100644 --- a/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -159,14 +159,9 @@ class SendingQueue { return; } - $newsletter = Newsletter::findOne($newsletterEntity->getId()); - if (!$newsletter) { - return; - } - // pre-process newsletter (render, replace shortcodes/links, etc.) - $newsletter = $this->newsletterTask->preProcessNewsletter($newsletter, $queue); - if (!$newsletter) { + $newsletterEntity = $this->newsletterTask->preProcessNewsletter($newsletterEntity, $queue); + if (!$newsletterEntity) { $this->loggerFactory->getLogger(LoggerFactory::TOPIC_NEWSLETTERS)->info( 'delete task in sending queue', ['task_id' => $queue->taskId] @@ -174,6 +169,12 @@ class SendingQueue { $queue->delete(); return; } + + $newsletter = Newsletter::findOne($newsletterEntity->getId()); + if (!$newsletter) { + return; + } + // clone the original object to be used for processing $_newsletter = (object)$newsletter->asArray(); $_newsletter->options = $newsletterEntity->getOptionsAsArray(); @@ -255,10 +256,8 @@ class SendingQueue { 'completed newsletter sending', ['newsletter_id' => $newsletter->id, 'task_id' => $queue->taskId] ); - $newsletter = $this->newslettersRepository->findOneById($newsletter->id); - assert($newsletter instanceof NewsletterEntity); - $this->newsletterTask->markNewsletterAsSent($newsletter, $queue); - $this->statsNotificationsScheduler->schedule($newsletter); + $this->newsletterTask->markNewsletterAsSent($newsletterEntity, $queue); + $this->statsNotificationsScheduler->schedule($newsletterEntity); } $this->enforceSendingAndExecutionLimits($timer); } diff --git a/mailpoet/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php b/mailpoet/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php index 0f85d47ba4..27f820e716 100644 --- a/mailpoet/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php +++ b/mailpoet/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php @@ -114,7 +114,7 @@ class Newsletter { return $newsletter; } - public function preProcessNewsletter(\MailPoet\Models\Newsletter $newsletter, Sending $sendingTask) { + public function preProcessNewsletter(NewsletterEntity $newsletter, Sending $sendingTask) { // return the newsletter if it was previously rendered /** @phpstan-ignore-next-line - SendingQueue::getNewsletterRenderedBody() is called inside Sending using __call(). Sending will be refactored soon to stop using Paris models. */ if (!is_null($sendingTask->getNewsletterRenderedBody())) { @@ -124,57 +124,56 @@ class Newsletter { } $this->loggerFactory->getLogger(LoggerFactory::TOPIC_NEWSLETTERS)->info( 'pre-processing newsletter', - ['newsletter_id' => $newsletter->id, 'task_id' => $sendingTask->taskId] + ['newsletter_id' => $newsletter->getId(), 'task_id' => $sendingTask->taskId] ); - $newsletterEntity = $this->newslettersRepository->findOneById($newsletter->id); - if (!$newsletterEntity) return false; + // if tracking is enabled, do additional processing if ($this->trackingEnabled) { // hook to the newsletter post-processing filter and add tracking image $this->trackingImageInserted = OpenTracking::addTrackingImage(); // render newsletter - $renderedNewsletter = $this->renderer->render($newsletterEntity, $sendingTask); + $renderedNewsletter = $this->renderer->render($newsletter, $sendingTask); $renderedNewsletter = $this->wp->applyFilters( 'mailpoet_sending_newsletter_render_after', $renderedNewsletter, $newsletter ); - $renderedNewsletter = $this->gaTracking->applyGATracking($renderedNewsletter, $newsletterEntity); + $renderedNewsletter = $this->gaTracking->applyGATracking($renderedNewsletter, $newsletter); // hash and save all links - $renderedNewsletter = $this->linksTask->process($renderedNewsletter, $newsletterEntity, $sendingTask); + $renderedNewsletter = $this->linksTask->process($renderedNewsletter, $newsletter, $sendingTask); } else { // render newsletter - $renderedNewsletter = $this->renderer->render($newsletterEntity, $sendingTask); + $renderedNewsletter = $this->renderer->render($newsletter, $sendingTask); $renderedNewsletter = $this->wp->applyFilters( 'mailpoet_sending_newsletter_render_after', $renderedNewsletter, $newsletter ); - $renderedNewsletter = $this->gaTracking->applyGATracking($renderedNewsletter, $newsletterEntity); + $renderedNewsletter = $this->gaTracking->applyGATracking($renderedNewsletter, $newsletter); } // check if this is a post notification and if it contains at least 1 ALC post if ( - $newsletter->type === NewsletterEntity::TYPE_NOTIFICATION_HISTORY && - $this->postsTask->getAlcPostsCount($renderedNewsletter, $newsletterEntity) === 0 + $newsletter->getType() === NewsletterEntity::TYPE_NOTIFICATION_HISTORY && + $this->postsTask->getAlcPostsCount($renderedNewsletter, $newsletter) === 0 ) { // delete notification history record since it will never be sent $this->loggerFactory->getLogger(LoggerFactory::TOPIC_POST_NOTIFICATIONS)->info( 'no posts in post notification, deleting it', - ['newsletter_id' => $newsletter->id, 'task_id' => $sendingTask->taskId] + ['newsletter_id' => $newsletter->getId(), 'task_id' => $sendingTask->taskId] ); - $this->newslettersRepository->bulkDelete([(int)$newsletter->id]); + $this->newslettersRepository->bulkDelete([(int)$newsletter->getId()]); return false; } // extract and save newsletter posts - $this->postsTask->extractAndSave($renderedNewsletter, $newsletterEntity); + $this->postsTask->extractAndSave($renderedNewsletter, $newsletter); $sendingQueueEntity = $sendingTask->getSendingQueueEntity(); // update queue with the rendered and pre-processed newsletter $sendingTask->newsletterRenderedSubject = ShortcodesTask::process( - $newsletter->subject, + $newsletter->getSubject(), $renderedNewsletter['html'], - $newsletterEntity, + $newsletter, null, $sendingQueueEntity ); diff --git a/mailpoet/lib/Newsletter/NewsletterSaveController.php b/mailpoet/lib/Newsletter/NewsletterSaveController.php index 82e2cfba8e..67c0367908 100644 --- a/mailpoet/lib/Newsletter/NewsletterSaveController.php +++ b/mailpoet/lib/Newsletter/NewsletterSaveController.php @@ -403,7 +403,7 @@ class NewsletterSaveController { $queueModel->newsletterRenderedBody = null; $newsletterQueueTask = new NewsletterQueueTask(); - $newsletterQueueTask->preProcessNewsletter($newsletterModel, $queueModel); + $newsletterQueueTask->preProcessNewsletter($newsletter, $queueModel); // 'preProcessNewsletter' modifies queue by old model - let's reload it $this->entityManager->refresh($queue); diff --git a/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php b/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php index dcb405ca13..824f145978 100644 --- a/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php +++ b/mailpoet/tests/integration/Cron/Workers/SendingQueue/Tasks/NewsletterTest.php @@ -13,7 +13,6 @@ use MailPoet\DI\ContainerWrapper; use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\NewsletterLinkEntity; use MailPoet\Entities\NewsletterPostEntity; -use MailPoet\Entities\NewsletterSegmentEntity; use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Entities\SendingQueueEntity; use MailPoet\Entities\SubscriberEntity; @@ -27,10 +26,11 @@ use MailPoet\Router\Router; use MailPoet\Settings\SettingsRepository; use MailPoet\Tasks\Sending; use MailPoet\Tasks\Sending as SendingTask; -use MailPoet\Test\DataFactories\Segment as SegmentFactory; +use MailPoet\Test\DataFactories\Newsletter as NewsletterFactory; use MailPoet\Test\DataFactories\Subscriber as SubscriberFactory; use MailPoet\WP\Emoji; use MailPoet\WP\Functions as WPFunctions; +use MailPoetVendor\Carbon\Carbon; class NewsletterTest extends \MailPoetTest { /** @var NewsletterTask */ @@ -39,10 +39,10 @@ class NewsletterTest extends \MailPoetTest { /** @var SubscriberEntity */ private $subscriber; - /** @var Newsletter */ + /** @var NewsletterEntity */ private $newsletter; - /** @var Newsletter */ + /** @var NewsletterEntity */ private $parentNewsletter; /** @var SendingTask */ @@ -64,21 +64,21 @@ class NewsletterTest extends \MailPoetTest { parent::_before(); $this->newsletterTask = new NewsletterTask(); $this->subscriber = (new SubscriberFactory())->create(); - $this->newsletter = Newsletter::create(); - $this->newsletter->type = NewsletterEntity::TYPE_STANDARD; - $this->newsletter->status = NewsletterEntity::STATUS_ACTIVE; - $this->newsletter->subject = Fixtures::get('newsletter_subject_template'); - $this->newsletter->body = Fixtures::get('newsletter_body_template'); - $this->newsletter->preheader = ''; - $this->newsletter->save(); - $this->parentNewsletter = Newsletter::create(); - $this->parentNewsletter->type = NewsletterEntity::TYPE_STANDARD; - $this->parentNewsletter->status = NewsletterEntity::STATUS_ACTIVE; - $this->parentNewsletter->subject = 'parent newsletter'; - $this->parentNewsletter->preheader = ''; - $this->parentNewsletter->save(); + $this->newsletter = (new NewsletterFactory()) + ->withType(NewsletterEntity::TYPE_STANDARD) + ->withStatus(NewsletterEntity::STATUS_ACTIVE) + ->withSubject(Fixtures::get('newsletter_subject_template')) + ->withBody(json_decode(Fixtures::get('newsletter_body_template'), true)) + ->create(); + + $this->parentNewsletter = (new NewsletterFactory()) + ->withType(NewsletterEntity::TYPE_STANDARD) + ->withStatus(NewsletterEntity::STATUS_ACTIVE) + ->withSubject('parent newsletter') + ->create(); + $this->sendingTask = SendingTask::create(); - $this->sendingTask->newsletter_id = $this->newsletter->id; + $this->sendingTask->newsletter_id = $this->newsletter->getId(); $this->sendingTask->save(); $this->loggerFactory = LoggerFactory::getInstance(); $this->newslettersRepository = $this->diContainer->get(NewslettersRepository::class); @@ -92,7 +92,7 @@ class NewsletterTest extends \MailPoetTest { public function testItDoesNotGetNewsletterWhenStatusIsNotActiveOrSending() { // draft or any other status return false - $newsletterEntity = $this->newslettersRepository->findOneById($this->newsletter->id); + $newsletterEntity = $this->newslettersRepository->findOneById($this->newsletter->getId()); $this->assertInstanceOf(NewsletterEntity::class, $newsletterEntity); $newsletterEntity->setStatus(NewsletterEntity::STATUS_DRAFT); $this->newslettersRepository->persist($newsletterEntity); @@ -112,20 +112,20 @@ class NewsletterTest extends \MailPoetTest { } public function testItDoesNotGetDeletedNewsletter() { - $newsletter = $this->newsletter; - $newsletter->set_expr('deleted_at', 'NOW()'); - $newsletter->save(); + $this->newsletter->setDeletedAt(new Carbon()); + $this->newslettersRepository->persist($this->newsletter); + $this->newslettersRepository->flush(); expect($this->newsletterTask->getNewsletterFromQueue($this->sendingTask))->null(); } public function testItDoesNotGetNewsletterWhenParentNewsletterStatusIsNotActiveOrSending() { // draft or any other status return false - $parentNewsletterEntity = $this->newslettersRepository->findOneById($this->parentNewsletter->id); + $parentNewsletterEntity = $this->newslettersRepository->findOneById($this->parentNewsletter->getId()); $this->assertInstanceOf(NewsletterEntity::class, $parentNewsletterEntity); $parentNewsletterEntity->setStatus( NewsletterEntity::STATUS_DRAFT); $this->newslettersRepository->persist($parentNewsletterEntity); $this->newslettersRepository->flush(); - $newsletterEntity = $this->newslettersRepository->findOneById($this->newsletter->id); + $newsletterEntity = $this->newslettersRepository->findOneById($this->newsletter->getId()); $this->assertInstanceOf(NewsletterEntity::class, $newsletterEntity); $newsletterEntity->setType(NewsletterEntity::TYPE_NOTIFICATION_HISTORY); $newsletterEntity->setParent($parentNewsletterEntity); @@ -146,20 +146,21 @@ class NewsletterTest extends \MailPoetTest { } public function testItDoesNotGetDeletedNewsletterWhenParentNewsletterIsDeleted() { - $parentNewsletter = $this->parentNewsletter; - $parentNewsletter->set_expr('deleted_at', 'NOW()'); - $parentNewsletter->save(); + $this->parentNewsletter->setDeletedAt(new Carbon()); + $this->newslettersRepository->persist($this->parentNewsletter); + $this->newslettersRepository->flush(); $newsletter = $this->newsletter; - $newsletter->type = NewsletterEntity::TYPE_NOTIFICATION_HISTORY; - $newsletter->parentId = $parentNewsletter->id; - $newsletter->save(); + $newsletter->setType(NewsletterEntity::TYPE_NOTIFICATION_HISTORY); + $newsletter->setParent($this->parentNewsletter); + $this->newslettersRepository->persist($newsletter); + $this->newslettersRepository->flush(); expect($this->newsletterTask->getNewsletterFromQueue($this->sendingTask))->null(); } public function testItReturnsNewsletterObjectWhenRenderedNewsletterBodyExistsInTheQueue() { $this->sendingTask->newsletterRenderedBody = ['html' => 'test', 'text' => 'test']; $result = $this->newsletterTask->preProcessNewsletter($this->newsletter, $this->sendingTask); - expect($result instanceof \MailPoet\Models\Newsletter)->true(); + expect($result instanceof NewsletterEntity)->true(); } public function testItHashesLinksAndInsertsTrackingImageWhenTrackingIsEnabled() { @@ -169,9 +170,9 @@ class NewsletterTest extends \MailPoetTest { $newsletterTask = new NewsletterTask($wp); $newsletterTask->trackingEnabled = true; $newsletterTask->preProcessNewsletter($this->newsletter, $this->sendingTask); - $link = $this->newsletterLinkRepository->findOneBy(['newsletter' => $this->newsletter->id]); + $link = $this->newsletterLinkRepository->findOneBy(['newsletter' => $this->newsletter->getId()]); $this->assertInstanceOf(NewsletterLinkEntity::class, $link); - $updatedQueue = SendingTask::getByNewsletterId($this->newsletter->id); + $updatedQueue = SendingTask::getByNewsletterId($this->newsletter->getId()); $renderedNewsletter = $updatedQueue->getNewsletterRenderedBody(); expect($renderedNewsletter['html']) ->stringContainsString('[mailpoet_click_data]-' . $link->getHash()); @@ -191,9 +192,9 @@ class NewsletterTest extends \MailPoetTest { $newsletterTask = new NewsletterTask($wp); $newsletterTask->trackingEnabled = false; $newsletterTask->preProcessNewsletter($this->newsletter, $this->sendingTask); - $link = $this->newsletterLinkRepository->findOneBy(['newsletter' => $this->newsletter->id]); + $link = $this->newsletterLinkRepository->findOneBy(['newsletter' => $this->newsletter->getId()]); expect($link)->null(); - $updatedQueue = SendingTask::getByNewsletterId($this->newsletter->id); + $updatedQueue = SendingTask::getByNewsletterId($this->newsletter->getId()); $renderedNewsletter = $updatedQueue->getNewsletterRenderedBody(); expect($renderedNewsletter['html']) ->stringNotContainsString('[mailpoet_click_data]'); @@ -207,42 +208,44 @@ class NewsletterTest extends \MailPoetTest { } public function testItReturnsFalseAndDeletesNewsletterWhenPostNotificationContainsNoPosts() { - $newsletter = $this->newsletter; - - $newsletter->type = NewsletterEntity::TYPE_NOTIFICATION_HISTORY; - $newsletter->parentId = $newsletter->id; + $this->newsletter->setType(NewsletterEntity::TYPE_NOTIFICATION_HISTORY); + $this->newsletter->setParent($this->newsletter); // replace post id data tag with something else - $newsletter->body = str_replace('data-post-id', 'id', $newsletter->getBodyString()); - $newsletter->save(); + $body = $this->newsletter->getBody(); + $body['content'] = json_decode(str_replace('data-post-id', 'id', $this->newsletter->getContent()), true); + $this->newsletter->setBody($body); + $this->newslettersRepository->persist($this->newsletter); + $this->newslettersRepository->flush(); // returned result is false $result = $this->newsletterTask->preProcessNewsletter($this->newsletter, $this->sendingTask); expect($result)->false(); // newsletter is deleted. $this->entityManager->clear(); // needed while part of the code uses Paris models and part uses Doctrine - $newsletter = $this->newslettersRepository->findOneById($newsletter->id); + $newsletter = $this->newslettersRepository->findOneById($this->newsletter->getId()); expect($newsletter)->null(); } public function testItSavesNewsletterPosts() { $newsletterPostRepository = ContainerWrapper::getInstance()->get(NewsletterPostsRepository::class); - $this->newsletter->type = NewsletterEntity::TYPE_NOTIFICATION_HISTORY; - $this->newsletter->parentId = $this->newsletter->id; + $this->newsletter->setType(NewsletterEntity::TYPE_NOTIFICATION_HISTORY); + $this->newsletter->setParent($this->newsletter); + $this->newslettersRepository->persist($this->newsletter); + $this->newslettersRepository->flush(); $postsTask = $this->make(PostsTask::class, [ 'getAlcPostsCount' => 1, 'loggerFactory' => $this->loggerFactory, 'newsletterPostRepository' => $newsletterPostRepository, ]); - $this->newsletter->save(); $newsletterTask = new NewsletterTask(new WPFunctions, $postsTask); $result = $newsletterTask->preProcessNewsletter($this->newsletter, $this->sendingTask); - $newsletterPost = $newsletterPostRepository->findOneBy(['newsletter' => $this->newsletter->id]); + $newsletterPost = $newsletterPostRepository->findOneBy(['newsletter' => $this->newsletter->getId()]); expect($newsletterPost)->isInstanceOf(NewsletterPostEntity::class); expect($result)->notEquals(false); expect($newsletterPost->getPostId())->equals('10'); } public function testItUpdatesStatusAndSetsSentAtDateOnlyForStandardAndPostNotificationNewsletters() { - $newsletter = $this->newslettersRepository->findOneById($this->newsletter->id); + $newsletter = $this->newslettersRepository->findOneById($this->newsletter->getId()); $this->assertInstanceOf(NewsletterEntity::class, $newsletter); $sendingQueue = $this->sendingTask->queue(); $sendingQueue->processedAt = date('Y-m-d H:i:s'); @@ -285,26 +288,28 @@ class NewsletterTest extends \MailPoetTest { } public function testItDoesNotRenderSubscriberShortcodeInSubjectWhenPreprocessingNewsletter() { - $newsletter = $this->newsletter; - $newsletter->subject = 'Newsletter for [subscriber:firstname] [date:dordinal]'; - $newsletter = $this->newsletterTask->preProcessNewsletter($newsletter, $this->sendingTask); - $this->sendingTask = SendingTask::getByNewsletterId($newsletter->id); + $this->newsletter->setSubject('Newsletter for [subscriber:firstname] [date:dordinal]'); + $this->newslettersRepository->persist($this->newsletter); + $this->newslettersRepository->flush(); + $this->newsletter = $this->newsletterTask->preProcessNewsletter($this->newsletter, $this->sendingTask); + $this->sendingTask = SendingTask::getByNewsletterId($this->newsletter->getId()); $wp = new WPFunctions(); expect($this->sendingTask->newsletterRenderedSubject) ->stringContainsString(date_i18n('jS', $wp->currentTime('timestamp'))); } public function testItUsesADefaultSubjectIfRenderedSubjectIsEmptyWhenPreprocessingNewsletter() { - $newsletter = $this->newsletter; - $newsletter->subject = ' [custom_shortcode:should_render_empty] '; - $newsletter = $this->newsletterTask->preProcessNewsletter($newsletter, $this->sendingTask); - $this->sendingTask = SendingTask::getByNewsletterId($newsletter->id); + $this->newsletter->setSubject(' [custom_shortcode:should_render_empty] '); + $this->newslettersRepository->persist($this->newsletter); + $this->newslettersRepository->flush(); + $this->newsletter = $this->newsletterTask->preProcessNewsletter($this->newsletter, $this->sendingTask); + $this->sendingTask = SendingTask::getByNewsletterId($this->newsletter->getId()); expect($this->sendingTask->newsletterRenderedSubject) ->equals('No subject'); } public function testItUsesRenderedNewsletterBodyAndSubjectFromQueueObjectWhenPreparingNewsletterForSending() { - $newsletterEntity = $this->newslettersRepository->findOneById($this->newsletter->id); + $newsletterEntity = $this->newslettersRepository->findOneById($this->newsletter->getId()); $this->assertInstanceOf(NewsletterEntity::class, $newsletterEntity); $this->sendingTask->newsletterRenderedBody = [ 'html' => 'queue HTML body', @@ -330,7 +335,7 @@ class NewsletterTest extends \MailPoetTest { public function testItRendersShortcodesAndReplacesSubscriberDataInLinks() { $newsletter = $this->newsletterTask->preProcessNewsletter($this->newsletter, $this->sendingTask); - $newsletterEntity = $this->newslettersRepository->findOneById($newsletter->id); + $newsletterEntity = $this->newslettersRepository->findOneById($newsletter->getId()); $this->assertInstanceOf(NewsletterEntity::class, $newsletterEntity); $result = $this->newsletterTask->prepareNewsletterForSending( $newsletterEntity, @@ -348,7 +353,7 @@ class NewsletterTest extends \MailPoetTest { $newsletterTask = $this->newsletterTask; $newsletterTask->trackingEnabled = false; $newsletter = $newsletterTask->preProcessNewsletter($this->newsletter, $this->sendingTask); - $newsletterEntity = $this->newslettersRepository->findOneById($newsletter->id); + $newsletterEntity = $this->newslettersRepository->findOneById($newsletter->getId()); $this->assertInstanceOf(NewsletterEntity::class, $newsletterEntity); $result = $newsletterTask->prepareNewsletterForSending( $newsletterEntity,