From 4ef10f245ada0c7d5ae8dc31eebffa63790d40fb Mon Sep 17 00:00:00 2001 From: Anders Steinlein Date: Wed, 5 Mar 2014 21:19:40 +0100 Subject: [PATCH] Fixed #17642 -- Added min_num support to modelformsets, inlines, and the admin. Thanks Stephen Burrows for work on the patch as well. Forwardport of 2914f66983a92fcae55673c517dd8d01e8c238c4 from stable/1.7.x --- django/contrib/admin/checks.py | 13 +++++- django/contrib/admin/options.py | 6 +++ django/contrib/contenttypes/admin.py | 1 + django/contrib/contenttypes/forms.py | 9 ++-- django/forms/models.py | 12 ++++-- docs/ref/checks.txt | 3 +- docs/ref/contrib/admin/index.txt | 20 +++++++++ docs/ref/contrib/contenttypes.txt | 6 ++- docs/ref/forms/models.txt | 4 +- tests/admin_inlines/tests.py | 62 +++++++++++++++++++++++++++- tests/generic_inline_admin/admin.py | 17 +------- tests/generic_inline_admin/models.py | 20 --------- tests/generic_inline_admin/tests.py | 61 ++++++++++++++++++++++----- tests/model_formsets/tests.py | 27 ++++++++++++ tests/modeladmin/tests.py | 29 ++++++++++++- 15 files changed, 229 insertions(+), 61 deletions(-) diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index dfe4cd04ec..e49bb8fb6e 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -850,6 +850,7 @@ class InlineModelAdminChecks(BaseModelAdminChecks): errors.extend(self._check_exclude_of_parent_model(cls, parent_model)) errors.extend(self._check_extra(cls)) errors.extend(self._check_max_num(cls)) + errors.extend(self._check_min_num(cls)) errors.extend(self._check_formset(cls)) return errors @@ -909,12 +910,22 @@ class InlineModelAdminChecks(BaseModelAdminChecks): else: return [] + def _check_min_num(self, cls): + """ Check that min_num is an integer. """ + + if cls.min_num is None: + return [] + elif not isinstance(cls.min_num, int): + return must_be('an integer', option='min_num', obj=cls, id='admin.E205') + else: + return [] + def _check_formset(self, cls): """ Check formset is a subclass of BaseModelFormSet. """ if not issubclass(cls.formset, BaseModelFormSet): return must_inherit_from(parent='BaseModelFormSet', option='formset', - obj=cls, id='admin.E205') + obj=cls, id='admin.E206') else: return [] diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 016b73d987..2d5a5cf933 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -1706,6 +1706,7 @@ class InlineModelAdmin(BaseModelAdmin): fk_name = None formset = BaseInlineFormSet extra = 3 + min_num = None max_num = None template = None verbose_name = None @@ -1738,6 +1739,10 @@ class InlineModelAdmin(BaseModelAdmin): """Hook for customizing the number of extra inline forms.""" return self.extra + def get_min_num(self, request, obj=None, **kwargs): + """Hook for customizing the min number of inline forms.""" + return self.min_num + def get_max_num(self, request, obj=None, **kwargs): """Hook for customizing the max number of extra inline forms.""" return self.max_num @@ -1769,6 +1774,7 @@ class InlineModelAdmin(BaseModelAdmin): "exclude": exclude, "formfield_callback": partial(self.formfield_for_dbfield, request=request), "extra": self.get_extra(request, obj, **kwargs), + "min_num": self.get_min_num(request, obj, **kwargs), "max_num": self.get_max_num(request, obj, **kwargs), "can_delete": can_delete, } diff --git a/django/contrib/contenttypes/admin.py b/django/contrib/contenttypes/admin.py index 8da6546a21..1938fa7aae 100644 --- a/django/contrib/contenttypes/admin.py +++ b/django/contrib/contenttypes/admin.py @@ -119,6 +119,7 @@ class GenericInlineModelAdmin(InlineModelAdmin): "can_delete": can_delete, "can_order": False, "fields": fields, + "min_num": self.min_num, "max_num": self.max_num, "exclude": exclude } diff --git a/django/contrib/contenttypes/forms.py b/django/contrib/contenttypes/forms.py index 33df7528db..693628d3de 100644 --- a/django/contrib/contenttypes/forms.py +++ b/django/contrib/contenttypes/forms.py @@ -56,9 +56,9 @@ def generic_inlineformset_factory(model, form=ModelForm, ct_field="content_type", fk_field="object_id", fields=None, exclude=None, extra=3, can_order=False, can_delete=True, - max_num=None, - formfield_callback=None, validate_max=False, - for_concrete_model=True): + max_num=None, formfield_callback=None, + validate_max=False, for_concrete_model=True, + min_num=None, validate_min=False): """ Returns a ``GenericInlineFormSet`` for the given kwargs. @@ -81,7 +81,8 @@ def generic_inlineformset_factory(model, form=ModelForm, formset=formset, extra=extra, can_delete=can_delete, can_order=can_order, fields=fields, exclude=exclude, max_num=max_num, - validate_max=validate_max) + validate_max=validate_max, min_num=min_num, + validate_min=validate_min) FormSet.ct_field = ct_field FormSet.ct_fk_field = fk_field FormSet.for_concrete_model = for_concrete_model diff --git a/django/forms/models.py b/django/forms/models.py index f1a383fde6..263d3d4340 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -805,7 +805,8 @@ def modelformset_factory(model, form=ModelForm, formfield_callback=None, formset=BaseModelFormSet, extra=1, can_delete=False, can_order=False, max_num=None, fields=None, exclude=None, widgets=None, validate_max=False, localized_fields=None, - labels=None, help_texts=None, error_messages=None): + labels=None, help_texts=None, error_messages=None, + min_num=None, validate_min=False): """ Returns a FormSet class for the given Django model class. """ @@ -823,9 +824,9 @@ def modelformset_factory(model, form=ModelForm, formfield_callback=None, formfield_callback=formfield_callback, widgets=widgets, localized_fields=localized_fields, labels=labels, help_texts=help_texts, error_messages=error_messages) - FormSet = formset_factory(form, formset, extra=extra, max_num=max_num, + FormSet = formset_factory(form, formset, extra=extra, min_num=min_num, max_num=max_num, can_order=can_order, can_delete=can_delete, - validate_max=validate_max) + validate_min=validate_min, validate_max=validate_max) FormSet.model = model return FormSet @@ -969,7 +970,8 @@ def inlineformset_factory(parent_model, model, form=ModelForm, fields=None, exclude=None, extra=3, can_order=False, can_delete=True, max_num=None, formfield_callback=None, widgets=None, validate_max=False, localized_fields=None, - labels=None, help_texts=None, error_messages=None): + labels=None, help_texts=None, error_messages=None, + min_num=None, validate_min=False): """ Returns an ``InlineFormSet`` for the given kwargs. @@ -989,8 +991,10 @@ def inlineformset_factory(parent_model, model, form=ModelForm, 'can_order': can_order, 'fields': fields, 'exclude': exclude, + 'min_num': min_num, 'max_num': max_num, 'widgets': widgets, + 'validate_min': validate_min, 'validate_max': validate_max, 'localized_fields': localized_fields, 'labels': labels, diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 67dc335d80..c8e97df660 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -205,7 +205,8 @@ inline on a :class:`~django.contrib.admin.ModelAdmin`. * **admin.E202**: ```` has no ForeignKey to ````./```` has more than one ForeignKey to ````. * **admin.E203**: The value of ``extra`` must be an integer. * **admin.E204**: The value of ``max_num`` must be an integer. -* **admin.E205**: The value of ``formset`` must inherit from ``BaseModelFormSet``. +* **admin.E205**: The value of ``min_num`` must be an integer. +* **admin.E206**: The value of ``formset`` must inherit from ``BaseModelFormSet``. GenericInlineModelAdmin ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 1a02d3fc3b..876bf290c3 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -1966,6 +1966,16 @@ The ``InlineModelAdmin`` class adds: :meth:`InlineModelAdmin.get_max_num` also allows you to customize the maximum number of extra forms. +.. attribute:: InlineModelAdmin.min_num + + .. versionadded:: 1.7 + + This controls the minimum number of forms to show in the inline. + See :func:`~django.forms.models.modelformset_factory` for more information. + + :meth:`InlineModelAdmin.get_min_num` also allows you to customize the + minimum number of displayed forms. + .. attribute:: InlineModelAdmin.raw_id_fields By default, Django's admin uses a select-box interface (' + total_forms = '' + + request = self.factory.get('/admin/admin_inlines/binarytree/add/') + request.user = User(username='super', is_superuser=True) + response = modeladmin.changeform_view(request) + self.assertContains(response, min_forms) + self.assertContains(response, total_forms) + + 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) + + class MinNumInline(TabularInline): + model = BinaryTree + extra = 3 + + def get_min_num(self, request, obj=None, **kwargs): + if obj: + return 5 + return 2 + + modeladmin = ModelAdmin(BinaryTree, admin_site) + modeladmin.inlines = [MinNumInline] + + min_forms = '' + total_forms = '' + + request = self.factory.get('/admin/admin_inlines/binarytree/add/') + request.user = User(username='super', is_superuser=True) + response = modeladmin.changeform_view(request) + self.assertContains(response, min_forms % 2) + self.assertContains(response, total_forms % 5) + + request = self.factory.get("/admin/admin_inlines/binarytree/%d/" % bt_head.id) + 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) + def test_inline_nonauto_noneditable_pk(self): response = self.client.get('/admin/admin_inlines/author/add/') self.assertContains(response, diff --git a/tests/generic_inline_admin/admin.py b/tests/generic_inline_admin/admin.py index d0ce4c70ef..46f6cf3e7f 100644 --- a/tests/generic_inline_admin/admin.py +++ b/tests/generic_inline_admin/admin.py @@ -1,8 +1,8 @@ from django.contrib import admin from django.contrib.contenttypes.admin import GenericTabularInline -from .models import (Media, PhoneNumber, Episode, EpisodeExtra, Contact, - Category, EpisodePermanent, EpisodeMaxNum) +from .models import (Media, PhoneNumber, Episode, Contact, + Category, EpisodePermanent) site = admin.AdminSite(name="admin") @@ -18,17 +18,6 @@ class EpisodeAdmin(admin.ModelAdmin): ] -class MediaExtraInline(GenericTabularInline): - model = Media - extra = 0 - - -class MediaMaxNumInline(GenericTabularInline): - model = Media - extra = 5 - max_num = 2 - - class PhoneNumberInline(GenericTabularInline): model = PhoneNumber @@ -39,8 +28,6 @@ class MediaPermanentInline(GenericTabularInline): site.register(Episode, EpisodeAdmin) -site.register(EpisodeExtra, inlines=[MediaExtraInline]) -site.register(EpisodeMaxNum, inlines=[MediaMaxNumInline]) site.register(Contact, inlines=[PhoneNumberInline]) site.register(Category) site.register(EpisodePermanent, inlines=[MediaPermanentInline]) diff --git a/tests/generic_inline_admin/models.py b/tests/generic_inline_admin/models.py index 0ae3c4b052..059aadc5ba 100644 --- a/tests/generic_inline_admin/models.py +++ b/tests/generic_inline_admin/models.py @@ -27,26 +27,6 @@ class Media(models.Model): def __str__(self): return self.url -# -# These models let us test the different GenericInline settings at -# different urls in the admin site. -# - -# -# Generic inline with extra = 0 -# - - -class EpisodeExtra(Episode): - pass - - -# -# Generic inline with extra and max_num -# -class EpisodeMaxNum(Episode): - pass - # # Generic inline with unique_together diff --git a/tests/generic_inline_admin/tests.py b/tests/generic_inline_admin/tests.py index 30513041d6..ed2411ffdf 100644 --- a/tests/generic_inline_admin/tests.py +++ b/tests/generic_inline_admin/tests.py @@ -4,17 +4,17 @@ import warnings from django.contrib import admin from django.contrib.admin.sites import AdminSite +from django.contrib.auth.models import User from django.contrib.contenttypes.admin import GenericTabularInline from django.contrib.contenttypes.forms import generic_inlineformset_factory from django.forms.formsets import DEFAULT_MAX_NUM from django.forms.models import ModelForm -from django.test import TestCase, override_settings +from django.test import TestCase, override_settings, RequestFactory from django.utils.deprecation import RemovedInDjango19Warning # local test models -from .admin import MediaInline, MediaPermanentInline -from .models import (Episode, EpisodeExtra, EpisodeMaxNum, Media, - EpisodePermanent, Category) +from .admin import MediaInline, MediaPermanentInline, site as admin_site +from .models import Episode, Media, EpisodePermanent, Category @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), @@ -137,6 +137,7 @@ class GenericInlineAdminParametersTest(TestCase): def setUp(self): self.client.login(username='super', password='secret') + self.factory = RequestFactory() def tearDown(self): self.client.logout() @@ -166,9 +167,18 @@ class GenericInlineAdminParametersTest(TestCase): """ With extra=0, there should be one form. """ - e = self._create_object(EpisodeExtra) - response = self.client.get('/generic_inline_admin/admin/generic_inline_admin/episodeextra/%s/' % e.pk) - formset = response.context['inline_admin_formsets'][0].formset + class ExtraInline(GenericTabularInline): + model = Media + extra = 0 + + modeladmin = admin.ModelAdmin(Episode, admin_site) + modeladmin.inlines = [ExtraInline] + + e = self._create_object(Episode) + request = self.factory.get('/generic_inline_admin/admin/generic_inline_admin/episode/%s/' % e.pk) + 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(), 1) self.assertEqual(formset.initial_form_count(), 1) @@ -176,12 +186,43 @@ class GenericInlineAdminParametersTest(TestCase): """ With extra=5 and max_num=2, there should be only 2 forms. """ - e = self._create_object(EpisodeMaxNum) - response = self.client.get('/generic_inline_admin/admin/generic_inline_admin/episodemaxnum/%s/' % e.pk) - formset = response.context['inline_admin_formsets'][0].formset + class MaxNumInline(GenericTabularInline): + model = Media + extra = 5 + max_num = 2 + + modeladmin = admin.ModelAdmin(Episode, admin_site) + modeladmin.inlines = [MaxNumInline] + + e = self._create_object(Episode) + request = self.factory.get('/generic_inline_admin/admin/generic_inline_admin/episode/%s/' % e.pk) + 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(), 2) self.assertEqual(formset.initial_form_count(), 1) + def testMinNumParam(self): + """ + With extra=3 and min_num=2, there should be six forms. + See #22628 - this will change when that's fixed. + """ + class MinNumInline(GenericTabularInline): + model = Media + extra = 3 + min_num = 2 + + modeladmin = admin.ModelAdmin(Episode, admin_site) + modeladmin.inlines = [MinNumInline] + + e = self._create_object(Episode) + request = self.factory.get('/generic_inline_admin/admin/generic_inline_admin/episode/%s/' % e.pk) + 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.initial_form_count(), 1) + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), ROOT_URLCONF="generic_inline_admin.urls") diff --git a/tests/model_formsets/tests.py b/tests/model_formsets/tests.py index b5e410738e..ed74dff2ed 100644 --- a/tests/model_formsets/tests.py +++ b/tests/model_formsets/tests.py @@ -376,6 +376,33 @@ class ModelFormsetTest(TestCase): '', ]) + def test_min_num(self): + # Test the behavior of min_num with model formsets. It should be + # added to extra. + qs = Author.objects.none() + + AuthorFormSet = modelformset_factory(Author, fields="__all__", extra=0) + formset = AuthorFormSet(queryset=qs) + self.assertEqual(len(formset.forms), 0) + + AuthorFormSet = modelformset_factory(Author, fields="__all__", min_num=1, extra=0) + formset = AuthorFormSet(queryset=qs) + self.assertEqual(len(formset.forms), 1) + + AuthorFormSet = modelformset_factory(Author, fields="__all__", min_num=1, extra=1) + formset = AuthorFormSet(queryset=qs) + self.assertEqual(len(formset.forms), 2) + + 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) + def test_custom_save_method(self): class PoetForm(forms.ModelForm): def save(self, commit=True): diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index 9cf54c8b9f..1c3a21d374 100644 --- a/tests/modeladmin/tests.py +++ b/tests/modeladmin/tests.py @@ -1444,6 +1444,33 @@ class MaxNumCheckTests(CheckTestCase): self.assertIsValid(ValidationTestModelAdmin, ValidationTestModel) +class MinNumCheckTests(CheckTestCase): + + def test_not_integer(self): + class ValidationTestInline(TabularInline): + model = ValidationTestInlineModel + min_num = "hello" + + class ValidationTestModelAdmin(ModelAdmin): + inlines = [ValidationTestInline] + + self.assertIsInvalid( + ValidationTestModelAdmin, ValidationTestModel, + "The value of 'min_num' must be an integer.", + 'admin.E205', + invalid_obj=ValidationTestInline) + + def test_valid_case(self): + class ValidationTestInline(TabularInline): + model = ValidationTestInlineModel + min_num = 2 + + class ValidationTestModelAdmin(ModelAdmin): + inlines = [ValidationTestInline] + + self.assertIsValid(ValidationTestModelAdmin, ValidationTestModel) + + class FormsetCheckTests(CheckTestCase): def test_invalid_type(self): @@ -1460,7 +1487,7 @@ class FormsetCheckTests(CheckTestCase): self.assertIsInvalid( ValidationTestModelAdmin, ValidationTestModel, "The value of 'formset' must inherit from 'BaseModelFormSet'.", - 'admin.E205', + 'admin.E206', invalid_obj=ValidationTestInline) def test_valid_case(self):