From 1b7634a0d0e21faba71a27ae7951d7cb7aec0e49 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Sat, 15 Jun 2013 22:34:25 +0200 Subject: [PATCH] Fixed #20464 -- Added a `total_error_count` method on formsets. Thanks to frog32 for the report and to Tim Graham for the review. --- .../admin/templates/admin/change_list.html | 2 +- django/forms/formsets.py | 7 + docs/releases/1.6.txt | 3 + docs/topics/forms/formsets.txt | 17 ++ tests/forms_tests/tests/test_formsets.py | 154 +++++++++--------- 5 files changed, 106 insertions(+), 77 deletions(-) diff --git a/django/contrib/admin/templates/admin/change_list.html b/django/contrib/admin/templates/admin/change_list.html index 5d1a6b2714..200b4cd74d 100644 --- a/django/contrib/admin/templates/admin/change_list.html +++ b/django/contrib/admin/templates/admin/change_list.html @@ -64,7 +64,7 @@ {% endblock %} {% if cl.formset.errors %}

- {% if cl.formset.errors|length == 1 %}{% trans "Please correct the error below." %}{% else %}{% trans "Please correct the errors below." %}{% endif %} + {% if cl.formset.total_error_count == 1 %}{% trans "Please correct the error below." %}{% else %}{% trans "Please correct the errors below." %}{% endif %}

