Skip to content

Commit c041d48

Browse files
fabpotfabpot
committed
feature #26092 [Workflow] Add a MetadataStore to fetch some metadata (lyrixx)
This PR was merged into the 4.1-dev branch. Discussion ---------- [Workflow] Add a MetadataStore to fetch some metadata | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | yes (little) | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #23257 | License | MIT | Doc PR | TODO --- This is an attempt to fix #23257. I first started to implement `Ẁorkflow::getMetadata()`, `Transition::getMetadata()` and `Place::getMetadata()`. **BUT**, there are no `Place` class. For now it's just a `string`. So dealing with BC is a nightmare. So I tried to find another way to fix the issue. [This comment](symfony/symfony#23257 (comment)) summary well the two options. But this PR is (will be) a mix of theses 2 options. First it will be possible to configure the workflow/metadata like this: ```yaml blog_publishing: supports: - AppBundle\Entity\BlogPost metada: label: Blog publishing description: Manages blog publishing places: draft: metadata: description: Blog has just been created color: grey review: metadata: description: Blog is waiting for review color: blue transitions: to_review: from: draft to: review metadata: label: Submit for review route: admin.blog.review ``` I think is very good for the DX. Simple to understand. All metadata will live in a `MetadataStoreInterface`. If metadata are set via the configuration (workflows.yaml), then we will use the `InMemoryMetadataStore`. Having a MetadataStoreInterface allow user to get dynamic value for a place / transitions. It's really flexible. (But is it a valid use case ?) Then, to retrieve these data, the end user will have to write this code: ```php public function onReview(Event $event) { $metadataStore = $event->getWorkflow()->getMetadataStore(); foreach ($event->getTransition()->getTos() as $place) { $this->flashbag->add('info', $metadataStore->getPlaceMetadata($place)->get('description')); } } ``` Note: I might add some shortcut to the Event class or in twig: ```jinja {% for transition in workflow_transitions(post) %} <a href="{{ workflow_metadata_transition(post, route) }}"> {{ workflow_metadata_transition(post, transition) }} </a> {% endfor %} ``` --- WDYT ? Should I continue this way, or should I introduce a `Place` class (there will be so many deprecation ...) Commits ------- bd1f2c8583 [Workflow] Add a MetadataStore
2 parents e889dac + 8e3a5ab commit c041d48

12 files changed

+214
-58
lines changed

