Opened 14 years ago

Closed 14 years ago

Last modified 12 years ago

#12525 closed (fixed)

ModelForm validation doesn't work like documented

Reported by: Bernd Schlapsi <brot@…> Owned by: nobody
Component: Documentation Version: dev
Severity: Keywords: Validators
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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).']}

Change History (9)

comment:1 by Matt McClanahan, 14 years ago

Resolution: worksforme
Status: newclosed

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).']}

comment:2 by Bernd Schlapsi <brot@…>, 14 years ago

Resolution: worksforme
Status: closedreopened

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

comment:3 by Matt McClanahan, 14 years ago

Component: FormsDocumentation

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".

comment:4 by Bernd Schlapsi <brot@…>, 14 years ago

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.

comment:5 by jkocherhans, 14 years ago

Resolution: fixed
Status: reopenedclosed

(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.

comment:6 by korpios, 14 years ago

Resolution: fixed
Status: closedreopened

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.

in reply to:  6 comment:7 by Karen Tracey, 14 years ago

Resolution: fixed
Status: reopenedclosed

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.

comment:8 by korpios, 14 years ago

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

comment:9 by Jacob, 12 years ago

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top