[FrameworkBundle][Workflow] Deprecate the default type of a workflow #22416
Conversation
UPGRADE-3.3.md
Outdated
| @@ -168,6 +168,9 @@ FrameworkBundle | |||
|
|
|||
| * The "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter have been deprecated and will be removed in 4.0. Use the Request::setTrustedProxies() method in your front controller instead. | |||
|
|
|||
| * Not defining the "type" option of the "framework.workflows.*" configuration entry is deprecated. | |||
There was a problem hiding this comment.
of ``framework.workflows.*`` configuration entries
UPGRADE-4.0.md
Outdated
| @@ -228,6 +228,8 @@ FrameworkBundle | |||
|
|
|||
| * The "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter have been removed. Use the `Request::setTrustedProxies()` method in your front controller instead. | |||
|
|
|||
| * The default value of the "framework.workflows.[name].type" configuration option is now "state_machine". | |||
There was a problem hiding this comment.
of ``framework.workflows.*.type`` configuration options
619bcae to
88300fb
Compare
| @@ -440,6 +440,10 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde | |||
| $registryDefinition = $container->getDefinition('workflow.registry'); | |||
|
|
|||
| foreach ($workflows as $name => $workflow) { | |||
| if (!array_key_exists('type', $workflow)) { | |||
| $workflow['type'] = 'workflow'; | |||
| @trigger_error(sprintf('The "type" option of the "framework.workflows.%s" configuration entry must be defined since Symfony 3.3. The default value will be "state_machine" in Symfony 4.0.', $name), E_USER_DEPRECATED); | |||
There was a problem hiding this comment.
much better would be to put it in the Configuration class:
- use a
validatecallback (on the workflow node) to trigger a deprecation and set the type when it is not set - switch to
->isRequired()on the type node for Symfony 4 instead.
This would keep the deprecation warning in the same place than the new way of doing things in Symfony 4, making it easier to update the code
There was a problem hiding this comment.
and we should have a legacy test covering the case of the default to ensure it does not break in 3.x
There was a problem hiding this comment.
switch to ->isRequired() on the type node for Symfony 4 instead.
In symfony 4, The default value will be "state_machine". So the isRequired will not be needed, right?
There was a problem hiding this comment.
I tried to use the validate way, but the DX is worst as I'm not able to attache the workflow name in the deprecation message. For recall, the workflow name is the key of the prototype. So I prefer this way.
More over, there are more code and finally the argument "making it easier to update the code" is not really an issue, since I (I guess) will update the code for SF 4.0
Before this patch, the default type is "workflow". Most of the time a "state_machine" is better because it's simpler and it involves less knowledge to be able to use it. So this patch deprecate a missing type in Symfony 3.3. And In Symfony 4.0 the default value will become "state_machine".
88300fb to
004751c
Compare
| @@ -440,6 +440,10 @@ private function registerWorkflowConfiguration(array $workflows, ContainerBuilde | |||
| $registryDefinition = $container->getDefinition('workflow.registry'); | |||
|
|
|||
| foreach ($workflows as $name => $workflow) { | |||
| if (!array_key_exists('type', $workflow)) { | |||
There was a problem hiding this comment.
I don't know. It's a matter of personal style here, isn't ?
|
👍 |
|
Thank you @lyrixx. |
… of a workflow (lyrixx) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle][Workflow] Deprecate the default type of a workflow | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - --- Before this patch, the default type is "workflow". Most of the time a "state_machine" is better because it's simpler and it involves less knowledge to be able to use it. So this patch deprecate a missing type in Symfony 3.3. And In Symfony 4.0 the default value will become "state_machine". Commits ------- 004751c [FrameworkBundle][Workflow] Deprecate the default type of a workflow
Before this patch, the default type is "workflow". Most of the time a
"state_machine" is better because it's simpler and it involves less
knowledge to be able to use it.
So this patch deprecate a missing type in Symfony 3.3. And In Symfony
4.0 the default value will become "state_machine".