From be6ca89396c031619947921c81b8795d816e3285 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Wed, 21 Feb 2018 19:11:50 -0800 Subject: [PATCH] Fixed #27991 -- Added obj arg to InlineModelAdmin.has_add_permission(). Thanks Vladimir Ivanov for the initial patch. --- django/contrib/admin/checks.py | 18 ++++++ django/contrib/admin/options.py | 15 +++-- docs/internals/deprecation.txt | 3 + docs/ref/contrib/admin/index.txt | 9 ++- docs/releases/2.1.txt | 7 +++ ...test_has_add_permission_obj_deprecation.py | 63 +++++++++++++++++++ tests/modeladmin/tests.py | 19 ++++++ 7 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 tests/modeladmin/test_has_add_permission_obj_deprecation.py diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index 26c5c3eef6..fc22a28438 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -1,3 +1,4 @@ +import warnings from itertools import chain from django.apps import apps @@ -13,6 +14,8 @@ from django.forms.models import ( BaseModelForm, BaseModelFormSet, _get_foreign_key, ) from django.template.engine import Engine +from django.utils.deprecation import RemovedInDjango30Warning +from django.utils.inspect import get_func_args def check_admin_app(app_configs, **kwargs): @@ -884,6 +887,7 @@ class ModelAdminChecks(BaseModelAdminChecks): class InlineModelAdminChecks(BaseModelAdminChecks): def check(self, inline_obj, **kwargs): + self._check_has_add_permission(inline_obj) parent_model = inline_obj.parent_model return [ *super().check(inline_obj), @@ -968,6 +972,20 @@ class InlineModelAdminChecks(BaseModelAdminChecks): else: return [] + def _check_has_add_permission(self, obj): + cls = obj.__class__ + try: + func = cls.has_add_permission + except AttributeError: + pass + else: + args = get_func_args(func) + if 'obj' not in args: + warnings.warn( + "Update %s.has_add_permission() to accept a positional " + "`obj` argument." % cls.__name__, RemovedInDjango30Warning + ) + def must_be(type, option, obj, id): return [ diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index bbc402bd74..7cbfde4452 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -44,6 +44,7 @@ from django.urls import reverse from django.utils.decorators import method_decorator from django.utils.html import format_html from django.utils.http import urlencode +from django.utils.inspect import get_func_args from django.utils.safestring import mark_safe from django.utils.text import capfirst, format_lazy, get_text_list from django.utils.translation import gettext as _, ngettext @@ -559,12 +560,18 @@ class ModelAdmin(BaseModelAdmin): inline_instances = [] for inline_class in self.inlines: inline = inline_class(self.model, self.admin_site) + # RemovedInDjango30Warning: obj will be a required argument. + args = get_func_args(inline.has_add_permission) + if 'obj' in args: + inline_has_add_permission = inline.has_add_permission(request, obj) + else: + inline_has_add_permission = inline.has_add_permission(request) if request: - if not (inline.has_add_permission(request) or + if not (inline_has_add_permission or inline.has_change_permission(request, obj) or inline.has_delete_permission(request, obj)): continue - if not inline.has_add_permission(request): + if not inline_has_add_permission: inline.max_num = 0 inline_instances.append(inline) @@ -2007,13 +2014,13 @@ class InlineModelAdmin(BaseModelAdmin): queryset = queryset.none() return queryset - def has_add_permission(self, request): + def has_add_permission(self, request, obj): if self.opts.auto_created: # We're checking the rights to an auto-created intermediate model, # which doesn't have its own individual permissions. The user needs # to have the change permission for the related model in order to # be able to do anything with the intermediate model. - return self.has_change_permission(request) + return self.has_change_permission(request, obj) return super().has_add_permission(request) def has_change_permission(self, request, obj=None): diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 5f30a38a3e..2d6cbb8e87 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -41,6 +41,9 @@ details on these changes. * ``django.contrib.staticfiles.templatetags.static()`` will be removed. +* The shim to allow ``InlineModelAdmin.has_add_permission()`` to be defined + without an ``obj`` argument will be removed. + .. _deprecation-removed-in-2.1: 2.1 diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 1a74794754..11291c3992 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -2395,10 +2395,15 @@ The ``InlineModelAdmin`` class adds or customizes: inline forms. For example, this may be based on the model instance (passed as the keyword argument ``obj``). -.. method:: InlineModelAdmin.has_add_permission(request) +.. method:: InlineModelAdmin.has_add_permission(request, obj) Should return ``True`` if adding an inline object is permitted, ``False`` - otherwise. + otherwise. ``obj`` is the parent object being edited or ``None`` when + adding a new parent. + + .. versionchanged:: 2.1 + + The ``obj`` argument was added. .. method:: InlineModelAdmin.has_change_permission(request, obj=None) diff --git a/docs/releases/2.1.txt b/docs/releases/2.1.txt index 87181fb33d..f41d5178fa 100644 --- a/docs/releases/2.1.txt +++ b/docs/releases/2.1.txt @@ -64,6 +64,9 @@ Minor features with ``change_list_object_tools.html`` and ``change_form_object_tools.html`` templates. +* :meth:`.InlineModelAdmin.has_add_permission` is now passed the parent object + as the second positional argument, ``obj``. + :mod:`django.contrib.admindocs` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -398,6 +401,10 @@ Miscellaneous * ``django.contrib.staticfiles.templatetags.static()`` is deprecated in favor of ``django.templatetags.static.static()``. +* Support for :meth:`.InlineModelAdmin.has_add_permission` methods that don't + accept ``obj`` as the second positional argument will be removed in Django + 3.0. + .. _removed-features-2.1: Features removed in 2.1 diff --git a/tests/modeladmin/test_has_add_permission_obj_deprecation.py b/tests/modeladmin/test_has_add_permission_obj_deprecation.py new file mode 100644 index 0000000000..6d48f210d1 --- /dev/null +++ b/tests/modeladmin/test_has_add_permission_obj_deprecation.py @@ -0,0 +1,63 @@ +import warnings + +from django.contrib.admin.options import ModelAdmin, TabularInline +from django.utils.deprecation import RemovedInDjango30Warning + +from .models import Band, Song +from .test_checks import CheckTestCase + + +class HasAddPermissionObjTests(CheckTestCase): + def test_model_admin_inherited_valid(self): + class BandAdmin(ModelAdmin): + pass + + self.assertIsValid(BandAdmin, Band) + + def test_model_admin_valid(self): + class BandAdmin(ModelAdmin): + def has_add_permission(self, request): + return super().has_add_permission(request) + + self.assertIsValid(BandAdmin, Band) + + def test_inline_admin_inherited_valid(self): + class SongInlineAdmin(TabularInline): + model = Song + + class BandAdmin(ModelAdmin): + inlines = [SongInlineAdmin] + + self.assertIsValid(BandAdmin, Band) + + def test_inline_admin_valid(self): + class SongInlineAdmin(TabularInline): + model = Song + + def has_add_permission(self, request, obj): + return super().has_add_permission(request, obj) + + class BandAdmin(ModelAdmin): + inlines = [SongInlineAdmin] + + self.assertIsValid(BandAdmin, Band) + + def test_inline_admin_warning(self): + class SongInlineAdmin(TabularInline): + model = Song + + def has_add_permission(self, request): + return super().has_add_permission(request) + + class BandAdmin(ModelAdmin): + inlines = [SongInlineAdmin] + + with warnings.catch_warnings(record=True) as recorded: + warnings.simplefilter('always') + self.assertIsValid(BandAdmin, Band) + self.assertEqual(len(recorded), 1) + self.assertIs(recorded[0].category, RemovedInDjango30Warning) + self.assertEqual(str(recorded[0].message), ( + "Update SongInlineAdmin.has_add_permission() to accept a " + "positional `obj` argument." + )) diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index 81c28cb0ff..d0123de722 100644 --- a/tests/modeladmin/tests.py +++ b/tests/modeladmin/tests.py @@ -709,6 +709,25 @@ class ModelAdminPermissionTests(SimpleTestCase): request.user = self.MockDeleteUser() self.assertFalse(ma.has_add_permission(request)) + def test_inline_has_add_permission_uses_obj(self): + class ConcertInline(TabularInline): + model = Concert + + def has_add_permission(self, request, obj): + return bool(obj) + + class BandAdmin(ModelAdmin): + inlines = [ConcertInline] + + ma = BandAdmin(Band, AdminSite()) + request = MockRequest() + request.user = self.MockAddUser() + self.assertEqual(ma.get_inline_instances(request), []) + band = Band(name='The Doors', bio='', sign_date=date(1965, 1, 1)) + inline_instances = ma.get_inline_instances(request, band) + self.assertEqual(len(inline_instances), 1) + self.assertIsInstance(inline_instances[0], ConcertInline) + def test_has_change_permission(self): """ has_change_permission returns True for users who can edit objects and