Browse Source

Fixed #1873 -- Handled multi-valued query parameters in admin changelist filters.

sarahboyce 2 years ago
parent
commit
d2b688b966

+ 11 - 7
django/contrib/admin/filters.py

@@ -9,6 +9,7 @@ import datetime
 
 from django.contrib.admin.options import IncorrectLookupParameters
 from django.contrib.admin.utils import (
+    build_q_object_from_lookup_parameters,
     get_last_value_from_parameters,
     get_model_from_relation,
     prepare_lookup_value,
@@ -187,7 +188,8 @@ class FieldListFilter(FacetsMixin, ListFilter):
 
     def queryset(self, request, queryset):
         try:
-            return queryset.filter(**self.used_parameters)
+            q_object = build_q_object_from_lookup_parameters(self.used_parameters)
+            return queryset.filter(q_object)
         except (ValueError, ValidationError) as e:
             # Fields may raise a ValueError or ValidationError when converting
             # the parameters to the correct type.
@@ -220,7 +222,7 @@ class RelatedFieldListFilter(FieldListFilter):
         other_model = get_model_from_relation(field)
         self.lookup_kwarg = "%s__%s__exact" % (field_path, field.target_field.name)
         self.lookup_kwarg_isnull = "%s__isnull" % field_path
-        self.lookup_val = get_last_value_from_parameters(params, self.lookup_kwarg)
+        self.lookup_val = params.get(self.lookup_kwarg)
         self.lookup_val_isnull = get_last_value_from_parameters(
             params, self.lookup_kwarg_isnull
         )
@@ -293,7 +295,8 @@ class RelatedFieldListFilter(FieldListFilter):
                 count = facet_counts[f"{pk_val}__c"]
                 val = f"{val} ({count})"
             yield {
-                "selected": self.lookup_val == str(pk_val),
+                "selected": self.lookup_val is not None
+                and str(pk_val) in self.lookup_val,
                 "query_string": changelist.get_query_string(
                     {self.lookup_kwarg: pk_val}, [self.lookup_kwarg_isnull]
                 ),
@@ -391,7 +394,7 @@ class ChoicesFieldListFilter(FieldListFilter):
     def __init__(self, field, request, params, model, model_admin, field_path):
         self.lookup_kwarg = "%s__exact" % field_path
         self.lookup_kwarg_isnull = "%s__isnull" % field_path
-        self.lookup_val = get_last_value_from_parameters(params, self.lookup_kwarg)
+        self.lookup_val = params.get(self.lookup_kwarg)
         self.lookup_val_isnull = get_last_value_from_parameters(
             params, self.lookup_kwarg_isnull
         )
@@ -432,7 +435,8 @@ class ChoicesFieldListFilter(FieldListFilter):
                 none_title = title
                 continue
             yield {
-                "selected": str(lookup) == self.lookup_val,
+                "selected": self.lookup_val is not None
+                and str(lookup) in self.lookup_val,
                 "query_string": changelist.get_query_string(
                     {self.lookup_kwarg: lookup}, [self.lookup_kwarg_isnull]
                 ),
@@ -555,7 +559,7 @@ class AllValuesFieldListFilter(FieldListFilter):
     def __init__(self, field, request, params, model, model_admin, field_path):
         self.lookup_kwarg = field_path
         self.lookup_kwarg_isnull = "%s__isnull" % field_path
-        self.lookup_val = get_last_value_from_parameters(params, self.lookup_kwarg)
+        self.lookup_val = params.get(self.lookup_kwarg)
         self.lookup_val_isnull = get_last_value_from_parameters(
             params, self.lookup_kwarg_isnull
         )
@@ -609,7 +613,7 @@ class AllValuesFieldListFilter(FieldListFilter):
                 continue
             val = str(val)
             yield {
-                "selected": self.lookup_val == val,
+                "selected": self.lookup_val is not None and val in self.lookup_val,
                 "query_string": changelist.get_query_string(
                     {self.lookup_kwarg: val}, [self.lookup_kwarg_isnull]
                 ),

+ 10 - 1
django/contrib/admin/utils.py

@@ -2,6 +2,8 @@ import datetime
 import decimal
 import json
 from collections import defaultdict
+from functools import reduce
+from operator import or_
 
 from django.core.exceptions import FieldDoesNotExist
 from django.db import models, router
@@ -64,7 +66,7 @@ def prepare_lookup_value(key, value, separator=","):
     Return a lookup value prepared to be used in queryset filtering.
     """
     if isinstance(value, list):
-        value = value[-1]
+        return [prepare_lookup_value(key, v, separator=separator) for v in value]
     # if key ends with __in, split parameter into separate values
     if key.endswith("__in"):
         value = value.split(separator)
@@ -74,6 +76,13 @@ def prepare_lookup_value(key, value, separator=","):
     return value
 
 
+def build_q_object_from_lookup_parameters(parameters):
+    q_object = models.Q()
+    for param, param_item_list in parameters.items():
+        q_object &= reduce(or_, (models.Q((param, item)) for item in param_item_list))
+    return q_object
+
+
 def quote(s):
     """
     Ensure that primary key values do not confuse the admin URLs by escaping

+ 9 - 6
django/contrib/admin/views/main.py

@@ -16,6 +16,7 @@ from django.contrib.admin.options import (
     ShowFacets,
 )
 from django.contrib.admin.utils import (
+    build_q_object_from_lookup_parameters,
     get_fields_from_path,
     lookup_spawns_duplicates,
     prepare_lookup_value,
@@ -173,9 +174,10 @@ class ChangeList:
         may_have_duplicates = False
         has_active_filters = False
 
-        for key, value in lookup_params.items():
-            if not self.model_admin.lookup_allowed(key, value[-1]):
-                raise DisallowedModelAdminLookup("Filtering by %s not allowed" % key)
+        for key, value_list in lookup_params.items():
+            for value in value_list:
+                if not self.model_admin.lookup_allowed(key, value):
+                    raise DisallowedModelAdminLookup(f"Filtering by {key} not allowed")
 
         filter_specs = []
         for list_filter in self.list_filter:
@@ -246,8 +248,8 @@ class ChangeList:
                     to_date = make_aware(to_date)
                 lookup_params.update(
                     {
-                        "%s__gte" % self.date_hierarchy: from_date,
-                        "%s__lt" % self.date_hierarchy: to_date,
+                        "%s__gte" % self.date_hierarchy: [from_date],
+                        "%s__lt" % self.date_hierarchy: [to_date],
                     }
                 )
 
@@ -534,7 +536,8 @@ class ChangeList:
             # Finally, we apply the remaining lookup parameters from the query
             # string (i.e. those that haven't already been processed by the
             # filters).
-            qs = qs.filter(**remaining_lookup_params)
+            q_object = build_q_object_from_lookup_parameters(remaining_lookup_params)
+            qs = qs.filter(q_object)
         except (SuspiciousOperation, ImproperlyConfigured):
             # Allow certain types of errors to be re-raised as-is so that the
             # caller can treat them in a special way.

+ 5 - 0
docs/releases/5.0.txt

@@ -54,6 +54,11 @@ Minor features
 * The new :meth:`.AdminSite.get_log_entries` method allows customizing the
   queryset for the site's listed log entries.
 
+* The ``django.contrib.admin.AllValuesFieldListFilter``,
+  ``ChoicesFieldListFilter``, ``RelatedFieldListFilter``, and
+  ``RelatedOnlyFieldListFilter`` admin filters now handle multi-valued query
+  parameters.
+
 :mod:`django.contrib.admindocs`
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

+ 2 - 2
tests/admin_changelist/test_date_hierarchy.py

@@ -25,8 +25,8 @@ class DateHierarchyTests(TestCase):
         request.user = self.superuser
         changelist = EventAdmin(Event, custom_site).get_changelist_instance(request)
         _, _, lookup_params, *_ = changelist.get_filters(request)
-        self.assertEqual(lookup_params["date__gte"], expected_from_date)
-        self.assertEqual(lookup_params["date__lt"], expected_to_date)
+        self.assertEqual(lookup_params["date__gte"], [expected_from_date])
+        self.assertEqual(lookup_params["date__lt"], [expected_to_date])
 
     def test_bounded_params(self):
         tests = (

+ 94 - 0
tests/admin_filters/tests.py

@@ -1533,6 +1533,100 @@ class ListFiltersTests(TestCase):
                     expected_displays,
                 )
 
+    def test_multi_related_field_filter(self):
+        modeladmin = DecadeFilterBookAdmin(Book, site)
+        request = self.request_factory.get(
+            "/",
+            [("author__id__exact", self.alfred.pk), ("author__id__exact", self.bob.pk)],
+        )
+        request.user = self.alfred
+        changelist = modeladmin.get_changelist_instance(request)
+        queryset = changelist.get_queryset(request)
+        self.assertSequenceEqual(
+            queryset,
+            list(
+                Book.objects.filter(
+                    author__pk__in=[self.alfred.pk, self.bob.pk]
+                ).order_by("-id")
+            ),
+        )
+        filterspec = changelist.get_filters(request)[0][0]
+        choices = list(filterspec.choices(changelist))
+        expected_choice_values = [
+            ("All", False, "?"),
+            ("alfred", True, f"?author__id__exact={self.alfred.pk}"),
+            ("bob", True, f"?author__id__exact={self.bob.pk}"),
+            ("lisa", False, f"?author__id__exact={self.lisa.pk}"),
+        ]
+        for i, (display, selected, query_string) in enumerate(expected_choice_values):
+            self.assertEqual(choices[i]["display"], display)
+            self.assertIs(choices[i]["selected"], selected)
+            self.assertEqual(choices[i]["query_string"], query_string)
+
+    def test_multi_choice_field_filter(self):
+        modeladmin = DecadeFilterBookAdmin(Book, site)
+        request = self.request_factory.get(
+            "/",
+            [("category__exact", "non-fiction"), ("category__exact", "fiction")],
+        )
+        request.user = self.alfred
+        changelist = modeladmin.get_changelist_instance(request)
+        queryset = changelist.get_queryset(request)
+        self.assertSequenceEqual(
+            queryset,
+            list(
+                Book.objects.filter(category__in=["non-fiction", "fiction"]).order_by(
+                    "-id"
+                )
+            ),
+        )
+        filterspec = changelist.get_filters(request)[0][3]
+        choices = list(filterspec.choices(changelist))
+        expected_choice_values = [
+            ("All", False, "?"),
+            ("Non-Fictional", True, "?category__exact=non-fiction"),
+            ("Fictional", True, "?category__exact=fiction"),
+            ("We don't know", False, "?category__exact="),
+            ("Not categorized", False, "?category__isnull=True"),
+        ]
+        for i, (display, selected, query_string) in enumerate(expected_choice_values):
+            self.assertEqual(choices[i]["display"], display)
+            self.assertIs(choices[i]["selected"], selected)
+            self.assertEqual(choices[i]["query_string"], query_string)
+
+    def test_multi_all_values_field_filter(self):
+        modeladmin = DecadeFilterBookAdmin(Book, site)
+        request = self.request_factory.get(
+            "/",
+            [
+                ("author__email", "bob@example.com"),
+                ("author__email", "lisa@example.com"),
+            ],
+        )
+        request.user = self.alfred
+        changelist = modeladmin.get_changelist_instance(request)
+        queryset = changelist.get_queryset(request)
+        self.assertSequenceEqual(
+            queryset,
+            list(
+                Book.objects.filter(
+                    author__email__in=["bob@example.com", "lisa@example.com"]
+                ).order_by("-id")
+            ),
+        )
+        filterspec = changelist.get_filters(request)[0][5]
+        choices = list(filterspec.choices(changelist))
+        expected_choice_values = [
+            ("All", False, "?"),
+            ("alfred@example.com", False, "?author__email=alfred%40example.com"),
+            ("bob@example.com", True, "?author__email=bob%40example.com"),
+            ("lisa@example.com", True, "?author__email=lisa%40example.com"),
+        ]
+        for i, (display, selected, query_string) in enumerate(expected_choice_values):
+            self.assertEqual(choices[i]["display"], display)
+            self.assertIs(choices[i]["selected"], selected)
+            self.assertEqual(choices[i]["query_string"], query_string)
+
     def test_two_characters_long_field(self):
         """
         list_filter works with two-characters long field names (#16080).

+ 15 - 0
tests/admin_utils/tests.py

@@ -7,6 +7,7 @@ from django.contrib import admin
 from django.contrib.admin import helpers
 from django.contrib.admin.utils import (
     NestedObjects,
+    build_q_object_from_lookup_parameters,
     display_for_field,
     display_for_value,
     flatten,
@@ -424,3 +425,17 @@ class UtilsTests(SimpleTestCase):
 
     def test_quote(self):
         self.assertEqual(quote("something\nor\nother"), "something_0Aor_0Aother")
+
+    def test_build_q_object_from_lookup_parameters(self):
+        parameters = {
+            "title__in": [["Article 1", "Article 2"]],
+            "hist__iexact": ["history"],
+            "site__pk": [1, 2],
+        }
+        q_obj = build_q_object_from_lookup_parameters(parameters)
+        self.assertEqual(
+            q_obj,
+            models.Q(title__in=["Article 1", "Article 2"])
+            & models.Q(hist__iexact="history")
+            & (models.Q(site__pk=1) | models.Q(site__pk=2)),
+        )