[Workflow] Choose which Workflow events should be dispatched

[stewartmalik]

Description
Provide a way to choose which events are to be dispatched by the Workflow component.

Background
Currently I have an application that processes orders. See below for Workflow places and transitions:

            places:
                - new
                - sent
                - saved_order_id
                - in_progress
                - order_complete
                - activated
                - complete
                - failed
            transitions:
                send:
                    from: new
                    to:   sent
                save_order_id:
                    from: sent
                    to: saved_order_id
                check:
                    from: saved_order_id
                    to: in_progress
                complete_order:
                    from: in_progress
                    to: order_complete
                activate:
                    from: order_complete
                    to: activated
                complete:
                    from: activated
                    to:   complete
                fail:
                    from: [ 'new', 'sent', 'saved_order_id', 'in_progress', 'order_complete', 'activated' ]
                    to: failed

I have workflows that assist in maintaining the state of the order. Currently my code listens for Guard Events and performs some action based on the state of the workflow.

For example when attempting to place an order via a third party API I have implemented a Guard Event Listener for the send transition. In the EventListener I attempt to call the third party API to place the order, if this fails I return a TransitionBlocker which prevents the transition from taking place. This all works perfectly.

My current issue is that when a transition has taken place successfully, the Workflow component calls the announce() method so that we can find which new transitions are now possible for the new place. The issue with this is that the announce() method eventually calls buildTransitionBlockerListForTransition() but with possible transitions for the new place the order is in. In the above example if the send transition completes successfully buildTransitionBlockerListForTransition() is called with save_order_id as the transition.

This then dispatches the Guard Event for the save_order_id transition even though we’ve only explicitly called Workflow->apply() on the send transition. The effect of this is that Guard Event Listeners for the save_order_id transition fire prematurely.

While this isn’t an issue for some transitions of the Workflow, for Guard Event Listeners that contact a third party API, or perform some kind of state change with the order, it means this state change happens before we explicitly call the transition where it should occur. Then when we actually call the next transition ourselves sometime later it happens for a second time.

Proposal
Provide some method of choosing which events the Workflow component should dispatch for a specific workflow.

In an ideal scenario I think this could be added as an optional configuration item for each workflow definition. An array that contains all events that should be dispatched, or the inverse, an array that defines which events should be excluded from being dispatched.

For example:

framework:
    workflows:
        some_workflow:
            type: state_machine
            events: ['leave', 'enter', 'entered', 'completed']
            audit_trail:
                enabled: true
...

Thank you for taking the time to read this proposal :)

[lyrixx]

I think this is a good idea.

Do you want to try to submit a pr? If you can't it not an issue at all. I will do it

[stewartmalik]

Thanks @lyrixx

I would love to submit a PR, do you have any suggestions on how I would approach this?

Do you have a preference on whether the optional config item should be a, "which events should be fired", or a "which events shouldn't be fired"?

My current thinking is to add a new method into Workflow.php called shouldDispatchEvent() which will be called within each event function such as completed() and announce() where the EventDispatcher is checked:

    private function announce(object $subject, Transition $initialTransition, Marking $marking): void
    {
        if (null === $this->dispatcher || !$this->shouldDispatchEvent(__FUNCTION__)) {
            return;
        }
...

shouldDispatchEvent() will be hooked up to the config item mentioned in the original comment events

Thoughts?

[lyrixx]

Do you have a preference on whether the optional config item should be a, "which events should be fired", or a "which events shouldn't be fired"?

Let's be positive:

• null => fire all events
• [] => fire 0 event
• [leave] => fire only the leave event

The configuration could live in the metadata key of the configuration.
Then why not for the shouldDispatchEvent method, but it should be private.
But I don't like the __FUNCTION__. I prefer to hardcode the value here.

Thanks

[noniagriconomie]

Maybe also remove the code that handle the announce alone?
In favor of this full config list 👍🏻

[stewartmalik]

Hi both,

Have submitted a draft PR beginning this work. Have removed the code that handled announce alone.

A few things to note:

• I have left the Guard Events untouched as I feel these are a special case that should always be fired.
• I'm not sure if we should use the metadata key of the configuration, I feel this space should be left for end users and we shouldn't mix component configuration in with it.
• Doc PR to follow after we've decided how this should be configured.

Thoughts?

[noniagriconomie]

I have left the Guard Events untouched as I feel these are a special case that should always be fired.

i think it is ok to keep it, only apply should be altered IMO

I'm not sure if we should use the metadata key of the configuration, I feel this space should be left for end users and we shouldn't mix component configuration in with it.

agreed, metadata must be "external" config, not framework config