[FrameworkBundle][Workflow] Fix deprectation when checking workflow.registry service in dump command #24620
Conversation
|
Well, forcing to keep the service as public just for the BC layer is bad, as it means that any people using the service in a public way would not be warned about that (and so we cannot make the service private at all in 4.0). A better solution would be to improve the way the command gets disabled in 3.4. |
| @@ -27,5 +27,9 @@ | |||
| <service id="Symfony\Component\Workflow\Registry" alias="workflow.registry" /> | |||
|
|
|||
| <service id="workflow.security.expression_language" class="Symfony\Component\Workflow\EventListener\ExpressionLanguage" /> | |||
|
|
|||
| <service id="Symfony\Bundle\FrameworkBundle\Command\WorkflowDumpCommand"> | |||
There was a problem hiding this comment.
moving this requires $container->removeDefinition(WorkflowDumpCommand::class); call to be updated, as console.xml is also conditionally loaded.
There was a problem hiding this comment.
well it still needs to be removed if console is not available ;-) not sure about the current effect of this service without console available. I guess it doesnt hurt.
There was a problem hiding this comment.
Argh! I think we can keep command definition in console.xml and remove it if workflow is disabled.
|
Thanks for your feedback @stof. So, I moved command in the conditionnally loaded |
| @@ -624,7 +624,7 @@ private function registerProfilerConfiguration(array $config, ContainerBuilder $ | |||
| private function registerWorkflowConfiguration(array $config, ContainerBuilder $container, XmlFileLoader $loader) | |||
| { | |||
| if (!$config['enabled']) { | |||
| if (!class_exists(Workflow\Workflow::class)) { | |||
| if ($container->hasDefinition(WorkflowDumpCommand::class)) { | |||
There was a problem hiding this comment.
the "if" can be removed: removeDefinition already does it internally
There was a problem hiding this comment.
Correct. Thks @nicolas-grekas !
|
Thank you @Jean-Beru. |
… workflow.registry service in dump command (Jean-Beru) This PR was squashed before being merged into the 3.4 branch (closes #24620). Discussion ---------- [FrameworkBundle][Workflow] Fix deprectation when checking workflow.registry service in dump command | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget to update UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | This PR will declare `workflow.registry` as a public service to avoid deprecation when `WorkflowDumpCommand` checks its existence. It only concerns 3.4 since this `isEnabled` method will be removed in 4.0. Commits ------- 9e75847 [FrameworkBundle][Workflow] Fix deprectation when checking workflow.registry service in dump command
This PR will declare
workflow.registryas a public service to avoid deprecation whenWorkflowDumpCommandchecks its existence. It only concerns 3.4 since thisisEnabledmethod will be removed in 4.0.