Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #12596. Calling super from a ModelForm's clean method is once a…

…gain optional. Failing to call super only skips unique validation as documented. Thanks for the initial patch and tests, carljm.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@12269 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 408d4310291cd1287f3dbc05aaeb5d205eba8751 1 parent 856a39e
@jkocherhans jkocherhans authored
View
10 django/forms/forms.py
@@ -263,6 +263,12 @@ def full_clean(self):
# changed from the initial data, short circuit any validation.
if self.empty_permitted and not self.has_changed():
return
+ self._clean_fields()
+ self._clean_form()
+ if self._errors:
+ delattr(self, 'cleaned_data')
+
+ def _clean_fields(self):
for name, field in self.fields.items():
# value_from_datadict() gets the data from the data dictionaries.
# Each widget type knows how to retrieve its own data, because some
@@ -282,12 +288,12 @@ def full_clean(self):
self._errors[name] = self.error_class(e.messages)
if name in self.cleaned_data:
del self.cleaned_data[name]
+
+ def _clean_form(self):
try:
self.cleaned_data = self.clean()
except ValidationError, e:
self._errors[NON_FIELD_ERRORS] = self.error_class(e.messages)
- if self._errors:
- delattr(self, 'cleaned_data')
def clean(self):
"""
View
54 django/forms/models.py
@@ -250,6 +250,16 @@ def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
super(BaseModelForm, self).__init__(data, files, auto_id, prefix, object_data,
error_class, label_suffix, empty_permitted)
+ def _update_errors(self, message_dict):
+ for k, v in message_dict.items():
+ if k != NON_FIELD_ERRORS:
+ self._errors.setdefault(k, self.error_class()).extend(v)
+ # Remove the data from the cleaned_data dict since it was invalid
+ if k in self.cleaned_data:
+ del self.cleaned_data[k]
+ if NON_FIELD_ERRORS in message_dict:
+ messages = message_dict[NON_FIELD_ERRORS]
+ self._errors.setdefault(NON_FIELD_ERRORS, self.error_class()).extend(messages)
def _get_validation_exclusions(self):
"""
@@ -281,21 +291,45 @@ def _get_validation_exclusions(self):
return exclude
def clean(self):
+ self.validate_unique()
+ return self.cleaned_data
+
+ def _clean_fields(self):
+ """
+ Cleans the form fields, constructs the instance, then cleans the model
+ fields.
+ """
+ super(BaseModelForm, self)._clean_fields()
opts = self._meta
self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
exclude = self._get_validation_exclusions()
try:
- self.instance.full_clean(exclude=exclude)
+ self.instance.clean_fields(exclude=exclude)
except ValidationError, e:
- for k, v in e.message_dict.items():
- if k != NON_FIELD_ERRORS:
- self._errors.setdefault(k, ErrorList()).extend(v)
- # Remove the data from the cleaned_data dict since it was invalid
- if k in self.cleaned_data:
- del self.cleaned_data[k]
- if NON_FIELD_ERRORS in e.message_dict:
- raise ValidationError(e.message_dict[NON_FIELD_ERRORS])
- return self.cleaned_data
+ self._update_errors(e.message_dict)
+
+ def _clean_form(self):
+ """
+ Runs the instance's clean method, then the form's. This is becuase the
+ form will run validate_unique() by default, and we should run the
+ model's clean method first.
+ """
+ try:
+ self.instance.clean()
+ except ValidationError, e:
+ self._update_errors(e.message_dict)
+ super(BaseModelForm, self)._clean_form()
+
+ def validate_unique(self):
+ """
+ Calls the instance's validate_unique() method and updates the form's
+ validation errors if any were raised.
+ """
+ exclude = self._get_validation_exclusions()
+ try:
+ self.instance.validate_unique(exclude=exclude)
+ except ValidationError, e:
+ self._update_errors(e.message_dict)
def save(self, commit=True):
"""
View
21 tests/regressiontests/model_forms_regress/tests.py
@@ -50,6 +50,27 @@ def test_multiple_field_unique_together(self):
form = TripleForm({'left': '1', 'middle': '3', 'right': '1'})
self.failUnless(form.is_valid())
+class TripleFormWithCleanOverride(forms.ModelForm):
+ class Meta:
+ model = Triple
+
+ def clean(self):
+ if not self.cleaned_data['left'] == self.cleaned_data['right']:
+ raise forms.ValidationError('Left and right should be equal')
+ return self.cleaned_data
+
+class OverrideCleanTests(TestCase):
+ def test_override_clean(self):
+ """
+ Regression for #12596: Calling super from ModelForm.clean() should be
+ optional.
+ """
+ form = TripleFormWithCleanOverride({'left': 1, 'middle': 2, 'right': 1})
+ self.failUnless(form.is_valid())
+ # form.instance.left will be None if the instance was not constructed
+ # by form.full_clean().
+ self.assertEquals(form.instance.left, 1)
+
class FPForm(forms.ModelForm):
class Meta:
model = FilePathModel
Please sign in to comment.
Something went wrong with that request. Please try again.