Fixed #22915 -- Document backward incompatible changes in the ValidationError constructor.

This patch also fixes update_error_dict to better handle the use case described
in this ticket, previously the type of the provided container could be lost in
some conditions.

Thanks Russell Keith-Magee for the report and Tim Graham for review.

django/core/exceptions.py

@@ -142,13 +142,10 @@ class ValidationError(Exception):
 
     def update_error_dict(self, error_dict):
         if hasattr(self, 'error_dict'):
-            if error_dict:
+            for field, error_list in self.error_dict.items():
-                for field, errors in self.error_dict.items():
+                error_dict.setdefault(field, []).extend(error_list)
-                    error_dict.setdefault(field, []).extend(errors)
         else:
-                error_dict = self.error_dict
+            error_dict.setdefault(NON_FIELD_ERRORS, []).extend(self.error_list)
-        else:
-            error_dict[NON_FIELD_ERRORS] = self.error_list
         return error_dict
 
     def __iter__(self):

docs/releases/1.7.txt

@@ -279,6 +279,74 @@ example ``qs.filter(author__birthdate__year__lte=1981)``.
 For more information about both custom lookups and transforms refer to
 :doc:`custom lookups </ref/models/custom-lookups>` documentation.
 
+Improvements to ``Form`` error handling
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``Form.add_error()``
+^^^^^^^^^^^^^^^^^^^^
+
+Previously there were two main patterns for handling errors in forms:
+
+* Raising a :exc:`~django.core.exceptions.ValidationError` from within certain
+  functions (e.g. ``Field.clean()``, ``Form.clean_<fieldname>()``, or
+  ``Form.clean()`` for non-field errors.)
+
+* Fiddling with ``Form._errors`` when targeting a specific field in
+  ``Form.clean()`` or adding errors from outside of a "clean" method
+  (e.g. directly from a view).
+
+Using the former pattern was straightforward since the form can guess from the
+context (i.e. which method raised the exception) where the errors belong and
+automatically process them. This remains the canonical way of adding errors
+when possible. However the latter was fiddly and error-prone, since the burden
+of handling edge cases fell on the user.
+
+The new :meth:`~django.forms.Form.add_error()` method allows adding errors
+to specific form fields from anywhere without having to worry about the details
+such as creating instances of ``django.forms.utils.ErrorList`` or dealing with
+``Form.cleaned_data``. This new API replaces manipulating ``Form._errors``
+which now becomes a private API.
+
+See :ref:`validating-fields-with-clean` for an example using
+``Form.add_error()``.
+
+Error metadata
+^^^^^^^^^^^^^^
+
+The :exc:`~django.core.exceptions.ValidationError` constructor accepts metadata
+such as error ``code`` or ``params`` which are then available for interpolating
+into the error message (see :ref:`raising-validation-error` for more details);
+however, before Django 1.7 those metadata were discarded as soon as the errors
+were added to :attr:`Form.errors <django.forms.Form.errors>`.
+
+:attr:`Form.errors <django.forms.Form.errors>` and
+``django.forms.utils.ErrorList`` now store the ``ValidationError`` instances
+so these metadata can be retrieved at any time through the new
+:meth:`Form.errors.as_data <django.forms.Form.errors.as_data()>` method.
+
+The retrieved ``ValidationError`` instances can then be identified thanks to
+their error ``code`` which enables things like rewriting the error's message
+or writing custom logic in a view when a given error is present. It can also
+be used to serialize the errors in a custom format such as XML.
+
+The new :meth:`Form.errors.as_json() <django.forms.Form.errors.as_json()>`
+method is a convenience method which returns error messages along with error
+codes serialized as JSON. ``as_json()`` uses ``as_data()`` and gives an idea
+of how the new system could be extended.
+
+Error containers and backward compatibility
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Heavy changes to the various error containers were necessary in order
+to support the features above, specifically
+:attr:`Form.errors <django.forms.Form.errors>`,
+``django.forms.utils.ErrorList``, and the internal storages of
+:exc:`~django.core.exceptions.ValidationError`. These containers which used
+to store error strings now store ``ValidationError`` instances and public APIs
+have been adapted to make this as transparent as possible, but if you've been
+using private APIs, some of the changes are backwards incompatible; see
+:ref:`validation-error-constructor-and-internal-storage` for more details.
+
 Minor features
 ~~~~~~~~~~~~~~
 
@@ -562,15 +630,6 @@ Forms
 * It's now possible to remove a field from a ``Form`` when subclassing by
   setting the name to ``None``.
 
-* The new :meth:`~django.forms.Form.add_error()` method allows adding errors
-  to specific form fields.
-
-* The dict-like attribute :attr:`~django.forms.Form.errors` now has two new
-  methods :meth:`~django.forms.Form.errors.as_data()` and
-  :meth:`~django.forms.Form.errors.as_json()`. The former returns a ``dict``
-  that maps fields to their original errors, complete with all metadata
-  (error code and params), the latter returns the errors serialized as json.
-
 * It's now possible to customize the error messages for ``ModelForm``’s
   ``unique``, ``unique_for_date``, and ``unique_together`` constraints.
   In order to support ``unique_together`` or any other ``NON_FIELD_ERROR``,
