Transition - From places definition

[klimenttoshkov]

Hi again,

When defining a transition, we can supply a list of places than can be either in sub-array or not.
This will effectively result in AND/OR join, demonstrated below for transition t2:

from => ['b', 'c']:

from => [['b', 'c']]:

However, in original Symfony YAML configuration there is actually only a way of specifying the "AND" transition, which here is demonstrated by sub-array in the array.

And personally I think that it does not make sense to have two different transitions with the same name, starting from different places and leading to the same place.

So my suggestion is to rework the code and treat the definition as it is always as sub-array [[]], resulting in the second example.

[klimenttoshkov]

Rationale is that while it is technically possible to configure the first example workflow, it actually would never work and will not be used in real world applications.

[zerodahero]

Yeah, the array syntax isn't great for this since it's only an approximation of the way Symfony handles it.

Happy to consider a PR for this, but I think we do still need to consider both the "AND" and the "OR" scenarios for workflows, since Symfony supports both. The main difference between the two would be that in the "OR" scenario, you can transition to another state when you are in one of the many places, which for transitions to close/discard/cleanup type states makes sense.

[klimenttoshkov]

Hi Zack,

In regard to one transition, with one name - Symfony does not allow you to define workflow like in first example (the OR way). You can only make it "AND". This is artificially (or engineered way) achieved in your package when defining one transition, but you can't configure it in Symfony and it is for good reason. Also you can't achieve transition from different places that is with the same name (t2 in my example) because transition name is the key of array and you can't have multiple keys with the same name. Take a look again in Symfony examples and you will see that they treat it the "AND" way in YAML config. This is the only reasonable way when defining the transition in "from->to" way. If we had "to->from" definitions, then yes, we should have AND/OR.

[klimenttoshkov]

Moreover - the guard for this transition t2 is the same, regardless of the starting place. So if you have different starting places for the same transition name (t2 in this case) then the guard should take into account the starting place and implement different functions, which means we break Single-responsibility principle.

[klimenttoshkov]

And one more thing: you can't define "OR split", only "AND split", so it doesn't make any sense to be able to make "OR join" when you started with "AND split".

[zerodahero]

I must be missing something in what you're saying.

The docs clearly show multiple places using the same transition via "OR": https://symfony.com/doc/current/workflow/workflow-and-state-machine.html#state-machines

It's something the code-side of the component fully supports as well.

Regarding the single responsibility principle, that's completely independent of state. If you have an order fulfillment workflow with a "cancel" state, any transition there doesn't necessarily need to care about where it came from, it just needs to clean up (maybe soft-delete the DB record, send a cancellation email, dispatch another workflow to handle any WIP, etc). Transitions can very much be state independent (albeit constrained by the workflow definition), in which case single responsibility is just a matter of structuring your code appropriately.

So, it seems the main concern here in this issue is that the PHP Array syntax adopted by this package doesn't match the YAML syntax of the Symfony component?

[klimenttoshkov]

Probably YES.

I think we should:

1. Remove the option to use array keys as transition name and enforce name attribute, it is already present in your service provider but is optional;
2. Treat all keys in from as in the way you treat them if enclosed in double array [[...]] in order to remove ambiguity in configuration.

When we need to define "OR join" we will use two different array items for transitions with the same name and it will work as expected.

[klimenttoshkov]

And that way it is pretty much better because we actually can now define two separate transitions with one transition definition:
't1' => 'from' => ['a','b'], 'to' => ['c']

and this will define two transitions t1, one from a to c and another from b to c.

This should be only available by:

[
'name' => 't1',
'from' => ['a'],
'to' => ['c'],
],
[
'name' => 't1',
'from' => ['b'],
'to' => ['c'],
],

[klimenttoshkov]

And when we have:

[
'name' => 't1',
'from' => ['a','b'],
'to' => ['c'],
]

it should define one transition from a and b to c (AND join).

[zerodahero]

• Remove the option to use array keys as transition name and enforce name attribute, it is already present in your service provider but is optional;

As this is the "simple" definition approach, I don't think it should be removed. It gives users with less demanding requirements of their workflow the option to state it more simply.

• Treat all keys in from as in the way you treat them if enclosed in double array [[...]] in order to remove ambiguity in configuration.

This is a really big breaking change. I'm still not convinced it's necessary, since it really doesn't conflict with anything in the Symfony docs. For those with very specific workflows that depend on a very careful distinction between the and/or split, it doesn't sound like there's anything that can't be configured?

Seems like you feel pretty strongly about this though--so I'd definitely consider a PR here. I just don't have the time to work on this package anymore (coming up on almost 5 years since I've used it in a project).

[klimenttoshkov]

Sending PR is easy, but you are right that it may introduce breaking change, however the initial design of Symfony is to treat all parameters as enclosed in array (as the to now). So IMHO it should have not been implemented this way on the first place, but we have it, so maybe it should stay.

You right about everything else, thank you for the time spent on discussion. it was helpful for me, as always.

[klimenttoshkov]

I just don't have the time to work on this package anymore (coming up on almost 5 years since I've used it in a project).

Can we get in touch somehow to have a short discussion - mostly about what alternatives you've come up to when it comes to controlling workflows? Or if you are not using workflows anymore, why is that?

[zerodahero]

@klimenttoshkov Sure! I'd be happy to! You can DM me on twitter @zero_dahero and we can exchange contact info and get something set up!

[klimenttoshkov]

I can’t send you direct message there, no option is available Sent from my iPhoneOn 2 Nov 2022, at 14:04, Zack Teska ***@***.***> wrote: @klimenttoshkov Sure! I'd be happy to! You can DM me on twitter @zero_dahero and we can exchange contact info and get something set up! —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[klimenttoshkov]