Skip to content

[WIP] [Workflow] Choose which Workflow events should be dispatched #36633

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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
Closed

[WIP] [Workflow] Choose which Workflow events should be dispatched #36633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
388069d
[Workflow] Choose which Workflow events should be dispatched
stewartmalik Apr 30, 2020
808e789
[Workflow] Removed previous change regarding not firing the `announce…
stewartmalik Apr 30, 2020
dc38230
[Workflow] Add feedback from Code Review. Add configuration support.
stewartmalik May 2, 2020
661bfb9
Update `Workflow` to `transition`
stewartmalik May 2, 2020
b93e33a
[Workflow] Style fixes suggested by CI
stewartmalik May 2, 2020
dfe0969
[Workflow] Fix merge conflict
stewartmalik May 2, 2020
6dde6db
[Workflow] Another code style fix after the first one broke on patch …
stewartmalik May 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
->fixXmlConfig('support')
->fixXmlConfig('place')
->fixXmlConfig('transition')
->fixXmlConfig('dispatched_event')
->children()
->arrayNode('audit_trail')
->canBeEnabled()
Expand Down Expand Up @@ -324,6 +325,22 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
->defaultValue([])
->prototype('scalar')->end()
->end()
->arrayNode('dispatched_events')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could definitely use input on configuration here, this is my first time working on the Symfony code base and this was somewhat confusing.

I ran into issues with a number of things such as expressing an empty array in the XML configuration format, as well as having a default null value for arrayNodes.

I have worked around this by specifying all and none as allowed values for this configuration option however these are mostly hidden from the end user. The only exception is with the XML configuration where you need to specify none to remove all events from being fired.

->beforeNormalization()
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should handle many way to configure the same thing.
to me, the perfect config would be:

  1. no configuration => all
  2. an empty array => non
  3. an array with few items => only theses items

So I would remove the case for string. And it would allow to remove special cases for all and none.

->ifString()
->then(function ($v) {
return [$v];
})
->end()
// We have to specify a default here as when this config option
// isn't set the default behaviour of `arrayNode()` is to return an empty
// array which conflicts with our Definition, and we cannot set a default
// of `null` for arrayNode()'s
->defaultValue(['all'])
->prototype('scalar')->end()
->info('Select which Transition events should be dispatched for this Workflow')
->example(['leave', 'completed'])
->end()
->arrayNode('places')
->beforeNormalization()
->always()
Expand Down
12 changes: 12 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,13 +713,25 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
$places = array_column($workflow['places'], 'name');
$initialMarking = $workflow['initial_marking'] ?? [];

// Record which events should be dispatched
if ($workflow['dispatched_events'] === ['all']) {
$dispatchedEvents = null;
} elseif ($workflow['dispatched_events'] === ['none']) {
$dispatchedEvents = [];
} else {
$dispatchedEvents = array_map(function (string $event) {
return 'workflow.'.$event;
}, $workflow['dispatched_events']);
}

// Create a Definition
$definitionDefinition = new Definition(Workflow\Definition::class);
$definitionDefinition->setPublic(false);
$definitionDefinition->addArgument($places);
$definitionDefinition->addArgument($transitions);
$definitionDefinition->addArgument($initialMarking);
$definitionDefinition->addArgument($metadataStoreDefinition);
$definitionDefinition->addArgument($dispatchedEvents);
$definitionDefinition->addTag('workflow.definition', [
'name' => $name,
'type' => $type,
Expand Down
14 changes: 14 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@
<xsd:element name="initial-marking" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
<xsd:element name="marking-store" type="marking_store" minOccurs="0" maxOccurs="1" />
<xsd:element name="support" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
<xsd:element name="dispatched-event" type="dispatched_event" minOccurs="0" maxOccurs="unbounded" />
<xsd:element name="place" type="place" minOccurs="0" maxOccurs="unbounded" />
<xsd:element name="transition" type="transition" minOccurs="0" maxOccurs="unbounded" />
<xsd:element name="metadata" type="metadata" minOccurs="0" maxOccurs="unbounded" />
Expand Down Expand Up @@ -340,6 +341,19 @@
</xsd:sequence>
</xsd:complexType>

<xsd:simpleType name="dispatched_event">
<xsd:restriction base="xsd:string">
<xsd:enumeration value="all" />
<xsd:enumeration value="none" />
<xsd:enumeration value="leave" />
<xsd:enumeration value="transition" />
<xsd:enumeration value="enter" />
<xsd:enumeration value="entered" />
<xsd:enumeration value="completed" />
<xsd:enumeration value="announce" />
</xsd:restriction>
</xsd:simpleType>

<xsd:simpleType name="default_middleware">
<xsd:restriction base="xsd:string">
<xsd:enumeration value="true" />
Expand Down
41 changes: 41 additions & 0 deletions ...workBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_all_dispatched_events.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

use Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest;

$container->loadFromExtension('framework', [
'workflows' => [
'my_workflow' => [
'type' => 'state_machine',
'marking_store' => [
'type' => 'method',
'property' => 'state'
],
'supports' => [
FrameworkExtensionTest::class,
],
'places' => [
'one',
'two',
'three',
],
'transitions' => [
'count_to_two' => [
'from' => [
'one',
],
'to' => [
'two',
],
],
'count_to_three' => [
'from' => [
'two',
],
'to' => [
'three'
]
]
],
],
],
]);
42 changes: 42 additions & 0 deletions ...eworkBundle/Tests/DependencyInjection/Fixtures/php/workflow_with_no_dispatched_events.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

use Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest;

$container->loadFromExtension('framework', [
'workflows' => [
'my_workflow' => [
'type' => 'state_machine',
'marking_store' => [
'type' => 'method',
'property' => 'state'
],
'supports' => [
FrameworkExtensionTest::class,
],
'dispatched_events' => [],
'places' => [
'one',
'two',
'three',
],
'transitions' => [
'count_to_two' => [
'from' => [
'one',
],
'to' => [
'two',
],
],
'count_to_three' => [
'from' => [
'two',
],
'to' => [
'three'
]
]
],
],
],
]);
45 changes: 45 additions & 0 deletions ...ndle/Tests/DependencyInjection/Fixtures/php/workflow_with_specified_dispatched_events.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

use Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest;

$container->loadFromExtension('framework', [
'workflows' => [
'my_workflow' => [
'type' => 'state_machine',
'marking_store' => [
'type' => 'method',
'property' => 'state'
],
'supports' => [
FrameworkExtensionTest::class,
],
'dispatched_events' => [
'leave',
'completed'
],
'places' => [
'one',
'two',
'three',
],
'transitions' => [
'count_to_two' => [
'from' => [
'one',
],
'to' => [
'two',
],
],
'count_to_three' => [
'from' => [
'two',
],
'to' => [
'three'
]
]
],
],
],
]);
27 changes: 27 additions & 0 deletions ...workBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_all_dispatched_events.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:framework="http://symfony.com/schema/dic/symfony"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">

<framework:config>
<framework:workflow name="my_workflow" type="state_machine">
<framework:initial-marking>one</framework:initial-marking>
<framework:marking-store type="method" property="state" />
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
<framework:place name="one" />
<framework:place name="two" />
<framework:place name="three" />
<framework:transition name="count_to_two">
<framework:from>one</framework:from>
<framework:to>two</framework:to>
</framework:transition>
<framework:transition name="count_to_three">
<framework:from>two</framework:from>
<framework:to>three</framework:to>
</framework:transition>
</framework:workflow>
</framework:config>
</container>
28 changes: 28 additions & 0 deletions ...eworkBundle/Tests/DependencyInjection/Fixtures/xml/workflow_with_no_dispatched_events.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:framework="http://symfony.com/schema/dic/symfony"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">

<framework:config>
<framework:workflow name="my_workflow" type="state_machine">
<framework:initial-marking>one</framework:initial-marking>
<framework:marking-store type="method" property="state" />
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
<framework:dispatched-event>none</framework:dispatched-event>
<framework:place name="one" />
<framework:place name="two" />
<framework:place name="three" />
<framework:transition name="count_to_two">
<framework:from>one</framework:from>
<framework:to>two</framework:to>
</framework:transition>
<framework:transition name="count_to_three">
<framework:from>two</framework:from>
<framework:to>three</framework:to>
</framework:transition>
</framework:workflow>
</framework:config>
</container>
29 changes: 29 additions & 0 deletions ...ndle/Tests/DependencyInjection/Fixtures/xml/workflow_with_specified_dispatched_events.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:framework="http://symfony.com/schema/dic/symfony"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">

<framework:config>
<framework:workflow name="my_workflow" type="state_machine">
<framework:initial-marking>one</framework:initial-marking>
<framework:marking-store type="method" property="state" />
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
<framework:dispatched-event>leave</framework:dispatched-event>
<framework:dispatched-event>completed</framework:dispatched-event>
<framework:place name="one" />
<framework:place name="two" />
<framework:place name="three" />
<framework:transition name="count_to_two">
<framework:from>one</framework:from>
<framework:to>two</framework:to>
</framework:transition>
<framework:transition name="count_to_three">
<framework:from>two</framework:from>
<framework:to>three</framework:to>
</framework:transition>
</framework:workflow>
</framework:config>
</container>
21 changes: 21 additions & 0 deletions ...workBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_all_dispatched_events.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
framework:
workflows:
my_workflow:
type: state_machine
initial_marking: one
marking_store:
type: method
property: state
supports:
- Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest
places:
- one
- two
- three
transitions:
count_to_two:
from: one
to: two
count_to_three:
from: two
to: three
22 changes: 22 additions & 0 deletions ...eworkBundle/Tests/DependencyInjection/Fixtures/yml/workflow_with_no_dispatched_events.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
framework:
workflows:
my_workflow:
type: state_machine
initial_marking: one
dispatched_events: []
marking_store:
type: method
property: state
supports:
- Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest
places:
- one
- two
- three
transitions:
count_to_two:
from: one
to: two
count_to_three:
from: two
to: three
22 changes: 22 additions & 0 deletions ...ndle/Tests/DependencyInjection/Fixtures/yml/workflow_with_specified_dispatched_events.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
framework:
workflows:
my_workflow:
type: state_machine
initial_marking: one
dispatched_events: ['leave', 'completed']
marking_store:
type: method
property: state
supports:
- Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest
places:
- one
- two
- three
transitions:
count_to_two:
from: one
to: two
count_to_three:
from: two
to: three
Loading