Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #17828 -- Ensured that when a list filter's `queryset()` method…

… fails, it does so loudly instead of getting swallowed by a `IncorrectLookupParameters` exception. This also properly fixes #16705, which hadn't been addressed correctly in [16705].

git-svn-id: http://code.djangoproject.com/svn/django/trunk@17763 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 1ff9be1144e80802ea810e6e70ef2b54b6e42047 1 parent b452439
@jphalip jphalip authored
View
20 django/contrib/admin/options.py
@@ -247,7 +247,12 @@ def lookup_allowed(self, lookup, value):
# the pk attribute name.
pk_attr_name = None
for part in parts[:-1]:
- field, _, _, _ = model._meta.get_field_by_name(part)
+ try:
+ field, _, _, _ = model._meta.get_field_by_name(part)
+ except FieldDoesNotExist:
+ # Lookups on non-existants fields are ok, since they're ignored
+ # later.
+ return True
if hasattr(field, 'rel'):
model = field.rel.to
pk_attr_name = model._meta.pk.name
@@ -259,17 +264,10 @@ def lookup_allowed(self, lookup, value):
if pk_attr_name and len(parts) > 1 and parts[-1] == pk_attr_name:
parts.pop()
- try:
- self.model._meta.get_field_by_name(parts[0])
- except FieldDoesNotExist:
- # Lookups on non-existants fields are ok, since they're ignored
- # later.
+ if len(parts) == 1:
return True
- else:
- if len(parts) == 1:
- return True
- clean_lookup = LOOKUP_SEP.join(parts)
- return clean_lookup in self.list_filter or clean_lookup == self.date_hierarchy
+ clean_lookup = LOOKUP_SEP.join(parts)
+ return clean_lookup in self.list_filter or clean_lookup == self.date_hierarchy
def has_add_permission(self, request):
"""
View
41 django/contrib/admin/views/main.py
@@ -3,6 +3,7 @@
from django.core.exceptions import SuspiciousOperation, ImproperlyConfigured
from django.core.paginator import InvalidPage
from django.db import models
+from django.db.models.fields import FieldDoesNotExist
from django.utils.datastructures import SortedDict
from django.utils.encoding import force_unicode, smart_str
from django.utils.translation import ugettext, ugettext_lazy
@@ -130,13 +131,16 @@ def get_filters(self, request):
# have been removed from lookup_params, which now only contains other
# parameters passed via the query string. We now loop through the
# remaining parameters both to ensure that all the parameters are valid
- # fields and to determine if at least one of them needs distinct().
- 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
+ # fields and to determine if at least one of them needs distinct(). If
+ # the lookup parameters aren't real fields, then bail out.
+ try:
+ 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
+ except FieldDoesNotExist, e:
+ raise IncorrectLookupParameters(e)
def get_query_string(self, new_params=None, remove=None):
if new_params is None: new_params = {}
@@ -292,18 +296,18 @@ def get_ordering_field_columns(self):
return ordering_fields
def get_query_set(self, request):
- try:
- # First, we collect all the declared list filters.
- (self.filter_specs, self.has_filters, remaining_lookup_params,
- use_distinct) = self.get_filters(request)
+ # First, we collect all the declared list filters.
+ (self.filter_specs, self.has_filters, remaining_lookup_params,
+ use_distinct) = self.get_filters(request)
- # Then, we let every list filter modify the qs 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
+ # Then, we let every list filter modify the queryset 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
+ try:
# Finally, we apply the remaining lookup parameters from the query
# string (i.e. those that haven't already been processed by the
# filters).
@@ -317,8 +321,7 @@ def get_query_set(self, request):
# have any other way of validating lookup parameters. 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, ValidationError, or ? from a custom field that raises
- # yet something else when handed impossible data.
+ # ValueError, ValidationError, or ?.
raise IncorrectLookupParameters(e)
# Use select_related() if one of the list_display options is a field
View
14 tests/regressiontests/admin_filters/tests.py
@@ -56,7 +56,7 @@ def lookups(self, request, model_admin):
class DecadeListFilterWithFailingQueryset(DecadeListFilterWithTitleAndParameter):
def queryset(self, request, queryset):
- raise Exception
+ raise 1/0
class DecadeListFilterWithQuerysetBasedLookups(DecadeListFilterWithTitleAndParameter):
@@ -77,6 +77,7 @@ class DecadeListFilterParameterEndsWith__Isnull(DecadeListFilter):
title = 'publication decade'
parameter_name = 'decade__isnull' # Ends with '__isnull"
+
class CustomUserAdmin(UserAdmin):
list_filter = ('books_authored', 'books_contributed')
@@ -112,6 +113,8 @@ class DecadeFilterBookAdminParameterEndsWith__In(ModelAdmin):
class DecadeFilterBookAdminParameterEndsWith__Isnull(ModelAdmin):
list_filter = (DecadeListFilterParameterEndsWith__Isnull,)
+
+
class ListFiltersTests(TestCase):
def setUp(self):
@@ -542,14 +545,13 @@ def test_simplelistfilter_with_none_returning_lookups(self):
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.
+ Ensure that when a filter's queryset method fails, it fails loudly and
+ the corresponding exception doesn't get swallowed.
+ Refs #17828.
"""
modeladmin = DecadeFilterBookAdminWithFailingQueryset(Book, site)
request = self.request_factory.get('/', {})
- self.assertRaises(IncorrectLookupParameters, self.get_changelist, request, Book, modeladmin)
+ self.assertRaises(ZeroDivisionError, self.get_changelist, request, Book, modeladmin)
def test_simplelistfilter_with_queryset_based_lookups(self):
modeladmin = DecadeFilterBookAdminWithQuerysetBasedLookups(Book, site)
Please sign in to comment.
Something went wrong with that request. Please try again.