Skip to content

[Validator] Override the default option of the choice constraint #39002

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 7, 2020
Merged

[Validator] Override the default option of the choice constraint #39002

merged 1 commit into from
Nov 7, 2020

Conversation

benji07
Copy link
Contributor

@benji07 benji07 commented Nov 5, 2020 •
edited
Loading

Q A
Branch? 5.2
Bug fix? yes
New feature? no
Deprecations? no
Tickets N/A
License MIT
Doc PR TODO

We have a bundle that run test against the last version of symfony and we detect BC Break when passing a string as the first argument of the class Choice

Our code extends the Choice class and change the defaultOption.

I saw that others constraints class had new construct signature (for php8 attributes), but only some of them kept the array options as their first arguments. Why ?

ping @ogizanagi @derrabus

https://travis-ci.com/github/Elao/PhpEnums/jobs/410045368

1) Elao\Enum\Tests\Unit\Bridge\Symfony\Validator\Constraint\EnumTest::testDefaultValueIsEnumClass
Symfony\Component\Validator\Exception\MissingOptionsException: The options "class" must be set for constraint "Elao\Enum\Bridge\Symfony\Validator\Constraint\Enum".
/home/travis/build/Elao/PhpEnums/vendor/symfony/symfony/src/Symfony/Component/Validator/Constraint.php:171
/home/travis/build/Elao/PhpEnums/vendor/symfony/symfony/src/Symfony/Component/Validator/Constraint.php:110
/home/travis/build/Elao/PhpEnums/vendor/symfony/symfony/src/Symfony/Component/Validator/Constraints/Choice.php:75
/home/travis/build/Elao/PhpEnums/src/Bridge/Symfony/Validator/Constraint/Enum.php:39
/home/travis/build/Elao/PhpEnums/tests/Unit/Bridge/Symfony/Validator/Constraint/EnumTest.php:22

@benji07 benji07 changed the title [Validator][Choice] Override the default option [Validator] Override the default option of the choice constraint Nov 5, 2020
@ogizanagi ogizanagi added this to the 5.2 milestone Nov 5, 2020
ogizanagi
ogizanagi reviewed Nov 5, 2020
View reviewed changes
Copy link
Contributor

@ogizanagi ogizanagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(AppVeyor failure is unrelated and fabbot doesn't like PHP 8 attributes yet)

@benji07
Copy link
Contributor Author

benji07 commented Nov 6, 2020

i just saw that we have a special key inside the $options array value, maybe i can use it instead of the getDefaultOption() method (that's the way used in constraints like Count, DateTime, Isbn, ...)

@derrabus
Copy link
Member

derrabus commented Nov 6, 2020

i just saw that we have a special key inside the $options array value, maybe i can use it instead of the getDefaultOption() method (that's the way used in constraints like Count, DateTime, Isbn, ...)

Yes, value should work as well.

@derrabus
Copy link
Member

derrabus commented Nov 6, 2020

Oh, for the two new files: Can you please add the usual file header (This file is part of the Symfony package. etc.) that you can find in other PHP files?

@benji07 @ogizanagi
Allow user to override default options when extending the Choice Cons…
d553750 
…traint

Co-authored-by: Maxime Steinhausser <maxime.steinhausser@gmail.com>
@ogizanagi ogizanagi force-pushed the bug-choice-constraint branch from fdd640e to d553750 Compare November 7, 2020 16:03
@ogizanagi
Copy link
Contributor

Thank you Benjamin.

@ogizanagi ogizanagi merged commit 50c7c3d into symfony:5.x Nov 7, 2020
@benji07 benji07 deleted the bug-choice-constraint branch November 8, 2020 16:14
@fabpot fabpot mentioned this pull request Nov 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus approved these changes

@ogizanagi ogizanagi ogizanagi approved these changes

@chalasr chalasr chalasr approved these changes

Assignees
No one assigned
Labels
Bug Status: Reviewed Validator
Projects
None yet
Milestone
5.2
Development

Successfully merging this pull request may close these issues.
6 participants
@benji07 @derrabus @ogizanagi @chalasr @xabbuh @carsonbot