Skip to content
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

[Validator] Fixed grouped composite constraints #29499

Merged
merged 1 commit into from Dec 8, 2018

Conversation

HeahDude
Copy link
Contributor

@HeahDude HeahDude commented Dec 7, 2018

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #17675, #25888, #23480
License MIT
Doc PR ~

From Lisbon :). Thanks @stof, @xabbuh for your help to finally fix this old issue.

@stof
Copy link
Member

stof commented Dec 8, 2018

Thanks @HeahDude.

@stof stof merged commit b53d911 into symfony:3.4 Dec 8, 2018
stof added a commit that referenced this pull request Dec 8, 2018
This PR was merged into the 3.4 branch.

Discussion
----------

[Validator] Fixed grouped composite constraints

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #17675, #25888, #23480
| License       | MIT
| Doc PR        | ~

From Lisbon :). Thanks @stof, @xabbuh for your help to finally fix this old issue.

Commits
-------

b53d911 [Validator] Fixed grouped composite constraints
@HeahDude HeahDude deleted the validation-groups-collection branch December 8, 2018 15:50
@backbone87
Copy link
Contributor

I think this change completely obsoletes the caching. I am not sure, but a constraint that has two grps would now be validated twice, if the both grps are the validation target.

@xabbuh
Copy link
Member

xabbuh commented Dec 10, 2018

Most constraints don't extend the Composite constraint so are not affected here.

@backbone87
Copy link
Contributor

Yes you are right, i was desperately searching for the check for Composites, while its right the line above.

@conradkleinespel
Copy link
Contributor

conradkleinespel commented May 2, 2019

Thanks for the fix @stof !

Just FYI, I think (I'm still investigating) this might have been a breaking change for us. Suddenly, some of our forms return unexpected validation errors. I think our code depended on this bug. Hopefully the fix should be quick. Nevertheless it is a bit surprising to see this in a patch release.

I think we might have to check every single validation rule that uses collections in our project, because some previously unknown validations might come into action silently, preventing customers from doing things that were previously possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants