Skip to content

Commit

Permalink
bug #28322 [Workflow] Make sure we do not run the next transition on …
Browse files Browse the repository at this point in the history
…an updated state (Nyholm)

This PR was squashed before being merged into the 4.1 branch (closes #28322).

Discussion
----------

[Workflow] Make sure we do not run the next transition on an updated state

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        |

When you have a workflow like this, where all transitions are named the same.

![php](https://user-images.githubusercontent.com/1275206/44908106-87914680-ad1a-11e8-9c6f-e58de4b255e8.png)

The current behavior on `$stateMachine->apply($object, 'next')` is that it will go to "done".

It will do this because it checks the current "froms" at each iteration in the loop in `apply()`. It should check the valid transition before it actually apply them.

Commits
-------

d9dda76 [Workflow] Make sure we do not run the next transition on an updated state
  • Loading branch information
fabpot committed Sep 4, 2018
2 parents 1c3d3db + d9dda76 commit 824dbbd
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
19 changes: 19 additions & 0 deletions src/Symfony/Component/Workflow/Tests/WorkflowTest.php
Expand Up @@ -345,6 +345,25 @@ public function testApplyWithSameNameTransition2()
$this->assertTrue($marking->has('d'));
}

public function testApplyWithSameNameTransition3()
{
$subject = new \stdClass();
$subject->marking = array('a' => 1);

$places = range('a', 'd');
$transitions = array();
$transitions[] = new Transition('t', 'a', 'b');
$transitions[] = new Transition('t', 'b', 'c');
$transitions[] = new Transition('t', 'c', 'd');
$definition = new Definition($places, $transitions);
$workflow = new Workflow($definition, new MultipleStateMarkingStore());

$marking = $workflow->apply($subject, 't');
// We want to make sure we do not end up in "d"
$this->assertTrue($marking->has('b'));
$this->assertFalse($marking->has('d'));
}

public function testApplyWithEventDispatcher()
{
$definition = $this->createComplexWorkflowDefinition();
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Component/Workflow/Workflow.php
Expand Up @@ -139,6 +139,7 @@ public function apply($subject, $transitionName)

$transitionBlockerList = null;
$applied = false;
$approvedTransitionQueue = array();

foreach ($this->definition->getTransitions() as $transition) {
if ($transition->getName() !== $transitionName) {
Expand All @@ -149,7 +150,10 @@ public function apply($subject, $transitionName)
if (!$transitionBlockerList->isEmpty()) {
continue;
}
$approvedTransitionQueue[] = $transition;
}

foreach ($approvedTransitionQueue as $transition) {
$applied = true;

$this->leave($subject, $transition, $marking);
Expand Down

0 comments on commit 824dbbd

Please sign in to comment.