-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Validator] add number constraints #28637
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
Conversation
ac485ee
to
77ba93a
Compare
src/Symfony/Component/Validator/Constraints/NegativeValidator.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Validator/Constraints/NegativeValidator.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Validator/Constraints/NegativeOrZeroValidator.php
Outdated
Show resolved
Hide resolved
6f25560
to
5a92e1b
Compare
What about null values? |
5a92e1b
to
0b6e408
Compare
@ostrolucky I think it should do nothing for null values. I will add a condition and a test. |
I think we could save plenty of code here if the constraints were just specialised versions of the already existing |
like |
yes |
Better fit is GreaterThan*/LessThan*. No new validators would be required then |
indeed, those fit better than |
@ostrolucky Do you mean we don't need the new |
73e1e81
to
43aef65
Compare
43aef65
to
40aa6cd
Compare
@jschaedl actually i expected it like:
then |
Yep, like @ro0NL says. Well, not exactly (validatedBy needs to be specified and minimum value maybe tweaked) but mostly just that. |
84de93c
to
63fa241
Compare
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.
Nice improvement
63fa241
to
d2073e5
Compare
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.
Like I explained in #28637 (comment) since these constraints don't accept any default value, their constructor shouldn't default to null
IMHO it should, so we get the expected exception here https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Constraint.php#L136 Also we could make |
These constraints are designed in a way they will never pass It would make some sense in case message is default property, but I don't think somebody would approve such change. |
8f7388a
to
2c07ac2
Compare
7119701
to
35bfa5a
Compare
189d578
to
0e31c3b
Compare
e476040
to
c77ef3b
Compare
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.
Ready to be merged after the translation files are fixed.
src/Symfony/Component/Validator/Resources/translations/validators.de.xlf
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Validator/Resources/translations/validators.vi.xlf
Outdated
Show resolved
Hide resolved
@jschaedl could you please send a Docs PR for this new feature? |
e718297
to
c973cee
Compare
@OskarStark I am already working on it. :-) |
c973cee
to
0187039
Compare
Thank you @jschaedl. |
This PR was squashed before being merged into the 4.3-dev branch (closes #28637). Discussion ---------- [Validator] add number constraints | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28608 | License | MIT | Doc PR | tbd. I added the following constraints: * `Positive` * `PositiveOrZero` * `Negative` * `NegativeOrZero` Commits ------- 0187039 [Validator] add number constraints
This PR was squashed before being merged into the master branch (closes #11254). Discussion ---------- [Validator] Add docs for number constraints Feature PR: symfony/symfony#28637 - [x] Positive - [x] PositiveOrZero - [x] Negative - [x] NegativeOrZero ### Todo: - [x] find a better example for `NegativeOrZero` constraint Commits ------- 8022292 [Validator] Add docs for number constraints
I added the following constraints:
Positive
PositiveOrZero
Negative
NegativeOrZero