@@ -1005,6 +1064,65 @@ This brings discovery of management commands in line with other parts of
 Django that rely on the order of :setting:`INSTALLED_APPS`, such as static
 files, templates, and translations.
 
+.. _validation-error-constructor-and-internal-storage:
+
+``ValidationError`` constructor and internal storage
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The behavior of the ``ValidationError`` constructor has changed when it
+receives a container of errors as an argument (e.g. a ``list`` or an
+``ErrorList``):
+
+* It converts any strings it finds to instances of ``ValidationError``
+  before adding them to its internal storage.
+
+* It doesn't store the given container but rather copies its content to its
+  own internal storage; previously the container itself was added to the
+  ``ValidationError`` instance and used as internal storage.
+
+This means that if you access the ``ValidationError`` internal storages, such
+as ``error_list``; ``error_dict``; or the return value of
+``update_error_dict()`` you may find instances of ``ValidationError`` where you
+would have previously found strings.
+
+Also if you directly assigned the return value of ``update_error_dict()``
+to ``Form._errors`` you may inadvertently add `list` instances where
+``ErrorList`` instances are expected. This is a problem because unlike a
+simple `list`, an ``ErrorList`` knows how to handle instances of
+``ValidationError``.
+
+Most use-cases that warranted using these private APIs are now covered by
+the newly introduced :meth:`Form.add_error() <django.forms.Form.add_error()>`
+method::
+
+    # Old pattern:
+    try:
+        # ...
+    except ValidationError as e:
+        self._errors = e.update_error_dict(self._errors)
+
+    # New pattern:
+    try:
+        # ...
+    except ValidationError as e:
+        self.add_error(None, e)
+
+If you need both Django <= 1.6 and 1.7 compatibility you can't use
+:meth:`Form.add_error() <django.forms.Form.add_error()>` since it
+wasn't available before Django 1.7, but you can use the following
+workaround to convert any ``list`` into ``ErrorList``::
+
+    try:
+        # ...
+    except ValidationError as e:
+        self._errors = e.update_error_dict(self._errors)
+
+    # Additional code to ensure ``ErrorDict`` is exclusively
+    # composed of ``ErrorList`` instances.
+    for field, error_list in self._errors.items():
+        if not isinstance(error_list, self.error_class):
+            self._errors[field] = self.error_class(error_list)
+
 Behavior of ``LocMemCache`` regarding pickle errors
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

tests/forms_tests/tests/test_forms.py

@@ -740,6 +740,53 @@ class FormsTestCase(TestCase):
         with six.assertRaisesRegex(self, ValueError, "has no field named"):
             f.add_error('missing_field', 'Some error.')
 
+    def test_update_error_dict(self):
+        class CodeForm(Form):
+            code = CharField(max_length=10)
+
+            def clean(self):
+                try:
+                    raise ValidationError({'code': [ValidationError('Code error 1.')]})
+                except ValidationError as e:
+                    self._errors = e.update_error_dict(self._errors)
+
+                try:
+                    raise ValidationError({'code': [ValidationError('Code error 2.')]})
+                except ValidationError as e:
+                    self._errors = e.update_error_dict(self._errors)
+
+                try:
+                    raise ValidationError({'code': forms.ErrorList(['Code error 3.'])})
+                except ValidationError as e:
+                    self._errors = e.update_error_dict(self._errors)
+
+                try:
+                    raise ValidationError('Non-field error 1.')
+                except ValidationError as e:
+                    self._errors = e.update_error_dict(self._errors)
+
+                try:
+                    raise ValidationError([ValidationError('Non-field error 2.')])
+                except ValidationError as e:
+                    self._errors = e.update_error_dict(self._errors)
+
+                # Ensure that the newly added list of errors is an instance of ErrorList.
+                for field, error_list in self._errors.items():
+                    if not isinstance(error_list, self.error_class):
+                        self._errors[field] = self.error_class(error_list)
+
+        form = CodeForm({'code': 'hello'})
+        # Trigger validation.
+        self.assertFalse(form.is_valid())
+
+        # Check that update_error_dict didn't lose track of the ErrorDict type.
+        self.assertTrue(isinstance(form._errors, forms.ErrorDict))
+
+        self.assertEqual(dict(form.errors), {
+            'code': ['Code error 1.', 'Code error 2.', 'Code error 3.'],
+            NON_FIELD_ERRORS: ['Non-field error 1.', 'Non-field error 2.'],
+        })
+
     def test_has_error(self):
         class UserRegistration(Form):
             username = CharField(max_length=10)