laravel-workflow icon indicating copy to clipboard operation
laravel-workflow copied to clipboard

PR#22 broke the correct definition of workflow's multiple from.

Open cirdog opened this issue 6 years ago • 3 comments

I'm not sure what bug this should have fixed but it broke something else.

Instead of having 1 transition with multiple froms you get 2 transition with 1 from. This way you can't force a transition to have multiple froms.

This changed should be reverted or it a better fix should be created.

To be as precise as possible when using the example config:

<?php

return [
    'straight'   => [
        'type'          => 'state_machine',
        'marking_store' => [
            'type'      => 'single_state',
            'arguments' => ['currentPlace']
        ],
        'supports'      => ['App\Payment'],
        'places'        => ['initial', 'payment', 'waiting'],
        'transitions'   => [
            'transitions'   => [
                'payment_state'      => [
                    'from' => 'initial',
                    'to'   => 'payment',
                ],
                'waiting_state'      => [
                    'from' => [
                        'initial',
                        'payment',
                    ],
                    'to'   => 'waiting',
                ],
            ],
        ],
    ]
];

This is wat i expect:

{Symfony\Component\Workflow\Transition} [3]
 name = "waiting_state"
 froms = {array} [2]
  0 = "initial"
  1 = "payment"
 tos = {array} [1]
  0 = "waiting"

And this is what i get:

{Symfony\Component\Workflow\Transition} [3]
 name = "waiting_state"
 froms = {array} [1]
  0 = "initial"
 tos = {array} [1]
  0 = "waiting"
{Symfony\Component\Workflow\Transition} [3]
 name = "waiting_state"
 froms = {array} [1]
  0 = "payment"
 tos = {array} [1]
  0 = "waiting"

For now I am reverting back to 1.2.0 as this works there.

_Originally posted by @MyDigitalLife in https://github.com/render_node/MDEyOklzc3VlQ29tbWVudDQxNzI5MjMyOA==/timeline/issue_comment#issuecomment-417292328

PR#22 breaks the SINGLE transition with multi-froms into 2 individual transitions with single-from , and this act broke the assumption of workflow requiring all froms to be fulfilled before leaving the place.

Now fulfilling any one of the froms will make it transition and it's obviously incorrect. Also I've reverted the code back to its previous state and it worked perfectly & correctly.

Please revert PR#22 to make workflow usable according to the specs again. The main point here is that multi-from doesn't mean "fulfilling any of the them" but "fulfilling ALL of them".

Thanks very much.

cirdog avatar Jan 09 '19 03:01 cirdog

Hi, I'm getting the same result as @cirdog . For now I'm retrieving this value directly from the workflow config file.

However, I have a doubt about how workflow with multiple froms configured should work: I understand the configuring multiple 'froms' for a particular transition means that transition can be applied from 'any' of the configure 'froms' places, is it right?

Maybe I'm confused by this sentence from @cirdog :

PR#22 breaks the SINGLE transition with multi-froms into 2 individual transitions with single-from , and this act broke the assumption of workflow requiring all froms to be fulfilled before leaving the place.

particularly the '...this act broke the assumption of workflow requiring all froms to be fulfilled before leaving the place.'.

Apologies if this is not the right place for asking this question, but it seems related to the issue in discussion.

Thanks in advance.

eworwa avatar Mar 04 '19 15:03 eworwa

Hi, I'm getting the same result as @cirdog too. I am on laravel version 6 , I can't come back to brexis/workflow version 1.2 Did someone find a solution or how to modify code to solve this issue ? Thanks

fabien44300 avatar Feb 28 '20 07:02 fabien44300

Hello,

To solve it, In the WorkflowRegistry class and addFromArray function , remove the foreach and only add the $builder->addTransition row

        $builder->addTransition(new Transition($transitionName, $transition['from'], $transition['to']));
        /*foreach ((array)$transition['from'] as $form) {

            $builder->addTransition(new Transition($transitionName, $form, $transition['to']));

        }*/

fabien44300 avatar Feb 28 '20 08:02 fabien44300