On Wed, Jan 6, 2010 at 12:49 PM, Simon Willison <si...@simonwillison.net> wrote: > A couple of related tickets filed today about model validation: > > http://code.djangoproject.com/ticket/12513 > http://code.djangoproject.com/ticket/12521 > > The first one describes the issue best - the new model validation code > breaks the following common Django convention: > > form = SecretQuestionForm( {"secret_question":"foo", "answer":"bar"} ) > if form.is_valid(): > p = form.save(commit=False) > p.user = request.user > p.primary_contact = somecontact > p.save() > > The problem is that is_valid() notices that some of the required > fields in SecretQuestionForm (a ModelForm) have not been provided, > even if those fields are excluded from validation using the excludes= > or fields= properties. The exception raised is a > UnresolvableValidationError.
I'll start off with the reasoning behind the implementation, but I agree that the current implementation is going to bite too many people to just call the old implementation a bug. Form.is_valid() now triggers model validation, and the model isn't valid. It's missing a couple of required fields. Presenting those errors to the user filling out the form is unacceptable because there is nothing the user can do to correct the error, and the developer will never get a notification about a problem that can only be solved with code. > This definitely needs to be fixed as it presumably breaks backwards > compatibility with lots of existing code (it breaks a common > ModelAdmin subclass convention as well, see #12521). Can we just > change the is_valid() logic to ignore model validation errors raised > against fields which aren't part of the ModelForm, or is it more > complicated than that? It shouldn't be much more complicated than that. Model.full_validate() takes an exclude parameter that we can use to provide a list of fields that aren't on the form. Or we can catch those errors and just throw them away. However, this means that part of the problem that model-validation was meant to solve will no longer be solved. ModelForm.is_valid() will only mean that your *form* is valid, not your *model* (which is the pre-merge semantics anyhow). Once again, that means ModelForm won't really validate your model, only your form. form.save() might still throw a database error just like before the merge. We can slap a big warning in the ModelForm docs though. One (probably unhelpful) way to make ModelForm validate your whole model would be to add a Meta flag to ModelForm: class UserForm(ModelForm): class Meta: # Defaults to False validate_model = True That would make it easy to trigger model validation, but it doesn't really help with the convention you're talking about. I don't know. Do people think triggering model validation in a ModelForm is something they need to do in a practical sense? Or is validating your form enough? Joseph
-- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.