Added missing events description #7528
Conversation
There was a problem hiding this comment.
@Guite thanks for your contribution!
Although I don't use the Workflow component, I've proposed some changes. The reason is that we must be extra careful when describing an event to explain exactly when it's triggered. That's why I used "just before" and "just after" in the description. Please tell me if I did it right. Thanks!
|
@javiereguiluz looks fine for me, but should be reviewed by someone who is more experienced with the internals. |
workflow/usage.rst
Outdated
| The following events are dispatched for all workflows: | ||
|
|
||
| * ``workflow.guard``: occurs just before starting a transition. It allows you to | ||
| prevent the transition by calling ``$event->setBlocked(true);`` as shown above. |
There was a problem hiding this comment.
It is also fired when testing available transitions.
workflow/usage.rst
Outdated
|
|
||
| * ``workflow.guard``: occurs just before starting a transition. It allows you to | ||
| prevent the transition by calling ``$event->setBlocked(true);`` as shown above. | ||
| * ``workflow.leave``: occurs just after an object has left it's current state. |
There was a problem hiding this comment.
We should mention that all events give carries the subject and the transition.
Also I think we should clarify what state is available in the event:
* ``workflow.leave``: carries the marking with the initial places, occurs just before the transition.
* ``workflow.transition``: carries the marking with the current places, occurs during the transition.
* ``workflow.enter``: carries the marking with the new places, occurs just after the transition.Actually while checking this I've opened symfony/symfony#21793.
There was a problem hiding this comment.
Thanks @Guite, I've made some other comments, but please wait for other members opinion before doing any changes, as we also need to see if symfony/symfony#21793 gets merged.
workflow/usage.rst
Outdated
| * ``workflow.transition``: occurs just before starting to transition to the new state. | ||
| * ``workflow.enter``: occurs just after the object has entered into the new state. | ||
| * ``workflow.guard``: occurs just before a transition is started and when testing which transitions are available. It allows you to define that the transition is not allowed by calling ``$event->setBlocked(true);`` as shown above. | ||
| * ``workflow.leave``: carries the marking with the initial places, occurs just after an object has left it's current state. |
There was a problem hiding this comment.
it's => its.
To avoid confusion, maybe we should not use "initial" and say something like:
carries the marking with the places held before an object leaves it's current state.
workflow/usage.rst
Outdated
| * ``workflow.enter``: occurs just after the object has entered into the new state. | ||
| * ``workflow.guard``: occurs just before a transition is started and when testing which transitions are available. It allows you to define that the transition is not allowed by calling ``$event->setBlocked(true);`` as shown above. | ||
| * ``workflow.leave``: carries the marking with the initial places, occurs just after an object has left it's current state. | ||
| * ``workflow.transition``: carries the marking with the current places, occurs just before starting to transition to the new state. |
There was a problem hiding this comment.
carries the marking with the places held just after leaving the previous state, and just before entering into the new state.
workflow/usage.rst
Outdated
| * ``workflow.guard``: occurs just before a transition is started and when testing which transitions are available. It allows you to define that the transition is not allowed by calling ``$event->setBlocked(true);`` as shown above. | ||
| * ``workflow.leave``: carries the marking with the initial places, occurs just after an object has left it's current state. | ||
| * ``workflow.transition``: carries the marking with the current places, occurs just before starting to transition to the new state. | ||
| * ``workflow.enter``: carries the marking with the new places, occurs just after the object has entered into the new state. |
There was a problem hiding this comment.
This one needs no "correction", but to be consistent with others, what about:
carries the marking with the places held after the object has entered into the new state.
There was a problem hiding this comment.
So symfony/symfony#21793 is merged now and this should be reworded to:
carries the marking with the current places, occurs just before the object enters into the new places.…HeahDude) This PR was merged into the 3.2 branch. Discussion ---------- [Workflow] Fixed marking state on leave and enter events | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | related to symfony/symfony-docs#7528 It seems weird to me to dispatch an event while the marking is not yet marked by the new places. The event is still "marked" by old places when leaving occurs, my guess is that the symmetry should be mirrored. Do you agree? Should I add a test? Commits ------- 175858a [Workflow] Fixed marking state on leave and enter events
|
The PR has been merged. And BTW, if you can add a note in the doc about symfony/symfony#21925 it would be awesome ! |
|
The change @lyrixx suggests should be done in another PR targeting master since this one needs to be merged in 3.2. |
|
This is ready for the final review. Thanks! |
|
This is rebased for Should I add |
|
I think this should be rearranged to fit with the existing Using Events section. |
|
@xabbuh you are right. I think this one can be closed, since the existing section covers the whole topic already. |
|
@Guite Thanks for the feedback. Closing then. If you want to add more information to the existing section, please feel free to create new PRs. :) |
This PR adds descriptions for all events dispatched by the workflow component.