Cavemanon/piratepoet
7
Fork 0
Code Pull Requests Actions Releases 16 Activity

Refactor Sending::setSubscribers() to use Doctrine instead of Paris

Browse Source 
[MAILPOET-4368]
This commit is contained in:
Rodrigo Primo
2023-06-22 14:57:27 -03:00
committed by Aschepikov
parent b252043dac
commit 8fd8b8f193
8 changed files with 145 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
mailpoet/lib/Newsletter/Scheduler/WelcomeScheduler.php
View File

@@ -4,6 +4,7 @@ namespace MailPoet\Newsletter\Scheduler;
use MailPoet\Entities\NewsletterEntity;
use MailPoet\Entities\NewsletterOptionFieldEntity;
use MailPoet\Entities\ScheduledTaskEntity;
use MailPoet\Entities\SegmentEntity;
use MailPoet\Entities\SendingQueueEntity;
use MailPoet\Entities\SubscriberEntity;
@@ -126,6 +127,19 @@ class WelcomeScheduler {
$newsletter->getOptionValue(NewsletterOptionFieldEntity::NAME_AFTER_TIME_TYPE),
$newsletter->getOptionValue(NewsletterOptionFieldEntity::NAME_AFTER_TIME_NUMBER)
);
return $sendingTask->save();
$savedSendingTask = $sendingTask->save();
// Refreshing this entity here is needed while we are still using Paris to create the scheduled tasks and queues
// in the code above using \MailPoet\Tasks\Sending class. Doing this should avoid bugs where the loaded entity contain
// stale data after the corresponding entry in the database is updated using Paris. This code can be removed once
// https://mailpoet.atlassian.net/browse/MAILPOET-4375 is finished. Currently, if this code is removed a few integration
// tests fail (see https://app.circleci.com/pipelines/github/mailpoet/mailpoet/14806/workflows/0d441848-16db-461a-88ec-87bed101fe36/jobs/251385/tests#failed-test-0).
$scheduledTaskEntity = $this->scheduledTasksRepository->findOneScheduledByNewsletterAndSubscriber($newsletter, $subscriber);
if ($scheduledTaskEntity instanceof ScheduledTaskEntity) {
$this->scheduledTasksRepository->refresh($scheduledTaskEntity);
}
return $savedSendingTask;
}
}

11
mailpoet/lib/Newsletter/Sending/ScheduledTaskSubscribersRepository.php
View File

