8000 [Form] removed validation of form children by kriswallsmith · Pull Request #797 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Form] removed validation of form children #797

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

Closed
wants to merge 1 commit into from
Closed

[Form] removed validation of form children #797

wants to merge 1 commit into from

Conversation

kriswallsmith
Copy link
Contributor

Validation of form children is not necessary. You can use the Valid constraint to accomplish this when desired.

@ghost ghost assigned webmozart May 6, 2011
@kriswallsmith
Copy link
Contributor Author

@bschussek: Can you review this patch? We're currently forcing the equivalent of an @assert:Valid constraint on all embedded forms, which defeats the purpose of having that constraint at all.

@webmozart
Copy link
Contributor

Validation of form children is necessary in case forms are working on decoupled objects. If form a has object X as data and child form a.b has object Y as data, and there is no connection between X and Y, Y won't be validated after your patch.

What's the idea behind this PR? We should solve it in a different way.

@kriswallsmith
Copy link
Contributor Author

The problem is that child objects are validated and there is not way to disable that behavior. The validation component includes a @Valid constraint that allows users to specify where validation should cascade. Always validating all form children is inconsistent with that.

@beberlei
Copy link
Contributor

I had a problem with this constraint aswell, if you define an embeded form where the object should be validated only if some other field is true/false this current feature makes it impossible to do so.

@kriswallsmith
Copy link
Contributor Author

We at least need a way to control whether validation cascades into each form child. Would this mean checking a new constraint that checks child forms based on a certain attribute or property on the parent?

@schmittjoh
Copy link
Contributor

We have a related problem here: #1151

@lsmith77
Copy link
Contributor

Whats going to happen with this issue?

@stof
Copy link
Member
stof commented Sep 4, 2011

up, what is the status here @bschussek @kriswallsmith ?

@mdhooge
Copy link
mdhooge commented Sep 21, 2011

The commit proposed by Kris (797#commits-pushed-99fa9e0) solved the problem I described here:
https://groups.google.com/d/topic/symfony2/slQtH6ximEU/discussion

In my case, there were no explicit child forms.
However, when adding an entity field, it also happens that all required validations of this sub-entity was also pulled, with a nasty hair-pulling effect ;-)

@webmozart
Copy link
Contributor

@kriswallsmith: This is now fixed in the referenced pull request.

@webmozart webmozart closed this Jan 16, 2012
fabpot added a commit that referenced this pull request Jan 16, 2012
Commits
-------

0c70a41 [Form] Made validation of form children configurable. Set the option "cascade_validation" to `true` if you need it.

Discussion
----------

[Form] Made validation of form children configurable

Bug fix: yes
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #797
Todo: adapt documentation

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=optional-form-child-validation)

Child forms now aren't validated anymore by default. This is not a problem as long as @Valid constraints are properly put in your model. If you want to enable cascading validation, for example when there is no connection between the parent and the child model, you can set the option "cascade_validation" in the parent form to true.

This change is not backwards compatible, but from my estimation the break should not affect many applications.

---------------------------------------------------------------------------

by kriswallsmith at 2012-01-16T19:59:25Z

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

Successfully merging this pull request may close these issues.

7 participants
0