ModelForm validation doesn't work like documented

[Bernd Schlapsi <brot@…>]

On ​http://docs.djangoproject.com/en/dev//ref/models/instances/#validating-objects there is the following sentence

However, if you are using a ModelForm, it will call full_validate for you, and present any errors along with the other form error messages.

But the ModelForm doesn't use the validators which are defined on my model. This is my sample code:

from django import forms
from django.core.validators import MinLengthValidator
from django.db import models
   
class Person(models.Model):
    first_name = models.CharField(validators=[MinLengthValidator(3)])
    last_name = models.CharField(validators=[MinLengthValidator(3)])
    email = models.EmailField()
 
class PersonForm(forms.ModelForm):
    class Meta:
        model = Person

f = PersonForm({'first_name':'a', 'last_name':'b', 'email':'a.b@test.com'})
f.full_clean()
f.errors

This is the output from the last statement

{'email': [u'This field is required.']}

But the output should look something like, if the documentation is right

{'email': [u'This field is required.'],
 'first_name': [u'Ensure this value has at least 3 characters (it has 1).'],
 'last_name': [u'Ensure this value has at least 3 characters (it has 1).']}

[Matt McClanahan]

I can't reproduce this, I get the expected result:

In [1]: from validatetest.models import PersonForm

In [2]: f = PersonForm({'first_name':'a', 'last_name':'b', 'email':'a.b@test.com'})

In [3]: f.full_clean()

In [4]: f.errors
Out[4]: 
{'first_name': [u'Ensure this value has at least 3 characters (it has 1).'],
 'last_name': [u'Ensure this value has at least 3 characters (it has 1).']}

[Bernd Schlapsi <brot@…>]

Ok, you're right. I missed something in my example. The problem occurs if you add the clean method to the form class

class PersonForm(forms.ModelForm):
    class Meta:
        model = Person

    def clean(self):
        return self.cleaned_data

Then the output is:

{}

But the documentation (​http://docs.djangoproject.com/en/dev//ref/forms/validation/#cleaning-and-validating-fields-that-depend-on-each-other) doesn't mention to call the superclass method in my clean method.
My example only works if the form class looks like:

class PersonForm(forms.ModelForm):
    class Meta:
        model = Person

    def clean(self):
        super(PersonForm, self).clean()
        return self.cleaned_data

[Matt McClanahan]

Yes, for a regular forms.Form it's true you don't need to call up to the parent's clean, because Form.clean doesn't do anything. And the example forms in the validation docs that you link to are plain Form classes. However, this issue is specific to a ModelForm, and the docs for modelforms actually do mention that you need to call the parent's clean method. ​http://docs.djangoproject.com/en/1.1/topics/forms/modelforms/#overriding-the-clean-method

It might be worth making a reference in the validation docs to that part of the modelform docs. Pretending to mark as "Documentation Decision Needed".

[Bernd Schlapsi <brot@…>]

OK, you're right again. I looked at the wrong documentation.
But my starting point for this issue was the new Validators section (​http://docs.djangoproject.com/en/dev//ref/validators/#validators). And this section only links to the regular Form documentation. A reference to the validation docs of the modelform would be helpful.

[jkocherhans]

(In [12212]) Fixed #12525. Added a note to the validators doucmentation that models will not automatically run validators when saved, and a link to the ModelForm docs.

[korpios]

Actually, there's a deeper problem here as well: if you don't call the super .clean() method from a ModelForm subclass that overrides .clean(), your ModelForm instance never gets updated with the values from your passed-in data(!). This is because the update happens via a call to construct_instance in ModelForm.clean(), and ModelForm.save() assumes that cleaning has already happened and therefore passes construct=False to save_instance. This is a fairly significant backward-incompatible change that needs to be documented.

[Karen Tracey]

Replying to korpios:

This is a fairly significant backward-incompatible change that needs to be documented.

I believe this is #12596, and as I understand it the proposal is to fix it to match the doc, not change the doc to match the new reality.

[korpios]

Ah, excellent. It's easy enough to work around, at least.

[Jacob]

Milestone 1.2 deleted