Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Validator] Allow Unique constraint validation on all elements#59274
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
GromNaN left a comment• 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.
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.
Sometimes it could be useful to assert that every elements of a collection is not duplicated.
Is there a situation where it's better to have only the first error?
This PR adds a
multipleErrorsoption to the Unique constraint to avoid stopping at the first violation.
I think we can avoid adding a new option and simply modify the behavior in version 7.3.
That was my first idea but it will lead to a BC break like mentioned in#59037. |
| @@ -26,6 +26,7 @@ class Unique extends Constraint | |||
| public array|string $fields = []; | |||
| public ?string $errorPath = null; | |||
| public bool $multipleErrors = false; | |||
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.
naming :) what about this? multipleErrors looks like a consequence of something, not an option
| public bool$multipleErrors =false; | |
| public bool$quickCheck =true; |
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.
The current name doesn't seem right to me. It's about having all errors, not necessarily multiple ones.
Maybe$allErrors?
If we want to name it closer to the behavior, the current one is "stopping on first error", which is more or less similar to PHPUnit--no-on-error flag.
So, another suggestion might be:$stopOnFirstError?
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.
Maybe "$all" so all must be unique?
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.
Agreed with$stopOnFirstError 👍
The PR updated in that way +CHANGELOG.md
| @@ -26,6 +26,7 @@ class Unique extends Constraint | |||
| public array|string $fields = []; | |||
| public ?string $errorPath = null; | |||
| public bool $multipleErrors = false; | |||
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.
The current name doesn't seem right to me. It's about having all errors, not necessarily multiple ones.
Maybe$allErrors?
If we want to name it closer to the behavior, the current one is "stopping on first error", which is more or less similar to PHPUnit--no-on-error flag.
So, another suggestion might be:$stopOnFirstError?
8f36692 to523d94eCompare523d94e to3fc871eCompareThank you@Jean-Beru. |
fd9c5b4 intosymfony:7.3Uh oh!
There was an error while loading.Please reload this page.
Sometimes it could be useful to assert that every elements of a collection is not duplicated. This PR adds a
multipleErrorsoption to the Unique constraint to avoid stopping at the first violation.Its value is
falseby default to avoid BC breaks:Now