Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Validator] Add support for enum inChoice constraint#59633
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
8f76b52 to23f36abCompare23f36ab toeca1cd7Compare| @@ -86,7 +86,7 @@ protected function formatValue(mixed $value, int $format = 0): string | |||
| } | |||
| if ($value instanceof \UnitEnum) { | |||
| return $value->name; | |||
| $value = $value instanceof \BackedEnum ? $value->value : $value->name; | |||
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.
I personally don't think this is a good idea.
Enum case names are helpful for developers, scalar equivalent less so.
Moreover, this will have an impact on other constraints, right ?
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.
To be honest, I don't need this enum functionality in choice constraint (just had an idea & started coding), but this line of code should be fixed upstream. In theBackedEnumNormalizer enums are currently (de)normalized from/to->value, here it's->name. Using some{{ value }} parameter in the messages block can confuse some, because input value is not same as displayed.
Btw this should not break anything, may third-party tests depending on this functionality. Haven't seen much enum usages in constraints and tests worked smooth for the other part of code.
PS: Is there any reason why there's noUnitEnumNormalizer, justBackedEnumNormalizer? If it makes sense to implement (for me it does), I'll create a new MR for that :-)
| @@ -56,6 +56,7 @@ CHANGELOG | |||
| ``` | |||
| * Add support for ratio checks for SVG files to the `Image` constraint | |||
| * Add the `Slug` constraint | |||
| * Add support for enums in `Choice` constraint | |||
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.
Enums are already supported
This PR add supports for using a class-string to specify the list of possible values ( integer or string )
Duplicate of#54226. |
Uh oh!
There was an error while loading.Please reload this page.
Feature
Supporting enum
\class-stringforchoicesattribute inChoiceconstraint.Fix
ConstraintValidator->formatValue()returning correct value for enum typeNote
Using
Choiceconstraint for attributes already defined as enum does not make sense (BackedEnumNormalizeralready throws exception ->The data must belong to a backed enumeration of type App\\CustomEnum). It's only possible additionally using enum-classes instead of just arrays & callbacks.Example