Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #16716 -- Fixed two small regressions in the development versio…

…n introduced in r16144 where the changelist crashed with a 500 error instead of nicely operating a 302 redirection back to the changelist.

The two specific cases were:

* a lookup through a non-existing field and apparently spanning multiple relationships (e.g. "?nonexistant__whatever=xxxx").
* a proper list_filter's queryset failing with an exception. In Django 1.3 the queryset was only directly manipulated by the changelist, whereas in 1.4 the list_filters may manipulate the queryset themselves. The fix here implies catching potential failures from the list_filters too.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@16705 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 93fbb77d9b5f4ab936c8f29b20a704d9b11be0c7 1 parent ce477f0
Julien Phalip jphalip authored
43 django/contrib/admin/views/main.py
View
@@ -267,6 +267,7 @@ def get_lookup_params(self, use_distinct=False):
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 @@ def get_lookup_params(self, use_distinct=False):
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 @@ def get_query_set(self, request):
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
21 tests/regressiontests/admin_filters/tests.py
View
@@ -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 @@ def test_simplelistfilter_with_none_returning_lookups(self):
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('/', {})
5 tests/regressiontests/admin_views/tests.py
View
@@ -410,6 +410,11 @@ def testIncorrectLookupParameters(self):
"""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)
Please sign in to comment.
Something went wrong with that request. Please try again.