From 3c5862ccb0fc77418c3b20c21c6b57c40b483464 Mon Sep 17 00:00:00 2001 From: Keryn Knight Date: Thu, 30 Apr 2015 08:57:30 +0100 Subject: [PATCH] Fixed #24706 -- Made ModelForm._post_clean() handle a ValidationError raised when constructing the model instance. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks Loïc Bistuer for review and advice. --- django/forms/models.py | 16 +++++++++++--- tests/model_forms/models.py | 21 +++++++++++++++++++ tests/model_forms/tests.py | 42 +++++++++++++++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/django/forms/models.py b/django/forms/models.py index 217676f06b..651749a356 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -347,7 +347,15 @@ class BaseModelForm(BaseForm): # Override any validation error messages defined at the model level # with those defined at the form level. opts = self._meta - for field, messages in errors.error_dict.items(): + + # Allow the model generated by construct_instance() to raise + # ValidationError and have them handled in the same way as others. + if hasattr(errors, 'error_dict'): + error_dict = errors.error_dict + else: + error_dict = {NON_FIELD_ERRORS: errors} + + for field, messages in error_dict.items(): if (field == NON_FIELD_ERRORS and opts.error_messages and NON_FIELD_ERRORS in opts.error_messages): error_messages = opts.error_messages[NON_FIELD_ERRORS] @@ -379,8 +387,10 @@ class BaseModelForm(BaseForm): if isinstance(field, InlineForeignKeyField): exclude.append(name) - # Update the model instance with self.cleaned_data. - self.instance = construct_instance(self, self.instance, opts.fields, exclude) + try: + self.instance = construct_instance(self, self.instance, opts.fields, exclude) + except ValidationError as e: + self._update_errors(e) try: self.instance.full_clean(exclude=exclude, validate_unique=False) diff --git a/tests/model_forms/models.py b/tests/model_forms/models.py index b68714af4c..d9c08ae9c0 100644 --- a/tests/model_forms/models.py +++ b/tests/model_forms/models.py @@ -453,3 +453,24 @@ class Photo(models.Model): class UUIDPK(models.Model): uuid = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) name = models.CharField(max_length=30) + + +# Models for #24706 +class StrictAssignmentFieldSpecific(models.Model): + title = models.CharField(max_length=30) + _should_error = False + + def __setattr__(self, key, value): + if self._should_error is True: + raise ValidationError(message={key: "Cannot set attribute"}, code='invalid') + super(StrictAssignmentFieldSpecific, self).__setattr__(key, value) + + +class StrictAssignmentAll(models.Model): + title = models.CharField(max_length=30) + _should_error = False + + def __setattr__(self, key, value): + if self._should_error is True: + raise ValidationError(message="Cannot set attribute", code='invalid') + super(StrictAssignmentAll, self).__setattr__(key, value) diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index d118785bd4..76f575ac7b 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -29,8 +29,8 @@ from .models import ( DerivedBook, DerivedPost, Document, ExplicitPK, FilePathModel, FlexibleDatePost, Homepage, ImprovedArticle, ImprovedArticleWithParentLink, Inventory, Person, Photo, Post, Price, Product, Publication, - PublicationDefaults, Student, StumpJoke, TextFile, Triple, Writer, - WriterProfile, test_images, + PublicationDefaults, StrictAssignmentAll, StrictAssignmentFieldSpecific, + Student, StumpJoke, TextFile, Triple, Writer, WriterProfile, test_images, ) if test_images: @@ -2635,3 +2635,41 @@ class CustomMetaclassTestCase(SimpleTestCase): def test_modelform_factory_metaclass(self): new_cls = modelform_factory(Person, fields="__all__", form=CustomMetaclassForm) self.assertEqual(new_cls.base_fields, {}) + + +class StrictAssignmentTests(TestCase): + """ + Should a model do anything special with __setattr__() or descriptors which + raise a ValidationError, a model form should catch the error (#24706). + """ + + def test_setattr_raises_validation_error_field_specific(self): + """ + A model ValidationError using the dict form should put the error + message into the correct key of form.errors. + """ + form_class = modelform_factory(model=StrictAssignmentFieldSpecific, fields=['title']) + form = form_class(data={'title': 'testing setattr'}, files=None) + # This line turns on the ValidationError; it avoids the model erroring + # when its own __init__() is called when creating form.instance. + form.instance._should_error = True + self.assertFalse(form.is_valid()) + self.assertEqual(form.errors, { + 'title': ['Cannot set attribute', 'This field cannot be blank.'] + }) + + def test_setattr_raises_validation_error_non_field(self): + """ + A model ValidationError not using the dict form should put the error + message into __all__ (i.e. non-field errors) on the form. + """ + form_class = modelform_factory(model=StrictAssignmentAll, fields=['title']) + form = form_class(data={'title': 'testing setattr'}, files=None) + # This line turns on the ValidationError; it avoids the model erroring + # when its own __init__() is called when creating form.instance. + form.instance._should_error = True + self.assertFalse(form.is_valid()) + self.assertEqual(form.errors, { + '__all__': ['Cannot set attribute'], + 'title': ['This field cannot be blank.'] + })