From d2247a1c766890471c4a1a83ab4834665efad0ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lys=C3=BD?= Date: Thu, 29 Sep 2022 14:41:00 +0200 Subject: [PATCH] Refactor parameter from int to entity [MAILPOET-4372] --- .../WooCommerce/Events/AbandonedCart.php | 8 ++-- .../WooCommerce/Events/FirstPurchase.php | 2 +- .../Events/PurchasedInCategory.php | 2 +- .../WooCommerce/Events/PurchasedProduct.php | 2 +- .../Scheduler/AutomaticEmailScheduler.php | 39 ++++++++++--------- .../Sending/ScheduledTasksRepository.php | 5 ++- .../Blocks/AbandonedCartContentTest.php | 10 +++-- .../Scheduler/AutomaticEmailTest.php | 4 +- 8 files changed, 38 insertions(+), 34 deletions(-) diff --git a/mailpoet/lib/AutomaticEmails/WooCommerce/Events/AbandonedCart.php b/mailpoet/lib/AutomaticEmails/WooCommerce/Events/AbandonedCart.php index aae6679cae..3f61597d10 100644 --- a/mailpoet/lib/AutomaticEmails/WooCommerce/Events/AbandonedCart.php +++ b/mailpoet/lib/AutomaticEmails/WooCommerce/Events/AbandonedCart.php @@ -165,11 +165,11 @@ class AbandonedCart { } $meta = [self::TASK_META_NAME => $cartProductIds]; - $this->scheduler->scheduleOrRescheduleAutomaticEmail(WooCommerceEmail::SLUG, self::SLUG, (int)$subscriber->getId(), $meta); + $this->scheduler->scheduleOrRescheduleAutomaticEmail(WooCommerceEmail::SLUG, self::SLUG, $subscriber, $meta); } - private function rescheduleAbandonedCartEmail(SubscriberEntity $subscriberEntity) { - $this->scheduler->rescheduleAutomaticEmail(WooCommerceEmail::SLUG, self::SLUG, (int)$subscriberEntity->getId()); + private function rescheduleAbandonedCartEmail(SubscriberEntity $subscriber) { + $this->scheduler->rescheduleAutomaticEmail(WooCommerceEmail::SLUG, self::SLUG, $subscriber); } private function cancelAbandonedCartEmail() { @@ -177,7 +177,7 @@ class AbandonedCart { if (!$subscriber) { return; } - $this->scheduler->cancelAutomaticEmail(WooCommerceEmail::SLUG, self::SLUG, (int)$subscriber->getId()); + $this->scheduler->cancelAutomaticEmail(WooCommerceEmail::SLUG, self::SLUG, $subscriber); } private function getSubscriber(): ?SubscriberEntity { diff --git a/mailpoet/lib/AutomaticEmails/WooCommerce/Events/FirstPurchase.php b/mailpoet/lib/AutomaticEmails/WooCommerce/Events/FirstPurchase.php index 37a7ffea61..cce36d2581 100644 --- a/mailpoet/lib/AutomaticEmails/WooCommerce/Events/FirstPurchase.php +++ b/mailpoet/lib/AutomaticEmails/WooCommerce/Events/FirstPurchase.php @@ -188,7 +188,7 @@ class FirstPurchase { 'subscriber_id' => $subscriber->getId(), ] ); - $this->scheduler->scheduleAutomaticEmail(WooCommerce::SLUG, self::SLUG, $checkEmailWasNotScheduled, $subscriber->getId(), $meta); + $this->scheduler->scheduleAutomaticEmail(WooCommerce::SLUG, self::SLUG, $checkEmailWasNotScheduled, $subscriber, $meta); } public function getCustomerOrderCount($customerEmail) { diff --git a/mailpoet/lib/AutomaticEmails/WooCommerce/Events/PurchasedInCategory.php b/mailpoet/lib/AutomaticEmails/WooCommerce/Events/PurchasedInCategory.php index e4eb1d5d89..eddd6aa1b4 100644 --- a/mailpoet/lib/AutomaticEmails/WooCommerce/Events/PurchasedInCategory.php +++ b/mailpoet/lib/AutomaticEmails/WooCommerce/Events/PurchasedInCategory.php @@ -160,7 +160,7 @@ class PurchasedInCategory { WooCommerce::SLUG, self::SLUG, $schedulingCondition, - $subscriber->getId(), + $subscriber, ['orderedProductCategories' => $orderedProductCategories], [$this, 'metaModifier'] ); diff --git a/mailpoet/lib/AutomaticEmails/WooCommerce/Events/PurchasedProduct.php b/mailpoet/lib/AutomaticEmails/WooCommerce/Events/PurchasedProduct.php index a655b362b6..de26286184 100644 --- a/mailpoet/lib/AutomaticEmails/WooCommerce/Events/PurchasedProduct.php +++ b/mailpoet/lib/AutomaticEmails/WooCommerce/Events/PurchasedProduct.php @@ -162,7 +162,7 @@ class PurchasedProduct { WooCommerce::SLUG, self::SLUG, $schedulingCondition, - $subscriber->getId(), + $subscriber, ['orderedProducts' => $orderedProducts], [$this, 'metaModifier'] ); diff --git a/mailpoet/lib/Newsletter/Scheduler/AutomaticEmailScheduler.php b/mailpoet/lib/Newsletter/Scheduler/AutomaticEmailScheduler.php index c7e5098fc4..4b81110d08 100644 --- a/mailpoet/lib/Newsletter/Scheduler/AutomaticEmailScheduler.php +++ b/mailpoet/lib/Newsletter/Scheduler/AutomaticEmailScheduler.php @@ -8,10 +8,10 @@ use MailPoet\Entities\NewsletterOptionFieldEntity; use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Entities\ScheduledTaskSubscriberEntity; use MailPoet\Entities\SendingQueueEntity; +use MailPoet\Entities\SubscriberEntity; use MailPoet\Newsletter\Sending\ScheduledTasksRepository; use MailPoet\Newsletter\Sending\ScheduledTaskSubscribersRepository; use MailPoet\Newsletter\Sending\SendingQueuesRepository; -use MailPoet\Subscribers\SubscribersRepository; class AutomaticEmailScheduler { @@ -27,24 +27,26 @@ class AutomaticEmailScheduler { /** @var ScheduledTaskSubscribersRepository */ private $scheduledTaskSubscribersRepository; - /** @var SubscribersRepository */ - private $subscribersRepository; - public function __construct( Scheduler $scheduler, ScheduledTasksRepository $scheduledTasksRepository, ScheduledTaskSubscribersRepository $scheduledTaskSubscribersRepository, - SendingQueuesRepository $sendingQueuesRepository, - SubscribersRepository $subscribersRepository + SendingQueuesRepository $sendingQueuesRepository ) { $this->scheduler = $scheduler; $this->scheduledTasksRepository = $scheduledTasksRepository; $this->scheduledTaskSubscribersRepository = $scheduledTaskSubscribersRepository; $this->sendingQueuesRepository = $sendingQueuesRepository; - $this->subscribersRepository = $subscribersRepository; } - public function scheduleAutomaticEmail(string $group, string $event, $schedulingCondition = false, $subscriberId = false, $meta = false, $metaModifier = null) { + public function scheduleAutomaticEmail( + string $group, + string $event, + ?callable $schedulingCondition = null, + ?SubscriberEntity $subscriber = null, + ?array $meta = null, + ?callable $metaModifier = null + ) { $newsletters = $this->scheduler->getNewsletters(NewsletterEntity::TYPE_AUTOMATIC, $group); if (empty($newsletters)) return false; foreach ($newsletters as $newsletter) { @@ -62,11 +64,11 @@ class AutomaticEmailScheduler { if (is_callable($metaModifier)) { $meta = $metaModifier($newsletter, $meta); } - $this->createAutomaticEmailScheduledTask($newsletter, $subscriberId, $meta); + $this->createAutomaticEmailScheduledTask($newsletter, $subscriber, $meta); } } - public function scheduleOrRescheduleAutomaticEmail(string $group, string $event, int $subscriberId, array $meta): void { + public function scheduleOrRescheduleAutomaticEmail(string $group, string $event, SubscriberEntity $subscriber, array $meta): void { $newsletters = $this->scheduler->getNewsletters(NewsletterEntity::TYPE_AUTOMATIC, $group); if (empty($newsletters)) { return; @@ -78,16 +80,16 @@ class AutomaticEmailScheduler { } // try to find existing scheduled task for given subscriber - $task = $this->scheduledTasksRepository->findOneScheduledByNewsletterAndSubscriberId($newsletter, $subscriberId); + $task = $this->scheduledTasksRepository->findOneScheduledByNewsletterAndSubscriber($newsletter, $subscriber); if ($task) { $this->rescheduleAutomaticEmailSendingTask($newsletter, $task, $meta); } else { - $this->createAutomaticEmailScheduledTask($newsletter, $subscriberId, $meta); + $this->createAutomaticEmailScheduledTask($newsletter, $subscriber, $meta); } } } - public function rescheduleAutomaticEmail(string $group, string $event, int $subscriberId): void { + public function rescheduleAutomaticEmail(string $group, string $event, SubscriberEntity $subscriber): void { $newsletters = $this->scheduler->getNewsletters(NewsletterEntity::TYPE_AUTOMATIC, $group); if (empty($newsletters)) { return; @@ -99,14 +101,14 @@ class AutomaticEmailScheduler { } // try to find existing scheduled task for given subscriber - $task = $this->scheduledTasksRepository->findOneScheduledByNewsletterAndSubscriberId($newsletter, $subscriberId); + $task = $this->scheduledTasksRepository->findOneScheduledByNewsletterAndSubscriber($newsletter, $subscriber); if ($task) { $this->rescheduleAutomaticEmailSendingTask($newsletter, $task); } } } - public function cancelAutomaticEmail(string $group, string $event, int $subscriberId): void { + public function cancelAutomaticEmail(string $group, string $event, SubscriberEntity $subscriber): void { $newsletters = $this->scheduler->getNewsletters(NewsletterEntity::TYPE_AUTOMATIC, $group); if (empty($newsletters)) { return; @@ -118,7 +120,7 @@ class AutomaticEmailScheduler { } // try to find existing scheduled task for given subscriber - $task = $this->scheduledTasksRepository->findOneScheduledByNewsletterAndSubscriberId($newsletter, $subscriberId); + $task = $this->scheduledTasksRepository->findOneScheduledByNewsletterAndSubscriber($newsletter, $subscriber); if ($task) { $this->sendingQueuesRepository->deleteByTask($task); $this->scheduledTaskSubscribersRepository->deleteByTask($task); @@ -128,8 +130,7 @@ class AutomaticEmailScheduler { } } - public function createAutomaticEmailScheduledTask(NewsletterEntity $newsletter, $subscriberId, $meta = false): ScheduledTaskEntity { - $subscriber = $subscriberId ? $this->subscribersRepository->findOneById($subscriberId) : null; + public function createAutomaticEmailScheduledTask(NewsletterEntity $newsletter, ?SubscriberEntity $subscriber, ?array $meta = null): ScheduledTaskEntity { $scheduledTask = new ScheduledTaskEntity(); $scheduledTask->setType(SendingQueue::TASK_TYPE); $scheduledTask->setStatus(SendingQueueEntity::STATUS_SCHEDULED); @@ -165,7 +166,7 @@ class AutomaticEmailScheduler { return $scheduledTask; } - private function rescheduleAutomaticEmailSendingTask(NewsletterEntity $newsletter, ScheduledTaskEntity $scheduledTask, $meta = false) { + private function rescheduleAutomaticEmailSendingTask(NewsletterEntity $newsletter, ScheduledTaskEntity $scheduledTask, ?array $meta = null): void { $sendingQueue = $this->sendingQueuesRepository->findOneBy(['task' => $scheduledTask]); if (!$sendingQueue) { return; diff --git a/mailpoet/lib/Newsletter/Sending/ScheduledTasksRepository.php b/mailpoet/lib/Newsletter/Sending/ScheduledTasksRepository.php index 9896f6e641..8bafab98fb 100644 --- a/mailpoet/lib/Newsletter/Sending/ScheduledTasksRepository.php +++ b/mailpoet/lib/Newsletter/Sending/ScheduledTasksRepository.php @@ -9,6 +9,7 @@ use MailPoet\Entities\NewsletterEntity; use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Entities\ScheduledTaskSubscriberEntity; use MailPoet\Entities\SendingQueueEntity; +use MailPoet\Entities\SubscriberEntity; use MailPoet\WP\Functions as WPFunctions; use MailPoetVendor\Carbon\Carbon; use MailPoetVendor\Carbon\CarbonImmutable; @@ -96,7 +97,7 @@ class ScheduledTasksRepository extends Repository { ->getResult(); } - public function findOneScheduledByNewsletterAndSubscriberId(NewsletterEntity $newsletter, int $subscriberId): ?ScheduledTaskEntity { + public function findOneScheduledByNewsletterAndSubscriber(NewsletterEntity $newsletter, SubscriberEntity $subscriber): ?ScheduledTaskEntity { $scheduledTask = $this->doctrineRepository->createQueryBuilder('st') ->join(SendingQueueEntity::class, 'sq', Join::WITH, 'st = sq.task') ->join(ScheduledTaskSubscriberEntity::class, 'sts', Join::WITH, 'st = sts.task') @@ -106,7 +107,7 @@ class ScheduledTasksRepository extends Repository { ->setMaxResults(1) ->setParameter('status', ScheduledTaskEntity::STATUS_SCHEDULED) ->setParameter('newsletter', $newsletter) - ->setParameter('subscriber', $subscriberId) + ->setParameter('subscriber', $subscriber) ->getQuery() ->getOneOrNullResult(); // for phpstan because it detects mixed instead of entity diff --git a/mailpoet/tests/integration/Newsletter/Blocks/AbandonedCartContentTest.php b/mailpoet/tests/integration/Newsletter/Blocks/AbandonedCartContentTest.php index 2b20add577..5bf9d4d2d3 100644 --- a/mailpoet/tests/integration/Newsletter/Blocks/AbandonedCartContentTest.php +++ b/mailpoet/tests/integration/Newsletter/Blocks/AbandonedCartContentTest.php @@ -10,11 +10,13 @@ use MailPoet\Entities\NewsletterOptionFieldEntity; use MailPoet\Entities\NewsletterPostEntity; use MailPoet\Entities\ScheduledTaskEntity; use MailPoet\Entities\SendingQueueEntity; +use MailPoet\Entities\SubscriberEntity; use MailPoet\Models\ScheduledTask; use MailPoet\Newsletter\NewslettersRepository; use MailPoet\Newsletter\Scheduler\AutomaticEmailScheduler; use MailPoet\Tasks\Sending; use MailPoet\Test\DataFactories\NewsletterOption; +use MailPoet\Test\DataFactories\Subscriber; use MailPoet\WP\Functions as WPFunctions; class AbandonedCartContentTest extends \MailPoetTest { @@ -136,7 +138,7 @@ class AbandonedCartContentTest extends \MailPoetTest { $this->setGroupAndEventOptions($newsletter); $this->accBlock['displayType'] = 'titleOnly'; $this->accBlock['pricePosition'] = 'hidden'; - $sendingTask = $this->createSendingTask($newsletter, 1, [AbandonedCart::TASK_META_NAME => []]); + $sendingTask = $this->createSendingTask($newsletter, [AbandonedCart::TASK_META_NAME => []]); $result = $this->block->render($newsletter, $this->accBlock, false, $sendingTask); $encodedResult = json_encode($result); expect($encodedResult)->equals('[]'); @@ -183,10 +185,10 @@ class AbandonedCartContentTest extends \MailPoetTest { ]); } - private function createSendingTask($newsletter, $subscriberId = null, $meta = null) { - $subscriberId = $subscriberId ?: 1; // dummy default value + private function createSendingTask(NewsletterEntity $newsletter, ?array $meta = null) { + $subscriber = (new Subscriber())->create(); // dummy default value $meta = $meta ?: [AbandonedCart::TASK_META_NAME => array_slice($this->productIds, 0, 3)]; - $scheduledTask = $this->automaticEmailScheduler->createAutomaticEmailScheduledTask($newsletter, $subscriberId, $meta); + $scheduledTask = $this->automaticEmailScheduler->createAutomaticEmailScheduledTask($newsletter, $subscriber, $meta); // this can be removed when SendingTask usage is removed from AbandonedCartContent $parisTask = ScheduledTask::findOne($scheduledTask->getId()); $this->assertInstanceOf(ScheduledTask::class, $parisTask); diff --git a/mailpoet/tests/integration/Newsletter/Scheduler/AutomaticEmailTest.php b/mailpoet/tests/integration/Newsletter/Scheduler/AutomaticEmailTest.php index 161a48575b..36557cd319 100644 --- a/mailpoet/tests/integration/Newsletter/Scheduler/AutomaticEmailTest.php +++ b/mailpoet/tests/integration/Newsletter/Scheduler/AutomaticEmailTest.php @@ -67,7 +67,7 @@ class AutomaticEmailTest extends \MailPoetTest { $this->assertInstanceOf(NewsletterEntity::class, $newsletter); $subscriber = (new SubscriberFactory())->create(); - $this->automaticEmailScheduler->createAutomaticEmailScheduledTask($newsletter, $subscriber->getId()); + $this->automaticEmailScheduler->createAutomaticEmailScheduledTask($newsletter, $subscriber); // new scheduled task should be created $task = $this->scheduledTasksRepository->findOneByNewsletter($newsletter); $currentTime = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp')); @@ -92,7 +92,7 @@ class AutomaticEmailTest extends \MailPoetTest { $subscriber = (new SubscriberFactory())->create(); $meta = ['some' => 'value']; - $this->automaticEmailScheduler->createAutomaticEmailScheduledTask($newsletter, $subscriber->getId(), $meta); + $this->automaticEmailScheduler->createAutomaticEmailScheduledTask($newsletter, $subscriber, $meta); // new queue record should be created with meta data $queue = $this->sendingQueuesRepository->findOneBy(['newsletter' => $newsletter]); $this->assertInstanceOf(SendingQueueEntity::class, $queue);