Use step run args in existing actions

[MAILPOET-4629]
2022-09-19 13:57:15 +02:00
parent 68b5bab9cc
commit fe3d719a13
5 changed files with 71 additions and 54 deletions
--- 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,
      ],
    ]);
--- 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 {
--- 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);
    }
  }

--- 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 {
--- 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)),
    ];
  }