diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 090b238dec..f741fd0764 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -267,6 +267,7 @@ class ChangeList(object): del lookup_params[key] lookup_params[smart_str(key)] = value + field = None if not use_distinct: # Check if it's a relationship that might return more than one # instance @@ -291,7 +292,7 @@ class ChangeList(object): value = True lookup_params[key] = value - if not self.model_admin.lookup_allowed(key, value): + if field and not self.model_admin.lookup_allowed(key, value): raise SuspiciousOperation("Filtering by %s not allowed" % key) return lookup_params, use_distinct @@ -300,28 +301,30 @@ class ChangeList(object): lookup_params, use_distinct = self.get_lookup_params(use_distinct=False) self.filter_specs, self.has_filters = self.get_filters(request, use_distinct) - # Let every list filter modify the qs and params to its liking - qs = self.root_query_set - for filter_spec in self.filter_specs: - new_qs = filter_spec.queryset(request, qs) - if new_qs is not None: - qs = new_qs - for param in filter_spec.used_params(): - try: - del lookup_params[param] - except KeyError: - pass - - # Apply the remaining lookup parameters from the query string (i.e. - # those that haven't already been processed by the filters). try: + # First, let every list filter modify the qs and params to its + # liking. + qs = self.root_query_set + for filter_spec in self.filter_specs: + new_qs = filter_spec.queryset(request, qs) + if new_qs is not None: + qs = new_qs + for param in filter_spec.used_params(): + try: + del lookup_params[param] + except KeyError: + pass + + # Then, apply the remaining lookup parameters from the query string + # (i.e. those that haven't already been processed by the filters). qs = qs.filter(**lookup_params) - # Naked except! Because we don't have any other way of validating "params". - # They might be invalid if the keyword arguments are incorrect, or if the - # values are not in the correct type, so we might get FieldError, ValueError, - # ValicationError, or ? from a custom field that raises yet something else - # when handed impossible data. except Exception, e: + # Naked except! Because we don't have any other way of validating + # "lookup_params". They might be invalid if the keyword arguments + # are incorrect, or if the values are not in the correct type, so + # we might get FieldError, ValueError, ValicationError, or ? from a + # custom field that raises yet something else when handed + # impossible data. raise IncorrectLookupParameters(e) # Use select_related() if one of the list_display options is a field diff --git a/tests/regressiontests/admin_filters/tests.py b/tests/regressiontests/admin_filters/tests.py index fd82a7aba3..e089ce24fb 100644 --- a/tests/regressiontests/admin_filters/tests.py +++ b/tests/regressiontests/admin_filters/tests.py @@ -1,9 +1,9 @@ import datetime +from django.contrib.admin.options import IncorrectLookupParameters from django.core.exceptions import ImproperlyConfigured from django.test import TestCase, RequestFactory from django.utils.encoding import force_unicode - from django.contrib.auth.admin import UserAdmin from django.contrib.auth.models import User from django.contrib.admin.views.main import ChangeList @@ -50,6 +50,11 @@ class DecadeListFilterWithNoneReturningLookups(DecadeListFilterWithTitleAndParam def lookups(self, request, model_admin): pass +class DecadeListFilterWithFailingQueryset(DecadeListFilterWithTitleAndParameter): + + def queryset(self, request, queryset): + raise Exception + class DecadeListFilterWithQuerysetBasedLookups(DecadeListFilterWithTitleAndParameter): def lookups(self, request, model_admin): @@ -84,6 +89,9 @@ class DecadeFilterBookAdminWithoutParameter(ModelAdmin): class DecadeFilterBookAdminWithNoneReturningLookups(ModelAdmin): list_filter = (DecadeListFilterWithNoneReturningLookups,) +class DecadeFilterBookAdminWithFailingQueryset(ModelAdmin): + list_filter = (DecadeListFilterWithFailingQueryset,) + class DecadeFilterBookAdminWithQuerysetBasedLookups(ModelAdmin): list_filter = (DecadeListFilterWithQuerysetBasedLookups,) @@ -509,6 +517,17 @@ class ListFiltersTests(TestCase): filterspec = changelist.get_filters(request)[0] self.assertEqual(len(filterspec), 0) + def test_filter_with_failing_queryset(self): + """ + Ensure that a filter's failing queryset is interpreted as if incorrect + lookup parameters were passed (therefore causing a 302 redirection to + the changelist). + Refs #16716, #16714. + """ + modeladmin = DecadeFilterBookAdminWithFailingQueryset(Book, site) + request = self.request_factory.get('/', {}) + self.assertRaises(IncorrectLookupParameters, self.get_changelist, request, Book, modeladmin) + def test_simplelistfilter_with_queryset_based_lookups(self): modeladmin = DecadeFilterBookAdminWithQuerysetBasedLookups(Book, site) request = self.request_factory.get('/', {}) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index 0d1628c366..22b65a6cb8 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -410,6 +410,11 @@ class AdminViewBasicTest(TestCase): """Ensure incorrect lookup parameters are handled gracefully.""" response = self.client.get('/test_admin/%s/admin_views/thing/' % self.urlbit, {'notarealfield': '5'}) self.assertRedirects(response, '/test_admin/%s/admin_views/thing/?e=1' % self.urlbit) + + # Spanning relationships through an inexistant related object (Refs #16716) + response = self.client.get('/test_admin/%s/admin_views/thing/' % self.urlbit, {'notarealfield__whatever': '5'}) + self.assertRedirects(response, '/test_admin/%s/admin_views/thing/?e=1' % self.urlbit) + response = self.client.get('/test_admin/%s/admin_views/thing/' % self.urlbit, {'color__id__exact': 'StringNotInteger!'}) self.assertRedirects(response, '/test_admin/%s/admin_views/thing/?e=1' % self.urlbit)