Forms _get_validations_exclusions is still not backwards compatible

[Chris Beaven]

In my user registration form, I have a field named password, but the modelform's Meta has fields = ('first_name', 'last_name', 'email').

Submitting the form causes a "This field cannot be blank." validation error for the password field.

The current logic doesn't actually refer to those fields, but to the fields dictionary (which will also contain any user-generated fields).

[Chris Beaven]

The original code was introduced in [12206]

[Honza Král]

Closing this as worksforme - I cannot replicate it and when I look at UserCreationForm in contrib.auth (which isusing the same approach) at

http://code.djangoproject.com/browser/django/trunk/django/contrib/auth/forms.py#L23

It works and is actually tested at

http://code.djangoproject.com/browser/django/trunk/django/contrib/auth/tests/forms.py#L72

Could you please provide a test case or more detailed example of what you are doing?

[Chris Beaven]

Hi Honza,

UserCreationForm doesn't use password, it uses password1 which skirts around this backwards incompatibility.
I'll see if I can get around to a test case, but it's pretty obvious (for those that know the code and previous behaviour) that this is an error if you look at the patch.

[Honza Král]

test case and fix

[Honza Král]

I see what you had a problem with, managed to produce a test case and a fix. Sorry for the initial confusion.

[jkocherhans]

This fixes the original problem, but it also breaks unique validation. Here's the test that breaks ​http://code.djangoproject.com/browser/django/trunk/tests/modeltests/model_formsets/models.py?rev=12369#L935

I'm don't have a strong opinion here. By replacing a default form field should you be opting out of unique validation on that field as well? Seems like either way would be a surprise.

[Chris Beaven]

I'm not really "replacing a default form field" because of the meta option, but I understand what you're saying.

IMO, if the fields meta option is used, I think that it's pretty explicit that those are the *only* fields should be want tied to the model.

[jkocherhans]

(In [12496]) Fixed #12901. Exclude overridden form fields from model field validation. Thanks, Honza Král.

[ammarr]

From the test case, it works with the following code:

class IncompleteCategoryForm(ModelForm): 
    url = forms.CharField(required=False) 	 
    class Meta: 
        fields = ('name', 'slug') 
        model = Category 

but not this:

class IncompleteCategoryForm(ModelForm): 
    url = forms.CharField(required=False) 	 
    class Meta: 
        # removed field tuple
        model = Category 

Is this intentional? IMO, if you declare a field manually, then it should be excluded from validation in any case (regardless of whether the field tuple is present or not).

[Honza Král]

Replying to ammarr:

From the test case, it works with the following code:

class IncompleteCategoryForm(ModelForm): 
    url = forms.CharField(required=False) 	 
    class Meta: 
        fields = ('name', 'slug') 
        model = Category 

but not this:

class IncompleteCategoryForm(ModelForm): 
    url = forms.CharField(required=False) 	 
    class Meta: 
        # removed field tuple
        model = Category 

Is this intentional? IMO, if you declare a field manually, then it should be excluded from validation in any case (regardless of whether the field tuple is present or not).

No. The key difference here is that if you omit the field in Meta.fields, it doesn't get populated on the model so validation doesn't make sense. If you, however, just override the field definition, the model still get populated with this field and validation should then be run.

[ammarr]

I see (and agree with) what you're saying, but the resultant behavior is not backwards compatible. The code snippet without the fields tuple validates in 1.1.1.

[ammarr]

Replying to Honza_Kral:

No. The key difference here is that if you omit the field in Meta.fields, it doesn't get populated on the model so validation doesn't make sense. If you, however, just override the field definition, the model still get populated with this field and validation should then be run.

In addition to the previous message, the only way to exclude a field from model validation is to provide the entire fields tuple, exclude that one field, and then manually declare it. This behavior isn't symmetrical though (declare a field manually, and then just exclude it using the exclude tuple doesn't work). Perhaps this check should be added:

elif self._meta.exclude and field in self._meta.exclude:
exclude.append(f.name) 

[jkocherhans]

(In [12590]) Fixed #12901. Again. Model validation will not be performed on excluded fields that were overridden in the form. Thanks, ammarr.

[Jacob]

Milestone 1.2 deleted