diff --git a/mailpoet/lib/AdminPages/Pages/AutomationEditor.php b/mailpoet/lib/AdminPages/Pages/AutomationEditor.php index d29189f057..9635a57b56 100644 --- a/mailpoet/lib/AdminPages/Pages/AutomationEditor.php +++ b/mailpoet/lib/AdminPages/Pages/AutomationEditor.php @@ -4,6 +4,7 @@ namespace MailPoet\AdminPages\Pages; use DateTimeImmutable; use MailPoet\AdminPages\PageRenderer; +use MailPoet\Automation\Engine\Data\NextStep; use MailPoet\Automation\Engine\Data\Step; use MailPoet\Automation\Engine\Data\Workflow; use MailPoet\Automation\Engine\Registry; @@ -95,8 +96,10 @@ class AutomationEditor { 'id' => $step->getId(), 'type' => $step->getType(), 'key' => $step->getKey(), - 'next_step_id' => $step->getNextStepId(), - 'args' => $step->getArgs() ?: new \stdClass(), + 'args' => $step->getArgs(), + 'next_steps' => array_map(function (NextStep $nextStep) { + return $nextStep->toArray(); + }, $step->getNextSteps()), ]; }, $workflow->getSteps()), ]; diff --git a/mailpoet/lib/Automation/Engine/Builder/UpdateStepsController.php b/mailpoet/lib/Automation/Engine/Builder/UpdateStepsController.php index c94756a325..471d3a7cf8 100644 --- a/mailpoet/lib/Automation/Engine/Builder/UpdateStepsController.php +++ b/mailpoet/lib/Automation/Engine/Builder/UpdateStepsController.php @@ -33,13 +33,6 @@ class UpdateStepsController { if (!$step) { throw Exceptions::workflowStepNotFound($key); } - - return new Step( - $data['id'], - $data['type'], - $data['key'], - $data['next_step_id'] ?? null, - $data['args'] ?? null - ); + return Step::fromArray($data); } } diff --git a/mailpoet/lib/Automation/Engine/Builder/UpdateWorkflowController.php b/mailpoet/lib/Automation/Engine/Builder/UpdateWorkflowController.php index 3caa6bc427..56ebaf3d2b 100644 --- a/mailpoet/lib/Automation/Engine/Builder/UpdateWorkflowController.php +++ b/mailpoet/lib/Automation/Engine/Builder/UpdateWorkflowController.php @@ -2,6 +2,7 @@ namespace MailPoet\Automation\Engine\Builder; +use MailPoet\Automation\Engine\Data\Step; use MailPoet\Automation\Engine\Data\Workflow; use MailPoet\Automation\Engine\Exceptions; use MailPoet\Automation\Engine\Exceptions\UnexpectedValueException; @@ -81,15 +82,17 @@ class UpdateWorkflowController { foreach ($steps as $id => $data) { $existingStep = $existingSteps[$id] ?? null; - if ( - !$existingStep - || $data['id'] !== $existingStep->getId() - || $data['type'] !== $existingStep->getType() - || $data['key'] !== $existingStep->getKey() - || $data['next_step_id'] !== $existingStep->getNextStepId() - ) { + if (!$existingStep || !$this->stepChanged(Step::fromArray($data), $existingStep)) { throw Exceptions::workflowStructureModificationNotSupported(); } } } + + private function stepChanged(Step $a, Step $b): bool { + $aData = $a->toArray(); + $bData = $b->toArray(); + unset($aData['args']); + unset($bData['args']); + return $aData === $bData; + } } diff --git a/mailpoet/lib/Automation/Engine/Data/NextStep.php b/mailpoet/lib/Automation/Engine/Data/NextStep.php new file mode 100644 index 0000000000..9cbb55c4e2 --- /dev/null +++ b/mailpoet/lib/Automation/Engine/Data/NextStep.php @@ -0,0 +1,28 @@ +id = $id; + } + + public function getId(): string { + return $this->id; + } + + public function toArray(): array { + return [ + 'id' => $this->id, + ]; + } + + public static function fromArray(array $data): self { + return new self($data['id']); + } +} diff --git a/mailpoet/lib/Automation/Engine/Data/Step.php b/mailpoet/lib/Automation/Engine/Data/Step.php index b27eeabb8d..3383c59e88 100644 --- a/mailpoet/lib/Automation/Engine/Data/Step.php +++ b/mailpoet/lib/Automation/Engine/Data/Step.php @@ -15,24 +15,28 @@ class Step { /** @var string */ private $key; - /** @var string|null */ - protected $nextStepId; - /** @var array */ protected $args; + /** @var NextStep[] */ + protected $nextSteps; + + /** + * @param array $args + * @param NextStep[] $nextSteps + */ public function __construct( string $id, string $type, string $key, - ?string $nextStepId = null, - array $args = [] + array $args, + array $nextSteps ) { $this->id = $id; $this->type = $type; $this->key = $key; - $this->nextStepId = $nextStepId; $this->args = $args; + $this->nextSteps = $nextSteps; } public function getId(): string { @@ -47,12 +51,14 @@ class Step { return $this->key; } - public function getNextStepId(): ?string { - return $this->nextStepId; + /** @return NextStep[] */ + public function getNextSteps(): array { + return $this->nextSteps; } - public function setNextStepId(string $id): void { - $this->nextStepId = $id; + /** @param NextStep[] $nextSteps */ + public function setNextSteps(array $nextSteps): void { + $this->nextSteps = $nextSteps; } public function getArgs(): array { @@ -64,8 +70,22 @@ class Step { 'id' => $this->id, 'type' => $this->type, 'key' => $this->key, - 'next_step_id' => $this->nextStepId, 'args' => $this->args, + 'next_steps' => array_map(function (NextStep $nextStep) { + return $nextStep->toArray(); + }, $this->nextSteps), ]; } + + public static function fromArray(array $data): self { + return new self( + $data['id'], + $data['type'], + $data['key'], + $data['args'], + array_map(function (array $nextStep) { + return NextStep::fromArray($nextStep); + }, $data['next_steps']) + ); + } } diff --git a/mailpoet/lib/Automation/Engine/Data/Workflow.php b/mailpoet/lib/Automation/Engine/Data/Workflow.php index 30cfe1ff7c..132ceaef4a 100644 --- a/mailpoet/lib/Automation/Engine/Data/Workflow.php +++ b/mailpoet/lib/Automation/Engine/Data/Workflow.php @@ -186,7 +186,9 @@ class Workflow { // TODO: validation $workflow = new self( $data['name'], - self::parseSteps(Json::decode($data['steps'])), + array_map(function (array $stepData): Step { + return Step::fromArray($stepData); + }, Json::decode($data['steps'])), new \WP_User((int)$data['author']) ); $workflow->id = (int)$data['id']; @@ -197,18 +199,4 @@ class Workflow { $workflow->activatedAt = $data['activated_at'] !== null ? new DateTimeImmutable($data['activated_at']) : null; return $workflow; } - - private static function parseSteps(array $data): array { - $steps = []; - foreach ($data as $step) { - $steps[] = new Step( - $step['id'], - $step['type'], - $step['key'], - $step['next_step_id'] ?? null, - $step['args'] ?? [] - ); - } - return $steps; - } } diff --git a/mailpoet/lib/Automation/Engine/Endpoints/Workflows/WorkflowsPutEndpoint.php b/mailpoet/lib/Automation/Engine/Endpoints/Workflows/WorkflowsPutEndpoint.php index abd041be36..eaa2e956c7 100644 --- a/mailpoet/lib/Automation/Engine/Endpoints/Workflows/WorkflowsPutEndpoint.php +++ b/mailpoet/lib/Automation/Engine/Endpoints/Workflows/WorkflowsPutEndpoint.php @@ -7,10 +7,11 @@ use MailPoet\Automation\Engine\API\Endpoint; use MailPoet\Automation\Engine\API\Request; use MailPoet\Automation\Engine\API\Response; use MailPoet\Automation\Engine\Builder\UpdateWorkflowController; +use MailPoet\Automation\Engine\Data\NextStep; use MailPoet\Automation\Engine\Data\Step; use MailPoet\Automation\Engine\Data\Workflow; +use MailPoet\Automation\Engine\Validators\WorkflowSchema; use MailPoet\Validator\Builder; -use stdClass; class WorkflowsPutEndpoint extends Endpoint { /** @var UpdateWorkflowController */ @@ -29,19 +30,11 @@ class WorkflowsPutEndpoint extends Endpoint { } public static function getRequestSchema(): array { - $step = Builder::object([ - 'id' => Builder::string()->required(), - 'type' => Builder::string()->required(), - 'key' => Builder::string()->required(), - 'args' => Builder::object(), - 'next_step_id' => Builder::string()->nullable(), - ]); - return [ 'id' => Builder::integer()->required(), 'name' => Builder::string()->minLength(1), 'status' => Builder::string(), - 'steps' => Builder::object()->additionalProperties($step), + 'steps' => WorkflowSchema::getStepsSchema(), ]; } @@ -62,8 +55,10 @@ class WorkflowsPutEndpoint extends Endpoint { 'id' => $step->getId(), 'type' => $step->getType(), 'key' => $step->getKey(), - 'next_step_id' => $step->getNextStepId(), - 'args' => $step->getArgs() ?: new stdClass(), + 'args' => $step->getArgs(), + 'next_steps' => array_map(function (NextStep $nextStep) { + return $nextStep->toArray(); + }, $step->getNextSteps()), ]; }, $workflow->getSteps()), ]; diff --git a/mailpoet/lib/Automation/Integrations/MailPoet/Templates/WorkflowBuilder.php b/mailpoet/lib/Automation/Integrations/MailPoet/Templates/WorkflowBuilder.php index 0019b700af..e7a2e70fbf 100644 --- a/mailpoet/lib/Automation/Integrations/MailPoet/Templates/WorkflowBuilder.php +++ b/mailpoet/lib/Automation/Integrations/MailPoet/Templates/WorkflowBuilder.php @@ -2,6 +2,7 @@ namespace MailPoet\Automation\Integrations\MailPoet\Templates; +use MailPoet\Automation\Engine\Data\NextStep; use MailPoet\Automation\Engine\Data\Step; use MailPoet\Automation\Engine\Data\Workflow; use MailPoet\Automation\Engine\Registry; @@ -22,7 +23,7 @@ class WorkflowBuilder { public function createFromSequence(string $name, array $sequence, array $sequenceArgs = []): Workflow { $steps = []; - $nextStep = null; + $nextSteps = []; foreach (array_reverse($sequence) as $index => $stepKey) { $workflowStep = $this->registry->getStep($stepKey); if (!$workflowStep) { @@ -33,12 +34,13 @@ class WorkflowBuilder { $this->uniqueId(), in_array(Trigger::class, (array)class_implements($workflowStep)) ? Step::TYPE_TRIGGER : Step::TYPE_ACTION, $stepKey, - $nextStep, - $args + $args, + $nextSteps ); - $nextStep = $step->getId(); + $nextSteps = [new NextStep($step->getId())]; $steps[] = $step; } + $steps[] = new Step('root', 'root', 'core:root', [], $nextSteps); $steps = array_reverse($steps); return new Workflow( $name, diff --git a/mailpoet/tests/integration/Automation/Engine/Storage/WorkflowStorageTest.php b/mailpoet/tests/integration/Automation/Engine/Storage/WorkflowStorageTest.php index 148382fd84..c3d38d3409 100644 --- a/mailpoet/tests/integration/Automation/Engine/Storage/WorkflowStorageTest.php +++ b/mailpoet/tests/integration/Automation/Engine/Storage/WorkflowStorageTest.php @@ -20,7 +20,7 @@ class WorkflowStorageTest extends \MailPoetTest public function testItLoadsLatestVersion() { $workflow = $this->createEmptyWorkflow(); - $step1 = new Step('id', Step::TYPE_ACTION, 'key'); + $step1 = new Step('id', Step::TYPE_ACTION, 'key', [], []); $workflow->setSteps(['id' => $step1]); $this->testee->updateWorkflow($workflow); $updatedWorkflow = $this->testee->getWorkflow($workflow->getId()); @@ -28,7 +28,7 @@ class WorkflowStorageTest extends \MailPoetTest $this->assertTrue($workflow->getVersionId() < $updatedWorkflow->getVersionId()); $this->assertEquals(1, count($updatedWorkflow->getSteps())); - $step2 = new Step('id-2', Step::TYPE_ACTION, 'key'); + $step2 = new Step('id-2', Step::TYPE_ACTION, 'key', [], []); $workflow->setSteps(['id' => $step1, 'id-2' => $step2]); $this->testee->updateWorkflow($workflow); $latestWorkflow = $this->testee->getWorkflow($workflow->getId()); @@ -40,7 +40,7 @@ class WorkflowStorageTest extends \MailPoetTest public function testItLoadsCorrectVersion() { $workflow = $this->createEmptyWorkflow(); - $step1 = new Step('id', Step::TYPE_ACTION, 'key'); + $step1 = new Step('id', Step::TYPE_ACTION, 'key', [], []); $workflow->setSteps(['id' => $step1]); $this->testee->updateWorkflow($workflow); $updatedWorkflow = $this->testee->getWorkflow($workflow->getId()); @@ -48,7 +48,7 @@ class WorkflowStorageTest extends \MailPoetTest $this->assertTrue($workflow->getVersionId() < $updatedWorkflow->getVersionId()); $this->assertEquals(1, count($updatedWorkflow->getSteps())); - $step2 = new Step('id-2', Step::TYPE_ACTION, 'key'); + $step2 = new Step('id-2', Step::TYPE_ACTION, 'key', [], []); $workflow->setSteps(['id' => $step1, 'id-2' => $step2]); $this->testee->updateWorkflow($workflow); $correctWorkflow = $this->testee->getWorkflow($workflow->getId(), $updatedWorkflow->getVersionId()); @@ -60,7 +60,7 @@ class WorkflowStorageTest extends \MailPoetTest public function testItLoadsOnlyActiveWorkflowsByTrigger() { $workflow = $this->createEmptyWorkflow(); $subscriberTrigger = $this->diContainer->get(SegmentSubscribedTrigger::class); - $trigger = new Step('id', Step::TYPE_TRIGGER, $subscriberTrigger->getKey()); + $trigger = new Step('id', Step::TYPE_TRIGGER, $subscriberTrigger->getKey(), [], []); $workflow->setSteps(['id' => $trigger]); $workflow->setStatus(Workflow::STATUS_INACTIVE); $this->testee->updateWorkflow($workflow); diff --git a/mailpoet/tests/integration/Automation/Integrations/Core/Actions/DelayActionTest.php b/mailpoet/tests/integration/Automation/Integrations/Core/Actions/DelayActionTest.php index 76d7251657..c4a320d0ba 100644 --- a/mailpoet/tests/integration/Automation/Integrations/Core/Actions/DelayActionTest.php +++ b/mailpoet/tests/integration/Automation/Integrations/Core/Actions/DelayActionTest.php @@ -3,6 +3,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\Step; use MailPoet\Automation\Engine\Data\Workflow; use MailPoet\Automation\Engine\Data\WorkflowRun; @@ -15,11 +16,16 @@ class DelayActionTest extends \MailPoetTest { * @dataProvider dataForTestItCalculatesDelayTypesCorrectly */ public function testItCalculatesDelayTypesCorrectly(int $delay, string $type, int $expectation) { - - $step = new Step("1", 'core:delay', 'core:delay', 'next-step', [ - 'delay' => $delay, - 'delay_type' => $type, - ]); + $step = new Step( + '1', + 'core:delay', + 'core:delay', + [ + 'delay' => $delay, + 'delay_type' => $type, + ], + [new NextStep('next-step')] + ); $workflow = $this->createMock(Workflow::class); $workflowRun = $this->createMock(WorkflowRun::class); $workflowRun->expects($this->atLeastOnce())->method('getId')->willReturn(1); @@ -81,10 +87,16 @@ class DelayActionTest extends \MailPoetTest { */ public function testDelayActionInvalidatesOutsideOfBoundaries(int $delay, bool $expectation) { - $step = new Step("1", 'core:delay', 'core:delay', 'next-step', [ - 'delay' => $delay, - 'delay_type' => "HOURS", - ]); + $step = new Step( + '1', + 'core:delay', + 'core:delay', + [ + 'delay' => $delay, + 'delay_type' => "HOURS", + ], + [new NextStep('next-step')] + ); $workflow = $this->createMock(Workflow::class); $actionScheduler = $this->createMock(ActionScheduler::class); $testee = new DelayAction($actionScheduler); diff --git a/mailpoet/tests/integration/Automation/Integrations/MailPoet/Actions/SendEmailActionTest.php b/mailpoet/tests/integration/Automation/Integrations/MailPoet/Actions/SendEmailActionTest.php index 283ee92283..4afd688ae8 100644 --- a/mailpoet/tests/integration/Automation/Integrations/MailPoet/Actions/SendEmailActionTest.php +++ b/mailpoet/tests/integration/Automation/Integrations/MailPoet/Actions/SendEmailActionTest.php @@ -66,7 +66,7 @@ class SendEmailActionTest extends \MailPoetTest { $this->segmentSubject = $this->diContainer->get(SegmentSubject::class); $this->email = (new Newsletter())->withAutomationType()->create(); - $this->step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $this->email->getId()]); + $this->step = new Step('step-id', Step::TYPE_ACTION, 'step-key',['email_id' => $this->email->getId()], []); $this->workflow = new Workflow('test-workflow', [], new \WP_User()); } @@ -84,13 +84,13 @@ class SendEmailActionTest extends \MailPoetTest { } public function testItIsNotValidIfStepHasNoEmail(): void { - $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, []); + $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', [], []); expect($this->action->isValid($this->getSubjects(), $step, $this->workflow))->false(); } public function testItRequiresAutomationEmailType(): void { $newsletter = (new Newsletter())->withPostNotificationsType()->create(); - $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $newsletter->getId()]); + $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $newsletter->getId()], []); expect($this->action->isValid($this->getSubjects(), $step, $this->workflow))->false(); } @@ -103,7 +103,7 @@ class SendEmailActionTest extends \MailPoetTest { $subjects = $this->getLoadedSubjects($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); - $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); + $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); $workflow = new Workflow('some-workflow', [$step], new \WP_User()); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects); @@ -125,7 +125,7 @@ class SendEmailActionTest extends \MailPoetTest { $subjects = $this->getLoadedSubjects($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); - $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); + $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); $workflow = new Workflow('some-workflow', [$step], new \WP_User()); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects); @@ -157,7 +157,7 @@ class SendEmailActionTest extends \MailPoetTest { $subjects = $this->getLoadedSubjects($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); - $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); + $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); $workflow = new Workflow('some-workflow', [$step], new \WP_User()); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects); @@ -186,7 +186,7 @@ class SendEmailActionTest extends \MailPoetTest { $subjects = $this->getLoadedSubjects($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); - $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); + $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); $workflow = new Workflow('some-workflow', [$step], new \WP_User()); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects); @@ -224,7 +224,7 @@ class SendEmailActionTest extends \MailPoetTest { $subjects = $this->getLoadedSubjects($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); - $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); + $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); $workflow = new Workflow('some-workflow', [$step], new \WP_User()); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects); @@ -253,7 +253,7 @@ class SendEmailActionTest extends \MailPoetTest { $subjects = $this->getLoadedSubjects($subscriber, $segment); $email = (new Newsletter())->withAutomationType()->create(); - $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); + $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', ['email_id' => $email->getId()], []); $workflow = new Workflow('some-workflow', [$step], new \WP_User()); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects);