Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FrameworkBundle] fixed guard event names for transitions #28007

Merged
merged 1 commit into from Jul 29, 2018

Conversation

Projects
None yet
5 participants
@destillat
Copy link
Contributor

commented Jul 19, 2018

Q A
Branch? 3.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Framework yaml configuration support workflow transitions as both indexed and associative array e.g

transitions:
    -    name: test
         from: open
         to: test
    -
transitions:
    test:
         from: open
         to: test

Then it's used in foreach loop to register guard event listeners

foreach ($workflow['transitions'] as $transitionName => $config) {

Array keys are used as transition names but it's wrong for indexed array so we get event names like these
workflow.workflow_name.guard.transition_index instead of workflow.workflow_name.guard.tranision_name

@ro0NL

ro0NL approved these changes Jul 19, 2018

@destillat

This comment has been minimized.

Copy link
Contributor Author

commented Jul 20, 2018

BTW, if transition names are not unique, the latest guard expression per transition name is evaluated for all transitions

@nicolas-grekas
Copy link
Member

left a comment

(for 3.4)

@nicolas-grekas nicolas-grekas changed the base branch from 3.3 to 3.4 Jul 29, 2018

@nicolas-grekas nicolas-grekas force-pushed the destillat:3.3 branch from 757907b to 9bbb1e5 Jul 29, 2018

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jul 29, 2018

Thank you @destillat.

@nicolas-grekas nicolas-grekas merged commit 9bbb1e5 into symfony:3.4 Jul 29, 2018

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Jul 29, 2018

bug #28007 [FrameworkBundle] fixed guard event names for transitions …
…(destillat)

This PR was submitted for the 3.3 branch but it was merged into the 3.4 branch instead (closes #28007).

Discussion
----------

[FrameworkBundle] fixed guard event names for transitions

| Q             | A
| ------------- | ---
| Branch?       | 3.3|4
| Bug fix?      | yes
| New feature?  |no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Framework yaml configuration support workflow transitions as both indexed and associative array e.g
```yaml
transitions:
    -    name: test
         from: open
         to: test
    -
```
```yaml
transitions:
    test:
         from: open
         to: test
```
Then it's used in foreach loop to register guard event listeners https://github.com/symfony/symfony/blob/4b92b96796d381b225b6665b15160c4c9b06cf41/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L609
Array keys are used as transition names but it's wrong for indexed array so we get event names like these
workflow.workflow_name.guard.transition_index instead of workflow.workflow_name.guard.tranision_name

Commits
-------

9bbb1e5 [FrameworkBundle] fixed guard event names for transitions

This was referenced Aug 1, 2018

lyrixx added a commit that referenced this pull request Nov 13, 2018

bug #29137 [Workflow][FrameworkBundle] fixed guard event names for tr…
…ansitions (destillat, lyrixx)

This PR was merged into the 3.4 branch.

Discussion
----------

[Workflow][FrameworkBundle] fixed guard event names for transitions

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #28018 #28007 (comment)
| License       | MIT
| Doc PR        |

There is a bug when many transitions are defined with the same name.
I finished destillat's work and rebase against 3.4 as it's a bug fix.

There another point of failure, but it could not be fixed on 3.4. I will
be a need feature. The issue is related to `Workflow::can($subject, $transitionName)`.
Since the transitionName could be not unique, we will need to support
passing an instance of Transition. A new PR is incomming

Commits
-------

83dc473 [FrameworkBundle] fixed guard event names for transitions
fb88bfc [FrameworkBundle] fixed guard event names for transitions

symfony-splitter pushed a commit to symfony/workflow that referenced this pull request Nov 13, 2018

bug #29137 [Workflow][FrameworkBundle] fixed guard event names for tr…
…ansitions (destillat, lyrixx)

This PR was merged into the 3.4 branch.

Discussion
----------

[Workflow][FrameworkBundle] fixed guard event names for transitions

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #28018 symfony/symfony#28007 (comment)
| License       | MIT
| Doc PR        |

There is a bug when many transitions are defined with the same name.
I finished destillat's work and rebase against 3.4 as it's a bug fix.

There another point of failure, but it could not be fixed on 3.4. I will
be a need feature. The issue is related to `Workflow::can($subject, $transitionName)`.
Since the transitionName could be not unique, we will need to support
passing an instance of Transition. A new PR is incomming

Commits
-------

83dc473dd6 [FrameworkBundle] fixed guard event names for transitions
fb88bfc79a [FrameworkBundle] fixed guard event names for transitions

symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Nov 13, 2018

bug #29137 [Workflow][FrameworkBundle] fixed guard event names for tr…
…ansitions (destillat, lyrixx)

This PR was merged into the 3.4 branch.

Discussion
----------

[Workflow][FrameworkBundle] fixed guard event names for transitions

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #28018 symfony/symfony#28007 (comment)
| License       | MIT
| Doc PR        |

There is a bug when many transitions are defined with the same name.
I finished destillat's work and rebase against 3.4 as it's a bug fix.

There another point of failure, but it could not be fixed on 3.4. I will
be a need feature. The issue is related to `Workflow::can($subject, $transitionName)`.
Since the transitionName could be not unique, we will need to support
passing an instance of Transition. A new PR is incomming

Commits
-------

83dc473dd6 [FrameworkBundle] fixed guard event names for transitions
fb88bfc79a [FrameworkBundle] fixed guard event names for transitions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.