diff --git a/mailpoet/lib/Automation/Integrations/Core/Actions/DelayAction.php b/mailpoet/lib/Automation/Integrations/Core/Actions/DelayAction.php index 0a0dbbea56..befca304a2 100644 --- a/mailpoet/lib/Automation/Integrations/Core/Actions/DelayAction.php +++ b/mailpoet/lib/Automation/Integrations/Core/Actions/DelayAction.php @@ -4,8 +4,8 @@ namespace MailPoet\Automation\Integrations\Core\Actions; use MailPoet\Automation\Engine\Control\ActionScheduler; use MailPoet\Automation\Engine\Data\Step; +use MailPoet\Automation\Engine\Data\StepRunArgs; use MailPoet\Automation\Engine\Data\Workflow; -use MailPoet\Automation\Engine\Data\WorkflowRun; use MailPoet\Automation\Engine\Hooks; use MailPoet\Automation\Engine\Workflows\Action; use MailPoet\Validator\Builder; @@ -40,11 +40,12 @@ class DelayAction implements Action { return []; } - public function run(Workflow $workflow, WorkflowRun $workflowRun, Step $step): void { + public function run(StepRunArgs $args): void { + $step = $args->getStep(); $nextStep = $step->getNextSteps()[0] ?? null; $this->actionScheduler->schedule(time() + $this->calculateSeconds($step), Hooks::WORKFLOW_STEP, [ [ - 'workflow_run_id' => $workflowRun->getId(), + 'workflow_run_id' => $args->getWorkflowRun()->getId(), 'step_id' => $nextStep ? $nextStep->getId() : null, ], ]); diff --git a/mailpoet/lib/Automation/Integrations/MailPoet/Actions/SendEmailAction.php b/mailpoet/lib/Automation/Integrations/MailPoet/Actions/SendEmailAction.php index aa51b4819a..0e8044d694 100644 --- a/mailpoet/lib/Automation/Integrations/MailPoet/Actions/SendEmailAction.php +++ b/mailpoet/lib/Automation/Integrations/MailPoet/Actions/SendEmailAction.php @@ -3,10 +3,12 @@ namespace MailPoet\Automation\Integrations\MailPoet\Actions; use MailPoet\Automation\Engine\Data\Step; +use MailPoet\Automation\Engine\Data\StepRunArgs; use MailPoet\Automation\Engine\Data\Workflow; -use MailPoet\Automation\Engine\Data\WorkflowRun; use MailPoet\Automation\Engine\Workflows\Action; use MailPoet\Automation\Engine\Workflows\Subject; +use MailPoet\Automation\Integrations\MailPoet\Payloads\SegmentPayload; +use MailPoet\Automation\Integrations\MailPoet\Payloads\SubscriberPayload; use MailPoet\Automation\Integrations\MailPoet\Subjects\SegmentSubject; use MailPoet\Automation\Integrations\MailPoet\Subjects\SubscriberSubject; use MailPoet\Entities\NewsletterEntity; @@ -96,30 +98,44 @@ class SendEmailAction implements Action { return count($segmentSubjects) === 1 && count($subscriberSubjects) === 1; } - public function run(Workflow $workflow, WorkflowRun $workflowRun, Step $step): void { - $newsletter = $this->getEmailForStep($step); - $subscriberSubject = $workflowRun->requireSingleSubject(SubscriberSubject::class); - $subscriber = $subscriberSubject->getSubscriber(); + public function run(StepRunArgs $args): void { + $newsletter = $this->getEmailForStep($args->getStep()); - if ($subscriber->getStatus() !== SubscriberEntity::STATUS_SUBSCRIBED) { - throw InvalidStateException::create()->withMessage(sprintf("Cannot schedule a newsletter for subscriber ID '%s' because their status is '%s'.", $subscriber->getId(), $subscriber->getStatus())); + $segmentPayload = $args->getSingleSubjectEntry('mailpoet:segment')->getPayload(); + if (!$segmentPayload instanceof SegmentPayload) { + throw new InvalidStateException(); } + $segmentId = $segmentPayload->getId(); + + $subscriberPayload = $args->getSingleSubjectEntry('mailpoet:subscriber')->getPayload(); + if (!$subscriberPayload instanceof SubscriberPayload) { + throw new InvalidStateException(); + } + $subscriberId = $subscriberPayload->getId(); - $segmentSubject = $workflowRun->requireSingleSubject(SegmentSubject::class); - $segmentId = $segmentSubject->getSegment()->getId(); $subscriberSegment = $this->subscriberSegmentRepository->findOneBy([ - 'subscriber' => $subscriber, + 'subscriber' => $subscriberId, 'segment' => $segmentId, 'status' => SubscriberEntity::STATUS_SUBSCRIBED, ]); - if ($subscriberSegment === null) { - throw InvalidStateException::create()->withMessage(sprintf("Subscriber ID '%s' is not subscribed to segment ID '%s'.", $subscriber->getId(), $segmentId)); + if (!$subscriberSegment) { + throw InvalidStateException::create()->withMessage(sprintf("Subscriber ID '%s' is not subscribed to segment ID '%s'.", $subscriberId, $segmentId)); } - $previouslyScheduledNotification = $this->scheduledTasksRepository->findByNewsletterAndSubscriberId($newsletter, (int)$subscriber->getId()); + $subscriber = $subscriberSegment->getSubscriber(); + if (!$subscriber) { + throw InvalidStateException::create(); + } + + $subscriberStatus = $subscriber->getStatus(); + if ($subscriberStatus !== SubscriberEntity::STATUS_SUBSCRIBED) { + throw InvalidStateException::create()->withMessage(sprintf("Cannot schedule a newsletter for subscriber ID '%s' because their status is '%s'.", $subscriberId, $subscriberStatus)); + } + + $previouslyScheduledNotification = $this->scheduledTasksRepository->findByNewsletterAndSubscriberId($newsletter, $subscriberId); if (!empty($previouslyScheduledNotification)) { - throw InvalidStateException::create()->withMessage(sprintf("Subscriber ID '%s' was already scheduled to receive newsletter ID '%s'.", $subscriber->getId(), $newsletter->getId())); + throw InvalidStateException::create()->withMessage(sprintf("Subscriber ID '%s' was already scheduled to receive newsletter ID '%s'.", $subscriberId, $newsletter->getId())); } try { diff --git a/mailpoet/tests/integration/Automation/Engine/Data/WorkflowRunLogTest.php b/mailpoet/tests/integration/Automation/Engine/Data/WorkflowRunLogTest.php index d2695e69c1..2b7ae8135e 100644 --- a/mailpoet/tests/integration/Automation/Engine/Data/WorkflowRunLogTest.php +++ b/mailpoet/tests/integration/Automation/Engine/Data/WorkflowRunLogTest.php @@ -3,6 +3,7 @@ namespace MailPoet\Test\Automation\Engine\Data; use MailPoet\Automation\Engine\Control\StepHandler; +use MailPoet\Automation\Engine\Data\StepRunArgs; use MailPoet\Automation\Engine\Data\Step; use MailPoet\Automation\Engine\Data\Workflow; use MailPoet\Automation\Engine\Data\WorkflowRun; @@ -294,9 +295,9 @@ class TestAction implements Action { return true; } - public function run(Workflow $workflow, WorkflowRun $workflowRun, Step $step): void { + public function run(StepRunArgs $args): void { if ($this->callback) { - ($this->callback)($workflow, $workflowRun, $step); + ($this->callback)($args); } } diff --git a/mailpoet/tests/integration/Automation/Integrations/Core/Actions/DelayActionTest.php b/mailpoet/tests/integration/Automation/Integrations/Core/Actions/DelayActionTest.php index c4a320d0ba..211669ab90 100644 --- a/mailpoet/tests/integration/Automation/Integrations/Core/Actions/DelayActionTest.php +++ b/mailpoet/tests/integration/Automation/Integrations/Core/Actions/DelayActionTest.php @@ -4,6 +4,7 @@ namespace MailPoet\Test\Automation\Integrations\Core\Actions; use MailPoet\Automation\Engine\Control\ActionScheduler; use MailPoet\Automation\Engine\Data\NextStep; +use MailPoet\Automation\Engine\Data\StepRunArgs; use MailPoet\Automation\Engine\Data\Step; use MailPoet\Automation\Engine\Data\Workflow; use MailPoet\Automation\Engine\Data\WorkflowRun; @@ -40,11 +41,7 @@ class DelayActionTest extends \MailPoetTest { ]] ); $testee = new DelayAction($actionScheduler); - $testee->run( - $workflow, - $workflowRun, - $step - ); + $testee->run(new StepRunArgs($workflow, $workflowRun, $step, [])); } public function dataForTestItCalculatesDelayTypesCorrectly() : array { diff --git a/mailpoet/tests/integration/Automation/Integrations/MailPoet/Actions/SendEmailActionTest.php b/mailpoet/tests/integration/Automation/Integrations/MailPoet/Actions/SendEmailActionTest.php index 4afd688ae8..8658655bc7 100644 --- a/mailpoet/tests/integration/Automation/Integrations/MailPoet/Actions/SendEmailActionTest.php +++ b/mailpoet/tests/integration/Automation/Integrations/MailPoet/Actions/SendEmailActionTest.php @@ -2,7 +2,10 @@ namespace MailPoet\Test\Automation\Integrations\MailPoet\Actions; +use MailPoet\Automation\Engine\Data\StepRunArgs; use MailPoet\Automation\Engine\Data\Step; +use MailPoet\Automation\Engine\Data\Subject; +use MailPoet\Automation\Engine\Data\SubjectEntry; use MailPoet\Automation\Engine\Data\Workflow; use MailPoet\Automation\Engine\Data\WorkflowRun; use MailPoet\Automation\Integrations\MailPoet\Actions\SendEmailAction; @@ -100,7 +103,7 @@ class SendEmailActionTest extends \MailPoetTest { ->withStatus(SubscriberEntity::STATUS_SUBSCRIBED) ->withSegments([$segment]) ->create(); - $subjects = $this->getLoadedSubjects($subscriber, $segment); + $subjects = $this->getSubjectData($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); @@ -110,7 +113,7 @@ class SendEmailActionTest extends \MailPoetTest { $scheduled = $this->scheduledTasksRepository->findByNewsletterAndSubscriberId($email, (int)$subscriber->getId()); expect($scheduled)->count(0); - $this->action->run($workflow, $run, $step); + $this->action->run(new StepRunArgs($workflow, $run, $step, $this->getSubjectEntries($subjects))); $scheduled = $this->scheduledTasksRepository->findByNewsletterAndSubscriberId($email, (int)$subscriber->getId()); expect($scheduled)->count(1); @@ -122,7 +125,7 @@ class SendEmailActionTest extends \MailPoetTest { ->withStatus(SubscriberEntity::STATUS_SUBSCRIBED) ->withSegments([$segment]) ->create(); - $subjects = $this->getLoadedSubjects($subscriber, $segment); + $subjects = $this->getSubjectData($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); @@ -133,13 +136,13 @@ class SendEmailActionTest extends \MailPoetTest { expect($scheduled)->count(0); $action = ContainerWrapper::getInstance()->get(SendEmailAction::class); - $action->run($workflow, $run, $step); + $action->run(new StepRunArgs($workflow, $run, $step, $this->getSubjectEntries($subjects))); $scheduled = $this->scheduledTasksRepository->findByNewsletterAndSubscriberId($email, (int)$subscriber->getId()); expect($scheduled)->count(1); try { - $action->run($workflow, $run, $step); + $action->run(new StepRunArgs($workflow, $run, $step, $this->getSubjectEntries($subjects))); } catch (InvalidStateException $exception) { // The exception itself isn't as important as the outcome } @@ -154,7 +157,7 @@ class SendEmailActionTest extends \MailPoetTest { ->withStatus(SubscriberEntity::STATUS_SUBSCRIBED) ->withSegments([$segment]) ->create(); - $subjects = $this->getLoadedSubjects($subscriber, $segment); + $subjects = $this->getSubjectData($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); @@ -168,7 +171,7 @@ class SendEmailActionTest extends \MailPoetTest { $action = ContainerWrapper::getInstance()->get(SendEmailAction::class); try { - $action->run($workflow, $run, $step); + $action->run(new StepRunArgs($workflow, $run, $step, $this->getSubjectEntries($subjects))); } catch (Exception $exception) { // The exception itself isn't as important as the outcome } @@ -183,7 +186,7 @@ class SendEmailActionTest extends \MailPoetTest { ->withStatus(SubscriberEntity::STATUS_SUBSCRIBED) ->withSegments([$segment]) ->create(); - $subjects = $this->getLoadedSubjects($subscriber, $segment); + $subjects = $this->getSubjectData($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); @@ -197,7 +200,7 @@ class SendEmailActionTest extends \MailPoetTest { $action = ContainerWrapper::getInstance()->get(SendEmailAction::class); try { - $action->run($workflow, $run, $step); + $action->run(new StepRunArgs($workflow, $run, $step, $this->getSubjectEntries($subjects))); } catch (Exception $exception) { // The exception itself isn't as important as the outcome } @@ -221,7 +224,7 @@ class SendEmailActionTest extends \MailPoetTest { ->withStatus($status) ->withSegments([$segment]) ->create(); - $subjects = $this->getLoadedSubjects($subscriber, $segment); + $subjects = $this->getSubjectData($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); @@ -235,7 +238,7 @@ class SendEmailActionTest extends \MailPoetTest { $action = ContainerWrapper::getInstance()->get(SendEmailAction::class); try { - $action->run($workflow, $run, $step); + $action->run(new StepRunArgs($workflow, $run, $step, $this->getSubjectEntries($subjects))); } catch (Exception $exception) { // The exception itself isn't as important as the outcome } @@ -250,7 +253,7 @@ class SendEmailActionTest extends \MailPoetTest { $subscriber = (new Subscriber()) ->withStatus(SubscriberEntity::STATUS_SUBSCRIBED) ->create(); - $subjects = $this->getLoadedSubjects($subscriber, $segment); + $subjects = $this->getSubjectData($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); @@ -263,7 +266,7 @@ class SendEmailActionTest extends \MailPoetTest { $action = ContainerWrapper::getInstance()->get(SendEmailAction::class); try { - $action->run($workflow, $run, $step); + $action->run(new StepRunArgs($workflow, $run, $step, $this->getSubjectEntries($subjects))); } catch (Exception $exception) { // The exception itself isn't as important as the outcome } @@ -272,20 +275,6 @@ class SendEmailActionTest extends \MailPoetTest { expect($scheduled)->count(0); } - private function getLoadedSubscriberSubject(SubscriberEntity $subscriber): SubscriberSubject { - $subscriberSubject = $this->diContainer->get(SubscriberSubject::class); - $subscriberSubject->load(['subscriber_id' => $subscriber->getId()]); - - return $subscriberSubject; - } - - private function getLoadedSegmentSubject(SegmentEntity $segment): SegmentSubject { - $segmentSubject = $this->diContainer->get(SegmentSubject::class); - $segmentSubject->load(['segment_id' => $segment->getId()]); - - return $segmentSubject; - } - private function getSubjects(): array { return [ $this->segmentSubject, @@ -293,10 +282,23 @@ class SendEmailActionTest extends \MailPoetTest { ]; } - private function getLoadedSubjects(SubscriberEntity $subscriber, SegmentEntity $segment): array { + private function getSubjectData(SubscriberEntity $subscriber, SegmentEntity $segment): array { return [ - $this->getLoadedSubscriberSubject($subscriber), - $this->getLoadedSegmentSubject($segment), + new Subject('mailpoet:segment', ['segment_id' => $segment->getId()]), + new Subject('mailpoet:subscriber', ['subscriber_id'=> $subscriber->getId()]), + ]; + } + + private function getSubjectEntries(array $subjects): array { + $segmentData = array_filter($subjects, function (Subject $subject) { + return $subject->getKey() === 'mailpoet:segment'; + }); + $subscriberData = array_filter($subjects, function (Subject $subject) { + return $subject->getKey() === 'mailpoet:subscriber'; + }); + return [ + new SubjectEntry($this->segmentSubject, reset($segmentData)), + new SubjectEntry($this->subscriberSubject, reset($subscriberData)), ]; }