Fixed #31597, #31603 -- Corrected admin clear all filters link behavior.

- Show "Clear all filters" only when any filter is selected.
- Preserve query string not related with filters.

Co-Authored-By: Hasan Ramezani <hasan.r67@gmail.com>

django/contrib/admin/templates/admin/change_list.html

@@ -60,8 +60,8 @@
         {% if cl.has_filters %}
           <div id="changelist-filter">
             <h2>{% translate 'Filter' %}</h2>
-            {% if cl.preserved_filters %}<h3 id="changelist-filter-clear">
+            {% if cl.has_active_filters %}<h3 id="changelist-filter-clear">
-              <a href="?{% if cl.is_popup %}_popup=1{% endif %}">&#10006; {% translate "Clear all filters" %}</a>
+              <a href="{{ cl.clear_all_filters_qs }}">&#10006; {% translate "Clear all filters" %}</a>
             </h3>{% endif %}
             {% for spec in cl.filter_specs %}{% admin_list_filter cl spec %}{% endfor %}
           </div>

django/contrib/admin/views/main.py

@@ -59,6 +59,8 @@ class ChangeList:
         self.list_display_links = list_display_links
         self.list_filter = list_filter
         self.has_filters = None
+        self.has_active_filters = None
+        self.clear_all_filters_qs = None
         self.date_hierarchy = date_hierarchy
         self.search_fields = search_fields
         self.list_select_related = list_select_related
@@ -121,6 +123,7 @@ class ChangeList:
     def get_filters(self, request):
         lookup_params = self.get_filters_params()
         use_distinct = False
+        has_active_filters = False
 
         for key, value in lookup_params.items():
             if not self.model_admin.lookup_allowed(key, value):
@@ -128,6 +131,7 @@ class ChangeList:
 
         filter_specs = []
         for list_filter in self.list_filter:
+            lookup_params_count = len(lookup_params)
             if callable(list_filter):
                 # This is simply a custom list filter class.
                 spec = list_filter(request, lookup_params, self.model, self.model_admin)
@@ -145,7 +149,6 @@ class ChangeList:
                     field_path = field
                     field = get_fields_from_path(self.model, field_path)[-1]
 
