-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[RFC] New constraints for Symfony validator #28608
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
Comments
if regarding the CreditCardNumber alias, I would point you to the discussion on #26436, which seems to tell me we don't want to make it easier to discover that a credit card number is using luhn. Regarding EAN, in Hibernate, this is just a Length constraint with only 2 possible allowed values for the length: https://github.com/hibernate/hibernate-validator/blob/eaa2cb6e45f95007c18297c2e4faea33e1c6b5ab/engine/src/main/java/org/hibernate/validator/internal/constraintvalidators/hv/EANValidator.java |
About the EAN validator, the last digit is a checksum digit ... so there's something to validate there. See https://gist.github.com/damonjones/7566978 |
Well, but there could be indeed. But Hibernate does not, making this constraint quite useless. |
1- for the Date Validator, do we have always a interest to keep DateTimeValidator and DateValidator ? Should we keep just one ? DateValidator with format accepted by date() ? 2- Past, Future can be options of DateConstraint ? or should it be annotation using the validation of DateValidator ? |
Well, these should not be options of the Date constraints, as these constraints make sense on a property holding a DateTime object too, only on properties holding a string representing a date (which is what the Date constraint is about). |
From https://docs.jboss.org/hibernate/stable/beanvalidation/api/javax/validation/constraints/Past.html i'd say yes, but maybe we can add a format option to these to allow strings. edit: actually we can just create dates from strings as is: https://3v4l.org/vk0AO |
@ro0NL I do not see your point regard to https://3v4l.org/vk0AO |
well.. it shows we dont need a format option
the first excepts any date string (e.g. https://3v4l.org/vk0AO), the second is narrowed to a formatted stirng |
About the I don't see a common use case which would require to check first if it is already safe. |
I have an Event entity and I wanted to use https://symfony.com/blog/new-in-symfony-3-4-improved-comparison-constraints to replace my own callback validator by However, the callback also validates that In summary: the |
@javiereguiluz : Do you mean |
🤦♂️ Thanks Maxime 😊 |
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 shouldn't have been closed since it's mentioning EAN (AKA GTIN), it's quite common for developers to work with "products" and having a dedicated validator for this specific identificator seems like something you'd want. It's not just length + checksum, it also has various flavors, etc. |
Also that HtmlSanitizer is now proposed as new Symfony component (#44144) |
Given that Symfony's validator is based on Java's validator, I was checking the Bean Validation 2.0 standard to see if they introduced new features that we could implement too.
These are the new constraints that Symfony doesn't define yet (although you can implement them indirectly with other existing constraints):
Dates:
@Past
@PastOrPresent
@Future
@FutureOrPresent
Numbers:
@Positive
@PositiveOrZero
@Negative
@NegativeOrZero
In addition, Hibernate Validator, the only official implementation of Bean Validator 2.0, defines other constraints:
Are we interested in implementing any of these constraints?
The text was updated successfully, but these errors were encountered: