Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion django/contrib/comments/forms.py
Expand Up @@ -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]
Expand Down
6 changes: 3 additions & 3 deletions django/forms/forms.py
Expand Up @@ -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 = {}
Expand Down Expand Up @@ -291,15 +291,15 @@ 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]

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
Expand Down
8 changes: 4 additions & 4 deletions django/forms/models.py
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions 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
Expand All @@ -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.
Expand All @@ -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.

Expand Down
30 changes: 11 additions & 19 deletions docs/ref/forms/validation.txt
Expand Up @@ -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?
Expand Down Expand Up @@ -432,23 +426,21 @@ 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.
del cleaned_data["cc_myself"]
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.
6 changes: 6 additions & 0 deletions docs/releases/1.7.txt
Expand Up @@ -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
=====================================

Expand Down
23 changes: 21 additions & 2 deletions 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
Expand Down Expand Up @@ -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>'
)