Commit 93fbb77d authored by Julien Phalip's avatar Julien Phalip
Browse files

Fixed #16716 -- Fixed two small regressions in the development version... 

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.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@16705 bcc190cf-cafb-0310-a4f2-bffc1f526a37
parent ce477f08

django/contrib/admin/views/main.py

+23 −20
Original line number Diff line number Diff line
@@ -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,7 +301,9 @@ 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

        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)
@@ -312,16 +315,16 @@ class ChangeList(object): 
                        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

tests/regressiontests/admin_filters/tests.py

+20 −1
Original line number Diff line number Diff line 
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('/', {})

tests/regressiontests/admin_views/tests.py

+5 −0
Original line number Diff line number Diff line
@@ -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)