@@ -125,6 +125,17 @@ class ScheduledTaskSubscribersRepository extends Repository {
->execute();
}
public function setSubscribers(ScheduledTaskEntity $task, array $subscriberIds): void {
$this->deleteByScheduledTask($task);
foreach ($subscriberIds as $subscriberId) {
$this->createOrUpdate([
'task_id' => $task->getId(),
'subscriber_id' => $subscriberId,
]);
}
}
private function checkCompleted(ScheduledTaskEntity $task): void {
$count = $this->countBy(['task' => $task, 'processed' => ScheduledTaskSubscriberEntity::STATUS_UNPROCESSED]);
if ($count === 0) {

26
mailpoet/lib/Tasks/Sending.php
View File

@@ -4,6 +4,7 @@ namespace MailPoet\Tasks;
use MailPoet\Cron\Workers\SendingQueue\SendingQueue as SendingQueueAlias;
use MailPoet\DI\ContainerWrapper;
use MailPoet\Entities\ScheduledTaskEntity;
use MailPoet\Entities\ScheduledTaskSubscriberEntity;
use MailPoet\Entities\SendingQueueEntity;
use MailPoet\InvalidStateException;
@@ -11,6 +12,7 @@ use MailPoet\Logging\LoggerFactory;
use MailPoet\Models\ScheduledTask;
use MailPoet\Models\ScheduledTaskSubscriber;
use MailPoet\Models\SendingQueue;
use MailPoet\Newsletter\Sending\ScheduledTasksRepository;
use MailPoet\Newsletter\Sending\ScheduledTaskSubscribersRepository;
use MailPoet\Newsletter\Sending\SendingQueuesRepository;
use MailPoet\Util\Helpers;
@@ -58,6 +60,12 @@ class Sending {
'deleted_at',
];
/** @var ScheduledTaskSubscribersRepository */
private $scheduledTaskSubscribersRepository;
/** @var ScheduledTasksRepository */
private $scheduledTasksRepository;
private function __construct(
ScheduledTask $task = null,
SendingQueue $queue = null
@@ -81,6 +89,8 @@ class Sending {
$this->task = $task;
$this->queue = $queue;
$this->taskSubscribers = new Subscribers($task);
$this->scheduledTaskSubscribersRepository = ContainerWrapper::getInstance()->get(ScheduledTaskSubscribersRepository::class);
$this->scheduledTasksRepository = ContainerWrapper::getInstance()->get(ScheduledTasksRepository::class);
}
public static function create(ScheduledTask $task = null, SendingQueue $queue = null) {
@@ -214,16 +224,14 @@ class Sending {
}
public function getSubscribers($processed = null) {
$scheduledTaskSubscribersRepository = ContainerWrapper::getInstance()->get(ScheduledTaskSubscribersRepository::class);
if (is_null($processed)) {
$subscribers = $scheduledTaskSubscribersRepository->findBy(['task' => $this->task->id]);
$subscribers = $this->scheduledTaskSubscribersRepository->findBy(['task' => $this->task->id]);
} else if ($processed) {
$subscribers = $scheduledTaskSubscribersRepository->findBy(
$subscribers = $this->scheduledTaskSubscribersRepository->findBy(
['task' => $this->task->id, 'processed' => ScheduledTaskSubscriberEntity::STATUS_PROCESSED]
);
} else {
$subscribers = $scheduledTaskSubscribersRepository->findBy(
$subscribers = $this->scheduledTaskSubscribersRepository->findBy(
['task' => $this->task->id, 'processed' => ScheduledTaskSubscriberEntity::STATUS_UNPROCESSED]
);
}
@@ -237,8 +245,12 @@ class Sending {
}
public function setSubscribers(array $subscriberIds) {
$this->taskSubscribers->setSubscribers($subscriberIds);
$this->updateCount();
$scheduledTaskEntity = $this->scheduledTasksRepository->findOneById($this->task->id);
if ($scheduledTaskEntity instanceof ScheduledTaskEntity) {
$this->scheduledTaskSubscribersRepository->setSubscribers($scheduledTaskEntity, $subscriberIds);
$this->updateCount();
}
}
public function removeSubscribers(array $subscriberIds) {

4
mailpoet/lib/Tasks/Subscribers.php
View File

@@ -14,10 +14,6 @@ class Subscribers {
$this->task = $task;
}
public function setSubscribers(array $subscriberIds) {
ScheduledTaskSubscriber::setSubscribers($this->task->id, $subscriberIds);
}
public function getSubscribers() {
return ScheduledTaskSubscriber::where('task_id', $this->task->id);
}

73
mailpoet/tests/integration/Newsletter/Sending/ScheduledTaskSubscribersRepositoryTest.php Normal file
View File

@@ -0,0 +1,73 @@
<?php declare(strict_types = 1);
namespace integration\Newsletter\Sending;
use MailPoet\Entities\ScheduledTaskEntity;
use MailPoet\Entities\SubscriberEntity;
use MailPoet\Newsletter\Sending\ScheduledTaskSubscribersRepository;
use MailPoet\Test\DataFactories\ScheduledTask as ScheduledTaskFactory;
use MailPoet\Test\DataFactories\ScheduledTaskSubscriber as TaskSubscriberFactory;
use MailPoet\Test\DataFactories\Subscriber as SubscriberFactory;
use MailPoetVendor\Carbon\Carbon;
class ScheduledTaskSubscribersRepositoryTest extends \MailPoetTest {
/** @var ScheduledTaskSubscribersRepository */
private $repository;
/** @var SubscriberFactory */
private $subscriberFactory;
/** @var ScheduledTaskEntity */
private $scheduledTask1;
/** @var ScheduledTaskEntity */
private $scheduledTask2;
/** @var SubscriberEntity */
private $subscriberUnprocessed;
/** @var SubscriberEntity */
private $subscriberProcessed;
public function _before() {
parent::_before();
$this->repository = $this->diContainer->get(ScheduledTaskSubscribersRepository::class);
$scheduledTaskFactory = new ScheduledTaskFactory();
$this->subscriberFactory = new SubscriberFactory();
$taskSubscriberFactory = new TaskSubscriberFactory();
$this->subscriberUnprocessed = $this->subscriberFactory->withEmail('subscriberUprocessed@email.com')->create();
$this->subscriberProcessed = $this->subscriberFactory->withEmail('subscriberProcessed@email.com')->create();
$subscriberFailed = $this->subscriberFactory->withEmail('subscriberFailed@email.com')->create();
$this->subscriberFactory->withEmail('subscriberNotIncluded@email.com')->create();
$this->scheduledTask1 = $scheduledTaskFactory->create('sending', ScheduledTaskEntity::STATUS_COMPLETED, Carbon::now()->subDay());
$this->scheduledTask2 = $scheduledTaskFactory->create('sending', ScheduledTaskEntity::STATUS_COMPLETED, Carbon::now()->subDay());
$taskSubscriberFactory->createUnprocessed($this->scheduledTask1, $this->subscriberUnprocessed);
$taskSubscriberFactory->createProcessed($this->scheduledTask1, $this->subscriberProcessed);
$taskSubscriberFactory->createFailed($this->scheduledTask1, $subscriberFailed, 'Error Message');
$taskSubscriberFactory->createUnprocessed($this->scheduledTask2, $this->subscriberUnprocessed);
$taskSubscriberFactory->createProcessed($this->scheduledTask2, $this->subscriberProcessed);
}
public function testItSetsSubscribers() {
$subscriber = $this->subscriberFactory->withEmail('newsubscriber@email.com')->create();
$this->assertCount(3, $this->repository->findBy(['task' => $this->scheduledTask1]));
$this->repository->setSubscribers($this->scheduledTask1, [$subscriber->getId()]);
$task1Subscribers = $this->repository->findBy(['task' => $this->scheduledTask1]);
$this->assertCount(1, $task1Subscribers);
$this->assertInstanceOf(SubscriberEntity::class, $task1Subscribers[0]->getSubscriber());
$this->assertEquals($subscriber->getId(), $task1Subscribers[0]->getSubscriber()->getId());
// check that setSubscribers() does not delete subscribers from other tasks
$task2Subscribers = $this->repository->findBy(['task' => $this->scheduledTask2]);
$this->assertCount(2, $task2Subscribers);
$this->assertInstanceOf(SubscriberEntity::class, $task2Subscribers[0]->getSubscriber());
$this->assertInstanceOf(SubscriberEntity::class, $task2Subscribers[1]->getSubscriber());
$this->assertEquals($this->subscriberUnprocessed->getId(), $task2Subscribers[0]->getSubscriber()->getId());
$this->assertEquals($this->subscriberProcessed->getId(), $task2Subscribers[1]->getSubscriber()->getId());
}
}

6
mailpoet/tests/integration/Newsletter/ViewInBrowser/ViewInBrowserRendererTest.php
View File

@@ -4,11 +4,13 @@ namespace MailPoet\Newsletter\ViewInBrowser;
use Codeception\Stub\Expected;
use MailPoet\Entities\NewsletterEntity;
use MailPoet\Entities\ScheduledTaskEntity;
use MailPoet\Entities\SendingQueueEntity;
use MailPoet\Entities\SubscriberEntity;
use MailPoet\Newsletter\Links\Links;
use MailPoet\Newsletter\NewslettersRepository;
use MailPoet\Newsletter\Renderer\Renderer;
use MailPoet\Newsletter\Sending\ScheduledTasksRepository;
use MailPoet\Newsletter\Sending\SendingQueuesRepository;
use MailPoet\Newsletter\Shortcodes\Shortcodes;
use MailPoet\Router\Router;
@@ -127,6 +129,10 @@ class ViewInBrowserRendererTest extends \MailPoetTest {
$queue->setSubscribers([$subscriber->getId()]);
$this->sendingTask = $queue->save();
$this->newsletterRepository->refresh($newsletter);
$scheduledTasksRepository = $this->diContainer->get(ScheduledTasksRepository::class);
$scheduledTask = $scheduledTasksRepository->findOneById($this->sendingTask->task()->id);
$this->assertInstanceOf(ScheduledTaskEntity::class, $scheduledTask);
$scheduledTasksRepository->refresh($scheduledTask);
$this->newsletter = $newsletter;
// create newsletter link associations

7
mailpoet/tests/integration/Statistics/Track/UnsubscribesTest.php
View File

@@ -2,8 +2,10 @@
namespace MailPoet\Test\Statistics\Track;
use MailPoet\Entities\ScheduledTaskEntity;
use MailPoet\Entities\StatisticsUnsubscribeEntity;
use MailPoet\Entities\SubscriberEntity;
use MailPoet\Newsletter\Sending\ScheduledTasksRepository;
use MailPoet\Statistics\StatisticsUnsubscribesRepository;
use MailPoet\Statistics\Track\Unsubscribes;
use MailPoet\Tasks\Sending as SendingTask;
@@ -43,6 +45,11 @@ class UnsubscribesTest extends \MailPoetTest {
$queue->setSubscribers([$this->subscriber->getId()]);
$queue->updateProcessedSubscribers([$this->subscriber->getId()]);
$this->queue = $queue->save();
$scheduledTasksRepository = $this->diContainer->get(ScheduledTasksRepository::class);
$scheduledTask = $scheduledTasksRepository->findOneById($this->queue->task()->id);
$this->assertInstanceOf(ScheduledTaskEntity::class, $scheduledTask);
$scheduledTasksRepository->refresh($scheduledTask);
// instantiate class
$this->unsubscribes = $this->diContainer->get(Unsubscribes::class);
$this->statisticsUnsubscribesRepository = $this->diContainer->get(StatisticsUnsubscribesRepository::class);

18
mailpoet/tests/integration/Tasks/SendingTest.php
View File

@@ -37,13 +37,16 @@ class SendingTest extends \MailPoetTest {
/** SubscriberEntity */
private $subscriber2;
/** @var SubscriberFactory */
private $subscriberFactory;
public function _before() {
parent::_before();
$this->newsletterFactory = new NewsletterFactory();
$this->newsletter = $this->newsletterFactory->create();
$subscriberFactory = new SubscriberFactory();
$this->subscriber1 = $subscriberFactory->withEmail('subscriber1@test.com')->create();
$this->subscriber2 = $subscriberFactory->withEmail('subscriber2@test.com')->create();
$this->subscriberFactory = new SubscriberFactory();
$this->subscriber1 = $this->subscriberFactory->withEmail('subscriber1@test.com')->create();
$this->subscriber2 = $this->subscriberFactory->withEmail('subscriber2@test.com')->create();
$this->task = $this->createNewScheduledTask();
$this->queue = $this->createNewSendingQueue([
'newsletter' => $this->newsletter,
@@ -155,10 +158,17 @@ class SendingTest extends \MailPoetTest {
}
public function testItSetsSubscribers() {
$subscriberIds = [$this->subscriber1->getId(), $this->subscriber2->getId()];
$subscriber3 = $this->subscriberFactory->withEmail('subscriber3@test.com')->create();
$subscriber4 = $this->subscriberFactory->withEmail('subscriber4@test.com')->create();
$subscriber5 = $this->subscriberFactory->withEmail('subscriber5@test.com')->create();
$subscriberIds = [$subscriber3->getId(), $subscriber4->getId(), $subscriber5->getId()];
$this->sending->setSubscribers($subscriberIds);
verify($this->sending->getSubscribers())->equals($subscriberIds);
verify($this->sending->count_total)->equals(count($subscriberIds));
verify($this->sending->count_processed)->equals(0);
verify($this->sending->count_to_process)->equals(3);
}
public function testItRemovesSubscribers() {