Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Validator] Choices constraint improvement#29658
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
HeahDude commentedDec 20, 2018 • 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.
This can be closed as this feature is supported by the annotation reader already, just remove the quotes :). /** * @var string * * @Assert\Choice(choices=App\Constants::AVAILABLE_STATES) */private$state; |
yceruto commentedDec 20, 2018
@HeahDude Do you meant |
HeahDude commentedDec 20, 2018
yes x). |
| }elseif (!\in_array($value,$choices,true)) { | ||
| $this->context->buildViolation($constraint->message) | ||
| ->setParameter('{{ value }}',$this->formatValue($value)) | ||
| ->setParameter('{{ choices }}',implode(',',$choices)) |
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 this line is still a nice addition indeed!
nikophil commentedDec 21, 2018
yeah indeed, you're right, and i 've learnt something new :) |
HeahDude commentedDec 21, 2018
I'm 👍 for supporting a default choices implode, as long as we don't change the default message for both BC and let it opt-in performance wise. |
nikophil commentedDec 21, 2018
i've updated the PR |
| }elseif (!\in_array($value,$choices,true)) { | ||
| $this->context->buildViolation($constraint->message) | ||
| ->setParameter('{{ value }}',$this->formatValue($value)) | ||
| ->setParameter('{{ choices }}',implode(',',$choices)) |
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.
should also be added to the multiple=true case
choices can be any value, i think we need to map all choices usingformatValue() also.
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.
yep, you're totally right - i've added to the multiple=true case and formatted the value
xabbuh commentedDec 27, 2018
@nikophil Can you please update the PR title and description to match the recent changes (they will be part of the git history)? |
nikophil commentedDec 27, 2018
done 👍 |
fabpot commentedJan 2, 2019
Thank you@nikophil. |
This PR was merged into the 4.3-dev branch.Discussion----------[Validator] Choices constraint improvement| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| License | MITHi,here is a little improvement for the choice constraint: expose a new `choices` wildcard for the messages, in order to provide a way to display easily the valid choices to the user.thanks.Commits-------71dfa35 add new `choices` wildcard in message
…int (nikophil)This PR was submitted for the 3.4 branch but it was merged into the master branch instead (closes#10836).Discussion----------document "choices" wildcard in Choice validation constraintHello,here is a little doc update in order to document the `choices` wildcard introduced bysymfony/symfony#29658Commits-------7764519 document "choices" wildcard in Choice validation constraint
…ation (nikophil)This PR was squashed before being merged into the 3.4 branch (closes#10810).Discussion----------[Validator] Explained how to use array constant in annotationHi,few days ago i tried to add a new feature to the `choice` validator constraint, by adding a new `choicesFromConstant` option.symfony/symfony#29658But then, i realized this was already supported.I think this should be documented, as i think a lot of people don't know this feature.thanks.Commits-------7774aab [Validator] Explained how to use array constant in annotation
It was added in this PR:symfony/symfony#29658
It was added in this PR:symfony/symfony#29658
…s (przemyslaw-bogusz)This PR was submitted for the 5.0 branch but it was merged into the 4.4 branch instead (closes#13081).Discussion----------[Constraints][Choice] Added choices to message parametersIt was added in this PR:symfony/symfony#29658<!--If your pull request fixes a BUG, use the oldest maintained branch that containsthe bug (seehttps://symfony.com/roadmap for the list of maintained branches).If your pull request documents a NEW FEATURE, use the same Symfony branch wherethe feature was introduced (and `master` for features of unreleased versions).-->Commits-------3ba67f7 Added choices to message parameters
Uh oh!
There was an error while loading.Please reload this page.
Hi,
here is a little improvement for the choice constraint: expose a new
choiceswildcard for the messages, in order to provide a way to display easily the valid choices to the user.thanks.