Ticket #17828: 17828.listfilter-proper-error-handling.3.diff

File 17828.listfilter-proper-error-handling.3.diff, 7.5 KB (added by Julien Phalip, 13 years ago)

Without naked except

  • django/contrib/admin/options.py

    diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
    index f5f6256..73c2958 100644
    a b class BaseModelAdmin(object):  
    247247        # the pk attribute name.
    248248        pk_attr_name = None
    249249        for part in parts[:-1]:
    250             field, _, _, _ = model._meta.get_field_by_name(part)
     250            try:
     251                field, _, _, _ = model._meta.get_field_by_name(part)
     252            except FieldDoesNotExist:
     253                # Lookups on non-existants fields are ok, since they're ignored
     254                # later.
     255                return True
    251256            if hasattr(field, 'rel'):
    252257                model = field.rel.to
    253258                pk_attr_name = model._meta.pk.name
    class BaseModelAdmin(object):  
    259264        if pk_attr_name and len(parts) > 1 and parts[-1] == pk_attr_name:
    260265            parts.pop()
    261266
    262         try:
    263             self.model._meta.get_field_by_name(parts[0])
    264         except FieldDoesNotExist:
    265             # Lookups on non-existants fields are ok, since they're ignored
    266             # later.
     267        if len(parts) == 1:
    267268            return True
    268         else:
    269             if len(parts) == 1:
    270                 return True
    271             clean_lookup = LOOKUP_SEP.join(parts)
    272             return clean_lookup in self.list_filter or clean_lookup == self.date_hierarchy
     269        clean_lookup = LOOKUP_SEP.join(parts)
     270        return clean_lookup in self.list_filter or clean_lookup == self.date_hierarchy
    273271
    274272    def has_add_permission(self, request):
    275273        """
  • django/contrib/admin/views/main.py

    diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py
    index 56f13f8..b11f9d5 100644
    a b import operator  
    33from django.core.exceptions import SuspiciousOperation, ImproperlyConfigured
    44from django.core.paginator import InvalidPage
    55from django.db import models
     6from django.db.models.fields import FieldDoesNotExist
    67from django.utils.datastructures import SortedDict
    78from django.utils.encoding import force_unicode, smart_str
    89from django.utils.translation import ugettext, ugettext_lazy
    class ChangeList(object):  
    130131        # have been removed from lookup_params, which now only contains other
    131132        # parameters passed via the query string. We now loop through the
    132133        # remaining parameters both to ensure that all the parameters are valid
    133         # fields and to determine if at least one of them needs distinct().
    134         for key, value in lookup_params.items():
    135             lookup_params[key] = prepare_lookup_value(key, value)
    136             use_distinct = (use_distinct or
    137                             lookup_needs_distinct(self.lookup_opts, key))
    138 
    139         return filter_specs, bool(filter_specs), lookup_params, use_distinct
     134        # fields and to determine if at least one of them needs distinct(). If
     135        # the lookup parameters aren't real fields, then bail out.
     136        try:
     137            for key, value in lookup_params.items():
     138                lookup_params[key] = prepare_lookup_value(key, value)
     139                use_distinct = (use_distinct or
     140                                lookup_needs_distinct(self.lookup_opts, key))
     141            return filter_specs, bool(filter_specs), lookup_params, use_distinct
     142        except FieldDoesNotExist, e:
     143            raise IncorrectLookupParameters(e)
    140144
    141145    def get_query_string(self, new_params=None, remove=None):
    142146        if new_params is None: new_params = {}
    class ChangeList(object):  
    292296        return ordering_fields
    293297
    294298    def get_query_set(self, request):
    295         try:
    296             # First, we collect all the declared list filters.
    297             (self.filter_specs, self.has_filters, remaining_lookup_params,
    298              use_distinct) = self.get_filters(request)
     299        # First, we collect all the declared list filters.
     300        (self.filter_specs, self.has_filters, remaining_lookup_params,
     301         use_distinct) = self.get_filters(request)
    299302
    300             # Then, we let every list filter modify the qs to its liking.
    301             qs = self.root_query_set
    302             for filter_spec in self.filter_specs:
    303                 new_qs = filter_spec.queryset(request, qs)
    304                 if new_qs is not None:
    305                     qs = new_qs
     303        # Then, we let every list filter modify the queryset to its liking.
     304        qs = self.root_query_set
     305        for filter_spec in self.filter_specs:
     306            new_qs = filter_spec.queryset(request, qs)
     307            if new_qs is not None:
     308                qs = new_qs
    306309
     310        try:
    307311            # Finally, we apply the remaining lookup parameters from the query
    308312            # string (i.e. those that haven't already been processed by the
    309313            # filters).
    class ChangeList(object):  
    317321            # have any other way of validating lookup parameters. They might be
    318322            # invalid if the keyword arguments are incorrect, or if the values
    319323            # are not in the correct type, so we might get FieldError,
    320             # ValueError, ValidationError, or ? from a custom field that raises
    321             # yet something else when handed impossible data.
     324            # ValueError, ValidationError, or ?.
    322325            raise IncorrectLookupParameters(e)
    323326
    324327        # Use select_related() if one of the list_display options is a field
  • tests/regressiontests/admin_filters/tests.py

    diff --git a/tests/regressiontests/admin_filters/tests.py b/tests/regressiontests/admin_filters/tests.py
    index 539bb0c..b42dfd7 100644
    a b class DecadeListFilterWithNoneReturningLookups(DecadeListFilterWithTitleAndParam  
    5656class DecadeListFilterWithFailingQueryset(DecadeListFilterWithTitleAndParameter):
    5757
    5858    def queryset(self, request, queryset):
    59         raise Exception
     59        raise 1/0
    6060
    6161class DecadeListFilterWithQuerysetBasedLookups(DecadeListFilterWithTitleAndParameter):
    6262
    class DecadeListFilterParameterEndsWith__Isnull(DecadeListFilter):  
    7777    title = 'publication decade'
    7878    parameter_name = 'decade__isnull' # Ends with '__isnull"
    7979
     80
    8081class CustomUserAdmin(UserAdmin):
    8182    list_filter = ('books_authored', 'books_contributed')
    8283
    class DecadeFilterBookAdminParameterEndsWith__In(ModelAdmin):  
    112113class DecadeFilterBookAdminParameterEndsWith__Isnull(ModelAdmin):
    113114    list_filter = (DecadeListFilterParameterEndsWith__Isnull,)
    114115
     116
     117
    115118class ListFiltersTests(TestCase):
    116119
    117120    def setUp(self):
    class ListFiltersTests(TestCase):  
    542545
    543546    def test_filter_with_failing_queryset(self):
    544547        """
    545         Ensure that a filter's failing queryset is interpreted as if incorrect
    546         lookup parameters were passed (therefore causing a 302 redirection to
    547         the changelist).
    548         Refs #16716, #16714.
     548        Ensure that when a filter's queryset method fails, it fails loudly and
     549        the corresponding exception doesn't get swallowed.
     550        Refs #17828.
    549551        """
    550552        modeladmin = DecadeFilterBookAdminWithFailingQueryset(Book, site)
    551553        request = self.request_factory.get('/', {})
    552         self.assertRaises(IncorrectLookupParameters, self.get_changelist, request, Book, modeladmin)
     554        self.assertRaises(ZeroDivisionError, self.get_changelist, request, Book, modeladmin)
    553555
    554556    def test_simplelistfilter_with_queryset_based_lookups(self):
    555557        modeladmin = DecadeFilterBookAdminWithQuerysetBasedLookups(Book, site)
Back to Top