Fix canceling multiple automatic emails
When we deleted sending queue using SQL it remained in the entity manager and subsequent flush (not the first one) triggered the error, because it didn't know the ScheduledTask entity attached to the orphaned SendingQueue entity. This commit fixes this by refactoring deletion of sending queue using standard repository method. After fixing the issue for sending queue there was another issue with SchedulesTaskSubscriberEntity that remained in memory. I fixed that by detaching those. Theoretically there might be many SchedulesTaskSubscriberEntities for an Automatic email so I consider still safer to delete using SQL and if there are some loaded (in this case there is one) detach them. [MAILPOET-4741]
This commit is contained in:
committed by
Aschepikov
parent
655641737b
commit
cd3652eaa6
@@ -122,8 +122,15 @@ class AutomaticEmailScheduler {
|
||||
// try to find existing scheduled task for given subscriber
|
||||
$task = $this->scheduledTasksRepository->findOneScheduledByNewsletterAndSubscriber($newsletter, $subscriber);
|
||||
if ($task) {
|
||||
$this->sendingQueuesRepository->deleteByTask($task);
|
||||
$queue = $task->getSendingQueue();
|
||||
if ($queue instanceof SendingQueueEntity) {
|
||||
$this->sendingQueuesRepository->remove($queue);
|
||||
}
|
||||
$this->scheduledTaskSubscribersRepository->deleteByTask($task);
|
||||
// In case any of task associated SchedulesTaskSubscriberEntity was loaded we need to detach them
|
||||
foreach ($task->getSubscribers() as $taskSubscriber) {
|
||||
$this->scheduledTaskSubscribersRepository->detach($taskSubscriber);
|
||||
}
|
||||
$this->scheduledTasksRepository->remove($task);
|
||||
$this->scheduledTasksRepository->flush();
|
||||
}
|
||||
|
@@ -347,6 +347,7 @@ class AbandonedCartTest extends \MailPoetTest {
|
||||
$this->entityManager->flush();
|
||||
|
||||
$scheduledTask->setScheduledAt($scheduleAt);
|
||||
$scheduledTask->setSendingQueue($sendingQueue);
|
||||
$scheduledTask->setStatus(($this->currentTime < $scheduleAt) ? ScheduledTaskEntity::STATUS_SCHEDULED : ScheduledTaskEntity::STATUS_COMPLETED);
|
||||
$this->entityManager->flush();
|
||||
|
||||
|
@@ -50,16 +50,7 @@ class AutomaticEmailTest extends \MailPoetTest {
|
||||
$this->scheduledTasksRepository = $this->diContainer->get(ScheduledTasksRepository::class);
|
||||
|
||||
$this->newsletterFactory = new NewsletterFactory();
|
||||
$this->newsletter = $this->newsletterFactory->withActiveStatus()->withAutomaticType()->create();
|
||||
$this->newsletterOptionFactory = new NewsletterOptionFactory();
|
||||
$this->newsletterOptionFactory->createMultipleOptions(
|
||||
$this->newsletter,
|
||||
[
|
||||
'sendTo' => 'user',
|
||||
'afterTimeType' => 'hours',
|
||||
'afterTimeNumber' => 2,
|
||||
]
|
||||
);
|
||||
$this->newsletter = $this->createAutomaticNewsletter();
|
||||
}
|
||||
|
||||
public function testItCreatesScheduledAutomaticEmailSendingTaskForUser() {
|
||||
@@ -149,6 +140,30 @@ class AutomaticEmailTest extends \MailPoetTest {
|
||||
$this->assertCount(0, $this->sendingQueuesRepository->findAll());
|
||||
}
|
||||
|
||||
public function testItCanCancelMultipleAutomaticEmails() {
|
||||
$newsletter = $this->newslettersRepository->findOneById($this->newsletter->getId());
|
||||
$this->newsletterOptionFactory->createMultipleOptions(
|
||||
$this->newsletter,
|
||||
[
|
||||
'group' => 'some_group',
|
||||
'event' => 'some_event',
|
||||
]
|
||||
);
|
||||
$newsletter2 = $this->createAutomaticNewsletter();
|
||||
$this->newsletterOptionFactory->createMultipleOptions(
|
||||
$newsletter2,
|
||||
[
|
||||
'group' => 'some_group',
|
||||
'event' => 'some_event',
|
||||
]
|
||||
);
|
||||
$this->assertInstanceOf(NewsletterEntity::class, $newsletter);
|
||||
$subscriber = (new SubscriberFactory())->create();
|
||||
$this->automaticEmailScheduler->createAutomaticEmailScheduledTask($newsletter, $subscriber);
|
||||
$this->automaticEmailScheduler->createAutomaticEmailScheduledTask($newsletter2, $subscriber);
|
||||
$this->automaticEmailScheduler->cancelAutomaticEmail('some_group', 'some_event', $subscriber);
|
||||
}
|
||||
|
||||
public function testItSchedulesAutomaticEmailWhenConditionMatches() {
|
||||
$currentTime = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp'));
|
||||
$this->newsletterOptionFactory->createMultipleOptions(
|
||||
@@ -200,6 +215,20 @@ class AutomaticEmailTest extends \MailPoetTest {
|
||||
->equals($currentTime->addHours(2)->format('Y-m-d H:i'));
|
||||
}
|
||||
|
||||
private function createAutomaticNewsletter(): NewsletterEntity {
|
||||
$newsletter = $this->newsletterFactory->withActiveStatus()->withAutomaticType()->create();
|
||||
$this->newsletterOptionFactory = new NewsletterOptionFactory();
|
||||
$this->newsletterOptionFactory->createMultipleOptions(
|
||||
$newsletter,
|
||||
[
|
||||
'sendTo' => 'user',
|
||||
'afterTimeType' => 'hours',
|
||||
'afterTimeNumber' => 2,
|
||||
]
|
||||
);
|
||||
return $newsletter;
|
||||
}
|
||||
|
||||
public function _after() {
|
||||
Carbon::setTestNow();
|
||||
$this->truncateEntity(NewsletterEntity::class);
|
||||
|
Reference in New Issue
Block a user