Author: julien
Date: 2011-08-28 00:57:56 -0700 (Sun, 28 Aug 2011)
New Revision: 16705
Modified:
django/trunk/django/contrib/admin/views/main.py
django/trunk/tests/regressiontests/admin_filters/tests.py
django/trunk/tests/regressiontests/admin_views/tests.py
Log:
Fixed #16716 -- Fixed two small regressions in the development version
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.
Modified: django/trunk/django/contrib/admin/views/main.py
===================================================================
--- django/trunk/django/contrib/admin/views/main.py 2011-08-28 06:49:29 UTC
(rev 16704)
+++ django/trunk/django/contrib/admin/views/main.py 2011-08-28 07:57:56 UTC
(rev 16705)
@@ -267,6 +267,7 @@
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 @@
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 @@
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
+ 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
- # Apply the remaining lookup parameters from the query string (i.e.
- # those that haven't already been processed by the filters).
- try:
+ # 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
Modified: django/trunk/tests/regressiontests/admin_filters/tests.py
===================================================================
--- django/trunk/tests/regressiontests/admin_filters/tests.py 2011-08-28
06:49:29 UTC (rev 16704)
+++ django/trunk/tests/regressiontests/admin_filters/tests.py 2011-08-28
07:57:56 UTC (rev 16705)
@@ -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 @@
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 DecadeFilterBookAdminWithNoneReturningLookups(ModelAdmin):
list_filter = (DecadeListFilterWithNoneReturningLookups,)
+class DecadeFilterBookAdminWithFailingQueryset(ModelAdmin):
+ list_filter = (DecadeListFilterWithFailingQueryset,)
+
class DecadeFilterBookAdminWithQuerysetBasedLookups(ModelAdmin):
list_filter = (DecadeListFilterWithQuerysetBasedLookups,)
@@ -509,6 +517,17 @@
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('/', {})
Modified: django/trunk/tests/regressiontests/admin_views/tests.py
===================================================================
--- django/trunk/tests/regressiontests/admin_views/tests.py 2011-08-28
06:49:29 UTC (rev 16704)
+++ django/trunk/tests/regressiontests/admin_views/tests.py 2011-08-28
07:57:56 UTC (rev 16705)
@@ -410,6 +410,11 @@
"""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)
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
