Skip to content

[FrameworkBundle] fixed guard event names for transitions #28007

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

Merged
merged 1 commit into from
Jul 29, 2018

Conversation

destillat
Copy link
Contributor

@destillat destillat 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

@destillat
Copy link
Contributor Author

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

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(for 3.4)

@nicolas-grekas nicolas-grekas changed the base branch from 3.3 to 3.4 July 29, 2018 15:24
@nicolas-grekas
Copy link
Member

Thank you @destillat.

@nicolas-grekas nicolas-grekas merged commit 9bbb1e5 into symfony:3.4 Jul 29, 2018
nicolas-grekas added a commit that referenced this pull request Jul 29, 2018
…(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
…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
…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
…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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants