[Workflow] Added Context to Workflow Event #37539

epitre · 2020-07-09T10:36:35Z

There's also a default context for the initial marking event.

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #37421
License	MIT
Doc PR	symfony/symfony-docs#13947

This time, I need context in the events, because, sometimes, the transition is automatic and sometimes, it is manual. I want to be able to make the difference. I figured using the context for that is a good idea. I hope you do too :)

By the way, I believe it also fixes #37421 . I took @pluk77 request to be able to differentiate an initial marking by having a default context in that case

src/Symfony/Bridge/Twig/Extension/WorkflowExtension.php

src/Symfony/Component/Workflow/Event/Event.php

epitre · 2020-07-09T11:00:48Z

appveyor is not happy but I believe it has nothing to do with me. Does it ?

src/Symfony/Component/Workflow/WorkflowInterface.php

pluk77 · 2020-07-09T12:08:18Z

src/Symfony/Component/Workflow/Workflow.php

 class Workflow implements WorkflowInterface
 {
    public const DISABLE_ANNOUNCE_EVENT = 'workflow_disable_announce_event';
+    public const DEFAULT_INITIAL_CONTEXT = ['initial' => true];


Is there a reason you made this an associative array and not an indexed one?
public const DEFAULT_INITIAL_CONTEXT = ['initial'];

I see in one of your tests you did use an indexed array as $context.

Are yuo

this way, you can do :
if ($context['initial']??false)

lyrixx

Thanks for your PR but I already rejected in the past

src/Symfony/Component/Workflow/WorkflowInterface.php

src/Symfony/Component/Workflow/Workflow.php

epitre · 2020-07-09T13:53:40Z

Thanks for your PR but I already rejected in the past

O_O Does it mean you are rejecting this one ?

What I want, is to be able to differentiate transitions that will be done automatically from others that will be done manually.
If not with the context, how do you suggest to do that ?

epitre · 2020-07-09T14:14:31Z

Thanks for your PR but I already rejected in the past

I found an old PR and realized the TransitionEvent knows the context ^^

I'll use this event, then. But I really don't understand why the other events do not deserve to know the context ^^

lyrixx · 2020-07-09T14:53:46Z

Oups, I write the comment but I forgot to remove it 👍

I'm against to put the context in the guard event but for other event I guess this is fine 👍

O_O Does it mean you are rejecting this one ?

nope, it's OK 👍

Sorry for my bad comment

epitre · 2020-07-09T15:11:58Z

Oups, I write the comment but I forgot to remove it +1

I'm against to put the context in the guard event but for other event I guess this is fine +1

O_O Does it mean you are rejecting this one ?

nope, it's OK +1

Sorry for my bad comment

Great ! So, GuardEvent is not aware of the context anymore. I simplified TransitionEvent. And TransitionEvent is still the only event which can change the context.

src/Symfony/Component/Workflow/Event/Event.php

src/Symfony/Component/Workflow/Workflow.php

lyrixx · 2020-07-09T21:45:32Z

src/Symfony/Component/Workflow/Workflow.php

            foreach ($transition->getTos() as $place) {
                $this->dispatcher->dispatch($event, sprintf('workflow.%s.entered.%s', $this->name, $place));
            }
+        } elseif (!empty($this->definition->getInitialPlaces())) {


This is not related to the current PR. Could you revert it? thanks

without that, the DEFAULT_INITIAL_CONTEXT makes no sense.

it's a little far fetched, but I believe adding this context enhancement is a good moment to add this default context

Unless you don't want a DEFAULT_INITIAL_CONTEXT ?

There's also a default context for the initial marking event.

epitre · 2020-07-20T09:29:01Z

@lyrixx up !

lyrixx · 2020-08-12T13:16:15Z

I'm gonna merge your PR, but I also gonna tweak 2/3 things

Thanks for your work and your patience

lyrixx · 2020-08-12T13:17:34Z

Thank you @epitre.

This PR was merged into the 5.2-dev branch. Discussion ---------- [Workflow] Simplify code + Updated README.md | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | Fix #37539 (cleaning previous PR) | License | MIT | Doc PR | Commits ------- 3268bdb [Workflow] Simplify code + Updated README.md

epitre · 2020-08-12T14:58:49Z

@lyrixx Thank you :)

Could you link me the changes you make, just to be informed, please ?

lyrixx · 2020-08-12T16:40:30Z

Just above: #37813

philepsybo · 2025-09-22T13:15:43Z

Oups, I write the comment but I forgot to remove it 👍

I'm against to put the context in the guard event but for other event I guess this is fine 👍

O_O Does it mean you are rejecting this one ?

nope, it's OK 👍

Sorry for my bad comment

I know this is a suuuper old issue but i would be interested to know the reasoning why the context should not be included in GuardEvents. If a transition can be requested by a user that is hidden behind some permission while the system should always be able to apply it if triggered by internal logic, it feels quite unintuitive to do this via TransitionEvents (at least for me). If i have a guard that prevents users from applying transitions they are not permitted to apply, i'd find it intuitive to have the option to convey some flag the guard so it knows "check, this is Dev explicitly telling me that this is an internal thing and i don't have to be afraid about the logged in user".
Maybe I am misinterpreting the intended semantics of GuardEvents and this would be an abuse of the concept.

