[Validator] Add warning for null and blank values for minimum string length #5789
Conversation
|
As this is the case for most of constraints, instead of changing this single constraint, I'd rather add a warning or note section to all of them. Not sure what the docs team thinks about it though :) |
|
Thank you for opening this pull request @theofidry. However, I am not sure if this is the best way to fix it. Please note that we already have the following sentence in the description of the min option (maybe we should wrap it in a
Also, I agree with @jakzal that we should not only document this for the |
|
@xabbuh I agree the explanation in the min options are well described. However the issue is that the introduction sentence is misleading. If you stop at that and the option names, you will miss that part. Maybe adding a warning between the introduction part and the table "Warning: null and blank values are not handled by this constraint" would do though. |
|
I agree that sounds like a good idea. Would you like to create a generic warning that we can include in all the constraints it applies too? |
|
Sure. I'll update the PR shortly |
|
I'm going to close this PR because of the lack of activity. There is an open issue about this (#4244), feel free to create a new PR to fix that issue. I would still like to thank you for trying to add this missing piece to the documentation! |
|
Oh my indeed an old one. I'll make sure to take care of this this week :) |
|
@theofidry Unfortunately, I cannot click the button. So you would have to start a new PR. |
…um string length (theofidry) This PR was submitted for the 2.3 branch but it was merged into the 2.7 branch instead (closes #6270). Discussion ---------- [Validator] Add warning for null and blank values for minimum string length | Q | A | | --- | --- | | Doc fix? | yes | | New docs? | no | | Applies to | 2.3+ | | Fixed tickets | symfony/symfony#16157 | Follow up of #5789. Summary of what has been said: even though the [Length#min](http://symfony.com/doc/current/reference/constraints/Length.html#min) option is well detailed and mention the case of `null` values, the introduction is still confusing: > Validates that a given string length is between some minimum and maximum value. As a result, a warning note is being added under this introduction. I've listed all the Symfony constraints. For some this `null` and `blank` values specificity has already been mentioned. I've noted the ones on which this warning does not apply, but for some I am not sure so it needs to be doubled checked. - Basic Contraints - [x] NotBlank: already done - [x] Blank: already done - [x] NotNull: already done - [x] IsNull: already done - ~~[ ] IsTrue~~ - ~~[ ] IsFalse~~ - ~~[ ] Type~~ - String Constraints¶ - ~~[ ] Email~~ - [x] Length - ~~[ ] Url~~ - ~~[ ] Regex~~ - ~~[ ] Ip~~ - ~~[ ] Uuid~~ - Number Constraints¶ - [ ] Range - Comparison Constraints¶ - ~~[ ] EqualTo~~ - ~~[ ] NotEqualTo~~ - ~~[ ] IdenticalTo~~ - ~~[ ] NotIdenticalTo~~ - [ ] LessThan: needed? How `@Assert\LessThan("-18 years")` behaves if the value is null? - [ ] LessThanOrEqual: same as `LessThan` - [ ] GreaterThan: same as `LessThan` - [ ] GreaterThanOrEqual: same as `LessThan` - Date Constraints¶ - ~~[ ] Date~~ - ~~[ ] DateTime~~ - ~~[ ] Time~~ - Collection Constraints¶ - ~~[ ] Choice~~ - ~~[ ] Collection~~ - ~~[ ] Count~~ - ~~[ ] UniqueEntity~~ - ~~[ ] Language~~ - ~~[ ] Locale~~ - ~~[ ] Country~~ - File Constraints¶ - ~~[ ] File~~ - ~~[ ] Image~~ - Financial and other Number Constraints¶ - ~~[ ] Bic~~ - ~~[ ] CardScheme~~ - ~~[ ] Currency~~ - ~~[ ] Luhn~~ - ~~[ ] Iban~~ - ~~[ ] Isbn~~ - ~~[ ] Issn~~ - Other Constraints¶ - ~~[ ] Callback~~ - ~~[ ] Expression~~ - ~~[ ] All~~ - ~~[ ] UserPassword~~ - ~~[ ] Valid~~ Commits ------- 38d44c2 [Validator] Add warning for null and blank values for minimum string length
As explained in the ticket, although the doc is very precise for the min option of the string Length constraint, the introducing line following the title is very misleading.
cc @jakzal