Index: django/db/models/related.py =================================================================== --- django/db/models/related.py (revision 13428) +++ django/db/models/related.py (working copy) @@ -1,3 +1,6 @@ +from django.utils.encoding import smart_unicode +from django.db.models.fields import BLANK_CHOICE_DASH + class BoundRelatedObject(object): def __init__(self, related_object, field_mapping, original): self.relation = related_object @@ -18,6 +21,22 @@ self.name = '%s:%s' % (self.opts.app_label, self.opts.module_name) self.var_name = self.opts.object_name.lower() + def get_choices(self, include_blank=True, blank_choice=BLANK_CHOICE_DASH, + limit_to_currently_related=False): + """Returns choices with a default blank choices included, for use + as SelectField choices for this field. + + Analogue of django.db.models.fields.Field.get_choices, provided + initially for utilisation by RelatedFilterSpec. + """ + first_choice = include_blank and blank_choice or [] + queryset = self.model._default_manager.all() + if limit_to_currently_related: + queryset = queryset.complex_filter( + {'%s__isnull' % self.parent_model._meta.module_name: False}) + lst = [(x._get_pk_val(), smart_unicode(x)) for x in queryset] + return first_choice + lst + def get_db_prep_lookup(self, lookup_type, value, connection, prepared=False): # Defer to the actual field definition for db prep return self.field.get_db_prep_lookup(lookup_type, value, Index: django/contrib/admin/validation.py =================================================================== --- django/contrib/admin/validation.py (revision 13428) +++ django/contrib/admin/validation.py (working copy) @@ -54,8 +54,7 @@ # list_filter if hasattr(cls, 'list_filter'): check_isseq(cls, 'list_filter', cls.list_filter) - for idx, field in enumerate(cls.list_filter): - get_field(cls, model, opts, 'list_filter[%d]' % idx, field) + # strict validation removed; same as search_fields now # list_per_page = 100 if hasattr(cls, 'list_per_page') and not isinstance(cls.list_per_page, int): Index: django/contrib/admin/util.py =================================================================== --- django/contrib/admin/util.py (revision 13428) +++ django/contrib/admin/util.py (working copy) @@ -1,5 +1,6 @@ from django.core.exceptions import ObjectDoesNotExist from django.db import models +from django.db.models.sql.constants import LOOKUP_SEP from django.forms.forms import pretty_name from django.utils import formats from django.utils.html import escape @@ -333,3 +334,95 @@ return formats.number_format(value) else: return smart_unicode(value) + + +class NotRelationField(Exception): + pass + + +def get_model_from_relation(field): + if isinstance(field, models.related.RelatedObject): + return field.model + elif getattr(field, 'rel'): # or isinstance? + return field.rel.to + else: + raise NotRelationField + + +def reverse_field_path(model, path): + """ Create a reversed field path. + + E.g. Given (Order, "user__groups"), + return (Group, "user__order"). + + Final field must be a related model, not a data field. + + """ + reversed_path = [] + parent = model + pieces = path.split(LOOKUP_SEP) + for piece in pieces: + field, model, direct, m2m = parent._meta.get_field_by_name(piece) + # skip trailing data field if extant: + if len(reversed_path) == len(pieces)-1: # final iteration + try: + get_model_from_relation(field) + except NotRelationField: + break + if direct: + related_name = field.related_query_name() + parent = field.rel.to + else: + related_name = field.field.name + parent = field.model + reversed_path.insert(0, related_name) + return (parent, LOOKUP_SEP.join(reversed_path)) + + +def get_fields_from_path(model, path): + """ Return list of Fields given path relative to model. + + e.g. (ModelX, "user__groups__name") -> [ + , + , + , + ] + """ + pieces = path.split(LOOKUP_SEP) + fields = [] + for piece in pieces: + if fields: + parent = get_model_from_relation(fields[-1]) + else: + parent = model + fields.append(parent._meta.get_field_by_name(piece)[0]) + return fields + + +def remove_trailing_data_field(fields): + """ Discard trailing non-relation field if extant. """ + try: + get_model_from_relation(fields[-1]) + except NotRelationField: + fields = fields[:-1] + return fields + + +def get_limit_choices_to_from_path(model, path): + """ Return Q object for limiting choices if applicable. + + If final model in path is linked via a ForeignKey or ManyToManyField which + has a `limit_choices_to` attribute, return it as a Q object. + """ + + fields = get_fields_from_path(model, path) + fields = remove_trailing_data_field(fields) + limit_choices_to = ( + fields and hasattr(fields[-1], 'rel') and + getattr(fields[-1].rel, 'limit_choices_to', None)) + if not limit_choices_to: + return models.Q() # empty Q + elif isinstance(limit_choices_to, models.Q): + return limit_choices_to # already a Q + else: + return models.Q(**limit_choices_to) # convert dict to Q Index: django/contrib/admin/filterspecs.py =================================================================== --- django/contrib/admin/filterspecs.py (revision 13428) +++ django/contrib/admin/filterspecs.py (working copy) @@ -11,22 +11,32 @@ from django.utils.translation import ugettext as _ from django.utils.html import escape from django.utils.safestring import mark_safe +from django.contrib.admin.util import get_model_from_relation, \ + reverse_field_path, get_limit_choices_to_from_path import datetime class FilterSpec(object): filter_specs = [] - def __init__(self, f, request, params, model, model_admin): + def __init__(self, f, request, params, model, model_admin, + field_path=None): self.field = f self.params = params - + self.field_path = field_path + if field_path is None: + if isinstance(f, models.related.RelatedObject): + self.field_path = f.var_name + else: + self.field_path = f.name + def register(cls, test, factory): cls.filter_specs.append((test, factory)) register = classmethod(register) - def create(cls, f, request, params, model, model_admin): + def create(cls, f, request, params, model, model_admin, field_path=None): for test, factory in cls.filter_specs: if test(f): - return factory(f, request, params, model, model_admin) + return factory(f, request, params, model, model_admin, + field_path=field_path) create = classmethod(create) def has_output(self): @@ -52,17 +62,23 @@ return mark_safe("".join(t)) class RelatedFilterSpec(FilterSpec): - def __init__(self, f, request, params, model, model_admin): - super(RelatedFilterSpec, self).__init__(f, request, params, model, model_admin) - if isinstance(f, models.ManyToManyField): - self.lookup_title = f.rel.to._meta.verbose_name + def __init__(self, f, request, params, model, model_admin, + field_path=None): + super(RelatedFilterSpec, self).__init__( + f, request, params, model, model_admin, field_path=field_path) + + other_model = get_model_from_relation(f) + if isinstance(f, (models.ManyToManyField, + models.related.RelatedObject)): + # no direct field on this model, get name from other model + self.lookup_title = other_model._meta.verbose_name else: - self.lookup_title = f.verbose_name - rel_name = f.rel.get_related_field().name - self.lookup_kwarg = '%s__%s__exact' % (f.name, rel_name) + self.lookup_title = f.verbose_name # use field name + rel_name = other_model._meta.pk.name + self.lookup_kwarg = '%s__%s__exact' % (self.field_path, rel_name) self.lookup_val = request.GET.get(self.lookup_kwarg, None) self.lookup_choices = f.get_choices(include_blank=False) - + def has_output(self): return len(self.lookup_choices) > 1 @@ -78,12 +94,17 @@ 'query_string': cl.get_query_string({self.lookup_kwarg: pk_val}), 'display': val} -FilterSpec.register(lambda f: bool(f.rel), RelatedFilterSpec) +FilterSpec.register(lambda f: ( + hasattr(f, 'rel') and bool(f.rel) or + isinstance(f, models.related.RelatedObject)), RelatedFilterSpec) class ChoicesFilterSpec(FilterSpec): - def __init__(self, f, request, params, model, model_admin): - super(ChoicesFilterSpec, self).__init__(f, request, params, model, model_admin) - self.lookup_kwarg = '%s__exact' % f.name + def __init__(self, f, request, params, model, model_admin, + field_path=None): + super(ChoicesFilterSpec, self).__init__(f, request, params, model, + model_admin, + field_path=field_path) + self.lookup_kwarg = '%s__exact' % self.field_path self.lookup_val = request.GET.get(self.lookup_kwarg, None) def choices(self, cl): @@ -98,10 +119,13 @@ FilterSpec.register(lambda f: bool(f.choices), ChoicesFilterSpec) class DateFieldFilterSpec(FilterSpec): - def __init__(self, f, request, params, model, model_admin): - super(DateFieldFilterSpec, self).__init__(f, request, params, model, model_admin) + def __init__(self, f, request, params, model, model_admin, + field_path=None): + super(DateFieldFilterSpec, self).__init__(f, request, params, model, + model_admin, + field_path=field_path) - self.field_generic = '%s__' % self.field.name + self.field_generic = '%s__' % self.field_path self.date_params = dict([(k, v) for k, v in params.items() if k.startswith(self.field_generic)]) @@ -111,14 +135,15 @@ self.links = ( (_('Any date'), {}), - (_('Today'), {'%s__year' % self.field.name: str(today.year), - '%s__month' % self.field.name: str(today.month), - '%s__day' % self.field.name: str(today.day)}), - (_('Past 7 days'), {'%s__gte' % self.field.name: one_week_ago.strftime('%Y-%m-%d'), - '%s__lte' % f.name: today_str}), - (_('This month'), {'%s__year' % self.field.name: str(today.year), - '%s__month' % f.name: str(today.month)}), - (_('This year'), {'%s__year' % self.field.name: str(today.year)}) + (_('Today'), {'%s__year' % self.field_path: str(today.year), + '%s__month' % self.field_path: str(today.month), + '%s__day' % self.field_path: str(today.day)}), + (_('Past 7 days'), {'%s__gte' % self.field_path: + one_week_ago.strftime('%Y-%m-%d'), + '%s__lte' % self.field_path: today_str}), + (_('This month'), {'%s__year' % self.field_path: str(today.year), + '%s__month' % self.field_path: str(today.month)}), + (_('This year'), {'%s__year' % self.field_path: str(today.year)}) ) def title(self): @@ -133,10 +158,13 @@ FilterSpec.register(lambda f: isinstance(f, models.DateField), DateFieldFilterSpec) class BooleanFieldFilterSpec(FilterSpec): - def __init__(self, f, request, params, model, model_admin): - super(BooleanFieldFilterSpec, self).__init__(f, request, params, model, model_admin) - self.lookup_kwarg = '%s__exact' % f.name - self.lookup_kwarg2 = '%s__isnull' % f.name + def __init__(self, f, request, params, model, model_admin, + field_path=None): + super(BooleanFieldFilterSpec, self).__init__(f, request, params, model, + model_admin, + field_path=field_path) + self.lookup_kwarg = '%s__exact' % self.field_path + self.lookup_kwarg2 = '%s__isnull' % self.field_path self.lookup_val = request.GET.get(self.lookup_kwarg, None) self.lookup_val2 = request.GET.get(self.lookup_kwarg2, None) @@ -159,21 +187,33 @@ # if a field is eligible to use the BooleanFieldFilterSpec, that'd be much # more appropriate, and the AllValuesFilterSpec won't get used for it. class AllValuesFilterSpec(FilterSpec): - def __init__(self, f, request, params, model, model_admin): - super(AllValuesFilterSpec, self).__init__(f, request, params, model, model_admin) - self.lookup_val = request.GET.get(f.name, None) - self.lookup_choices = model_admin.queryset(request).distinct().order_by(f.name).values(f.name) + def __init__(self, f, request, params, model, model_admin, + field_path=None): + super(AllValuesFilterSpec, self).__init__(f, request, params, model, + model_admin, + field_path=field_path) + self.lookup_val = request.GET.get(self.field_path, None) + parent_model, reverse_path = reverse_field_path(model, field_path) + queryset = parent_model._default_manager.all() + # optional feature: limit choices base on existing relationships + # queryset = queryset.complex_filter( + # {'%s__isnull' % reverse_path: False}) + limit_choices_to = get_limit_choices_to_from_path(model, field_path) + queryset = queryset.filter(limit_choices_to) + + self.lookup_choices = \ + queryset.distinct().order_by(f.name).values(f.name) def title(self): return self.field.verbose_name def choices(self, cl): yield {'selected': self.lookup_val is None, - 'query_string': cl.get_query_string({}, [self.field.name]), + 'query_string': cl.get_query_string({}, [self.field_path]), 'display': _('All')} for val in self.lookup_choices: val = smart_unicode(val[self.field.name]) yield {'selected': self.lookup_val == val, - 'query_string': cl.get_query_string({self.field.name: val}), + 'query_string': cl.get_query_string({self.field_path: val}), 'display': val} FilterSpec.register(lambda f: True, AllValuesFilterSpec) Index: django/contrib/admin/views/main.py =================================================================== --- django/contrib/admin/views/main.py (revision 13428) +++ django/contrib/admin/views/main.py (working copy) @@ -1,6 +1,6 @@ from django.contrib.admin.filterspecs import FilterSpec from django.contrib.admin.options import IncorrectLookupParameters -from django.contrib.admin.util import quote +from django.contrib.admin.util import quote, get_fields_from_path from django.core.paginator import Paginator, InvalidPage from django.db import models from django.db.models.query import QuerySet @@ -69,9 +69,11 @@ def get_filters(self, request): filter_specs = [] if self.list_filter: - filter_fields = [self.lookup_opts.get_field(field_name) for field_name in self.list_filter] - for f in filter_fields: - spec = FilterSpec.create(f, request, self.params, self.model, self.model_admin) + for filter_name in self.list_filter: + field = get_fields_from_path(self.model, filter_name)[-1] + spec = FilterSpec.create(field, request, self.params, + self.model, self.model_admin, + field_path=filter_name) if spec and spec.has_output(): filter_specs.append(spec) return filter_specs, bool(filter_specs) Index: tests/regressiontests/admin_views/fixtures/admin-views-books.xml =================================================================== --- tests/regressiontests/admin_views/fixtures/admin-views-books.xml (revision 0) +++ tests/regressiontests/admin_views/fixtures/admin-views-books.xml (revision 0) @@ -0,0 +1,45 @@ + + + + Book 1 + + + Book 2 + + + Promo 1 + 1 + + + Promo 2 + 2 + + + Chapter 1 + [ insert contents here ] + 1 + + + Chapter 2 + [ insert contents here ] + 1 + + + Chapter 1 + [ insert contents here ] + 2 + + + Chapter 2 + [ insert contents here ] + 2 + + + ChapterXtra1 1 + 1 + + + ChapterXtra1 2 + 3 + + Index: tests/regressiontests/admin_views/tests.py =================================================================== --- tests/regressiontests/admin_views/tests.py (revision 13428) +++ tests/regressiontests/admin_views/tests.py (working copy) @@ -17,18 +17,21 @@ from django.utils.cache import get_max_age from django.utils.encoding import iri_to_uri from django.utils.html import escape +from django.utils.http import urlencode from django.utils.translation import get_date_formats, activate, deactivate +from django.db.models.sql.constants import LOOKUP_SEP # local test models from models import Article, BarAccount, CustomArticle, EmptyModel, \ FooAccount, Gallery, ModelWithStringPrimaryKey, \ Person, Persona, Picture, Podcast, Section, Subscriber, Vodcast, \ Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit, \ - Category, Post, Plot, FunkyTag + Category, Post, Plot, FunkyTag, Chapter, Book, Promo class AdminViewBasicTest(TestCase): - fixtures = ['admin-views-users.xml', 'admin-views-colors.xml', 'admin-views-fabrics.xml'] + fixtures = ['admin-views-users.xml', 'admin-views-colors.xml', + 'admin-views-fabrics.xml', 'admin-views-books.xml'] # Store the bit of the URL where the admin is registered as a class # variable. That way we can test a second AdminSite just by subclassing @@ -201,7 +204,9 @@ ) def testLimitedFilter(self): - """Ensure admin changelist filters do not contain objects excluded via limit_choices_to.""" + """Ensure admin changelist filters do not contain objects excluded via limit_choices_to. + This also tests relation-spanning filters (e.g. 'color__value'). + """ response = self.client.get('/test_admin/%s/admin_views/thing/' % self.urlbit) self.failUnlessEqual(response.status_code, 200) self.failUnless( @@ -213,6 +218,47 @@ "Changelist filter not correctly limited by limit_choices_to." ) + def testRelationSpanningFilters(self): + response = self.client.get('/test_admin/%s/admin_views/chapterxtra1/' % + self.urlbit) + self.failUnlessEqual(response.status_code, 200) + self.assertContains(response, '
') + filters = { + 'chap__id__exact': dict( + values=[c.id for c in Chapter.objects.all()], + test=lambda obj, value: obj.chap.id == value), + 'chap__title': dict( + values=[c.title for c in Chapter.objects.all()], + test=lambda obj, value: obj.chap.title == value), + 'chap__book__id__exact': dict( + values=[b.id for b in Book.objects.all()], + test=lambda obj, value: obj.chap.book.id == value), + 'chap__book__name': dict( + values=[b.name for b in Book.objects.all()], + test=lambda obj, value: obj.chap.book.name == value), + 'chap__book__promo__id__exact': dict( + values=[p.id for p in Promo.objects.all()], + test=lambda obj, value: + obj.chap.book.promo_set.filter(id=value).exists()), + 'chap__book__promo__name': dict( + values=[p.name for p in Promo.objects.all()], + test=lambda obj, value: + obj.chap.book.promo_set.filter(name=value).exists()), + } + for filter_path, params in filters.items(): + for value in params['values']: + query_string = urlencode({filter_path: value}) + # ensure filter link exists + self.assertContains(response, '' % query_string) + # ensure link works + filtered_response = self.client.get( + '/test_admin/%s/admin_views/chapterxtra1/?%s' % ( + self.urlbit, query_string)) + self.failUnlessEqual(filtered_response.status_code, 200) + # ensure changelist contains only valid objects + for obj in filtered_response.context['cl'].query_set.all(): + self.assertTrue(params['test'](obj, value)) + def testIncorrectLookupParameters(self): """Ensure incorrect lookup parameters are handled gracefully.""" response = self.client.get('/test_admin/%s/admin_views/thing/' % self.urlbit, {'notarealfield': '5'}) Index: tests/regressiontests/admin_views/customadmin.py =================================================================== --- tests/regressiontests/admin_views/customadmin.py (revision 13428) +++ tests/regressiontests/admin_views/customadmin.py (working copy) @@ -32,3 +32,4 @@ site.register(models.Section, inlines=[models.ArticleInline]) site.register(models.Thing, models.ThingAdmin) site.register(models.Fabric, models.FabricAdmin) +site.register(models.ChapterXtra1, models.ChapterXtra1Admin) Index: tests/regressiontests/admin_views/models.py =================================================================== --- tests/regressiontests/admin_views/models.py (revision 13428) +++ tests/regressiontests/admin_views/models.py (working copy) @@ -89,6 +89,14 @@ class ChapterInline(admin.TabularInline): model = Chapter +class ChapterXtra1Admin(admin.ModelAdmin): + list_filter = ('chap', + 'chap__title', + 'chap__book', + 'chap__book__name', + 'chap__book__promo', + 'chap__book__promo__name',) + class ArticleAdmin(admin.ModelAdmin): list_display = ('content', 'date', callable_year, 'model_year', 'modeladmin_year') list_filter = ('date',) @@ -148,7 +156,7 @@ return self.title class ThingAdmin(admin.ModelAdmin): - list_filter = ('color',) + list_filter = ('color', 'color__warm', 'color__value') class Fabric(models.Model): NG_CHOICES = ( @@ -622,6 +630,6 @@ # contrib.admin.util's get_deleted_objects function. admin.site.register(Book, inlines=[ChapterInline]) admin.site.register(Promo) -admin.site.register(ChapterXtra1) +admin.site.register(ChapterXtra1, ChapterXtra1Admin) admin.site.register(Pizza, PizzaAdmin) admin.site.register(Topping) Index: tests/regressiontests/modeladmin/models.py =================================================================== --- tests/regressiontests/modeladmin/models.py (revision 13428) +++ tests/regressiontests/modeladmin/models.py (working copy) @@ -677,13 +677,6 @@ ImproperlyConfigured: 'ValidationTestModelAdmin.list_filter' must be a list or tuple. >>> class ValidationTestModelAdmin(ModelAdmin): -... list_filter = ('non_existent_field',) ->>> validate(ValidationTestModelAdmin, ValidationTestModel) -Traceback (most recent call last): -... -ImproperlyConfigured: 'ValidationTestModelAdmin.list_filter[0]' refers to field 'non_existent_field' that is missing from model 'ValidationTestModel'. - ->>> class ValidationTestModelAdmin(ModelAdmin): ... list_filter = ('is_active',) >>> validate(ValidationTestModelAdmin, ValidationTestModel) Index: docs/ref/contrib/admin/index.txt =================================================================== --- docs/ref/contrib/admin/index.txt (revision 13428) +++ docs/ref/contrib/admin/index.txt (working copy) @@ -461,6 +461,11 @@ list_display = ('username', 'email', 'first_name', 'last_name', 'is_staff') list_filter = ('is_staff', 'is_superuser') +In ``list_filter`` can be defined lookup separator as well:: + + class UserAdminWithLookup(UserAdmin): + list_filter = ('groups__name') + The above code results in an admin change list page that looks like this: .. image:: _images/users_changelist.png