From 55f2d83abbaa4751e84fa35628dff685915216d3 Mon Sep 17 00:00:00 2001 From: Rostislav Wolny Date: Thu, 28 Nov 2024 12:41:43 +0100 Subject: [PATCH] Prevent resending emails in case the email is not active Automation email needs to be active in order to be sent. If we allow resending to inactive emails, we would need to activate them first, but this could be an unexpected side effect. And activating may cause sending other paused tasks as a side effect. I expect the email to be active in the majority of cases, so users should not meet this notice very often. [MAILPOET-6241] --- .../API/JSON/v1/SendingTaskSubscribers.php | 14 +++++++--- .../JSON/v1/SendingTaskSubscribersTest.php | 28 +++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/mailpoet/lib/API/JSON/v1/SendingTaskSubscribers.php b/mailpoet/lib/API/JSON/v1/SendingTaskSubscribers.php index 7668554d7b..8df2974004 100644 --- a/mailpoet/lib/API/JSON/v1/SendingTaskSubscribers.php +++ b/mailpoet/lib/API/JSON/v1/SendingTaskSubscribers.php @@ -4,6 +4,7 @@ namespace MailPoet\API\JSON\v1; use MailPoet\API\JSON\Endpoint as APIEndpoint; use MailPoet\API\JSON\Error as APIError; +use MailPoet\API\JSON\Response; use MailPoet\API\JSON\ResponseBuilders\ScheduledTaskSubscriberResponseBuilder; use MailPoet\Config\AccessControl; use MailPoet\Cron\CronHelper; @@ -125,12 +126,17 @@ class SendingTaskSubscribers extends APIEndpoint { ]); } + if ($newsletter->canBeSetActive() && $newsletter->getStatus() !== NewsletterEntity::STATUS_ACTIVE) { + return $this->errorResponse([ + APIError::BAD_REQUEST => __('Failed to resend! The email is not active. Please activate it first.', 'mailpoet'), + ], [], Response::STATUS_BAD_REQUEST); + } + $taskSubscriber->resetToUnprocessed(); $taskSubscriber->getTask()->setStatus(null); - $newsletter->setStatus( - $newsletter->canBeSetActive() ? NewsletterEntity::STATUS_ACTIVE : NewsletterEntity::STATUS_SENDING - ); - + if (!$newsletter->canBeSetActive()) { + $newsletter->setStatus(NewsletterEntity::STATUS_SENDING); + } // Each repository flushes all changes $this->scheduledTaskSubscribersRepository->flush(); return $this->successResponse([]); diff --git a/mailpoet/tests/integration/API/JSON/v1/SendingTaskSubscribersTest.php b/mailpoet/tests/integration/API/JSON/v1/SendingTaskSubscribersTest.php index 2e46135ad7..d672b46844 100644 --- a/mailpoet/tests/integration/API/JSON/v1/SendingTaskSubscribersTest.php +++ b/mailpoet/tests/integration/API/JSON/v1/SendingTaskSubscribersTest.php @@ -244,4 +244,32 @@ class SendingTaskSubscribersTest extends \MailPoetTest { verify($failedSubscriberTask->getProcessed())->equals(0); verify($failedSubscriberTask->getFailed())->equals(0); } + + public function testItPreventResendingInactiveAutomationEmail() { + $newsletter = (new NewsletterFactory())->withSubject('My Automatic Newsletter') + ->withType(NewsletterEntity::TYPE_AUTOMATION) + ->withStatus(NewsletterEntity::STATUS_DRAFT) + ->withBody(Fixtures::get('newsletter_body_template')) + ->withSendingQueue() + ->create(); + + $queue = $newsletter->getLatestQueue(); + $task = $queue->getTask(); + + $failedSubscriber = $this->subscriberFactory + ->withEmail('failed2@example.com') + ->withFirstName('Failed') + ->withLastName('Test') + ->create(); + $this->taskSubscriberFactory->createFailed($task, $failedSubscriber, 'Something went wrong!'); + + $res = $this->endpoint->resend([ + 'taskId' => $task->getId(), + 'subscriberId' => $failedSubscriber->getId(), + ]); + + verify($res->status)->equals(APIResponse::STATUS_BAD_REQUEST); + verify($res->errors[0]['message']) + ->equals('Failed to resend! The email is not active. Please activate it first.'); + } }