[Workflow] Added an entered event #20787

Closed
wants to merge 1 commit into
from

Projects

None yet

6 participants

@Padam87
Contributor
Padam87 commented Dec 6, 2016 edited
Q A
Branch? 3.2
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #20774 (partially) ; #21433
License MIT
Doc PR -
@lyrixx
Member
lyrixx commented Dec 6, 2016

Hello @Padam87

1/ there is already the enter event. What is the difference?
2/ What is the use case for this new event?

@Padam87
Contributor
Padam87 commented Dec 6, 2016

Hi, I have expained in the issue. The enter event is called before the new markings are set, so you can't flush an entity in a listener because of this :)

@nicolas-grekas nicolas-grekas modified the milestone: 3.x Dec 7, 2016
@lyrixx
Member
lyrixx commented Dec 13, 2016

Thanks @Padam87 for your time. But sadly I'm still ๐Ÿ‘Ž about this event.

We already have many events. More over if you really want to to have your flush outside the controller, you can put it in the marking store. As well it makes more sense since the marking store, is all about the "store", and so the DB.

I need to close this PR as "won't fix" because of the -1 votes from the Symfony Core Team members.

The Workflow component is very recent, so we are very cautious about adding new features to it. In any case, if more users complain about this very same thing, we'll reconsider your proposal for Symfony 3.3. Thanks!

@lyrixx lyrixx closed this Dec 13, 2016
@Padam87 Padam87 referenced this pull request Feb 1, 2017
Closed

[Workflow] Enter events #21433

@lyrixx lyrixx reopened this Feb 9, 2017
@lyrixx
Member
lyrixx commented Feb 9, 2017

@Padam87 Could you rebase the PR? Thanks.

@@ -238,6 +240,24 @@ private function enter($subject, Transition $transition, Marking $marking)
}
}
+ private function entered($subject, Transition $transition, Marking $marking)
+ {
+ if (null !== $this->dispatcher) {
@lyrixx
lyrixx Feb 9, 2017 Member

if (null == $this->dispatcher) { return ; }

+ }
+
+ foreach ($transition->getTos() as $place) {
+ $marking->mark($place);
@lyrixx
lyrixx Feb 9, 2017 Member

The logic is wrong here, the marking is already set in the enter method

@Padam87
Padam87 Feb 9, 2017 Contributor

Oops, totally missed this, thx

@Padam87 Padam87 [Workflow] Added an entered event
7c7725c
@lyrixx
Member
lyrixx commented Feb 9, 2017

๐Ÿ‘

@xabbuh
xabbuh approved these changes Feb 10, 2017 View changes

๐Ÿ‘

Status: Reviewed

@lyrixx
lyrixx approved these changes Feb 13, 2017 View changes
@lyrixx
Member
lyrixx commented Feb 13, 2017

Thank you @Padam87.

@lyrixx lyrixx added a commit that referenced this pull request Feb 13, 2017
@lyrixx lyrixx feature #20787 [Workflow] Added an entered event (Padam87)
This PR was submitted for the 3.2 branch but it was merged into the 3.3-dev branch instead (closes #20787).

Discussion
----------

[Workflow] Added an entered event

| Q             | A
| ------------- | ---
| Branch?       |  3.2
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20774 (partially) ; #21433
| License       | MIT
| Doc PR        | -

Commits
-------

7a562bd [Workflow] Added an entered event
772d8e6
@lyrixx lyrixx added a commit that closed this pull request Feb 13, 2017
@lyrixx lyrixx feature #20787 [Workflow] Added an entered event (Padam87)
This PR was submitted for the 3.2 branch but it was merged into the 3.3-dev branch instead (closes #20787).

Discussion
----------

[Workflow] Added an entered event

| Q             | A
| ------------- | ---
| Branch?       |  3.2
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20774 (partially) ; #21433
| License       | MIT
| Doc PR        | -

Commits
-------

7a562bd [Workflow] Added an entered event
772d8e6
@lyrixx lyrixx closed this in 772d8e6 Feb 13, 2017
@fabpot
Member
fabpot commented Feb 13, 2017

@lyrixx Can you add a note in the component CHANGELOG?

@lyrixx
Member
lyrixx commented Feb 13, 2017

Yes, I will make a PR.

@lyrixx lyrixx added a commit to lyrixx/symfony that referenced this pull request Feb 13, 2017
@lyrixx lyrixx [Workflow] Updated changelog according to #20787 a0f1e85
@Padam87 Padam87 deleted the Padam87:workflow-event branch Feb 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment