From 12f2d1730ff017c8b23a2451f55221f4500b5512 Mon Sep 17 00:00:00 2001 From: Jan Jakes Date: Tue, 1 Nov 2022 09:49:47 +0100 Subject: [PATCH] Do not enforce workflow content for non-active workflows [MAILPOET-4757] --- .../Validation/WorkflowRules/AtLeastOneTriggerRule.php | 5 +++++ .../WorkflowRules/TriggerNeedsToBeFollowedByActionRule.php | 5 +++++ .../{AtLeastOnTriggerTest.php => AtLeastOneTriggerTest.php} | 5 +++-- .../WorkflowRules/TriggerNeedsNextStepsRuleTest.php | 3 +++ 4 files changed, 16 insertions(+), 2 deletions(-) rename mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/{AtLeastOnTriggerTest.php => AtLeastOneTriggerTest.php} (89%) diff --git a/mailpoet/lib/Automation/Engine/Validation/WorkflowRules/AtLeastOneTriggerRule.php b/mailpoet/lib/Automation/Engine/Validation/WorkflowRules/AtLeastOneTriggerRule.php index 9aba6f21b8..f0a13d74ca 100644 --- a/mailpoet/lib/Automation/Engine/Validation/WorkflowRules/AtLeastOneTriggerRule.php +++ b/mailpoet/lib/Automation/Engine/Validation/WorkflowRules/AtLeastOneTriggerRule.php @@ -25,6 +25,11 @@ class AtLeastOneTriggerRule implements WorkflowNodeVisitor { } public function complete(Workflow $workflow): void { + // run full step validation only for active workflows + if ($workflow->getStatus() !== Workflow::STATUS_ACTIVE) { + return; + } + if ($this->triggerFound) { return; } diff --git a/mailpoet/lib/Automation/Engine/Validation/WorkflowRules/TriggerNeedsToBeFollowedByActionRule.php b/mailpoet/lib/Automation/Engine/Validation/WorkflowRules/TriggerNeedsToBeFollowedByActionRule.php index 0e634ea0a4..e806208e98 100644 --- a/mailpoet/lib/Automation/Engine/Validation/WorkflowRules/TriggerNeedsToBeFollowedByActionRule.php +++ b/mailpoet/lib/Automation/Engine/Validation/WorkflowRules/TriggerNeedsToBeFollowedByActionRule.php @@ -15,6 +15,11 @@ class TriggerNeedsToBeFollowedByActionRule implements WorkflowNodeVisitor { } public function visitNode(Workflow $workflow, WorkflowNode $node): void { + // run full step validation only for active workflows + if ($workflow->getStatus() !== Workflow::STATUS_ACTIVE) { + return; + } + $step = $node->getStep(); if ($step->getType() !== Step::TYPE_TRIGGER) { return; diff --git a/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/AtLeastOnTriggerTest.php b/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/AtLeastOneTriggerTest.php similarity index 89% rename from mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/AtLeastOnTriggerTest.php rename to mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/AtLeastOneTriggerTest.php index cb3d78bb29..6d87ced6de 100644 --- a/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/AtLeastOnTriggerTest.php +++ b/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/AtLeastOneTriggerTest.php @@ -9,7 +9,7 @@ use MailPoet\Automation\Engine\Validation\WorkflowGraph\WorkflowWalker; require_once __DIR__ . '/WorkflowRuleTest.php'; -class AtLeastOnTriggerTest extends WorkflowRuleTest +class AtLeastOneTriggerTest extends WorkflowRuleTest { public function testItPassesWhenTriggerExists(): void { $steps = [ @@ -17,6 +17,7 @@ class AtLeastOnTriggerTest extends WorkflowRuleTest 't' => $this->createStep('t', Step::TYPE_TRIGGER), ]; $workflow = $this->make(Workflow::class, ['getSteps' => $steps, 'getStep' => function($id) use ($steps) { return $steps[$id]??null; }]); + $workflow->setStatus(Workflow::STATUS_ACTIVE); (new WorkflowWalker())->walk($workflow, [new AtLeastOneTriggerRule()]); //no exception thrown. @@ -27,7 +28,7 @@ class AtLeastOnTriggerTest extends WorkflowRuleTest 'root' => $this->createStep('root', Step::TYPE_ROOT) ]; $workflow = $this->make(Workflow::class, ['getSteps' => $steps, 'getStep' => function($id) use ($steps) { return $steps[$id]??null; }]); - + $workflow->setStatus(Workflow::STATUS_ACTIVE); $this->expectException(UnexpectedValueException::class); $this->expectExceptionMessage('Invalid automation structure: There must be at least one trigger in the automation.'); diff --git a/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/TriggerNeedsNextStepsRuleTest.php b/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/TriggerNeedsNextStepsRuleTest.php index faa859a11a..304e448e7c 100644 --- a/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/TriggerNeedsNextStepsRuleTest.php +++ b/mailpoet/tests/unit/Automation/Engine/Validation/WorkflowRules/TriggerNeedsNextStepsRuleTest.php @@ -18,6 +18,7 @@ class TriggerNeedsNextStepsRuleTest extends WorkflowRuleTest 'a' => $this->createStep('a', Step::TYPE_ACTION, []), ]; $workflow = $this->make(Workflow::class, ['getSteps' => $steps, 'getStep' => function($id) use ($steps) { return $steps[$id]??null; }]); + $workflow->setStatus(Workflow::STATUS_ACTIVE); (new WorkflowWalker())->walk($workflow, [new TriggerNeedsToBeFollowedByActionRule()]); //no exception thrown. @@ -29,6 +30,7 @@ class TriggerNeedsNextStepsRuleTest extends WorkflowRuleTest 't' => $this->createStep('t', Step::TYPE_TRIGGER, []), ]; $workflow = $this->make(Workflow::class, ['getSteps' => $steps, 'getStep' => function($id) use ($steps) { return $steps[$id]??null; }]); + $workflow->setStatus(Workflow::STATUS_ACTIVE); $this->expectException(UnexpectedValueException::class); $this->expectExceptionMessage('Invalid automation structure: A trigger needs to be followed by an action.'); @@ -43,6 +45,7 @@ class TriggerNeedsNextStepsRuleTest extends WorkflowRuleTest 't2' => $this->createStep('t2', Step::TYPE_TRIGGER, ['a']), ]; $workflow = $this->make(Workflow::class, ['getSteps' => $steps, 'getStep' => function($id) use ($steps) { return $steps[$id]??null; }]); + $workflow->setStatus(Workflow::STATUS_ACTIVE); $this->expectException(UnexpectedValueException::class);