Fixed #24706 -- Made ModelForm._post_clean() handle a ValidationError raised…

Fixed #24706 -- Made ModelForm._post_clean() handle a ValidationError raised when constructing the model instance.

Thanks Loïc Bistuer for review and advice.

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)

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)

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.']
+        })