lyrixx · 2025-10-01T12:24:17Z

I think a subject's transition should only be blocked via its own properties or behaviours. Not by something related to the context. This avoids scattering bits of data between the subject and the context.

Therefore, you should attach what you need to the subject itself.

epitre requested a review from lyrixx as a code owner July 9, 2020 10:36

carsonbot added Status: Needs Review Feature labels Jul 9, 2020

epitre force-pushed the eventWithContext branch from 33cc8f2 to 82b8a82 Compare July 9, 2020 10:47

epitre commented Jul 9, 2020

View reviewed changes

src/Symfony/Bridge/Twig/Extension/WorkflowExtension.php Show resolved Hide resolved

ro0NL reviewed Jul 9, 2020

View reviewed changes

src/Symfony/Component/Workflow/Event/Event.php Outdated Show resolved Hide resolved

epitre force-pushed the eventWithContext branch 2 times, most recently from ee4d388 to 348cb02 Compare July 9, 2020 11:07

epitre changed the title ~~Added Context to Workflow Event~~ [Workflow] Added Context to Workflow Event Jul 9, 2020

epitre commented Jul 9, 2020

View reviewed changes

src/Symfony/Component/Workflow/WorkflowInterface.php Outdated Show resolved Hide resolved

pluk77 reviewed Jul 9, 2020

View reviewed changes

lyrixx added the Workflow label Jul 9, 2020

lyrixx reviewed Jul 9, 2020

View reviewed changes

src/Symfony/Component/Workflow/WorkflowInterface.php Outdated Show resolved Hide resolved

src/Symfony/Component/Workflow/Workflow.php Outdated Show resolved Hide resolved

src/Symfony/Component/Workflow/Workflow.php Outdated Show resolved Hide resolved

epitre force-pushed the eventWithContext branch from 348cb02 to 6ece94a Compare July 9, 2020 14:06

epitre force-pushed the eventWithContext branch from 6ece94a to f53d995 Compare July 9, 2020 15:09

epitre force-pushed the eventWithContext branch 3 times, most recently from 7756024 to 2b1651c Compare July 9, 2020 15:23

nicolas-grekas added this to the next milestone Jul 9, 2020

lyrixx reviewed Jul 9, 2020

View reviewed changes

Added Context to Workflow Event

bfe07dd

There's also a default context for the initial marking event.

epitre force-pushed the eventWithContext branch from 2b1651c to bfe07dd Compare July 9, 2020 22:09

lyrixx approved these changes Aug 12, 2020

View reviewed changes

carsonbot removed the Status: Needs Review label Aug 12, 2020

carsonbot added the Status: Reviewed label Aug 12, 2020

lyrixx merged commit 25095d8 into symfony:master Aug 12, 2020

This was referenced Aug 12, 2020

[Worfklow] Add 'initiated' event which is dispatched upon entering inital place #37481

Closed

[Workflow] Simplify code + Updated README.md #37813

Merged

nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020

fabpot mentioned this pull request Oct 5, 2020

Release v5.2.0-BETA1 #38417

Merged

xabbuh mentioned this pull request Apr 9, 2021

Missing $context parameter in Workflow GuardEvent constructor #40742

Closed

xabbuh mentioned this pull request Nov 11, 2021

[Workflow] fixed $context not available on GuardEvent #43996

Closed

Uh oh!

[Workflow] Added Context to Workflow Event #37539

[Workflow] Added Context to Workflow Event #37539

Uh oh!

Conversation

epitre commented Jul 9, 2020

Uh oh!

Uh oh!

Uh oh!

epitre commented Jul 9, 2020

Uh oh!

Uh oh!

pluk77 Jul 9, 2020

Choose a reason for hiding this comment

Uh oh!

epitre Jul 9, 2020

Choose a reason for hiding this comment

Uh oh!

lyrixx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

epitre commented Jul 9, 2020

Uh oh!

epitre commented Jul 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lyrixx commented Jul 9, 2020

Uh oh!

epitre commented Jul 9, 2020

Uh oh!

Uh oh!

Uh oh!

lyrixx Jul 9, 2020

Choose a reason for hiding this comment

Uh oh!

epitre Jul 9, 2020

Choose a reason for hiding this comment

Uh oh!

epitre commented Jul 20, 2020

Uh oh!

lyrixx commented Aug 12, 2020

Uh oh!

lyrixx commented Aug 12, 2020

Uh oh!

epitre commented Aug 12, 2020

Uh oh!

lyrixx commented Aug 12, 2020

Uh oh!

philepsybo commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lyrixx commented Oct 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

epitre commented Jul 9, 2020 •

edited

Loading

philepsybo commented Sep 22, 2025 •

edited

Loading