{{ cl.formset.non_form_errors }} {% endif %} diff --git a/django/forms/formsets.py b/django/forms/formsets.py index fd98c43405..79e1c2298a 100644 --- a/django/forms/formsets.py +++ b/django/forms/formsets.py @@ -263,6 +263,13 @@ class BaseFormSet(object): self.full_clean() return self._errors + def total_error_count(self): + """ + Returns the number of errors across all forms in the formset. + """ + return len(self.non_form_errors()) +\ + sum(len(form_errors) for form_errors in self.errors) + def _should_delete_form(self, form): """ Returns whether or not the form was marked for deletion. diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index 458ccdc2a5..7d449edbe5 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -315,6 +315,9 @@ Minor features :class:`~django.contrib.admin.InlineModelAdmin` may be overridden to customize the extra and maximum number of inline forms. +* Formsets now have a + :meth:`~django.forms.formsets.BaseFormSet.total_error_count` method. + Backwards incompatible changes in 1.6 ===================================== diff --git a/docs/topics/forms/formsets.txt b/docs/topics/forms/formsets.txt index 77341cf55d..29139c5dea 100644 --- a/docs/topics/forms/formsets.txt +++ b/docs/topics/forms/formsets.txt @@ -164,6 +164,23 @@ As we can see, ``formset.errors`` is a list whose entries correspond to the forms in the formset. Validation was performed for each of the two forms, and the expected error message appears for the second item. +.. currentmodule:: django.forms.formsets.BaseFormSet + +.. method:: total_error_count(self) + +.. versionadded:: 1.6 + +To check how many errors there are in the formset, we can use the +``total_error_count`` method:: + + >>> # Using the previous example + >>> formset.errors + [{}, {'pub_date': [u'This field is required.']}] + >>> len(formset.errors) + 2 + >>> formset.total_error_count() + 1 + We can also check if form data differs from the initial data (i.e. the form was sent without any data):: diff --git a/tests/forms_tests/tests/test_formsets.py b/tests/forms_tests/tests/test_formsets.py index 1e9e7db30c..b26017bc78 100644 --- a/tests/forms_tests/tests/test_formsets.py +++ b/tests/forms_tests/tests/test_formsets.py @@ -55,81 +55,82 @@ SplitDateTimeFormSet = formset_factory(SplitDateTimeForm) class FormsFormsetTestCase(TestCase): + + def make_choiceformset(self, formset_data=None, formset_class=ChoiceFormSet, + total_forms=None, initial_forms=0, max_num_forms=0, **kwargs): + """ + Make a ChoiceFormset from the given formset_data. + The data should be given as a list of (choice, votes) tuples. + """ + kwargs.setdefault('prefix', 'choices') + kwargs.setdefault('auto_id', False) + + if formset_data is None: + return formset_class(**kwargs) + + if total_forms is None: + total_forms = len(formset_data) + + def prefixed(*args): + args = (kwargs['prefix'],) + args + return '-'.join(args) + + data = { + prefixed('TOTAL_FORMS'): str(total_forms), + prefixed('INITIAL_FORMS'): str(initial_forms), + prefixed('MAX_NUM_FORMS'): str(max_num_forms), + } + for i, (choice, votes) in enumerate(formset_data): + data[prefixed(str(i), 'choice')] = choice + data[prefixed(str(i), 'votes')] = votes + + return formset_class(data, **kwargs) + def test_basic_formset(self): # A FormSet constructor takes the same arguments as Form. Let's create a FormSet # for adding data. By default, it displays 1 blank form. It can display more, # but we'll look at how to do so later. - formset = ChoiceFormSet(auto_id=False, prefix='choices') + formset = self.make_choiceformset() + self.assertHTMLEqual(str(formset), """ Choice: Votes:""") - # On thing to note is that there needs to be a special value in the data. This - # value tells the FormSet how many forms were displayed so it can tell how - # many forms it needs to clean and validate. You could use javascript to create - # new forms on the client side, but they won't get validated unless you increment - # the TOTAL_FORMS field appropriately. - - data = { - 'choices-TOTAL_FORMS': '1', # the number of forms rendered - 'choices-INITIAL_FORMS': '0', # the number of forms with initial data - 'choices-MAX_NUM_FORMS': '0', # max number of forms - 'choices-0-choice': 'Calexico', - 'choices-0-votes': '100', - } # We treat FormSet pretty much like we would treat a normal Form. FormSet has an # is_valid method, and a cleaned_data or errors attribute depending on whether all # the forms passed validation. However, unlike a Form instance, cleaned_data and # errors will be a list of dicts rather than just a single dict. - formset = ChoiceFormSet(data, auto_id=False, prefix='choices') + formset = self.make_choiceformset([('Calexico', '100')]) self.assertTrue(formset.is_valid()) self.assertEqual([form.cleaned_data for form in formset.forms], [{'votes': 100, 'choice': 'Calexico'}]) # If a FormSet was not passed any data, its is_valid and has_changed # methods should return False. - formset = ChoiceFormSet() + formset = self.make_choiceformset() self.assertFalse(formset.is_valid()) self.assertFalse(formset.has_changed()) def test_formset_validation(self): # FormSet instances can also have an error attribute if validation failed for # any of the forms. - - data = { - 'choices-TOTAL_FORMS': '1', # the number of forms rendered - 'choices-INITIAL_FORMS': '0', # the number of forms with initial data - 'choices-MAX_NUM_FORMS': '0', # max number of forms - 'choices-0-choice': 'Calexico', - 'choices-0-votes': '', - } - - formset = ChoiceFormSet(data, auto_id=False, prefix='choices') + formset = self.make_choiceformset([('Calexico', '')]) self.assertFalse(formset.is_valid()) self.assertEqual(formset.errors, [{'votes': ['This field is required.']}]) def test_formset_has_changed(self): # FormSet instances has_changed method will be True if any data is # passed to his forms, even if the formset didn't validate - data = { - 'choices-TOTAL_FORMS': '1', # the number of forms rendered - 'choices-INITIAL_FORMS': '0', # the number of forms with initial data - 'choices-MAX_NUM_FORMS': '0', # max number of forms - 'choices-0-choice': '', - 'choices-0-votes': '', - } - blank_formset = ChoiceFormSet(data, auto_id=False, prefix='choices') + blank_formset = self.make_choiceformset([('', '')]) self.assertFalse(blank_formset.has_changed()) # invalid formset test - data['choices-0-choice'] = 'Calexico' - invalid_formset = ChoiceFormSet(data, auto_id=False, prefix='choices') + invalid_formset = self.make_choiceformset([('Calexico', '')]) self.assertFalse(invalid_formset.is_valid()) self.assertTrue(invalid_formset.has_changed()) # valid formset test - data['choices-0-votes'] = '100' - valid_formset = ChoiceFormSet(data, auto_id=False, prefix='choices') + valid_formset = self.make_choiceformset([('Calexico', '100')]) self.assertTrue(valid_formset.is_valid()) self.assertTrue(valid_formset.has_changed()) @@ -139,7 +140,7 @@ class FormsFormsetTestCase(TestCase): # an extra blank form is included. initial = [{'choice': 'Calexico', 'votes': 100}] - formset = ChoiceFormSet(initial=initial, auto_id=False, prefix='choices') + formset = self.make_choiceformset(initial=initial) form_output = [] for form in formset.forms: @@ -151,18 +152,7 @@ class FormsFormsetTestCase(TestCase):
  • Votes:
    • """) # Let's simulate what would happen if we submitted this form. - - data = { - 'choices-TOTAL_FORMS': '2', # the number of forms rendered - 'choices-INITIAL_FORMS': '1', # the number of forms with initial data - 'choices-MAX_NUM_FORMS': '0', # max number of forms - 'choices-0-choice': 'Calexico', - 'choices-0-votes': '100', - 'choices-1-choice': '', - 'choices-1-votes': '', - } - - formset = ChoiceFormSet(data, auto_id=False, prefix='choices') + formset = self.make_choiceformset([('Calexico', '100'), ('', '')], initial_forms=1) self.assertTrue(formset.is_valid()) self.assertEqual([form.cleaned_data for form in formset.forms], [{'votes': 100, 'choice': 'Calexico'}, {}]) @@ -172,18 +162,7 @@ class FormsFormsetTestCase(TestCase): # one of the fields of a blank form though, it will be validated. We may want to # required that at least x number of forms are completed, but we'll show how to # handle that later. - - data = { - 'choices-TOTAL_FORMS': '2', # the number of forms rendered - 'choices-INITIAL_FORMS': '1', # the number of forms with initial data - 'choices-MAX_NUM_FORMS': '0', # max number of forms - 'choices-0-choice': 'Calexico', - 'choices-0-votes': '100', - 'choices-1-choice': 'The Decemberists', - 'choices-1-votes': '', # missing value - } - - formset = ChoiceFormSet(data, auto_id=False, prefix='choices') + formset = self.make_choiceformset([('Calexico', '100'), ('The Decemberists', '')], initial_forms=1) self.assertFalse(formset.is_valid()) self.assertEqual(formset.errors, [{}, {'votes': ['This field is required.']}]) @@ -191,18 +170,7 @@ class FormsFormsetTestCase(TestCase): # If we delete data that was pre-filled, we should get an error. Simply removing # data from form fields isn't the proper way to delete it. We'll see how to # handle that case later. - - data = { - 'choices-TOTAL_FORMS': '2', # the number of forms rendered - 'choices-INITIAL_FORMS': '1', # the number of forms with initial data - 'choices-MAX_NUM_FORMS': '0', # max number of forms - 'choices-0-choice': '', # deleted value - 'choices-0-votes': '', # deleted value - 'choices-1-choice': '', - 'choices-1-votes': '', - } - - formset = ChoiceFormSet(data, auto_id=False, prefix='choices') + formset = self.make_choiceformset([('', ''), ('', '')], initial_forms=1) self.assertFalse(formset.is_valid()) self.assertEqual(formset.errors, [{'votes': ['This field is required.'], 'choice': ['This field is required.']}, {}]) @@ -1027,6 +995,40 @@ class FormsFormsetTestCase(TestCase): self.assertTrue(formset.is_valid()) + def test_formset_total_error_count(self): + """A valid formset should have 0 total errors.""" + data = [ # formset_data, expected error count + ([('Calexico', '100')], 0), + ([('Calexico', '')], 1), + ([('', 'invalid')], 2), + ([('Calexico', '100'), ('Calexico', '')], 1), + ([('Calexico', ''), ('Calexico', '')], 2), + ] + + for formset_data, expected_error_count in data: + formset = self.make_choiceformset(formset_data) + self.assertEqual(formset.total_error_count(), expected_error_count) + + def test_formset_total_error_count_with_non_form_errors(self): + data = { + 'choices-TOTAL_FORMS': '2', # the number of forms rendered + 'choices-INITIAL_FORMS': '0', # the number of forms with initial data + 'choices-MAX_NUM_FORMS': '2', # max number of forms - should be ignored + 'choices-0-choice': 'Zero', + 'choices-0-votes': '0', + 'choices-1-choice': 'One', + 'choices-1-votes': '1', + } + + ChoiceFormSet = formset_factory(Choice, extra=1, max_num=1, validate_max=True) + formset = ChoiceFormSet(data, auto_id=False, prefix='choices') + self.assertEqual(formset.total_error_count(), 1) + + data['choices-1-votes'] = '' + formset = ChoiceFormSet(data, auto_id=False, prefix='choices') + self.assertEqual(formset.total_error_count(), 2) + + data = { 'choices-TOTAL_FORMS': '1', # the number of forms rendered 'choices-INITIAL_FORMS': '0', # the number of forms with initial data @@ -1087,7 +1089,7 @@ class TestIsBoundBehavior(TestCase): self.assertEqual([{}], formset.cleaned_data) - def test_form_errors_are_cought_by_formset(self): + def test_form_errors_are_caught_by_formset(self): data = { 'form-TOTAL_FORMS': '2', 'form-INITIAL_FORMS': '0',