summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJon Dufresne <jon.dufresne@gmail.com>2020-11-05 01:40:41 -0800
committerGitHub <noreply@github.com>2020-11-05 10:40:41 +0100
commit859cd7c6b43bf70e2852eda10f635c70feeb397f (patch)
tree648e43ff9d72edcf162583a123fbdfe047fd0f02
parent76181308fb02e67794d0cc1471766a5d7e4c877e (diff)
downloaddjango-859cd7c6b43bf70e2852eda10f635c70feeb397f.tar.gz
Fixed #22276 -- Fixed crash when formset management form is invalid.
Co-authored-by: Patryk Zawadzki <patrys@room-303.com>
-rw-r--r--django/forms/formsets.py53
-rw-r--r--docs/releases/3.2.txt6
-rw-r--r--docs/topics/forms/formsets.txt37
-rw-r--r--tests/forms_tests/tests/test_formsets.py68
-rw-r--r--tests/model_formsets/tests.py9
5 files changed, 143 insertions, 30 deletions
diff --git a/django/forms/formsets.py b/django/forms/formsets.py
index c921da72f5..a89c35599f 100644
--- a/django/forms/formsets.py
+++ b/django/forms/formsets.py
@@ -6,7 +6,7 @@ from django.forms.widgets import HiddenInput, NumberInput
from django.utils.functional import cached_property
from django.utils.html import html_safe
from django.utils.safestring import mark_safe
-from django.utils.translation import gettext as _, ngettext
+from django.utils.translation import gettext_lazy as _, ngettext
__all__ = ('BaseFormSet', 'formset_factory', 'all_valid')
@@ -41,6 +41,14 @@ class ManagementForm(Form):
self.base_fields[MAX_NUM_FORM_COUNT] = IntegerField(required=False, widget=HiddenInput)
super().__init__(*args, **kwargs)
+ def clean(self):
+ cleaned_data = super().clean()
+ # When the management form is invalid, we don't know how many forms
+ # were submitted.
+ cleaned_data.setdefault(TOTAL_FORM_COUNT, 0)
+ cleaned_data.setdefault(INITIAL_FORM_COUNT, 0)
+ return cleaned_data
+
@html_safe
class BaseFormSet:
@@ -48,9 +56,16 @@ class BaseFormSet:
A collection of instances of the same Form class.
"""
ordering_widget = NumberInput
+ default_error_messages = {
+ 'missing_management_form': _(
+ 'ManagementForm data is missing or has been tampered with. Missing fields: '
+ '%(field_names)s. You may need to file a bug report if the issue persists.'
+ ),
+ }
def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
- initial=None, error_class=ErrorList, form_kwargs=None):
+ initial=None, error_class=ErrorList, form_kwargs=None,
+ error_messages=None):
self.is_bound = data is not None or files is not None
self.prefix = prefix or self.get_default_prefix()
self.auto_id = auto_id
@@ -62,6 +77,13 @@ class BaseFormSet:
self._errors = None
self._non_form_errors = None
+ messages = {}
+ for cls in reversed(type(self).__mro__):
+ messages.update(getattr(cls, 'default_error_messages', {}))
+ if error_messages is not None:
+ messages.update(error_messages)
+ self.error_messages = messages
+
def __str__(self):
return self.as_table()
@@ -88,18 +110,7 @@ class BaseFormSet:
"""Return the ManagementForm instance for this FormSet."""
if self.is_bound:
form = ManagementForm(self.data, auto_id=self.auto_id, prefix=self.prefix)
- if not form.is_valid():
- raise ValidationError(
- _(
- 'ManagementForm data is missing or has been tampered '
- 'with. Missing fields: %(field_names)s'
- ) % {
- 'field_names': ', '.join(
- form.add_prefix(field_name) for field_name in form.errors
- ),
- },
- code='missing_management_form',
- )
+ form.full_clean()
else:
form = ManagementForm(auto_id=self.auto_id, prefix=self.prefix, initial={
TOTAL_FORM_COUNT: self.total_form_count(),
@@ -327,6 +338,20 @@ class BaseFormSet:
if not self.is_bound: # Stop further processing.
return
+
+ if not self.management_form.is_valid():
+ error = ValidationError(
+ self.error_messages['missing_management_form'],
+ params={
+ 'field_names': ', '.join(
+ self.management_form.add_prefix(field_name)
+ for field_name in self.management_form.errors
+ ),
+ },
+ code='missing_management_form',
+ )
+ self._non_form_errors.append(error)
+
for i, form in enumerate(self.forms):
# Empty forms are unchanged forms beyond those with initial data.
if not form.has_changed() and i >= self.initial_form_count():
diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt
index e76223f6d4..f684486687 100644
--- a/docs/releases/3.2.txt
+++ b/docs/releases/3.2.txt
@@ -233,6 +233,12 @@ Forms
removal of the option to delete extra forms. See
:attr:`~.BaseFormSet.can_delete_extra` for more information.
+* :class:`~django.forms.formsets.BaseFormSet` now reports a user facing error,
+ rather than raising an exception, when the management form is missing or has
+ been tampered with. To customize this error message, pass the
+ ``error_messages`` argument with the key ``'missing_management_form'`` when
+ instantiating the formset.
+
Generic Views
~~~~~~~~~~~~~
diff --git a/docs/topics/forms/formsets.txt b/docs/topics/forms/formsets.txt
index 37f6e300c4..64f740a997 100644
--- a/docs/topics/forms/formsets.txt
+++ b/docs/topics/forms/formsets.txt
@@ -22,7 +22,7 @@ a formset out of an ``ArticleForm`` you would do::
>>> ArticleFormSet = formset_factory(ArticleForm)
You now have created a formset class named ``ArticleFormSet``.
-Instantiating the formset gives you the ability to iterate over the forms
+Instantiating the formset gives you the ability to iterate over the forms
in the formset and display them as you would with a regular form::
>>> formset = ArticleFormSet()
@@ -242,7 +242,7 @@ You may have noticed the additional data (``form-TOTAL_FORMS``,
in the formset's data above. This data is required for the
``ManagementForm``. This form is used by the formset to manage the
collection of forms contained in the formset. If you don't provide
-this management data, an exception will be raised::
+this management data, the formset will be invalid::
>>> data = {
... 'form-0-title': 'Test',
@@ -250,9 +250,7 @@ this management data, an exception will be raised::
... }
>>> formset = ArticleFormSet(data)
>>> formset.is_valid()
- Traceback (most recent call last):
- ...
- django.core.exceptions.ValidationError: ['ManagementForm data is missing or has been tampered with']
+ False
It is used to keep track of how many form instances are being displayed. If
you are adding new forms via JavaScript, you should increment the count fields
@@ -266,6 +264,11 @@ itself. When rendering a formset in a template, you can include all
the management data by rendering ``{{ my_formset.management_form }}``
(substituting the name of your formset as appropriate).
+.. versionchanged:: 3.2
+
+ ``formset.is_valid()`` now returns ``False`` rather than raising an
+ exception when the management form is missing or has been tampered with.
+
``total_form_count`` and ``initial_form_count``
-----------------------------------------------
@@ -287,6 +290,30 @@ sure you understand what they do before doing so.
a form instance with a prefix of ``__prefix__`` for easier use in dynamic
forms with JavaScript.
+``error_messages``
+------------------
+
+.. versionadded:: 3.2
+
+The ``error_messages`` argument lets you override the default messages that the
+formset will raise. Pass in a dictionary with keys matching the error messages
+you want to override. For example, here is the default error message when the
+management form is missing::
+
+ >>> formset = ArticleFormSet({})
+ >>> formset.is_valid()
+ False
+ >>> formset.non_form_errors()
+ ['ManagementForm data is missing or has been tampered with. Missing fields: form-TOTAL_FORMS, form-INITIAL_FORMS. You may need to file a bug report if the issue persists.']
+
+And here is a custom error message::
+
+ >>> formset = ArticleFormSet({}, error_messages={'missing_management_form': 'Sorry, something went wrong.'})
+ >>> formset.is_valid()
+ False
+ >>> formset.non_form_errors()
+ ['Sorry, something went wrong.']
+
Custom formset validation
-------------------------
diff --git a/tests/forms_tests/tests/test_formsets.py b/tests/forms_tests/tests/test_formsets.py
index a11c183f86..889560aa74 100644
--- a/tests/forms_tests/tests/test_formsets.py
+++ b/tests/forms_tests/tests/test_formsets.py
@@ -1300,13 +1300,69 @@ ArticleFormSet = formset_factory(ArticleForm)
class TestIsBoundBehavior(SimpleTestCase):
- def test_no_data_raises_validation_error(self):
- msg = (
- 'ManagementForm data is missing or has been tampered with. '
- 'Missing fields: form-TOTAL_FORMS, form-INITIAL_FORMS'
+ def test_no_data_error(self):
+ formset = ArticleFormSet({})
+ self.assertIs(formset.is_valid(), False)
+ self.assertEqual(
+ formset.non_form_errors(),
+ [
+ 'ManagementForm data is missing or has been tampered with. '
+ 'Missing fields: form-TOTAL_FORMS, form-INITIAL_FORMS. '
+ 'You may need to file a bug report if the issue persists.',
+ ],
+ )
+ self.assertEqual(formset.errors, [])
+ # Can still render the formset.
+ self.assertEqual(
+ str(formset),
+ '<tr><td colspan="2">'
+ '<ul class="errorlist nonfield">'
+ '<li>(Hidden field TOTAL_FORMS) This field is required.</li>'
+ '<li>(Hidden field INITIAL_FORMS) This field is required.</li>'
+ '</ul>'
+ '<input type="hidden" name="form-TOTAL_FORMS" id="id_form-TOTAL_FORMS">'
+ '<input type="hidden" name="form-INITIAL_FORMS" id="id_form-INITIAL_FORMS">'
+ '<input type="hidden" name="form-MIN_NUM_FORMS" id="id_form-MIN_NUM_FORMS">'
+ '<input type="hidden" name="form-MAX_NUM_FORMS" id="id_form-MAX_NUM_FORMS">'
+ '</td></tr>\n'
)
- with self.assertRaisesMessage(ValidationError, msg):
- ArticleFormSet({}).is_valid()
+
+ def test_management_form_invalid_data(self):
+ data = {
+ 'form-TOTAL_FORMS': 'two',
+ 'form-INITIAL_FORMS': 'one',
+ }
+ formset = ArticleFormSet(data)
+ self.assertIs(formset.is_valid(), False)
+ self.assertEqual(
+ formset.non_form_errors(),
+ [
+ 'ManagementForm data is missing or has been tampered with. '
+ 'Missing fields: form-TOTAL_FORMS, form-INITIAL_FORMS. '
+ 'You may need to file a bug report if the issue persists.',
+ ],
+ )
+ self.assertEqual(formset.errors, [])
+ # Can still render the formset.
+ self.assertEqual(
+ str(formset),
+ '<tr><td colspan="2">'
+ '<ul class="errorlist nonfield">'
+ '<li>(Hidden field TOTAL_FORMS) Enter a whole number.</li>'
+ '<li>(Hidden field INITIAL_FORMS) Enter a whole number.</li>'
+ '</ul>'
+ '<input type="hidden" name="form-TOTAL_FORMS" value="two" id="id_form-TOTAL_FORMS">'
+ '<input type="hidden" name="form-INITIAL_FORMS" value="one" id="id_form-INITIAL_FORMS">'
+ '<input type="hidden" name="form-MIN_NUM_FORMS" id="id_form-MIN_NUM_FORMS">'
+ '<input type="hidden" name="form-MAX_NUM_FORMS" id="id_form-MAX_NUM_FORMS">'
+ '</td></tr>\n',
+ )
+
+ def test_customize_management_form_error(self):
+ formset = ArticleFormSet({}, error_messages={'missing_management_form': 'customized'})
+ self.assertIs(formset.is_valid(), False)
+ self.assertEqual(formset.non_form_errors(), ['customized'])
+ self.assertEqual(formset.errors, [])
def test_with_management_data_attrs_work_fine(self):
data = {
diff --git a/tests/model_formsets/tests.py b/tests/model_formsets/tests.py
index aaf76fa7ec..9c06d45339 100644
--- a/tests/model_formsets/tests.py
+++ b/tests/model_formsets/tests.py
@@ -4,7 +4,7 @@ from datetime import date
from decimal import Decimal
from django import forms
-from django.core.exceptions import ImproperlyConfigured, ValidationError
+from django.core.exceptions import ImproperlyConfigured
from django.db import models
from django.forms.models import (
BaseModelFormSet, _get_foreign_key, inlineformset_factory,
@@ -1783,11 +1783,10 @@ class ModelFormsetTest(TestCase):
[{'id': ['Select a valid choice. That choice is not one of the available choices.']}],
)
- def test_initial_form_count_empty_data_raises_validation_error(self):
+ def test_initial_form_count_empty_data(self):
AuthorFormSet = modelformset_factory(Author, fields='__all__')
- msg = 'ManagementForm data is missing or has been tampered with'
- with self.assertRaisesMessage(ValidationError, msg):
- AuthorFormSet({}).initial_form_count()
+ formset = AuthorFormSet({})
+ self.assertEqual(formset.initial_form_count(), 0)
class TestModelFormsetOverridesTroughFormMeta(TestCase):