From d64f192df188903ea5a1ab8dfd6679e5d52aaf9b Mon Sep 17 00:00:00 2001 From: Stephen Burrows Date: Thu, 15 May 2014 20:12:32 -0700 Subject: [PATCH] Fixed #22628 -- Took initial forms into account when combining FormSet.min_num and FormSet.extra. Forwardport of 79f15ab1ef from stable/1.7.x --- django/forms/formsets.py | 8 ++++---- tests/admin_inlines/tests.py | 3 +-- tests/forms_tests/tests/test_formsets.py | 6 +++++- tests/generic_inline_admin/tests.py | 5 ++--- tests/model_formsets/tests.py | 3 +-- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/django/forms/formsets.py b/django/forms/formsets.py index 2309fa00bd..07cbb41409 100644 --- a/django/forms/formsets.py +++ b/django/forms/formsets.py @@ -114,7 +114,7 @@ class BaseFormSet(object): return min(self.management_form.cleaned_data[TOTAL_FORM_COUNT], self.absolute_max) else: initial_forms = self.initial_form_count() - total_forms = initial_forms + self.extra + total_forms = max(initial_forms, self.min_num) + self.extra # Allow all existing related objects/inlines to be displayed, # but don't allow extra beyond max_num. if initial_forms > self.max_num >= 0: @@ -158,8 +158,9 @@ class BaseFormSet(object): defaults['initial'] = self.initial[i] except IndexError: pass - # Allow extra forms to be empty. - if i >= self.initial_form_count(): + # Allow extra forms to be empty, unless they're part of + # the minimum forms. + if i >= self.initial_form_count() and i >= self.min_num: defaults['empty_permitted'] = True defaults.update(kwargs) form = self.form(**defaults) @@ -422,7 +423,6 @@ def formset_factory(form, formset=BaseFormSet, extra=1, can_order=False, # limit is simply max_num + DEFAULT_MAX_NUM (which is 2*DEFAULT_MAX_NUM # if max_num is None in the first place) absolute_max = max_num + DEFAULT_MAX_NUM - extra += min_num attrs = {'form': form, 'extra': extra, 'can_order': can_order, 'can_delete': can_delete, 'min_num': min_num, 'max_num': max_num, diff --git a/tests/admin_inlines/tests.py b/tests/admin_inlines/tests.py index 7025158aab..5d97381f18 100644 --- a/tests/admin_inlines/tests.py +++ b/tests/admin_inlines/tests.py @@ -247,7 +247,6 @@ class TestInline(TestCase): def test_custom_min_num(self): """ Ensure that get_min_num is called and used correctly. - See #22628 - this will change when that's fixed. """ bt_head = BinaryTree.objects.create(name="Tree Head") BinaryTree.objects.create(name="First Child", parent=bt_head) @@ -277,7 +276,7 @@ class TestInline(TestCase): request.user = User(username='super', is_superuser=True) response = modeladmin.changeform_view(request, object_id=str(bt_head.id)) self.assertContains(response, min_forms % 5) - self.assertContains(response, total_forms % 9) + self.assertContains(response, total_forms % 8) def test_inline_nonauto_noneditable_pk(self): response = self.client.get('/admin/admin_inlines/author/add/') diff --git a/tests/forms_tests/tests/test_formsets.py b/tests/forms_tests/tests/test_formsets.py index 52dfdf4044..6b82f57343 100644 --- a/tests/forms_tests/tests/test_formsets.py +++ b/tests/forms_tests/tests/test_formsets.py @@ -216,7 +216,7 @@ class FormsFormsetTestCase(TestCase): def test_min_num_displaying_more_than_one_blank_form(self): # We can also display more than 1 empty form passing min_num argument - # to formset_factory. It will increment the extra argument + # to formset_factory. It will (essentially) increment the extra argument ChoiceFormSet = formset_factory(Choice, extra=1, min_num=1) formset = ChoiceFormSet(auto_id=False, prefix='choices') @@ -225,6 +225,10 @@ class FormsFormsetTestCase(TestCase): for form in formset.forms: form_output.append(form.as_ul()) + # Min_num forms are required; extra forms can be empty. + self.assertFalse(formset.forms[0].empty_permitted) + self.assertTrue(formset.forms[1].empty_permitted) + self.assertHTMLEqual('\n'.join(form_output), """
  • Choice:
  • Votes:
  • Choice:
    • diff --git a/tests/generic_inline_admin/tests.py b/tests/generic_inline_admin/tests.py index ed2411ffdf..e73fc40421 100644 --- a/tests/generic_inline_admin/tests.py +++ b/tests/generic_inline_admin/tests.py @@ -204,8 +204,7 @@ class GenericInlineAdminParametersTest(TestCase): def testMinNumParam(self): """ - With extra=3 and min_num=2, there should be six forms. - See #22628 - this will change when that's fixed. + With extra=3 and min_num=2, there should be five forms. """ class MinNumInline(GenericTabularInline): model = Media @@ -220,7 +219,7 @@ class GenericInlineAdminParametersTest(TestCase): request.user = User(username='super', is_superuser=True) response = modeladmin.changeform_view(request, object_id=str(e.pk)) formset = response.context_data['inline_admin_formsets'][0].formset - self.assertEqual(formset.total_form_count(), 6) + self.assertEqual(formset.total_form_count(), 5) self.assertEqual(formset.initial_form_count(), 1) diff --git a/tests/model_formsets/tests.py b/tests/model_formsets/tests.py index ed74dff2ed..2913f6c0ab 100644 --- a/tests/model_formsets/tests.py +++ b/tests/model_formsets/tests.py @@ -395,13 +395,12 @@ class ModelFormsetTest(TestCase): def test_min_num_with_existing(self): # Test the behavior of min_num with existing objects. - # See #22628 - this will change when that's fixed. Author.objects.create(name='Charles Baudelaire') qs = Author.objects.all() AuthorFormSet = modelformset_factory(Author, fields="__all__", extra=0, min_num=1) formset = AuthorFormSet(queryset=qs) - self.assertEqual(len(formset.forms), 2) + self.assertEqual(len(formset.forms), 1) def test_custom_save_method(self): class PoetForm(forms.ModelForm):