Code

Ticket #12960: 12960.diff

File 12960.diff, 4.7 KB (added by jkocherhans, 4 years ago)

Complete patch. Still thinking about it, but this moves model validation to happen entirely after form validation. I think this is a good thing, but I'm not entirely sure yet.

Line 
1diff --git a/django/forms/forms.py b/django/forms/forms.py
2index 6f7a8ce..45bbbb4 100644
3--- a/django/forms/forms.py
4+++ b/django/forms/forms.py
5@@ -265,6 +265,7 @@ class BaseForm(StrAndUnicode):
6             return
7         self._clean_fields()
8         self._clean_form()
9+        self._post_clean()
10         if self._errors:
11             delattr(self, 'cleaned_data')
12 
13@@ -295,6 +296,13 @@ class BaseForm(StrAndUnicode):
14         except ValidationError, e:
15             self._errors[NON_FIELD_ERRORS] = self.error_class(e.messages)
16 
17+    def _post_clean(self):
18+        """
19+        An internal hook for performing additional cleaning after form cleaning
20+        is complete. Used for model validation in model forms.
21+        """
22+        pass
23+
24     def clean(self):
25         """
26         Hook for doing any extra form-wide cleaning after Field.clean() been
27diff --git a/django/forms/models.py b/django/forms/models.py
28index 65fe1a7..3c95b4d 100644
29--- a/django/forms/models.py
30+++ b/django/forms/models.py
31@@ -245,6 +245,11 @@ class BaseModelForm(BaseForm):
32         # if initial was provided, it should override the values from instance
33         if initial is not None:
34             object_data.update(initial)
35+        # This will be set to True by BaseModelForm.clean(). This makes it so
36+        # it will be set to False if someone overrides that method and doesn't
37+        # call super. Unfortunately the implementation detail of overriding
38+        # clean() to prevent unique valiation was documented.
39+        self._validate_unique = False
40         super(BaseModelForm, self).__init__(data, files, auto_id, prefix, object_data,
41                                             error_class, label_suffix, empty_permitted)
42 
43@@ -299,34 +304,36 @@ class BaseModelForm(BaseForm):
44         return exclude
45 
46     def clean(self):
47-        self.validate_unique()
48+        # Set self._validate_unique to True so self.validate_unique will be
49+        # called later, and so that overriding this method and failing to call
50+        # super will cause self.validate_unique() to not be called. This is an
51+        # unfortunate hack to keep backwards compatibility for an
52+        # implementation detail that shouldn't have been documented.
53+        self._validate_unique = True
54         return self.cleaned_data
55 
56-    def _clean_fields(self):
57-        """
58-        Cleans the form fields, constructs the instance, then cleans the model
59-        fields.
60-        """
61-        super(BaseModelForm, self)._clean_fields()
62+    def _post_clean(self):
63+        exclude = self._get_validation_exclusions()
64         opts = self._meta
65+
66+        # Update the model instance with self.cleaned_data.
67         self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
68-        exclude = self._get_validation_exclusions()
69+
70+        # Clean the model instance's fields.
71         try:
72             self.instance.clean_fields(exclude=exclude)
73         except ValidationError, e:
74             self._update_errors(e.message_dict)
75 
76-    def _clean_form(self):
77-        """
78-        Runs the instance's clean method, then the form's. This is becuase the
79-        form will run validate_unique() by default, and we should run the
80-        model's clean method first.
81-        """
82+        # Call the model instance's clean method.
83         try:
84             self.instance.clean()
85         except ValidationError, e:
86             self._update_errors({NON_FIELD_ERRORS: e.messages})
87-        super(BaseModelForm, self)._clean_form()
88+
89+        # Validate uniqueness if needed.
90+        if self._validate_unique:
91+            self.validate_unique()
92 
93     def validate_unique(self):
94         """
95diff --git a/tests/regressiontests/model_forms_regress/tests.py b/tests/regressiontests/model_forms_regress/tests.py
96index d03e61d..485160a 100644
97--- a/tests/regressiontests/model_forms_regress/tests.py
98+++ b/tests/regressiontests/model_forms_regress/tests.py
99@@ -72,6 +72,26 @@ class OverrideCleanTests(TestCase):
100         # by form.full_clean().
101         self.assertEquals(form.instance.left, 1)
102 
103+# Regression test for #12960.
104+# Make sure the cleaned_data returned from ModelForm.clean() is applied to the
105+# model instance.
106+
107+class PublicationForm(forms.ModelForm):
108+    def clean(self):
109+        print self.cleaned_data
110+        self.cleaned_data['title'] = self.cleaned_data['title'].upper()
111+        return self.cleaned_data
112+
113+    class Meta:
114+        model = Publication
115+
116+class ModelFormCleanTest(TestCase):
117+    def test_model_form_clean_applies_to_model(self):
118+        data = {'title': 'test', 'date_published': '2010-2-25'}
119+        form = PublicationForm(data)
120+        publication = form.save()
121+        self.assertEqual(publication.title, 'TEST')
122+
123 class FPForm(forms.ModelForm):
124     class Meta:
125         model = FilePathModel