Ticket #12596: 12596-simple.diff

django/forms/forms.py

@@ -263 +263 @@
         # 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 +290 @@
                 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):
         """

django/forms/models.py

@@ -280 +280 @@
                     exclude.append(f.name)
         return exclude
 
-    def clean(self):
+    def _clean_form(self):
+        # construct the instance after the form has been validated on the form level.
+        # this is a partial fix for #12596 which allows people to still use
+        # ModelForms without calling super().clean() even though they thus
+        # loose some validation (everything run on models, including validating
+        # the fields)
         opts = self._meta
         self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
+        super(BaseModelForm, self)._clean_form()
+
+    def clean(self):
         exclude = self._get_validation_exclusions()
         try:
             self.instance.full_clean(exclude=exclude)

tests/regressiontests/model_forms_regress/tests.py

@@ -50 +50 @@
         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