Skip to content

Commit

Permalink
bug #20760 [FrameworkBundle] [Workflow] Fix service marking store con…
Browse files Browse the repository at this point in the history
…figuration (fduch)

This PR was submitted for the master branch but it was merged into the 3.2 branch instead (closes #20760).

Discussion
----------

[FrameworkBundle] [Workflow] Fix service marking store configuration

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

Currently workflow marking store configuration [checks](https://github.com/symfony/symfony/blob/3.2/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php#L271-L272) that `service` and `arguments` fields are not specified simultaneously using `isset` function.

Since [arguments node](https://github.com/symfony/symfony/blob/3.2/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php#L253-L261) defines prototype node inside, even if `arguments` node is not specified, -  after processing it looks like empty array. If `service` setting is set it leads to failing build with validation error message (due to `isset([])` returns `true`):
`"arguments" and "service" cannot be used together.`.

This patch addresses this issue.

Commits
-------

3289b10 [FrameworkBundle] [Workflow] Fix service marking store configuration
  • Loading branch information
nicolas-grekas committed Dec 8, 2016
2 parents a28c522 + 3289b10 commit af0043b
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 1 deletion.
Expand Up @@ -268,7 +268,7 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
->thenInvalid('"type" and "service" cannot be used together.')
->end()
->validate()
->ifTrue(function ($v) { return isset($v['arguments']) && isset($v['service']); })
->ifTrue(function ($v) { return !empty($v['arguments']) && isset($v['service']); })
->thenInvalid('"arguments" and "service" cannot be used together.')
->end()
->end()
Expand Down
Expand Up @@ -88,5 +88,23 @@
),
),
),
'service_marking_store_workflow' => array(
'marking_store' => array(
'service' => 'workflow_service',
),
'supports' => array(
FrameworkExtensionTest::class,
),
'places' => array(
'first',
'last',
),
'transitions' => array(
'go' => array(
'from' => 'first',
'to' => 'last',
),
),
),
),
));
Expand Up @@ -79,5 +79,16 @@
<framework:to>review</framework:to>
</framework:transition>
</framework:workflow>

<framework:workflow name="service_marking_store_workflow">
<framework:marking-store service="workflow_service"/>
<framework:support>Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest</framework:support>
<framework:place>first</framework:place>
<framework:place>last</framework:place>
<framework:transition name="go">
<framework:from>first</framework:from>
<framework:to>last</framework:to>
</framework:transition>
</framework:workflow>
</framework:config>
</container>
Expand Up @@ -63,3 +63,17 @@ framework:
reopen:
from: closed
to: review
service_marking_store_workflow:
marking_store:
service: workflow_service
supports:
- Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\FrameworkExtensionTest
places:
- first
- last
transitions:
go:
from:
- first
to:
- last
Expand Up @@ -165,6 +165,12 @@ public function testWorkflows()
$this->assertSame(array('workflow.definition' => array(array('name' => 'pull_request', 'type' => 'state_machine', 'marking_store' => 'single_state'))), $stateMachineDefinition->getTags());
$this->assertCount(9, $stateMachineDefinition->getArgument(1));
$this->assertSame('start', $stateMachineDefinition->getArgument(2));

$serviceMarkingStoreWorkflowDefinition = $container->getDefinition('workflow.service_marking_store_workflow');
/** @var Reference $markingStoreRef */
$markingStoreRef = $serviceMarkingStoreWorkflowDefinition->getArgument(1);
$this->assertInstanceOf(Reference::class, $markingStoreRef);
$this->assertEquals('workflow_service', (string) $markingStoreRef);
}

/**
Expand Down

0 comments on commit af0043b

Please sign in to comment.