summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorsarahboyce <sarahvboyce95@gmail.com>2023-04-13 11:46:47 +0200
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2023-04-17 14:09:38 +0200
commit594fcc2b7427f7baf2cf1a2d7cd2be61467df0c3 (patch)
tree9406ab96668239a7fbdee33f2efb93592db22b36
parent57f2b935b34d148c3c0d906fc8256765004b7b77 (diff)
downloaddjango-594fcc2b7427f7baf2cf1a2d7cd2be61467df0c3.tar.gz
Fixed #22569 -- Made ModelAdmin.lookup_allowed() respect get_list_filter().
Thank you Simon Meers for the initial patch.
-rw-r--r--django/contrib/admin/options.py11
-rw-r--r--django/contrib/admin/views/main.py15
-rw-r--r--django/contrib/auth/admin.py6
-rw-r--r--docs/internals/deprecation.txt3
-rw-r--r--docs/ref/contrib/admin/index.txt15
-rw-r--r--docs/releases/5.0.txt4
-rw-r--r--tests/modeladmin/tests.py96
7 files changed, 133 insertions, 17 deletions
diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index 49c816dc9e..b0635669e9 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -436,7 +436,9 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass):
else self.get_list_display(request)
)
- def lookup_allowed(self, lookup, value):
+ # RemovedInDjango60Warning: when the deprecation ends, replace with:
+ # def lookup_allowed(self, lookup, value, request):
+ def lookup_allowed(self, lookup, value, request=None):
from django.contrib.admin.filters import SimpleListFilter
model = self.model
@@ -482,7 +484,12 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass):
# Either a local field filter, or no fields at all.
return True
valid_lookups = {self.date_hierarchy}
- for filter_item in self.list_filter:
+ # RemovedInDjango60Warning: when the deprecation ends, replace with:
+ # for filter_item in self.get_list_filter(request):
+ list_filter = (
+ self.get_list_filter(request) if request is not None else self.list_filter
+ )
+ for filter_item in list_filter:
if isinstance(filter_item, type) and issubclass(
filter_item, SimpleListFilter
):
diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py
index 9a130ae8a7..c99972c2bd 100644
--- a/django/contrib/admin/views/main.py
+++ b/django/contrib/admin/views/main.py
@@ -1,3 +1,4 @@
+import warnings
from datetime import datetime, timedelta
from django import forms
@@ -31,7 +32,9 @@ from django.core.paginator import InvalidPage
from django.db.models import Exists, F, Field, ManyToOneRel, OrderBy, OuterRef
from django.db.models.expressions import Combinable
from django.urls import reverse
+from django.utils.deprecation import RemovedInDjango60Warning
from django.utils.http import urlencode
+from django.utils.inspect import func_supports_parameter
from django.utils.timezone import make_aware
from django.utils.translation import gettext
@@ -174,9 +177,19 @@ class ChangeList:
may_have_duplicates = False
has_active_filters = False
+ supports_request = func_supports_parameter(
+ self.model_admin.lookup_allowed, "request"
+ )
+ if not supports_request:
+ warnings.warn(
+ f"`request` must be added to the signature of "
+ f"{self.model_admin.__class__.__qualname__}.lookup_allowed().",
+ RemovedInDjango60Warning,
+ )
for key, value_list in lookup_params.items():
for value in value_list:
- if not self.model_admin.lookup_allowed(key, value):
+ params = (key, value, request) if supports_request else (key, value)
+ if not self.model_admin.lookup_allowed(*params):
raise DisallowedModelAdminLookup(f"Filtering by {key} not allowed")
filter_specs = []
diff --git a/django/contrib/auth/admin.py b/django/contrib/auth/admin.py
index 5424711643..f9532abc14 100644
--- a/django/contrib/auth/admin.py
+++ b/django/contrib/auth/admin.py
@@ -106,10 +106,12 @@ class UserAdmin(admin.ModelAdmin):
),
] + super().get_urls()
- def lookup_allowed(self, lookup, value):
+ # RemovedInDjango60Warning: when the deprecation ends, replace with:
+ # def lookup_allowed(self, lookup, value, request):
+ def lookup_allowed(self, lookup, value, request=None):
# Don't allow lookups involving passwords.
return not lookup.startswith("password") and super().lookup_allowed(
- lookup, value
+ lookup, value, request
)
@sensitive_post_parameters_m
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index b151020375..168a5572c6 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -21,6 +21,9 @@ details on these changes.
* Support for passing positional arguments to ``BaseConstraint`` will be
removed.
+* ``request`` will be required in the signature of
+ ``ModelAdmin.lookup_allowed()`` subclasses.
+
.. _deprecation-removed-in-5.1:
5.1
diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt
index e856f0b640..636d44c750 100644
--- a/docs/ref/contrib/admin/index.txt
+++ b/docs/ref/contrib/admin/index.txt
@@ -1845,7 +1845,7 @@ templates used by the :class:`ModelAdmin` views:
kwargs["formset"] = MyAdminFormSet
return super().get_changelist_formset(request, **kwargs)
-.. method:: ModelAdmin.lookup_allowed(lookup, value)
+.. method:: ModelAdmin.lookup_allowed(lookup, value, request)
The objects in the changelist page can be filtered with lookups from the
URL's query string. This is how :attr:`list_filter` works, for example. The
@@ -1855,10 +1855,11 @@ templates used by the :class:`ModelAdmin` views:
unauthorized data exposure.
The ``lookup_allowed()`` method is given a lookup path from the query string
- (e.g. ``'user__email'``) and the corresponding value
- (e.g. ``'user@example.com'``), and returns a boolean indicating whether
- filtering the changelist's ``QuerySet`` using the parameters is permitted.
- If ``lookup_allowed()`` returns ``False``, ``DisallowedModelAdminLookup``
+ (e.g. ``'user__email'``), the corresponding value
+ (e.g. ``'user@example.com'``), and the request, and returns a boolean
+ indicating whether filtering the changelist's ``QuerySet`` using the
+ parameters is permitted. If ``lookup_allowed()`` returns ``False``,
+ ``DisallowedModelAdminLookup``
(subclass of :exc:`~django.core.exceptions.SuspiciousOperation`) is raised.
By default, ``lookup_allowed()`` allows access to a model's local fields,
@@ -1870,6 +1871,10 @@ templates used by the :class:`ModelAdmin` views:
Override this method to customize the lookups permitted for your
:class:`~django.contrib.admin.ModelAdmin` subclass.
+ .. versionchanged:: 5.0
+
+ The ``request`` argument was added.
+
.. method:: ModelAdmin.has_view_permission(request, obj=None)
Should return ``True`` if viewing ``obj`` is permitted, ``False`` otherwise.
diff --git a/docs/releases/5.0.txt b/docs/releases/5.0.txt
index 0d1e7ffda1..934da1bba9 100644
--- a/docs/releases/5.0.txt
+++ b/docs/releases/5.0.txt
@@ -387,6 +387,10 @@ Miscellaneous
:class:`~django.db.models.BaseConstraint` is deprecated in favor of
keyword-only arguments.
+* ``request`` is added to the signature of :meth:`.ModelAdmin.lookup_allowed`.
+ Support for ``ModelAdmin`` subclasses that do not accept this argument is
+ deprecated.
+
Features removed in 5.0
=======================
diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py
index 5604e39746..627029c9cf 100644
--- a/tests/modeladmin/tests.py
+++ b/tests/modeladmin/tests.py
@@ -19,8 +19,9 @@ from django.contrib.admin.widgets import (
from django.contrib.auth.models import User
from django.db import models
from django.forms.widgets import Select
-from django.test import SimpleTestCase, TestCase
+from django.test import RequestFactory, SimpleTestCase, TestCase
from django.test.utils import isolate_apps
+from django.utils.deprecation import RemovedInDjango60Warning
from .models import Band, Concert, Song
@@ -121,7 +122,10 @@ class ModelAdminTests(TestCase):
fields = ["name"]
ma = BandAdmin(Band, self.site)
- self.assertTrue(ma.lookup_allowed("name__nonexistent", "test_value"))
+ self.assertIs(
+ ma.lookup_allowed("name__nonexistent", "test_value", request),
+ True,
+ )
@isolate_apps("modeladmin")
def test_lookup_allowed_onetoone(self):
@@ -147,11 +151,15 @@ class ModelAdminTests(TestCase):
ma = EmployeeProfileAdmin(EmployeeProfile, self.site)
# Reverse OneToOneField
self.assertIs(
- ma.lookup_allowed("employee__employeeinfo__description", "test_value"), True
+ ma.lookup_allowed(
+ "employee__employeeinfo__description", "test_value", request
+ ),
+ True,
)
# OneToOneField and ForeignKey
self.assertIs(
- ma.lookup_allowed("employee__department__code", "test_value"), True
+ ma.lookup_allowed("employee__department__code", "test_value", request),
+ True,
)
@isolate_apps("modeladmin")
@@ -175,13 +183,87 @@ class ModelAdminTests(TestCase):
]
ma = WaiterAdmin(Waiter, self.site)
- self.assertIs(ma.lookup_allowed("restaurant__place__country", "1"), True)
self.assertIs(
- ma.lookup_allowed("restaurant__place__country__id__exact", "1"), True
+ ma.lookup_allowed("restaurant__place__country", "1", request),
+ True,
+ )
+ self.assertIs(
+ ma.lookup_allowed("restaurant__place__country__id__exact", "1", request),
+ True,
+ )
+ self.assertIs(
+ ma.lookup_allowed(
+ "restaurant__place__country__name", "test_value", request
+ ),
+ True,
+ )
+
+ def test_lookup_allowed_considers_dynamic_list_filter(self):
+ class ConcertAdmin(ModelAdmin):
+ list_filter = ["main_band__sign_date"]
+
+ def get_list_filter(self, request):
+ if getattr(request, "user", None):
+ return self.list_filter + ["main_band__name"]
+ return self.list_filter
+
+ model_admin = ConcertAdmin(Concert, self.site)
+ request_band_name_filter = RequestFactory().get(
+ "/", {"main_band__name": "test"}
+ )
+ self.assertIs(
+ model_admin.lookup_allowed(
+ "main_band__sign_date", "?", request_band_name_filter
+ ),
+ True,
)
self.assertIs(
- ma.lookup_allowed("restaurant__place__country__name", "test_value"), True
+ model_admin.lookup_allowed(
+ "main_band__name", "?", request_band_name_filter
+ ),
+ False,
+ )
+ request_with_superuser = request
+ self.assertIs(
+ model_admin.lookup_allowed(
+ "main_band__sign_date", "?", request_with_superuser
+ ),
+ True,
+ )
+ self.assertIs(
+ model_admin.lookup_allowed("main_band__name", "?", request_with_superuser),
+ True,
+ )
+
+ def test_lookup_allowed_without_request_deprecation(self):
+ class ConcertAdmin(ModelAdmin):
+ list_filter = ["main_band__sign_date"]
+
+ def get_list_filter(self, request):
+ return self.list_filter + ["main_band__name"]
+
+ def lookup_allowed(self, lookup, value):
+ return True
+
+ model_admin = ConcertAdmin(Concert, self.site)
+ msg = (
+ "`request` must be added to the signature of ModelAdminTests."
+ "test_lookup_allowed_without_request_deprecation.<locals>."
+ "ConcertAdmin.lookup_allowed()."
+ )
+ request_band_name_filter = RequestFactory().get(
+ "/", {"main_band__name": "test"}
+ )
+ request_band_name_filter.user = User.objects.create_superuser(
+ username="bob", email="bob@test.com", password="test"
)
+ with self.assertWarnsMessage(RemovedInDjango60Warning, msg):
+ changelist = model_admin.get_changelist_instance(request_band_name_filter)
+ filterspec = changelist.get_filters(request_band_name_filter)[0][0]
+ self.assertEqual(filterspec.title, "sign date")
+ filterspec = changelist.get_filters(request_band_name_filter)[0][1]
+ self.assertEqual(filterspec.title, "name")
+ self.assertSequenceEqual(filterspec.lookup_choices, [self.band.name])
def test_field_arguments(self):
# If fields is specified, fieldsets_add and fieldsets_change should