Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
[Workflow] Fix code blocks using an enum#21499
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
| ->name('to_review') | ||
| ->from(BlogPostStatus::Draft) | ||
| ->to([BlogPostStatus::Reviewed]); | ||
| ->from([BlogPostStatus::Draft->value]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
is this really needed ? The Yaml code snippet uses the enum case, not the value.
MatTheCatOct 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The PHP config would support an enum, but not in an array. Since arrays are used everywhere else I preferred to keep consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This fails with
The following keys are not supported by "Symfony\Config\Framework\Workflows\WorkflowConfig\TransitionConfig\FromConfig": 0 in .../config/packages/workflow.php.
My advice: the array-shape format is way more flexible: it allows to diverge from the shape and still have normalizers make it work. (Which means we can also improve the shape-generator independently)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We could also take this as an opportunity to improve the config generators, so that eg enums are accepted in theplace() method. Lists accepted from->from(). But I don't know if we'd have enough info in the config tree to know about this.
The array-shape format looks the best way to overcome this wall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
all the block works this way
$blogPublishing->transition() ->name('to_review') ->from(BlogPostStatus::Draft) ->to(BlogPostStatus::Reviewed);$blogPublishing->transition() ->name('publish') ->from(BlogPostStatus::Reviewed) ->to(BlogPostStatus::Published);$blogPublishing->transition() ->name('reject') ->from(BlogPostStatus::Reviewed) ->to(BlogPostStatus::Rejected);
nicolas-grekasOct 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Related:symfony/symfony#62082, to allow enums in->from(['place' => Enum::Case])
| ->property('status'); | ||
| $blogPublishing->places(BlogPostStatus::cases()); | ||
| $blogPublishing->place(BlogPostStatus::Draft->value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this isnot the equivalent of the Yaml code snippet, which references the enum itself instead of registering each case separately.
The equivalent of the Yaml snippet would beBlogPostStatus::class
MatTheCatOct 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
AFAIU there is no equivalent to the YAML syntax using PHP.
PassingBlogPostStatus::class toplace would create a place named as the class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@MatTheCat is right, there's no equivalent using the fluent PHP-DSL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ah, wait, this works:$blogPublishing->place(BlogPostStatus::class.'::*');
| <!-- or type="state_machine"--> | ||
| <framework:workflowname="blog_publishing"type="workflow"places="App\Enumeration\BlogPostStatus::*"> | ||
| <framework:marking-storetype="single_state"> | ||
| <framework:initial-marking>draft</framework:initial-marking> |
MatTheCatOct 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not sure whyinitial-marking is ina sequence; I moved it so that the XML is valid.
MatTheCat commentedOct 17, 2025
Closing this because I don’t know what is supposed to work and how. |
nicolas-grekas commentedOct 17, 2025
@MatTheCat I totally get it, that's a mess. And this PR pushed me to submitsymfony/symfony#62092 ! |
Follow-up of#21386 (comment)