diff --git a/django/forms/formsets.py b/django/forms/formsets.py index c921da72f5b..a89c35599f8 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 e76223f6d43..f6844866870 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 37f6e300c4e..64f740a9979 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 a11c183f86e..889560aa742 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.', + ], ) - with self.assertRaisesMessage(ValidationError, msg): - ArticleFormSet({}).is_valid() + self.assertEqual(formset.errors, []) + # Can still render the formset. + self.assertEqual( + str(formset), + '' + '' + '' + '' + '' + '' + '\n' + ) + + 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), + '' + '' + '' + '' + '' + '' + '\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 aaf76fa7ec1..9c06d45339d 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):