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

Remove an unnecessary condition in the batch processing loop

Browse Source 
The batch will be continued anyway because of the next condition
if (!$queue->getCountToProcess()) {
  continue;
}
The newsletter entity will be marked as sent (plus other stuff related to finished sending)
after the loop in $this->endSending.
This commit is contained in:
Rostislav Wolny
2024-05-30 15:31:15 +02:00
committed by Aschepikov
parent 55ae6264cc
commit e4cff6c14f
2 changed files with 53 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php
View File

@@ -291,11 +291,7 @@ class SendingQueue {
$this->scheduledTaskSubscribersRepository->deleteByScheduledTaskAndSubscriberIds($task, $subscribersToRemove);
$this->sendingQueuesRepository->updateCounts($queue);
if (!$queue->getCountToProcess()) {
$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
// if there aren't any subscribers to process in the batch (e.g. all unsubscribed or were deleted), continue with the next batch
if (count($foundSubscribersIds) === 0) {
continue;
}
@@ -350,6 +346,9 @@ class SendingQueue {
return;
}
}
// At this point all batches were processed or there are no batches to process
// Also none of the checks above paused or invalidated the task
$this->endSending($task, $newsletter);
}
public function getBatchSize(): int {

49
mailpoet/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php
View File

@@ -840,6 +840,39 @@ class SendingQueueTest extends \MailPoetTest {
verify($sendingQueue->getCountToProcess())->equals(0);
}
public function testItCompletesEverythingProperlyWhenThereIsNoOneToSendTo() {
// Set the only scheduled task subscriber to unsubscribed
$this->subscriber->setStatus(SubscriberEntity::STATUS_UNSUBSCRIBED);
$this->entityManager->flush();
$sendingQueueWorker = $this->getSendingQueueWorker(
$this->construct(
MailerTask::class,
[$this->diContainer->get(MailerFactory::class)],
[
'send' => Expected::never(),
]
)
);
$sendingQueueWorker->process();
// queue status is set to completed
$sendingQueue = $this->sendingQueuesRepository->findOneById($this->sendingQueue->getId());
$this->assertInstanceOf(SendingQueueEntity::class, $sendingQueue);
$scheduledTask = $this->scheduledTasksRepository->findOneBySendingQueue($sendingQueue);
$this->assertInstanceOf(ScheduledTaskEntity::class, $scheduledTask);
$this->sendingQueuesRepository->refresh($sendingQueue);
$this->scheduledTasksRepository->refresh($scheduledTask);
$newsletter = $sendingQueue->getNewsletter();
$this->assertInstanceOf(NewsletterEntity::class, $newsletter);
verify($sendingQueue->getCountTotal())->equals(0);
verify($sendingQueue->getCountProcessed())->equals(0);
verify($sendingQueue->getCountToProcess())->equals(0);
verify($scheduledTask->getStatus())->equals(SendingQueueEntity::STATUS_COMPLETED);
verify($newsletter->getStatus())->equals(NewsletterEntity::STATUS_SENT);
}
public function testItRemovesSubscribersFromProcessingListWhenNewsletterHasSegmentAndSubscriberIsNotPartOfIt() {
$subscriberNotPartOfNewsletterSegment = $this->createSubscriber('subscriber1@mailpoet.com', 'Subscriber', 'One');
@@ -951,6 +984,7 @@ class SendingQueueTest extends \MailPoetTest {
verify($sendingQueue->getCountTotal())->equals(1);
verify($sendingQueue->getCountProcessed())->equals(1);
verify($sendingQueue->getCountToProcess())->equals(0);
verify($scheduledTask->getStatus())->equals(SendingQueueEntity::STATUS_COMPLETED);
}
public function testItUpdatesQueueSubscriberCountWhenNoneOfSubscribersExist() {
@@ -975,6 +1009,9 @@ class SendingQueueTest extends \MailPoetTest {
verify($this->sendingQueue->getCountTotal())->equals(0);
verify($this->sendingQueue->getCountProcessed())->equals(0);
verify($this->sendingQueue->getCountToProcess())->equals(0);
$scheduledTask = $this->scheduledTasksRepository->findOneBySendingQueue($this->sendingQueue);
$this->assertInstanceOf(ScheduledTaskEntity::class, $scheduledTask);
verify($scheduledTask->getStatus())->equals(SendingQueueEntity::STATUS_COMPLETED);
}
public function testItDoesNotSendToTrashedSubscribers() {
@@ -995,6 +1032,9 @@ class SendingQueueTest extends \MailPoetTest {
$this->assertInstanceOf(SendingQueueEntity::class, $sendingQueue);
$this->sendingQueuesRepository->refresh($sendingQueue);
verify($sendingQueue->getCountTotal())->equals(0);
$scheduledTask = $this->scheduledTasksRepository->findOneBySendingQueue($this->sendingQueue);
$this->assertInstanceOf(ScheduledTaskEntity::class, $scheduledTask);
verify($scheduledTask->getStatus())->equals(SendingQueueEntity::STATUS_COMPLETED);
}
/**
@@ -1054,6 +1094,9 @@ class SendingQueueTest extends \MailPoetTest {
$this->assertInstanceOf(SendingQueueEntity::class, $sendingQueue);
$this->sendingQueuesRepository->refresh($sendingQueue);
verify($sendingQueue->getCountTotal())->equals(0);
$scheduledTask = $this->scheduledTasksRepository->findOneBySendingQueue($this->sendingQueue);
$this->assertInstanceOf(ScheduledTaskEntity::class, $scheduledTask);
verify($scheduledTask->getStatus())->equals(SendingQueueEntity::STATUS_COMPLETED);
}
public function testItDoesNotSendToSubscribersUnsubscribedFromSegments() {
@@ -1076,6 +1119,9 @@ class SendingQueueTest extends \MailPoetTest {
$this->assertInstanceOf(SendingQueueEntity::class, $sendingQueue);
$this->sendingQueuesRepository->refresh($sendingQueue);
verify($sendingQueue->getCountTotal())->equals(0);
$scheduledTask = $this->scheduledTasksRepository->findOneBySendingQueue($this->sendingQueue);
$this->assertInstanceOf(ScheduledTaskEntity::class, $scheduledTask);
verify($scheduledTask->getStatus())->equals(SendingQueueEntity::STATUS_COMPLETED);
}
public function testItDoesNotSendToInactiveSubscribers() {
@@ -1096,6 +1142,9 @@ class SendingQueueTest extends \MailPoetTest {
$this->assertInstanceOf(SendingQueueEntity::class, $sendingQueue);
$this->sendingQueuesRepository->refresh($sendingQueue);
verify($sendingQueue->getCountTotal())->equals(0);
$scheduledTask = $this->scheduledTasksRepository->findOneBySendingQueue($this->sendingQueue);
$this->assertInstanceOf(ScheduledTaskEntity::class, $scheduledTask);
verify($scheduledTask->getStatus())->equals(SendingQueueEntity::STATUS_COMPLETED);
}
public function testItPausesSendingWhenProcessedSubscriberListCannotBeUpdated() {