Fixed #12596. Calling super from a ModelForm's clean method is once again 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
This commit is contained in:
parent
856a39e841
commit
408d431029
|
@ -263,6 +263,12 @@ class BaseForm(StrAndUnicode):
|
||||||
# changed from the initial data, short circuit any validation.
|
# changed from the initial data, short circuit any validation.
|
||||||
if self.empty_permitted and not self.has_changed():
|
if self.empty_permitted and not self.has_changed():
|
||||||
return
|
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():
|
for name, field in self.fields.items():
|
||||||
# value_from_datadict() gets the data from the data dictionaries.
|
# value_from_datadict() gets the data from the data dictionaries.
|
||||||
# Each widget type knows how to retrieve its own data, because some
|
# Each widget type knows how to retrieve its own data, because some
|
||||||
|
@ -282,12 +288,12 @@ class BaseForm(StrAndUnicode):
|
||||||
self._errors[name] = self.error_class(e.messages)
|
self._errors[name] = self.error_class(e.messages)
|
||||||
if name in self.cleaned_data:
|
if name in self.cleaned_data:
|
||||||
del self.cleaned_data[name]
|
del self.cleaned_data[name]
|
||||||
|
|
||||||
|
def _clean_form(self):
|
||||||
try:
|
try:
|
||||||
self.cleaned_data = self.clean()
|
self.cleaned_data = self.clean()
|
||||||
except ValidationError, e:
|
except ValidationError, e:
|
||||||
self._errors[NON_FIELD_ERRORS] = self.error_class(e.messages)
|
self._errors[NON_FIELD_ERRORS] = self.error_class(e.messages)
|
||||||
if self._errors:
|
|
||||||
delattr(self, 'cleaned_data')
|
|
||||||
|
|
||||||
def clean(self):
|
def clean(self):
|
||||||
"""
|
"""
|
||||||
|
|
|
@ -250,6 +250,16 @@ class BaseModelForm(BaseForm):
|
||||||
super(BaseModelForm, self).__init__(data, files, auto_id, prefix, object_data,
|
super(BaseModelForm, self).__init__(data, files, auto_id, prefix, object_data,
|
||||||
error_class, label_suffix, empty_permitted)
|
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):
|
def _get_validation_exclusions(self):
|
||||||
"""
|
"""
|
||||||
|
@ -281,21 +291,45 @@ class BaseModelForm(BaseForm):
|
||||||
return exclude
|
return exclude
|
||||||
|
|
||||||
def clean(self):
|
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
|
opts = self._meta
|
||||||
self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
|
self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
|
||||||
exclude = self._get_validation_exclusions()
|
exclude = self._get_validation_exclusions()
|
||||||
try:
|
try:
|
||||||
self.instance.full_clean(exclude=exclude)
|
self.instance.clean_fields(exclude=exclude)
|
||||||
except ValidationError, e:
|
except ValidationError, e:
|
||||||
for k, v in e.message_dict.items():
|
self._update_errors(e.message_dict)
|
||||||
if k != NON_FIELD_ERRORS:
|
|
||||||
self._errors.setdefault(k, ErrorList()).extend(v)
|
def _clean_form(self):
|
||||||
# Remove the data from the cleaned_data dict since it was invalid
|
"""
|
||||||
if k in self.cleaned_data:
|
Runs the instance's clean method, then the form's. This is becuase the
|
||||||
del self.cleaned_data[k]
|
form will run validate_unique() by default, and we should run the
|
||||||
if NON_FIELD_ERRORS in e.message_dict:
|
model's clean method first.
|
||||||
raise ValidationError(e.message_dict[NON_FIELD_ERRORS])
|
"""
|
||||||
return self.cleaned_data
|
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):
|
def save(self, commit=True):
|
||||||
"""
|
"""
|
||||||
|
|
|
@ -50,6 +50,27 @@ class UniqueTogetherTests(TestCase):
|
||||||
form = TripleForm({'left': '1', 'middle': '3', 'right': '1'})
|
form = TripleForm({'left': '1', 'middle': '3', 'right': '1'})
|
||||||
self.failUnless(form.is_valid())
|
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 FPForm(forms.ModelForm):
|
||||||
class Meta:
|
class Meta:
|
||||||
model = FilePathModel
|
model = FilePathModel
|
||||||
|
|
Loading…
Reference in New Issue