Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixed #5335 -- Changed ErrorDict to be a subclass of defaultdict. #1481

Closed
wants to merge 1 commit into from

3 participants

@loic
Collaborator

No description provided.

docs/ref/forms/validation.txt
((4 lines not shown))
-extra design effort to create a sensible form display. The details are worth
-noting, however. Firstly, earlier we mentioned that you might need to check if
-the field name keys already exist in the ``_errors`` dictionary. In this case,
-since we know the fields exist in ``self.cleaned_data``, they must have been
-valid when cleaned as individual fields, so there will be no corresponding
-entries in ``_errors``.
-
-Secondly, once we have decided that the combined data in the two fields we are
-considering aren't valid, we must remember to remove them from the
-``cleaned_data``. `cleaned_data`` is present even if the form doesn't
-validate, but it contains only field values that did validate.
+extra design effort to create a sensible form display. Once we have decided
+that the combined data in the two fields we are considering aren't valid, we
+must remember to remove them from the ``cleaned_data``.
+
+.. versionchanged:: 1.5
@timgraham Owner

I think you resolved a merge conflict incorrectly. "versionchanged:: 1.5" annotations have been removed in master.

@loic Collaborator
loic added a note

Indeed, I thought it was an artifact from the rewind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mjtamlyn
Collaborator

Closed as discussed on ticket and ML - the main reason being that defaultdicts don't behave nicely with templates.

@mjtamlyn mjtamlyn closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  django/contrib/comments/forms.py
@@ -30,7 +30,7 @@ def __init__(self, target_object, data=None, initial=None):
def security_errors(self):
"""Return just those errors associated with security"""
- errors = ErrorDict()
+ errors = ErrorDict(self.error_class)
for f in ["honeypot", "timestamp", "security_hash"]:
if f in self.errors:
errors[f] = self.errors[f]
View
6 django/forms/forms.py
@@ -262,7 +262,7 @@ def full_clean(self):
Cleans all of self.data and populates self._errors and
self.cleaned_data.
"""
- self._errors = ErrorDict()
+ self._errors = ErrorDict(self.error_class)
if not self.is_bound: # Stop further processing.
return
self.cleaned_data = {}
@@ -291,7 +291,7 @@ def _clean_fields(self):
value = getattr(self, 'clean_%s' % name)()
self.cleaned_data[name] = value
except ValidationError as e:
- self._errors[name] = self.error_class(e.messages)
+ self._errors[name].extend(e.messages)
if name in self.cleaned_data:
del self.cleaned_data[name]
@@ -299,7 +299,7 @@ def _clean_form(self):
try:
cleaned_data = self.clean()
except ValidationError as e:
- self._errors[NON_FIELD_ERRORS] = self.error_class(e.messages)
+ self._errors[NON_FIELD_ERRORS].extend(e.messages)
else:
if cleaned_data is not None:
self.cleaned_data = cleaned_data
View
8 django/forms/models.py
@@ -327,13 +327,13 @@ def _update_errors(self, errors):
message_dict = errors.message_dict
for k, v in message_dict.items():
if k != NON_FIELD_ERRORS:
- self._errors.setdefault(k, self.error_class()).extend(v)
+ self._errors[k].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)
+ self._errors[NON_FIELD_ERRORS].extend(messages)
def _get_validation_exclusions(self):
"""
@@ -638,7 +638,7 @@ def validate_unique(self):
# poke error messages into the right places and mark
# the form as invalid
errors.append(self.get_unique_error_message(unique_check))
- form._errors[NON_FIELD_ERRORS] = self.error_class([self.get_form_error()])
+ form._errors[NON_FIELD_ERRORS].append(self.get_form_error())
# remove the data from the cleaned_data dict since it was invalid
for field in unique_check:
if field in form.cleaned_data:
@@ -667,7 +667,7 @@ def validate_unique(self):
# poke error messages into the right places and mark
# the form as invalid
errors.append(self.get_date_error_message(date_check))
- form._errors[NON_FIELD_ERRORS] = self.error_class([self.get_form_error()])
+ form._errors[NON_FIELD_ERRORS].append(self.get_form_error())
# remove the data from the cleaned_data dict since it was invalid
del form.cleaned_data[field]
# mark the data as seen
View
5 django/forms/util.py
@@ -1,5 +1,7 @@
from __future__ import unicode_literals
+from collections import defaultdict
+import sys
import warnings
from django.conf import settings
@@ -8,7 +10,6 @@
from django.utils import timezone
from django.utils.translation import ugettext_lazy as _
from django.utils import six
-import sys
# Import ValidationError so that it can be imported from this
# module to maintain backwards compatibility.
@@ -31,7 +32,7 @@ def flatatt(attrs):
return format_html_join('', ' {0}="{1}"', sorted(attrs.items()))
@python_2_unicode_compatible
-class ErrorDict(dict):
+class ErrorDict(defaultdict):
"""
A collection of errors that knows how to display itself in various formats.
View
30 docs/ref/forms/validation.txt
@@ -226,12 +226,6 @@ that has an error. Each value in the dictionary is a
display itself in different ways. So you can treat ``_errors`` as a dictionary
mapping field names to lists.
-If you want to add a new error to a particular field, you should check whether
-the key already exists in ``self._errors`` or not. If not, create a new entry
-for the given key, holding an empty ``ErrorList`` instance. In either case,
-you can then append your error message to the list for the field name in
-question and it will be displayed when the form is displayed.
-
There is an example of modifying ``self._errors`` in the following section.
.. admonition:: What's in a name?
@@ -432,8 +426,8 @@ sample) looks like this::
# We know these are not in self._errors now (see discussion
# below).
msg = u"Must put 'help' in subject when cc'ing yourself."
- self._errors["cc_myself"] = self.error_class([msg])
- self._errors["subject"] = self.error_class([msg])
+ self._errors["cc_myself"].append(msg)
+ self._errors["subject"].append(msg)
# These fields are no longer valid. Remove them from the
# cleaned data.
@@ -441,14 +435,12 @@ sample) looks like this::
del cleaned_data["subject"]
As you can see, this approach requires a bit more effort, not withstanding the
-extra design effort to create a sensible form display. The details are worth
-noting, however. Firstly, earlier we mentioned that you might need to check if
-the field name keys already exist in the ``_errors`` dictionary. In this case,
-since we know the fields exist in ``self.cleaned_data``, they must have been
-valid when cleaned as individual fields, so there will be no corresponding
-entries in ``_errors``.
-
-Secondly, once we have decided that the combined data in the two fields we are
-considering aren't valid, we must remember to remove them from the
-``cleaned_data``. `cleaned_data`` is present even if the form doesn't
-validate, but it contains only field values that did validate.
+extra design effort to create a sensible form display. Once we have decided
+that the combined data in the two fields we are considering aren't valid, we
+must remember to remove them from the ``cleaned_data``.
+
+.. versionchanged:: 1.7
+
+ Before adding new errors, it used to be necessary to check if a key already
+ existed in ``self._errors``, and if not, to create a new entry key holding an
+ empty ``ErrorList`` instance.
View
6 docs/releases/1.7.txt
@@ -159,6 +159,12 @@ Minor features
* The :ttag:`widthratio` template tag now accepts an "as" parameter to capture
the result in a variable.
+* ``django.forms.util.ErrorDict`` is now a subclass of
+ :class:`~collections.defaultdict` and the default factory for
+ :attr:`django.forms.Form.errors` is set to the ``error_class`` attribute of
+ the form instance. As a result it's no longer necessary to check if an
+ ``error_class`` already exist for a given field before adding errors.
+
Backwards incompatible changes in 1.7
=====================================
View
23 tests/forms_tests/tests/test_util.py
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
+from django import forms
from django.core.exceptions import ValidationError
from django.forms.util import flatatt, ErrorDict, ErrorList
from django.test import TestCase
@@ -61,7 +62,25 @@ def __str__(self): return "A very bad error."
'<ul class="errorlist"><li>Example of link: &lt;a href=&quot;http://www.example.com/&quot;&gt;example&lt;/a&gt;</li></ul>')
self.assertHTMLEqual(str(ErrorList([mark_safe(example)])),
'<ul class="errorlist"><li>Example of link: <a href="http://www.example.com/">example</a></li></ul>')
- self.assertHTMLEqual(str(ErrorDict({'name': example})),
+ self.assertHTMLEqual(str(ErrorDict(None, {'name': example})),
'<ul class="errorlist"><li>nameExample of link: &lt;a href=&quot;http://www.example.com/&quot;&gt;example&lt;/a&gt;</li></ul>')
- self.assertHTMLEqual(str(ErrorDict({'name': mark_safe(example)})),
+ self.assertHTMLEqual(str(ErrorDict(None, {'name': mark_safe(example)})),
'<ul class="errorlist"><li>nameExample of link: <a href="http://www.example.com/">example</a></li></ul>')
+
+ def test_errordict(self):
+ class MyForm(forms.Form):
+ pass
+
+ # Test that form
+ form = MyForm({})
+ self.assertEqual(str(form.errors), '')
+
+ # Test that a default `ErrorList` is created automatically.
+ form.errors['field'].append('error')
+ self.assertHTMLEqual(str(form.errors),
+ '<ul class="errorlist">'
+ ' <li>field'
+ ' <ul class="errorlist"><li>error</li></ul>'
+ ' </li>'
+ '</ul>'
+ )
Something went wrong with that request. Please try again.