UnresolvableValidationError far too greedy

[Skylar Saveland]

This should be an okay modelform:

from django.contrib.auth.models import User

class FailForm(forms.ModelForm):

    class Meta:
        model = User
        fields = ('username',)

I want to see:

>>> f = FailForm({})
>>> f.is_valid()
False

Instead, expecting False, I get:

UnresolvableValidationError: {'password': [u'This field cannot be blank.']}

line ~270 django.forms.models

[Honza Král]

Hi, this form will NEVER validate. If we just raised ValidationError (resulting in is_valid() == False) we would confuse both the programmer and the user.

The programmer will be confused because they would think it's the user's error and the user will get a validation error that wouldn't appear anywhere or, if we make it appear somewhere, will appear with no association to it's field (of whose existence the user doesn't have to be aware). Also the user has no way of fixing this validation error simply because this is the programmers fault - they contructed a FailForm and raising an exception is usually accepted behavior for mistakes in code.

I am changing this to Design decision needed

[Skylar Saveland]

>>> profile_form = SecretQuestionForm({})
>>> SecretQuestionForm.base_fields
{'secret_question': <django.forms.fields.CharField object at 0xa8557ec>, 'answer': <django.forms.fields.CharField object at 0
xa85588c>}
>>> profile_form.errors # I really only want errors from the included fields
{'answer': [u'This field is required.'], 'primary_contact': [u'This field cannot be null.'], 'secret_question': [u'This field
 is required.'], 'user': [u'This field cannot be null.']}
>>> form = SecretQuestionForm( {"secret_question":"foo", "answer":"bar"} ) # I want this to be .is_valid()
>>> form.errors
{'primary_contact': [u'This field cannot be null.'], 'user': [u'This field cannot be null.']}
>>> form.is_valid() # this would be Unresolvable but for my hacks ...
False

I think that maybe a Meta option in the modelform specifying whether or not to check fields that aren't included would be prudent.

The current trunk breaks vast swaths of my 1.1 code. So, for backwards compatibility I would vote for the modelform behaving like it used to as default.

[Honza Král]

I know what you mean, I just feel it's very wrong to have ModelForm validate and then fail horribly when calling .save() on it or on the model it produces. It maybe be backwards incompatible, but imo it would is a bug in 1.1.

In your case you should pass in an instance with primary_contact and user already set or set it in your form's .clean() before call to super.clean()

I leave it to core devs to decide this.

[Skylar Saveland]

I see what you're saying and have learned more about forms in investigating this. But, I think that the following should be a stable 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()

I process so many forms like that.

[simon]

That's certainly the convention I've been using.

This ticket looks like it relates to the one I just filed about the admin: #12521

[jkocherhans]

This is essentially the same as #12521 and I've added a patch over there, so I'm closing this one. I'll use the other ticket to track this issue.