Ticket #17091: 17091.changelist-lookup-filters-untangling.diff
File 17091.changelist-lookup-filters-untangling.diff, 25.5 KB (added by , 13 years ago) |
---|
-
django/contrib/admin/filters.py
diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py index ed98a9e..abe0e1b 100644
a b from django.utils.encoding import smart_unicode 13 13 from django.utils.translation import ugettext_lazy as _ 14 14 15 15 from django.contrib.admin.util import (get_model_from_relation, 16 reverse_field_path, get_limit_choices_to_from_path) 16 reverse_field_path, get_limit_choices_to_from_path, 17 prepare_lookup_value_for_queryset_filtering) 17 18 18 19 class ListFilter(object): 19 20 title = None # Human-readable title to appear in the right sidebar. 20 21 21 22 def __init__(self, request, params, model, model_admin): 22 self. params = params23 self.used_params = {} 23 24 if self.title is None: 24 25 raise ImproperlyConfigured( 25 26 "The list filter '%s' does not specify " … … class ListFilter(object): 43 44 """ 44 45 raise NotImplementedError 45 46 46 def used_param s(self):47 def used_param_names(self): 47 48 """ 48 49 Return a list of parameters to consume from the change list 49 50 querystring. 50 51 """ 51 52 raise NotImplementedError 52 53 53 54 55 54 class SimpleListFilter(ListFilter): 56 55 # The parameter that should be used in the query string for that filter. 57 56 parameter_name = None … … class SimpleListFilter(ListFilter): 67 66 if lookup_choices is None: 68 67 lookup_choices = () 69 68 self.lookup_choices = list(lookup_choices) 69 if self.parameter_name in params: 70 self.used_params[self.parameter_name] = params.get(self.parameter_name) 71 del params[self.parameter_name] 72 else: 73 self.used_params[self.parameter_name] = None 70 74 71 75 def has_output(self): 72 76 return len(self.lookup_choices) > 0 … … class SimpleListFilter(ListFilter): 76 80 Returns the value given in the query string for this filter, 77 81 if any. Returns None otherwise. 78 82 """ 79 return self. params.get(self.parameter_name, None)83 return self.used_params[self.parameter_name] 80 84 81 85 def lookups(self, request, model_admin): 82 86 """ … … class SimpleListFilter(ListFilter): 84 88 """ 85 89 raise NotImplementedError 86 90 87 def used_param s(self):91 def used_param_names(self): 88 92 return [self.parameter_name] 89 93 90 94 def choices(self, cl): … … class FieldListFilter(ListFilter): 112 116 self.field_path = field_path 113 117 self.title = getattr(field, 'verbose_name', field_path) 114 118 super(FieldListFilter, self).__init__(request, params, model, model_admin) 119 for p in self.used_param_names(): 120 if p in params: 121 self.used_params[p] = ( 122 prepare_lookup_value_for_queryset_filtering(p, params[p])) 123 del params[p] 115 124 116 125 def has_output(self): 117 126 return True 118 127 119 128 def queryset(self, request, queryset): 120 for p in self.used_params(): 121 if p in self.params: 122 return queryset.filter(**{p: self.params[p]}) 129 return queryset.filter(**self.used_params) 123 130 124 131 @classmethod 125 132 def register(cls, test, list_filter_class, take_priority=False): … … class FieldListFilter(ListFilter): 144 151 145 152 class RelatedFieldListFilter(FieldListFilter): 146 153 def __init__(self, field, request, params, model, model_admin, field_path): 147 super(RelatedFieldListFilter, self).__init__(148 field, request, params, model, model_admin, field_path)149 154 other_model = get_model_from_relation(field) 150 if hasattr(field, 'verbose_name'):151 self.lookup_title = field.verbose_name152 else:153 self.lookup_title = other_model._meta.verbose_name154 155 rel_name = other_model._meta.pk.name 155 self.lookup_kwarg = '%s__%s__exact' % ( self.field_path, rel_name)156 self.lookup_kwarg_isnull = '%s__isnull' % ( self.field_path)156 self.lookup_kwarg = '%s__%s__exact' % (field_path, rel_name) 157 self.lookup_kwarg_isnull = '%s__isnull' % (field_path) 157 158 self.lookup_val = request.GET.get(self.lookup_kwarg, None) 158 159 self.lookup_val_isnull = request.GET.get( 159 160 self.lookup_kwarg_isnull, None) 160 161 self.lookup_choices = field.get_choices(include_blank=False) 162 super(RelatedFieldListFilter, self).__init__( 163 field, request, params, model, model_admin, field_path) 164 if hasattr(field, 'verbose_name'): 165 self.lookup_title = field.verbose_name 166 else: 167 self.lookup_title = other_model._meta.verbose_name 161 168 self.title = self.lookup_title 162 169 163 170 def has_output(self): … … class RelatedFieldListFilter(FieldListFilter): 169 176 extra = 0 170 177 return len(self.lookup_choices) + extra > 1 171 178 172 def used_param s(self):179 def used_param_names(self): 173 180 return [self.lookup_kwarg, self.lookup_kwarg_isnull] 174 181 175 182 def choices(self, cl): … … FieldListFilter.register(lambda f: ( 206 213 207 214 class BooleanFieldListFilter(FieldListFilter): 208 215 def __init__(self, field, request, params, model, model_admin, field_path): 209 super(BooleanFieldListFilter, self).__init__(field, 210 request, params, model, model_admin, field_path) 211 self.lookup_kwarg = '%s__exact' % self.field_path 212 self.lookup_kwarg2 = '%s__isnull' % self.field_path 216 self.lookup_kwarg = '%s__exact' % field_path 217 self.lookup_kwarg2 = '%s__isnull' % field_path 213 218 self.lookup_val = request.GET.get(self.lookup_kwarg, None) 214 219 self.lookup_val2 = request.GET.get(self.lookup_kwarg2, None) 220 super(BooleanFieldListFilter, self).__init__(field, 221 request, params, model, model_admin, field_path) 215 222 216 def used_param s(self):223 def used_param_names(self): 217 224 return [self.lookup_kwarg, self.lookup_kwarg2] 218 225 219 226 def choices(self, cl): … … FieldListFilter.register(lambda f: isinstance(f, 243 250 244 251 class ChoicesFieldListFilter(FieldListFilter): 245 252 def __init__(self, field, request, params, model, model_admin, field_path): 253 self.lookup_kwarg = '%s__exact' % field_path 254 self.lookup_val = request.GET.get(self.lookup_kwarg) 246 255 super(ChoicesFieldListFilter, self).__init__( 247 256 field, request, params, model, model_admin, field_path) 248 self.lookup_kwarg = '%s__exact' % self.field_path249 self.lookup_val = request.GET.get(self.lookup_kwarg)250 257 251 def used_param s(self):258 def used_param_names(self): 252 259 return [self.lookup_kwarg] 253 260 254 261 def choices(self, cl): … … FieldListFilter.register(lambda f: bool(f.choices), ChoicesFieldListFilter) 269 276 270 277 class DateFieldListFilter(FieldListFilter): 271 278 def __init__(self, field, request, params, model, model_admin, field_path): 272 super(DateFieldListFilter, self).__init__( 273 field, request, params, model, model_admin, field_path) 274 275 self.field_generic = '%s__' % self.field_path 279 self.field_generic = '%s__' % field_path 276 280 self.date_params = dict([(k, v) for k, v in params.items() 277 281 if k.startswith(self.field_generic)]) 278 279 282 today = datetime.date.today() 280 283 one_week_ago = today - datetime.timedelta(days=7) 281 284 today_str = str(today) 282 if isinstance( self.field, models.DateTimeField):285 if isinstance(field, models.DateTimeField): 283 286 today_str += ' 23:59:59' 284 285 self.lookup_kwarg_year = '%s__year' % self.field_path 286 self.lookup_kwarg_month = '%s__month' % self.field_path 287 self.lookup_kwarg_day = '%s__day' % self.field_path 288 self.lookup_kwarg_past_7_days_gte = '%s__gte' % self.field_path 289 self.lookup_kwarg_past_7_days_lte = '%s__lte' % self.field_path 290 287 self.lookup_kwarg_year = '%s__year' % field_path 288 self.lookup_kwarg_month = '%s__month' % field_path 289 self.lookup_kwarg_day = '%s__day' % field_path 290 self.lookup_kwarg_past_7_days_gte = '%s__gte' % field_path 291 self.lookup_kwarg_past_7_days_lte = '%s__lte' % field_path 291 292 self.links = ( 292 293 (_('Any date'), {}), 293 294 (_('Today'), { … … class DateFieldListFilter(FieldListFilter): 307 308 self.lookup_kwarg_year: str(today.year), 308 309 }), 309 310 ) 311 super(DateFieldListFilter, self).__init__( 312 field, request, params, model, model_admin, field_path) 310 313 311 def used_param s(self):314 def used_param_names(self): 312 315 return [ 313 316 self.lookup_kwarg_year, self.lookup_kwarg_month, self.lookup_kwarg_day, 314 317 self.lookup_kwarg_past_7_days_gte, self.lookup_kwarg_past_7_days_lte 315 318 ] 316 319 317 def queryset(self, request, queryset):318 """319 Override the default behaviour since there can be multiple query320 string parameters used for the same date filter (e.g. year + month).321 """322 query_dict = {}323 for p in self.used_params():324 if p in self.params:325 query_dict[p] = self.params[p]326 if len(query_dict):327 return queryset.filter(**query_dict)328 329 320 def choices(self, cl): 330 321 for title, param_dict in self.links: 331 322 yield { … … FieldListFilter.register( 344 335 # more appropriate, and the AllValuesFieldListFilter won't get used for it. 345 336 class AllValuesFieldListFilter(FieldListFilter): 346 337 def __init__(self, field, request, params, model, model_admin, field_path): 347 super(AllValuesFieldListFilter, self).__init__( 348 field, request, params, model, model_admin, field_path) 349 self.lookup_kwarg = self.field_path 350 self.lookup_kwarg_isnull = '%s__isnull' % self.field_path 338 self.lookup_kwarg = field_path 339 self.lookup_kwarg_isnull = '%s__isnull' % field_path 351 340 self.lookup_val = request.GET.get(self.lookup_kwarg, None) 352 341 self.lookup_val_isnull = request.GET.get(self.lookup_kwarg_isnull, None) 353 parent_model, reverse_path = reverse_field_path(model, self.field_path)342 parent_model, reverse_path = reverse_field_path(model, field_path) 354 343 queryset = parent_model._default_manager.all() 355 344 # optional feature: limit choices base on existing relationships 356 345 # queryset = queryset.complex_filter( … … class AllValuesFieldListFilter(FieldListFilter): 360 349 361 350 self.lookup_choices = queryset.distinct( 362 351 ).order_by(field.name).values_list(field.name, flat=True) 352 super(AllValuesFieldListFilter, self).__init__( 353 field, request, params, model, model_admin, field_path) 363 354 364 def used_param s(self):355 def used_param_names(self): 365 356 return [self.lookup_kwarg, self.lookup_kwarg_isnull] 366 357 367 358 def choices(self, cl): -
django/contrib/admin/util.py
diff --git a/django/contrib/admin/util.py b/django/contrib/admin/util.py index 7204a12..eee2294 100644
a b from django.utils.encoding import force_unicode, smart_unicode, smart_str 11 11 from django.utils.translation import ungettext 12 12 from django.core.urlresolvers import reverse 13 13 14 def lookup_path_needs_distinct(opts, lookup_path): 15 field_name = lookup_path.split('__', 1)[0] 16 field = opts.get_field_by_name(field_name)[0] 17 if ((hasattr(field, 'rel') and 18 isinstance(field.rel, models.ManyToManyRel)) or 19 (isinstance(field, models.related.RelatedObject) and 20 not field.field.unique)): 21 return True 22 return False 23 24 def prepare_lookup_value_for_queryset_filtering(key, value): 25 """ 26 Returns a lookup value prepared to be used in queryset filtering. 27 """ 28 # if key ends with __in, split parameter into separate values 29 if key.endswith('__in'): 30 value = value.split(',') 31 # if key ends with __isnull, special case '' and false 32 if key.endswith('__isnull'): 33 if value.lower() in ('', 'false'): 34 value = False 35 else: 36 value = True 37 return value 38 14 39 15 40 def quote(s): 16 41 """ -
django/contrib/admin/views/main.py
diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 616b249..3bd14db 100644
a b 1 1 import operator 2 2 3 from django.core.exceptions import SuspiciousOperation 3 from django.core.exceptions import SuspiciousOperation, ImproperlyConfigured 4 4 from django.core.paginator import InvalidPage 5 5 from django.db import models 6 6 from django.utils.datastructures import SortedDict … … from django.utils.http import urlencode 10 10 11 11 from django.contrib.admin import FieldListFilter 12 12 from django.contrib.admin.options import IncorrectLookupParameters 13 from django.contrib.admin.util import quote, get_fields_from_path 13 from django.contrib.admin.util import (quote, get_fields_from_path, 14 lookup_path_needs_distinct, prepare_lookup_value_for_queryset_filtering) 14 15 15 16 # Changelist settings 16 17 ALL_VAR = 'all' … … IGNORED_PARAMS = ( 28 29 # Text to display within change-list table cells if the value is blank. 29 30 EMPTY_CHANGELIST_VALUE = ugettext_lazy('(None)') 30 31 31 def field_needs_distinct(field):32 if ((hasattr(field, 'rel') and33 isinstance(field.rel, models.ManyToManyRel)) or34 (isinstance(field, models.related.RelatedObject) and35 not field.field.unique)):36 return True37 return False38 39 32 40 33 class ChangeList(object): 41 34 def __init__(self, request, model, list_display, list_display_links, … … class ChangeList(object): 84 77 self.title = title % force_unicode(self.opts.verbose_name) 85 78 self.pk_attname = self.lookup_opts.pk.attname 86 79 87 def get_filters(self, request, use_distinct=False): 80 def get_filters(self, request): 81 lookup_params = self.params.copy() # a dictionary of the query string 82 use_distinct = False 83 84 # Remove all the parameters that are globally and systematically 85 # ignored. 86 for ignored in IGNORED_PARAMS: 87 if ignored in lookup_params: 88 del lookup_params[ignored] 89 90 # Normalize the types of keys 91 for key, value in lookup_params.items(): 92 if not isinstance(key, str): 93 # 'key' will be used as a keyword argument later, so Python 94 # requires it to be a string. 95 del lookup_params[key] 96 lookup_params[smart_str(key)] = value 97 98 if not self.model_admin.lookup_allowed(key, value): 99 raise SuspiciousOperation("Filtering by %s not allowed" % key) 100 88 101 filter_specs = [] 89 cleaned_params, use_distinct = self.get_lookup_params(use_distinct)90 102 if self.list_filter: 91 103 for list_filter in self.list_filter: 92 104 if callable(list_filter): 93 105 # This is simply a custom list filter class. 94 spec = list_filter(request, cleaned_params,106 spec = list_filter(request, lookup_params, 95 107 self.model, self.model_admin) 96 108 else: 97 109 field_path = None … … class ChangeList(object): 106 118 if not isinstance(field, models.Field): 107 119 field_path = field 108 120 field = get_fields_from_path(self.model, field_path)[-1] 109 spec = field_list_filter_class(field, request, cleaned_params,121 spec = field_list_filter_class(field, request, lookup_params, 110 122 self.model, self.model_admin, field_path=field_path) 123 # Check if we need to use distinct() 124 use_distinct = (use_distinct or 125 lookup_path_needs_distinct(self.lookup_opts, 126 field_path)) 111 127 if spec and spec.has_output(): 112 128 filter_specs.append(spec) 113 return filter_specs, bool(filter_specs) 129 130 # At this point, all the parameters used by the various ListFilters 131 # have been removed from lookup_params, which now only contains other 132 # parameters passed via the query string. We now loop through the 133 # remaining parameters both to ensure that all the parameters are valid 134 # fields and to determine if at least one of them needs distinct(). 135 for key, value in lookup_params.items(): 136 lookup_params[key] = prepare_lookup_value_for_queryset_filtering( 137 key, value) 138 use_distinct = (use_distinct or 139 lookup_path_needs_distinct(self.lookup_opts, key)) 140 141 return filter_specs, bool(filter_specs), lookup_params, use_distinct 114 142 115 143 def get_query_string(self, new_params=None, remove=None): 116 144 if new_params is None: new_params = {} … … class ChangeList(object): 250 278 ordering_fields[idx] = 'desc' if pfx == '-' else 'asc' 251 279 return ordering_fields 252 280 253 def get_lookup_params(self, use_distinct=False):254 lookup_params = self.params.copy() # a dictionary of the query string255 256 for ignored in IGNORED_PARAMS:257 if ignored in lookup_params:258 del lookup_params[ignored]259 260 for key, value in lookup_params.items():261 if not isinstance(key, str):262 # 'key' will be used as a keyword argument later, so Python263 # requires it to be a string.264 del lookup_params[key]265 lookup_params[smart_str(key)] = value266 267 field = None268 if not use_distinct:269 # Check if it's a relationship that might return more than one270 # instance271 field_name = key.split('__', 1)[0]272 try:273 field = self.lookup_opts.get_field_by_name(field_name)[0]274 use_distinct = field_needs_distinct(field)275 except models.FieldDoesNotExist:276 # It might be a custom NonFieldFilter277 pass278 279 # if key ends with __in, split parameter into separate values280 if key.endswith('__in'):281 value = value.split(',')282 lookup_params[key] = value283 284 # if key ends with __isnull, special case '' and false285 if key.endswith('__isnull'):286 if value.lower() in ('', 'false'):287 value = False288 else:289 value = True290 lookup_params[key] = value291 292 if field and not self.model_admin.lookup_allowed(key, value):293 raise SuspiciousOperation("Filtering by %s not allowed" % key)294 295 return lookup_params, use_distinct296 297 281 def get_query_set(self, request): 298 lookup_params, use_distinct = self.get_lookup_params(use_distinct=False)299 self.filter_specs, self.has_filters = self.get_filters(request, use_distinct)300 301 282 try: 302 # First, let every list filter modify the qs and params to its 303 # liking. 283 # First, we collect all the declared list filters. 284 (self.filter_specs, self.has_filters, remaining_lookup_params, 285 use_distinct) = self.get_filters(request) 286 287 # Then, we let every list filter modify the qs to its liking. 304 288 qs = self.root_query_set 305 289 for filter_spec in self.filter_specs: 306 290 new_qs = filter_spec.queryset(request, qs) 307 291 if new_qs is not None: 308 292 qs = new_qs 309 for param in filter_spec.used_params():310 try:311 del lookup_params[param]312 except KeyError:313 pass314 293 315 # Then, apply the remaining lookup parameters from the query string 316 # (i.e. those that haven't already been processed by the filters). 317 qs = qs.filter(**lookup_params) 294 # Finally, we apply the remaining lookup parameters from the query 295 # string (i.e. those that haven't already been processed by the 296 # filters). 297 qs = qs.filter(**remaining_lookup_params) 298 except (SuspiciousOperation, ImproperlyConfigured): 299 # Allow certain types of errors to be re-raised as-is so that the 300 # caller can treat them in a special way. 301 raise 318 302 except Exception, e: 319 # Naked except! Because we don't have any other way of validating320 # "lookup_params". They might be invalid if the keyword arguments321 # are incorrect, or if the values are not in the correct type, so322 # we might get FieldError, ValueError, ValicationError, or ? from a323 # custom field that raises yet something else when handed324 # impossible data.303 # Every other error is caught with a naked except, because we don't 304 # have any other way of validating lookup parameters. They might be 305 # invalid if the keyword arguments are incorrect, or if the values 306 # are not in the correct type, so we might get FieldError, 307 # ValueError, ValidationError, or ? from a custom field that raises 308 # yet something else when handed impossible data. 325 309 raise IncorrectLookupParameters(e) 326 310 327 311 # Use select_related() if one of the list_display options is a field … … class ChangeList(object): 365 349 qs = qs.filter(reduce(operator.or_, or_queries)) 366 350 if not use_distinct: 367 351 for search_spec in orm_lookups: 368 field_name = search_spec.split('__', 1)[0] 369 f = self.lookup_opts.get_field_by_name(field_name)[0] 370 if field_needs_distinct(f): 352 if lookup_path_needs_distinct(self.lookup_opts, 353 search_spec): 371 354 use_distinct = True 372 355 break 373 356 -
tests/regressiontests/admin_filters/tests.py
diff --git a/tests/regressiontests/admin_filters/tests.py b/tests/regressiontests/admin_filters/tests.py index 28693ae..4988e57 100644
a b class DecadeListFilterWithQuerysetBasedLookups(DecadeListFilterWithTitleAndParam 68 68 if qs.filter(year__gte=2000, year__lte=2009).exists(): 69 69 yield ('the 00s', "the 2000's") 70 70 71 class DecadeListFilterParameterEndsWith__In(DecadeListFilter): 72 title = 'publication decade' 73 parameter_name = 'decade__in' # Ends with '__in" 74 75 class DecadeListFilterParameterEndsWith__Isnull(DecadeListFilter): 76 title = 'publication decade' 77 parameter_name = 'decade__isnull' # Ends with '__isnull" 78 71 79 class CustomUserAdmin(UserAdmin): 72 80 list_filter = ('books_authored', 'books_contributed') 73 81 … … class DecadeFilterBookAdminWithFailingQueryset(ModelAdmin): 97 105 class DecadeFilterBookAdminWithQuerysetBasedLookups(ModelAdmin): 98 106 list_filter = (DecadeListFilterWithQuerysetBasedLookups,) 99 107 108 class DecadeFilterBookAdminParameterEndsWith__In(ModelAdmin): 109 list_filter = (DecadeListFilterParameterEndsWith__In,) 110 111 class DecadeFilterBookAdminParameterEndsWith__Isnull(ModelAdmin): 112 list_filter = (DecadeListFilterParameterEndsWith__Isnull,) 113 100 114 class ListFiltersTests(TestCase): 101 115 102 116 def setUp(self): … … class ListFiltersTests(TestCase): 570 584 choices = list(filterspec.choices(changelist)) 571 585 self.assertEqual(choices[2]['selected'], True) 572 586 self.assertEqual(choices[2]['query_string'], '?no=207') 587 588 def test_parameter_ends_with__in__or__isnull(self): 589 """ 590 Ensure that a SimpleListFilter's parameter name is not mistaken for a 591 model field if it ends with '__isnull' or '__in'. 592 Refs #17091. 593 """ 594 595 # When it ends with '__in' ----------------------------------------- 596 modeladmin = DecadeFilterBookAdminParameterEndsWith__In(Book, site) 597 request = self.request_factory.get('/', {'decade__in': 'the 90s'}) 598 changelist = self.get_changelist(request, Book, modeladmin) 599 600 # Make sure the correct queryset is returned 601 queryset = changelist.get_query_set(request) 602 self.assertEqual(list(queryset), [self.bio_book]) 603 604 # Make sure the correct choice is selected 605 filterspec = changelist.get_filters(request)[0][0] 606 self.assertEqual(force_unicode(filterspec.title), u'publication decade') 607 choices = list(filterspec.choices(changelist)) 608 self.assertEqual(choices[2]['display'], u'the 1990\'s') 609 self.assertEqual(choices[2]['selected'], True) 610 self.assertEqual(choices[2]['query_string'], '?decade__in=the+90s') 611 612 # When it ends with '__isnull' --------------------------------------- 613 modeladmin = DecadeFilterBookAdminParameterEndsWith__Isnull(Book, site) 614 request = self.request_factory.get('/', {'decade__isnull': 'the 90s'}) 615 changelist = self.get_changelist(request, Book, modeladmin) 616 617 # Make sure the correct queryset is returned 618 queryset = changelist.get_query_set(request) 619 self.assertEqual(list(queryset), [self.bio_book]) 620 621 # Make sure the correct choice is selected 622 filterspec = changelist.get_filters(request)[0][0] 623 self.assertEqual(force_unicode(filterspec.title), u'publication decade') 624 choices = list(filterspec.choices(changelist)) 625 self.assertEqual(choices[2]['display'], u'the 1990\'s') 626 self.assertEqual(choices[2]['selected'], True) 627 self.assertEqual(choices[2]['query_string'], '?decade__isnull=the+90s') 628 No newline at end of file