Do not handle workflow runs when workflow status is not active/deactivating

[MAILPOET-4731]
This commit is contained in:
David Remer
2022-10-24 11:28:36 +03:00
parent b573ce1038
commit 6df11bf3d6
3 changed files with 133 additions and 1 deletions

View File

@@ -8,6 +8,7 @@ use MailPoet\Automation\Engine\Data\Step;
use MailPoet\Automation\Engine\Data\StepRunArgs; use MailPoet\Automation\Engine\Data\StepRunArgs;
use MailPoet\Automation\Engine\Data\StepValidationArgs; use MailPoet\Automation\Engine\Data\StepValidationArgs;
use MailPoet\Automation\Engine\Data\SubjectEntry; use MailPoet\Automation\Engine\Data\SubjectEntry;
use MailPoet\Automation\Engine\Data\Workflow;
use MailPoet\Automation\Engine\Data\WorkflowRun; use MailPoet\Automation\Engine\Data\WorkflowRun;
use MailPoet\Automation\Engine\Data\WorkflowRunLog; use MailPoet\Automation\Engine\Data\WorkflowRunLog;
use MailPoet\Automation\Engine\Exceptions; use MailPoet\Automation\Engine\Exceptions;
@@ -98,7 +99,8 @@ class StepHandler {
try { try {
$this->handleStep($args); $this->handleStep($args);
} catch (Throwable $e) { } catch (Throwable $e) {
$this->workflowRunStorage->updateStatus((int)$args['workflow_run_id'], WorkflowRun::STATUS_FAILED); $status = $e instanceof InvalidStateException && $e->getErrorCode() === 'mailpoet_automation_workflow_not_active' ? WorkflowRun::STATUS_CANCELLED : WorkflowRun::STATUS_FAILED;
$this->workflowRunStorage->updateStatus((int)$args['workflow_run_id'], $status);
if (!$e instanceof Exception) { if (!$e instanceof Exception) {
throw new Exception($e->getMessage(), intval($e->getCode()), $e); throw new Exception($e->getMessage(), intval($e->getCode()), $e);
} }
@@ -123,6 +125,9 @@ class StepHandler {
if (!$workflow) { if (!$workflow) {
throw Exceptions::workflowVersionNotFound($workflowRun->getWorkflowId(), $workflowRun->getVersionId()); throw Exceptions::workflowVersionNotFound($workflowRun->getWorkflowId(), $workflowRun->getVersionId());
} }
if (!in_array($workflow->getStatus(), [Workflow::STATUS_ACTIVE, Workflow::STATUS_DEACTIVATING], true)) {
throw Exceptions::workflowNotActive($workflowRun->getWorkflowId());
}
// complete workflow run // complete workflow run
if (!$stepId) { if (!$stepId) {

View File

@@ -14,6 +14,7 @@ class Exceptions {
private const JSON_NOT_OBJECT = 'mailpoet_automation_json_not_object'; private const JSON_NOT_OBJECT = 'mailpoet_automation_json_not_object';
private const WORKFLOW_NOT_FOUND = 'mailpoet_automation_workflow_not_found'; private const WORKFLOW_NOT_FOUND = 'mailpoet_automation_workflow_not_found';
private const WORKFLOW_VERSION_NOT_FOUND = 'mailpoet_automation_workflow_version_not_found'; private const WORKFLOW_VERSION_NOT_FOUND = 'mailpoet_automation_workflow_version_not_found';
private const WORKFLOW_NOT_ACTIVE = 'mailpoet_automation_workflow_not_active';
private const WORKFLOW_RUN_NOT_FOUND = 'mailpoet_automation_workflow_run_not_found'; private const WORKFLOW_RUN_NOT_FOUND = 'mailpoet_automation_workflow_run_not_found';
private const WORKFLOW_STEP_NOT_FOUND = 'mailpoet_automation_workflow_step_not_found'; private const WORKFLOW_STEP_NOT_FOUND = 'mailpoet_automation_workflow_step_not_found';
private const WORKFLOW_TRIGGER_NOT_FOUND = 'mailpoet_automation_workflow_trigger_not_found'; private const WORKFLOW_TRIGGER_NOT_FOUND = 'mailpoet_automation_workflow_trigger_not_found';
@@ -72,6 +73,13 @@ class Exceptions {
->withMessage(sprintf(__('Workflow with ID "%1$s" in version "%2$s" not found.', 'mailpoet'), $workflow, $version)); ->withMessage(sprintf(__('Workflow with ID "%1$s" in version "%2$s" not found.', 'mailpoet'), $workflow, $version));
} }
public static function workflowNotActive(int $workflow): InvalidStateException {
return InvalidStateException::create()
->withErrorCode(self::WORKFLOW_NOT_ACTIVE)
// translators: %1$s is the ID of the workflow.
->withMessage(sprintf(__('Workflow with ID "%1$s" in no longer active.', 'mailpoet'), $workflow));
}
public static function workflowRunNotFound(int $id): NotFoundException { public static function workflowRunNotFound(int $id): NotFoundException {
return NotFoundException::create() return NotFoundException::create()
->withErrorCode(self::WORKFLOW_RUN_NOT_FOUND) ->withErrorCode(self::WORKFLOW_RUN_NOT_FOUND)

View File

@@ -0,0 +1,119 @@
<?php
namespace MailPoet\Test\Automation\Engine\Control;
use MailPoet\Automation\Engine\Control\StepHandler;
use MailPoet\Automation\Engine\Control\StepRunner;
use MailPoet\Automation\Engine\Data\NextStep;
use MailPoet\Automation\Engine\Data\Step;
use MailPoet\Automation\Engine\Data\Workflow;
use MailPoet\Automation\Engine\Data\WorkflowRun;
use MailPoet\Automation\Engine\Exceptions;
use MailPoet\Automation\Engine\Exceptions\InvalidStateException;
use MailPoet\Automation\Engine\Exceptions\NotFoundException;
use MailPoet\Automation\Engine\Storage\WorkflowRunStorage;
use MailPoet\Automation\Engine\Storage\WorkflowStorage;
use MailPoet\Automation\Integrations\Core\Actions\DelayAction;
use MailPoet\Automation\Integrations\MailPoet\Triggers\SomeoneSubscribesTrigger;
class StepHandlerTest extends \MailPoetTest
{
/** @var WorkflowStorage */
private $workflowStorage;
/** @var WorkflowRunStorage */
private $workflowRunStorage;
/** @var StepHandler */
private $testee;
public function _before() {
$this->testee = $this->diContainer->get(StepHandler::class);
$this->workflowStorage = $this->diContainer->get(WorkflowStorage::class);
$this->workflowRunStorage = $this->diContainer->get(WorkflowRunStorage::class);
}
public function testItDoesOnlyProcessActiveAndDeactivatingWorkflows() {
$workflow = $this->createWorkflow();
$this->assertInstanceOf(Workflow::class, $workflow);
$steps = $workflow->getSteps();
$workflowRun = $this->createWorkflowRun($workflow);
$this->assertInstanceOf(WorkflowRun::class, $workflowRun);
$currentStep = current($steps);
$this->assertInstanceOf(Step::class, $currentStep);
$runner = $this->createMock(StepRunner::class);
$runner->expects(self::exactly(2))->method('run'); // The run method will be called twice: Once for the active workflow and once for the deactivating workflow.
$this->testee->addStepRunner($currentStep->getType(), $runner);
$this->assertSame(Workflow::STATUS_ACTIVE, $workflow->getStatus());
$this->testee->handle(['workflow_run_id' => $workflowRun->getId(), 'step_id' => $currentStep->getId()]);
// no exception thrown.
$newWorkflowRun = $this->workflowRunStorage->getWorkflowRun($workflowRun->getId());
$this->assertInstanceOf(WorkflowRun::class, $newWorkflowRun);
$this->assertSame(WorkflowRun::STATUS_RUNNING, $newWorkflowRun->getStatus());
$workflow->setStatus(Workflow::STATUS_DEACTIVATING);
$this->workflowStorage->updateWorkflow($workflow);
$this->testee->handle(['workflow_run_id' => $workflowRun->getId(), 'step_id' => $currentStep->getId()]);
// no exception thrown.
$newWorkflowRun = $this->workflowRunStorage->getWorkflowRun($workflowRun->getId());
$this->assertInstanceOf(WorkflowRun::class, $newWorkflowRun);
$this->assertSame(WorkflowRun::STATUS_RUNNING, $newWorkflowRun->getStatus());
$invalidStati = array_filter(
Workflow::STATUS_ALL,
function(string $status) : bool {
return !in_array($status, [Workflow::STATUS_ACTIVE, Workflow::STATUS_DEACTIVATING], true);
}
);
foreach ($invalidStati as $status) {
$workflow->setStatus($status);
$this->workflowStorage->updateWorkflow($workflow);
$workflowRun = $this->createWorkflowRun($workflow);
$this->assertInstanceOf(WorkflowRun::class, $workflowRun);
$error = null;
try {
$this->testee->handle(['workflow_run_id' => $workflowRun->getId(), 'step_id' => $currentStep->getId()]);
} catch (InvalidStateException $error) {
$this->assertSame('mailpoet_automation_workflow_not_active', $error->getErrorCode(), "Workflow with '$status' did not return expected error code.");
}
$this->assertInstanceOf(InvalidStateException::class, $error);
$newWorkflowRun = $this->workflowRunStorage->getWorkflowRun($workflowRun->getId());
$this->assertInstanceOf(WorkflowRun::class, $newWorkflowRun);
$this->assertSame(WorkflowRun::STATUS_CANCELLED, $newWorkflowRun->getStatus());
}
}
private function createWorkflow(): ?Workflow {
$trigger = $this->diContainer->get(SomeoneSubscribesTrigger::class);
$delay = $this->diContainer->get(DelayAction::class);
$steps = [
'root' => new Step('root', Step::TYPE_ROOT, 'root', [], [new NextStep('someone-subscribes')]),
'someone-subscribes' => new Step('someone-subscribes', Step::TYPE_TRIGGER, $trigger->getKey(), [], [new NextStep('a')]),
'delay' => new Step('delay', Step::TYPE_ACTION, $delay->getKey(), [], []),
];
$workflow = new Workflow('test', $steps, wp_get_current_user());
$workflow->setStatus(Workflow::STATUS_ACTIVE);
return $this->workflowStorage->getWorkflow($this->workflowStorage->createWorkflow($workflow));
}
private function createWorkflowRun(Workflow $workflow, $subjects = []) : ?WorkflowRun {
$trigger = array_filter($workflow->getSteps(), function(Step $step) : bool { return $step->getType() === Step::TYPE_TRIGGER;});
$triggerKeys = array_map(function(Step $step) : string { return $step->getKey();}, $trigger);
$triggerKey = count($triggerKeys)>0?current($triggerKeys):'';
$workflowRun = new WorkflowRun(
$workflow->getId(),
$workflow->getVersionId(),
$triggerKey,
$subjects
);
return $this->workflowRunStorage->getWorkflowRun($this->workflowRunStorage->createWorkflowRun($workflowRun));
}
}