‎DependencyInjection/Configuration.php

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
* FrameworkExtension configuration structure.
3232
*
3333
* @author Jeremy Mikola <jmikola@gmail.com>
34+
* @author Grégoire Pineau <lyrixx@lyrixx.info>
3435
*/
3536
class Configuration implements ConfigurationInterface
3637
{
@@ -292,23 +293,61 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
292293
->defaultNull()
293294
->end()
294295
->arrayNode('places')
296+
->beforeNormalization()
297+
->always()
298+
->then(function ($places) {
299+
// It's an indexed array of shape ['place1', 'place2']
300+
if (isset($places[0]) && is_string($places[0])) {
301+
return array_map(function (string $place) {
302+
return array('name' => $place);
303+
}, $places);
304+
}
305+
306+
// It's an indexed array, we let the validation occur
307+
if (isset($places[0]) && is_array($places[0])) {
308+
return $places;
309+
}
310+
311+
foreach ($places as $name => $place) {
312+
if (is_array($place) && array_key_exists('name', $place)) {
313+
continue;
314+
}
315+
$place['name'] = $name;
316+
$places[$name] = $place;
317+
}
318+
319+
return array_values($places);
320+
})
321+
->end()
295322
->isRequired()
296323
->requiresAtLeastOneElement()
297-
->prototype('scalar')
298-
->cannotBeEmpty()
324+
->prototype('array')
325+
->children()
326+
->scalarNode('name')
327+
->isRequired()
328+
->cannotBeEmpty()
329+
->end()
330+
->arrayNode('metadata')
331+
->normalizeKeys(false)
332+
->defaultValue(array())
333+
->example(array('color' => 'blue', 'description' => 'Workflow to manage article.'))
334+
->prototype('variable')
335+
->end()
336+
->end()
337+
->end()
299338
->end()
300339
->end()
301340
->arrayNode('transitions')
302341
->beforeNormalization()
303342
->always()
304343
->then(function ($transitions) {
305-
// It's an indexed array, we let the validation occurs
306-
if (isset($transitions[0])) {
344+
// It's an indexed array, we let the validation occur
345+
if (isset($transitions[0]) && is_array($transitions[0])) {
307346
return $transitions;
308347
}
309348

310349
foreach ($transitions as $name => $transition) {
311-
if (array_key_exists('name', $transition)) {
350+
if (is_array($transition) && array_key_exists('name', $transition)) {
312351
continue;
313352
}
314353
$transition['name'] = $name;
@@ -351,9 +390,23 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
351390
->cannotBeEmpty()
352391
->end()
353392
->end()
393+
->arrayNode('metadata')
394+
->normalizeKeys(false)
395+
->defaultValue(array())
396+
->example(array('color' => 'blue', 'description' => 'Workflow to manage article.'))
397+
->prototype('variable')
398+
->end()
399+
->end()
354400
->end()
355401
->end()
356402
->end()
403+
->arrayNode('metadata')
404+
->normalizeKeys(false)
405+
->defaultValue(array())
406+
->example(array('color' => 'blue', 'description' => 'Workflow to manage article.'))
407+
->prototype('variable')
408+
->end()
409+
->end()
357410
->end()
358411
->validate()
359412
->ifTrue(function ($v) {

‎DependencyInjection/FrameworkExtension.php

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -466,32 +466,68 @@ private function registerWorkflowConfiguration(array $config, ContainerBuilder $
466466
foreach ($config['workflows'] as $name => $workflow) {
467467
$type = $workflow['type'];
468468

469+
// Process Metadata (workflow + places (transition is done in the "create transition" block))
470+
$metadataStoreDefinition = new Definition(Workflow\Metadata\InMemoryMetadataStore::class, array(null, null, null));
471+
if ($workflow['metadata']) {
472+
$metadataStoreDefinition->replaceArgument(0, $workflow['metadata']);
473+
}
474+
$placesMetadata = array();
475+
foreach ($workflow['places'] as $place) {
476+
if ($place['metadata']) {
477+
$placesMetadata[$place['name']] = $place['metadata'];
478+
}
479+
}
480+
if ($placesMetadata) {
481+
$metadataStoreDefinition->replaceArgument(1, $placesMetadata);
482+
}
483+
484+
// Create transitions
469485
$transitions = array();
486+
$transitionsMetadataDefinition = new Definition(\SplObjectStorage::class);
470487
foreach ($workflow['transitions'] as $transition) {
471488
if ('workflow' === $type) {
472-
$transitions[] = new Definition(Workflow\Transition::class, array($transition['name'], $transition['from'], $transition['to']));
489+
$transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $transition['from'], $transition['to']));
490+
$transitions[] = $transitionDefinition;
491+
if ($transition['metadata']) {
492+
$transitionsMetadataDefinition->addMethodCall('attach', array(
493+
$transitionDefinition,
494+
$transition['metadata'],
495+
));
496+
}
473497
} elseif ('state_machine' === $type) {
474498
foreach ($transition['from'] as $from) {
475499
foreach ($transition['to'] as $to) {
476-
$transitions[] = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to));
500+
$transitionDefinition = new Definition(Workflow\Transition::class, array($transition['name'], $from, $to));
501+
$transitions[] = $transitionDefinition;
502+
if ($transition['metadata']) {
503+
$transitionsMetadataDefinition->addMethodCall('attach', array(
504+
$transitionDefinition,
505+
$transition['metadata'],
506+
));
507+
}
477508
}
478509
}
479510
}
480511
}
512+
$metadataStoreDefinition->replaceArgument(2, $transitionsMetadataDefinition);
513+
514+
// Create places
515+
$places = array_map(function (array $place) {
516+
return $place['name'];
517+
}, $workflow['places']);
481518

482519
// Create a Definition
483520
$definitionDefinition = new Definition(Workflow\Definition::class);
484521
$definitionDefinition->setPublic(false);
485-
$definitionDefinition->addArgument($workflow['places']);
522+
$definitionDefinition->addArgument($places);
486523
$definitionDefinition->addArgument($transitions);
524+
$definitionDefinition->addArgument($workflow['initial_place'] ?? null);
525+
$definitionDefinition->addArgument($metadataStoreDefinition);
487526
$definitionDefinition->addTag('workflow.definition', array(
488527
'name' => $name,
489528
'type' => $type,
490529
'marking_store' => isset($workflow['marking_store']['type']) ? $workflow['marking_store']['type'] : null,
491530
));
492-
if (isset($workflow['initial_place'])) {
493-
$definitionDefinition->addArgument($workflow['initial_place']);
494-
}
495531

496532
// Create MarkingStore
497533
if (isset($workflow['marking_store']['type'])) {

‎Resources/config/schema/symfony-1.0.xsd

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,9 @@
273273
<xsd:sequence>
274274
<xsd:element name="marking-store" type="marking_store" minOccurs="0" maxOccurs="1" />
275275
<xsd:element name="support" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
276-
<xsd:element name="place" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
276+
<xsd:element name="place" type="place" minOccurs="0" maxOccurs="unbounded" />
277277
<xsd:element name="transition" type="transition" minOccurs="0" maxOccurs="unbounded" />
278+
<xsd:element name="metadata" type="metadata" minOccurs="0" maxOccurs="unbounded" />
278279
</xsd:sequence>
279280
<xsd:attribute name="name" type="xsd:string" />
280281
<xsd:attribute name="type" type="workflow_type" />
@@ -302,10 +303,24 @@
302303
<xsd:sequence>
303304
<xsd:element name="from" type="xsd:string" minOccurs="1" maxOccurs="unbounded" />
304305
<xsd:element name="to" type="xsd:string" minOccurs="1" maxOccurs="unbounded" />
306+
<xsd:element name="metadata" type="metadata" minOccurs="0" maxOccurs="unbounded" />
305307
</xsd:sequence>
306308
<xsd:attribute name="name" type="xsd:string" use="required" />
307309
</xsd:complexType>
308310

311+
<xsd:complexType name="place" mixed="true">
312+
<xsd:sequence>
313+
<xsd:element name="metadata" type="metadata" minOccurs="0" maxOccurs="unbounded" />
314+
</xsd:sequence>
315+
<xsd:attribute name="name" type="xsd:string" />
316+
</xsd:complexType>
317+
318+
<xsd:complexType name="metadata">
319+
<xsd:sequence>
320+
<xsd:any minOccurs="0" processContents="lax"/>
321+
</xsd:sequence>
322+
</xsd:complexType>
323+
309324
<xsd:simpleType name="workflow_type">
310325
<xsd:restriction base="xsd:string">
311326
<xsd:enumeration value="state_machine" />

‎Tests/DependencyInjection/Fixtures/php/workflows.php

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,29 @@
4848
FrameworkExtensionTest::class,
4949
),
5050
'initial_place' => 'start',
51+
'metadata' => array(
52+
'title' => 'workflow title',
53+
),
5154
'places' => array(
52-
'start',
53-
'coding',
54-
'travis',
55-
'review',
56-
'merged',
57-
'closed',
55+
'start_name_not_used' => array(
56+
'name' => 'start',
57+
'metadata' => array(
58+
'title' => 'place start title',
59+
),
60+
),
61+
'coding' => null,
62+
'travis' => null,
63+
'review' => null,
64+
'merged' => null,
65+
'closed' => null,
5866
),
5967
'transitions' => array(
6068
'submit' => array(
6169
'from' => 'start',
6270
'to' => 'travis',
71+
'metadata' => array(
72+
'title' => 'transition submit title',
73+
),
6374
),
6475
'update' => array(
6576
'from' => array('coding', 'travis', 'review'),
@@ -96,8 +107,8 @@
96107
FrameworkExtensionTest::class,
97108
),
98109
'places' => array(
99-
'first',
100-
'last',
110+
array('name' => 'first'),
111+
array('name' => 'last'),
101112
),
102113
'transitions' => array(
103114
'go' => array(

‎Tests/DependencyInjection/Fixtures/xml/workflow_with_arguments_and_service.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
<framework:argument>a</framework:argument>
1414
</framework:marking-store>
1515
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
16-
<framework:place>first</framework:place>
17-
<framework:place>last</framework:place>
16+
<framework:place name="first" />
17+
<framework:place name="last" />
1818
<framework:transition name="foobar">
1919
<framework:from>a</framework:from>
2020
<framework:to>a</framework:to>

‎Tests/DependencyInjection/Fixtures/xml/workflow_with_multiple_transitions_with_same_name.xml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@
1313
<framework:argument>a</framework:argument>
1414
</framework:marking-store>
1515
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
16-
<framework:place>draft</framework:place>
17-
<framework:place>wait_for_journalist</framework:place>
18-
<framework:place>approved_by_journalist</framework:place>
19-
<framework:place>wait_for_spellchecker</framework:place>
20-
<framework:place>approved_by_spellchecker</framework:place>
21-
<framework:place>published</framework:place>
16+
<framework:place name="draft" />
17+
<framework:place name="wait_for_journalist" />
18+
<framework:place name="approved_by_journalist" />
19+
<framework:place name="wait_for_spellchecker" />
20+
<framework:place name="approved_by_spellchecker" />
21+
<framework:place name="published" />
2222
<framework:transition name="request_review">
2323
<framework:from>draft</framework:from>
2424
<framework:to>wait_for_journalist</framework:to>

‎Tests/DependencyInjection/Fixtures/xml/workflow_with_support_and_support_strategy.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
<framework:workflow name="my_workflow" support-strategy="foobar">
1111
<framework:marking-store type="multiple_state"/>
1212
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
13-
<framework:place>first</framework:place>
14-
<framework:place>last</framework:place>
13+
<framework:place name="first" />
14+
<framework:place name="last" />
1515
<framework:transition name="foobar">
1616
<framework:from>a</framework:from>
1717
<framework:to>a</framework:to>

‎Tests/DependencyInjection/Fixtures/xml/workflow_with_type_and_service.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
<framework:workflow name="my_workflow">
1111
<framework:marking-store type="multiple_state" service="workflow_service" />
1212
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
13-
<framework:place>first</framework:place>
14-
<framework:place>last</framework:place>
13+
<framework:place name="first" />
14+
<framework:place name="last" />
1515
<framework:transition name="foobar">
1616
<framework:from>a</framework:from>
1717
<framework:to>a</framework:to>

‎Tests/DependencyInjection/Fixtures/xml/workflow_without_support_and_support_strategy.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
<framework:config>
1010
<framework:workflow name="my_workflow">
1111
<framework:marking-store type="multiple_state"/>
12-
<framework:place>first</framework:place>
13-
<framework:place>last</framework:place>
12+
<framework:place name="first" />
13+
<framework:place name="last" />
1414
<framework:transition name="foobar">
1515
<framework:from>a</framework:from>
1616
<framework:to>a</framework:to>

0 commit comments

Comments
 (0)