-                lookup_params_count = len(lookup_params)
                 spec = field_list_filter_class(
                     field, request, lookup_params,
                     self.model, self.model_admin, field_path=field_path,
@@ -157,6 +160,8 @@ class ChangeList:
                     use_distinct = use_distinct or lookup_needs_distinct(self.lookup_opts, field_path)
             if spec and spec.has_output():
                 filter_specs.append(spec)
+                if lookup_params_count > len(lookup_params):
+                    has_active_filters = True
 
         if self.date_hierarchy:
             # Create bounded lookup parameters so that the query is more
@@ -199,7 +204,10 @@ class ChangeList:
             for key, value in lookup_params.items():
                 lookup_params[key] = prepare_lookup_value(key, value)
                 use_distinct = use_distinct or lookup_needs_distinct(self.lookup_opts, key)
-            return filter_specs, bool(filter_specs), lookup_params, use_distinct
+            return (
+                filter_specs, bool(filter_specs), lookup_params, use_distinct,
+                has_active_filters,
+            )
         except FieldDoesNotExist as e:
             raise IncorrectLookupParameters(e) from e
 
@@ -433,9 +441,13 @@ class ChangeList:
 
     def get_queryset(self, request):
         # First, we collect all the declared list filters.
-        (self.filter_specs, self.has_filters, remaining_lookup_params,
+        (
-         filters_use_distinct) = self.get_filters(request)
+            self.filter_specs,
-
+            self.has_filters,
+            remaining_lookup_params,
+            filters_use_distinct,
+            self.has_active_filters,
+        ) = self.get_filters(request)
         # Then, we let every list filter modify the queryset to its liking.
         qs = self.root_queryset
         for filter_spec in self.filter_specs:
@@ -470,6 +482,11 @@ class ChangeList:
         # Apply search results
         qs, search_use_distinct = self.model_admin.get_search_results(request, qs, self.query)
 
+        # Set query string for clearing all filters.
+        self.clear_all_filters_qs = self.get_query_string(
+            new_params=remaining_lookup_params,
+            remove=self.get_filters_params(),
+        )
         # Remove duplicates from results, if necessary
         if filters_use_distinct | search_use_distinct:
             return qs.distinct()

tests/admin_changelist/admin.py

@@ -3,7 +3,7 @@ from django.contrib.auth.admin import UserAdmin
 from django.contrib.auth.models import User
 from django.core.paginator import Paginator
 
-from .models import Child, Event, Parent, Swallow
+from .models import Band, Child, Event, Parent, Swallow
 
 site = admin.AdminSite(name="admin")
 
@@ -59,6 +59,31 @@ class BandAdmin(admin.ModelAdmin):
     list_filter = ['genres']
 
 
+class NrOfMembersFilter(admin.SimpleListFilter):
+    title = 'number of members'
+    parameter_name = 'nr_of_members_partition'
+
+    def lookups(self, request, model_admin):
+        return [
+            ('5', '0 - 5'),
+            ('more', 'more than 5'),
+        ]
+
+    def queryset(self, request, queryset):
+        value = self.value()
+        if value == '5':
+            return queryset.filter(nr_of_members__lte=5)
+        if value == 'more':
+            return queryset.filter(nr_of_members__gt=5)
+
+
+class BandCallableFilterAdmin(admin.ModelAdmin):
+    list_filter = [NrOfMembersFilter]
+
+
+site.register(Band, BandCallableFilterAdmin)
+
+
 class GroupAdmin(admin.ModelAdmin):
     list_filter = ['members']
 

tests/admin_changelist/test_date_hierarchy.py

@@ -21,7 +21,7 @@ class DateHierarchyTests(TestCase):
         request = self.factory.get('/', query)
         request.user = self.superuser
         changelist = EventAdmin(Event, custom_site).get_changelist_instance(request)
-        _, _, lookup_params, _ = changelist.get_filters(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)
 

tests/admin_changelist/tests.py

@@ -5,7 +5,9 @@ from django.contrib.admin.models import LogEntry
 from django.contrib.admin.options import IncorrectLookupParameters
 from django.contrib.admin.templatetags.admin_list import pagination
 from django.contrib.admin.tests import AdminSeleniumTestCase
-from django.contrib.admin.views.main import ALL_VAR, SEARCH_VAR
+from django.contrib.admin.views.main import (
+    ALL_VAR, IS_POPUP_VAR, ORDER_VAR, PAGE_VAR, SEARCH_VAR, TO_FIELD_VAR,
+)
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.messages.storage.cookie import CookieStorage
@@ -708,15 +710,65 @@ class ChangeListTests(TestCase):
 
     def test_clear_all_filters_link(self):
         self.client.force_login(self.superuser)
-        link = '<a href="?">&#10006; Clear all filters</a>'
+        url = reverse('admin:auth_user_changelist')
-        response = self.client.get(reverse('admin:auth_user_changelist'))
+        response = self.client.get(url)
-        self.assertNotContains(response, link)
+        self.assertNotContains(response, '&#10006; Clear all filters')
+        link = '<a href="%s">&#10006; Clear all filters</a>'
+        for data, href in (
+            ({'is_staff__exact': '0'}, '?'),
+            (
+                {'is_staff__exact': '0', 'username__startswith': 'test'},
+                '?username__startswith=test',
+            ),
+            (
+                {'is_staff__exact': '0', SEARCH_VAR: 'test'},
+                '?%s=test' % SEARCH_VAR,
+            ),
+            (
+                {'is_staff__exact': '0', IS_POPUP_VAR: 'id'},
+                '?%s=id' % IS_POPUP_VAR,
+            ),
+        ):
+            with self.subTest(data=data):
+                response = self.client.get(url, data=data)
+                self.assertContains(response, link % href)
+
+    def test_clear_all_filters_link_callable_filter(self):
+        self.client.force_login(self.superuser)
+        url = reverse('admin:admin_changelist_band_changelist')
+        response = self.client.get(url)
+        self.assertNotContains(response, '&#10006; Clear all filters')
+        link = '<a href="%s">&#10006; Clear all filters</a>'
+        for data, href in (
+            ({'nr_of_members_partition': '5'}, '?'),
+            (
+                {'nr_of_members_partition': 'more', 'name__startswith': 'test'},
+                '?name__startswith=test',
+            ),
+            (
+                {'nr_of_members_partition': '5', IS_POPUP_VAR: 'id'},
+                '?%s=id' % IS_POPUP_VAR,
+            ),
+        ):
+            with self.subTest(data=data):
+                response = self.client.get(url, data=data)
+                self.assertContains(response, link % href)
+
+    def test_no_clear_all_filters_link(self):
+        self.client.force_login(self.superuser)
+        url = reverse('admin:auth_user_changelist')
+        link = '>&#10006; Clear all filters</a>'
         for data in (
             {SEARCH_VAR: 'test'},
-            {'is_staff__exact': '0'},
+            {ORDER_VAR: '-1'},
+            {TO_FIELD_VAR: 'id'},
+            {PAGE_VAR: '1'},
+            {IS_POPUP_VAR: '1'},
+            {'username__startswith': 'test'},
         ):
-            response = self.client.get(reverse('admin:auth_user_changelist'), data=data)
+            with self.subTest(data=data):
-            self.assertContains(response, link)
+                response = self.client.get(url, data=data)
+                self.assertNotContains(response, link)
 
     def test_tuple_list_display(self):
         swallow = Swallow.objects.create(origin='Africa', load='12.34', speed='22.2')