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

File 17828.listfilter-proper-error-handling.2.diff, 7.5 KB (added by Julien Phalip, 13 years ago)
  • 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..2d4ecee 100644
    a b class ChangeList(object):  
    130130        # have been removed from lookup_params, which now only contains other
    131131        # parameters passed via the query string. We now loop through the
    132132        # 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
     133        # fields and to determine if at least one of them needs distinct(). If
     134        # an error occurs then it means the lookup parameters are incorrect, so
     135        # we raise the proper exception.
     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 Exception, e:
     143            # Every other error is caught with a naked except, because we don't
     144            # have any other way of validating lookup parameters. They might be
     145            # invalid if the keyword arguments are incorrect, or if the values
     146            # are not in the correct type, so we might get FieldError,
     147            # ValueError, ValidationError, or ?
     148            raise IncorrectLookupParameters(e)
    140149
    141150    def get_query_string(self, new_params=None, remove=None):
    142151        if new_params is None: new_params = {}
    class ChangeList(object):  
    292301        return ordering_fields
    293302
    294303    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)
     304        # First, we collect all the declared list filters.
     305        (self.filter_specs, self.has_filters, remaining_lookup_params,
     306         use_distinct) = self.get_filters(request)
    299307
    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
     308        # Then, we let every list filter modify the queryset to its liking.
     309        qs = self.root_query_set
     310        for filter_spec in self.filter_specs:
     311            new_qs = filter_spec.queryset(request, qs)
     312            if new_qs is not None:
     313                qs = new_qs
    306314
     315        try:
    307316            # Finally, we apply the remaining lookup parameters from the query
    308317            # string (i.e. those that haven't already been processed by the
    309318            # filters).
    class ChangeList(object):  
    317326            # have any other way of validating lookup parameters. They might be
    318327            # invalid if the keyword arguments are incorrect, or if the values
    319328            # 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.
     329            # ValueError, ValidationError, or ?.
    322330            raise IncorrectLookupParameters(e)
    323331
    324332        # 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