Code

Ticket #12596: 12596-simple.diff

File 12596-simple.diff, 3.4 KB (added by Honza_Kral, 4 years ago)

Simpler variant of the patch that only goes half way

Line 
1diff --git a/django/forms/forms.py b/django/forms/forms.py
2index c53a901..73599d0 100644
3--- a/django/forms/forms.py
4+++ b/django/forms/forms.py
5@@ -263,6 +263,14 @@ class BaseForm(StrAndUnicode):
6         # changed from the initial data, short circuit any validation.
7         if self.empty_permitted and not self.has_changed():
8             return
9+
10+        self._clean_fields()
11+        self._clean_form()
12+
13+        if self._errors:
14+            delattr(self, 'cleaned_data')
15+
16+    def _clean_fields(self):
17         for name, field in self.fields.items():
18             # value_from_datadict() gets the data from the data dictionaries.
19             # Each widget type knows how to retrieve its own data, because some
20@@ -282,12 +290,12 @@ class BaseForm(StrAndUnicode):
21                 self._errors[name] = self.error_class(e.messages)
22                 if name in self.cleaned_data:
23                     del self.cleaned_data[name]
24+
25+    def _clean_form(self):
26         try:
27             self.cleaned_data = self.clean()
28         except ValidationError, e:
29             self._errors[NON_FIELD_ERRORS] = self.error_class(e.messages)
30-        if self._errors:
31-            delattr(self, 'cleaned_data')
32 
33     def clean(self):
34         """
35diff --git a/django/forms/models.py b/django/forms/models.py
36index cce2319..6aa08f7 100644
37--- a/django/forms/models.py
38+++ b/django/forms/models.py
39@@ -280,9 +280,17 @@ class BaseModelForm(BaseForm):
40                     exclude.append(f.name)
41         return exclude
42 
43-    def clean(self):
44+    def _clean_form(self):
45+        # construct the instance after the form has been validated on the form level.
46+        # this is a partial fix for #12596 which allows people to still use
47+        # ModelForms without calling super().clean() even though they thus
48+        # loose some validation (everything run on models, including validating
49+        # the fields)
50         opts = self._meta
51         self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
52+        super(BaseModelForm, self)._clean_form()
53+
54+    def clean(self):
55         exclude = self._get_validation_exclusions()
56         try:
57             self.instance.full_clean(exclude=exclude)
58diff --git a/tests/regressiontests/model_forms_regress/tests.py b/tests/regressiontests/model_forms_regress/tests.py
59index f8b6511..57d5655 100644
60--- a/tests/regressiontests/model_forms_regress/tests.py
61+++ b/tests/regressiontests/model_forms_regress/tests.py
62@@ -50,6 +50,27 @@ class UniqueTogetherTests(TestCase):
63         form = TripleForm({'left': '1', 'middle': '3', 'right': '1'})
64         self.failUnless(form.is_valid())
65 
66+class TripleFormWithCleanOverride(forms.ModelForm):
67+    class Meta:
68+        model = Triple
69+
70+    def clean(self):
71+        if not self.cleaned_data['left'] == self.cleaned_data['right']:
72+            raise forms.ValidationError('Left and right should be equal')
73+        return self.cleaned_data
74+
75+class OverrideCleanTests(TestCase):
76+    def test_override_clean(self):
77+        """
78+        Regression for #12596: Calling super from ModelForm.clean() should be
79+        optional.
80+        """
81+        form = TripleFormWithCleanOverride({'left': 1, 'middle': 2, 'right': 1})
82+        self.failUnless(form.is_valid())
83+        # form.instance.left will be None if the instance was not constructed
84+        # by form.full_clean().
85+        self.assertEquals(form.instance.left, 1)
86+
87 class FPForm(forms.ModelForm):
88     class Meta:
89         model = FilePathModel