diff --git a/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php b/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php index d5e45d7cab..efa75b7d3c 100644 --- a/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/mailpoet/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -228,7 +228,12 @@ class SendingQueue { // get subscribers $subscriberBatches = new BatchIterator($task->getId(), $this->getBatchSize()); - if ($subscriberBatches->count() === 0) { + + // Set invalid state for sending task for non-campaign (no-bulk) newsletters with no subscribers (e.g. welcome emails, automatic emails). + // This cover cases when a welcome or automatic email was scheduled but before processing it the subscriber was deleted. + // The non-campaign emails are sent only to a single recipient, and we count stats based on sending tasks statues, so we can't mark them as completed. + // At the same time we want to keep a record abut processing them + if ($subscriberBatches->count() === 0 && !in_array($newsletter->getType(), NewsletterEntity::CAMPAIGN_TYPES, true)) { $this->loggerFactory->getLogger(LoggerFactory::TOPIC_NEWSLETTERS)->info( 'no subscribers to process', ['task_id' => $task->getId()] @@ -640,6 +645,18 @@ class SendingQueue { } private function endSending(ScheduledTaskEntity $task, NewsletterEntity $newsletter): void { + // The task is running but there is no one to send to. + // This may happen when we send to all but the execution is interrupted (e.g. by PHP time limit) and we don't update the task status + // or if we trigger sending to a newsletter without any subscriber (e.g. scheduled for long time but all were deleted) + // Lets set status to completed and update the queue counts + if ($task->getStatus() === null && $this->scheduledTaskSubscribersRepository->countUnprocessed($task) === 0) { + $task->setStatus(ScheduledTaskEntity::STATUS_COMPLETED); + $queue = $task->getSendingQueue(); + if ($queue) { + $this->sendingQueuesRepository->updateCounts($queue); + } + $this->scheduledTasksRepository->flush(); + } // Task is completed let's do all the stuff for the completed task if ($task->getStatus() === ScheduledTaskEntity::STATUS_COMPLETED) { $this->loggerFactory->getLogger(LoggerFactory::TOPIC_NEWSLETTERS)->info( diff --git a/mailpoet/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php b/mailpoet/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php index 6af46dcf62..1b532485c2 100644 --- a/mailpoet/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php +++ b/mailpoet/tests/integration/Cron/Workers/SendingQueue/SendingQueueTest.php @@ -840,7 +840,7 @@ class SendingQueueTest extends \MailPoetTest { verify($sendingQueue->getCountToProcess())->equals(0); } - public function testItCompletesEverythingProperlyWhenThereIsNoOneToSendTo() { + public function testItCompletesEverythingProperlyWhenThereIsNoSubscribedSubscriberToSendTo() { // Set the only scheduled task subscriber to unsubscribed $this->subscriber->setStatus(SubscriberEntity::STATUS_UNSUBSCRIBED); $this->entityManager->flush(); @@ -873,6 +873,41 @@ class SendingQueueTest extends \MailPoetTest { verify($newsletter->getStatus())->equals(NewsletterEntity::STATUS_SENT); } + public function testItCompletesEverythingProperlyWhenThereIsNoOneToSendTo() { + // No subscribers in queue + $this->scheduledTaskSubscribersRepository->setSubscribers( + $this->scheduledTask, + [] + ); + + $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'); @@ -1405,8 +1440,8 @@ class SendingQueueTest extends \MailPoetTest { } } - public function testSendingGetsStuckWhenSubscribersAreUnsubscribed() { - $newsletter = $this->createNewsletter(NewsletterEntity::TYPE_STANDARD, 'Subject With Deleted', NewsletterEntity::STATUS_SENDING); + public function testItMarksNonCampaignEmailWithNoSubscribersToSendToAsInvalid() { + $newsletter = $this->createNewsletter(NewsletterEntity::TYPE_WELCOME, 'Subject With Deleted', NewsletterEntity::STATUS_SENDING); [$segment, $subscriber] = $this->createListWithSubscriber(); $this->addSegmentToNewsletter($newsletter, $segment); $queue = $this->createQueueWithTask($newsletter, null, ['html' => 'Hello', 'text' => 'Hello']);