Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#12525 closed (fixed)

ModelForm validation doesn't work like documented

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

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 Changed 6 years ago by mattmcc

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to worksforme
  • Status changed from new to closed

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 Changed 6 years ago by Bernd Schlapsi <brot@…>

  • Resolution worksforme deleted
  • Status changed from closed to reopened

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 Changed 6 years ago by mattmcc

  • Component changed from Forms to Documentation

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 Changed 6 years ago by 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.

comment:5 Changed 6 years ago by jkocherhans

  • Resolution set to fixed
  • Status changed from reopened to closed

(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 follow-up: Changed 6 years ago by korpios

  • Resolution fixed deleted
  • Status changed from closed to reopened

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.

comment:7 in reply to: ↑ 6 Changed 6 years ago by kmtracey

  • Resolution set to fixed
  • Status changed from reopened to closed

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 Changed 6 years ago by korpios

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

comment:9 Changed 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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