NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Validator] Fixed grouped composite constraints#29499

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

Jump to bottom

Merged

stof merged 1 commit intosymfony:3.4fromHeahDude:validation-groups-collection

Dec 8, 2018

Merged

[Validator] Fixed grouped composite constraints#29499

stof merged 1 commit intosymfony:3.4fromHeahDude:validation-groups-collection

Dec 8, 2018

Conversation

Copy link

Contributor

HeahDude commentedDec 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.

carsonbot added Status: Needs Review Validator Bug labels

Dec 7, 2018

[Validator] Fixed grouped composite constraints

b53d911

HeahDude force-pushed thevalidation-groups-collection branch from3adf789 tob53d911Compare

December 7, 2018 14:48

This was referencedDec 7, 2018

[Validator] Pass validation groups on Collection validate.#23480

Closed

[Validator] Propagate embedded groups for collection validator#25888

Closed

xabbuh approved these changes

Dec 7, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Dec 7, 2018

stof approved these changes

Dec 7, 2018

View reviewed changes

Copy link

Member

stof commentedDec 8, 2018

Thanks@HeahDude.

stof merged commitb53d911 intosymfony:3.4

Dec 8, 2018

stof added a commit that referenced this pull request

Dec 8, 2018

bug#29499[Validator] Fixed grouped composite constraints (HeahDude)

fc8dc91

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 deleted the validation-groups-collection branch

December 8, 2018 15:50

Copy link

Contributor

backbone87 commentedDec 10, 2018

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.

Copy link

Member

xabbuh commentedDec 10, 2018

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

Copy link

Contributor

backbone87 commentedDec 12, 2018

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

This was referencedJan 6, 2019

Release v3.4.21#29801

Merged

Release v4.1.10#29802

Merged

Release v4.2.2#29803

Merged

xabbuh mentioned this pull request

Feb 6, 2019

Valid groupSequence seems not to work properly#26205

Closed

Copy link

Contributor

conradkleinespel commentedMay 2, 2019•
edited
Loading

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.