-
-
Notifications
You must be signed in to change notification settings - Fork 5.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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 7.4
Are you sure you want to change the base?
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.
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 the place()
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);
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 is not 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 be BlogPostStatus::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.
AFAIU there is no equivalent to the YAML syntax using PHP.
Passing BlogPostStatus::class
to place
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:workflow name="blog_publishing" type="workflow" places="App\Enumeration\BlogPostStatus::*"> | ||
<framework:marking-store type="single_state"> | ||
<framework:initial-marking>draft</framework:initial-marking> |
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 why initial-marking
is in a sequence; I moved it so that the XML is valid.
Follow-up of #21386 (comment)