Fixed #3400 -- Support for lookup separator with list_filter admin op…

…tion. Thanks to DrMeers and vitek_pliska for the patch!

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

django/contrib/admin/filterspecs.py

@@ -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,14 +62,20 @@ def output(self, cl):
         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)
 
@@ -78,12 +94,17 @@ def choices(self, cl):
                    '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 @@ def choices(self, cl):
 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 @@ def __init__(self, f, request, params, model, model_admin):
 
         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 @@ def choices(self, cl):
 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 @@ def choices(self, cl):
 # 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)

django/contrib/admin/util.py

@@ -1,4 +1,5 @@
 from django.db import models
+from django.db.models.sql.constants import LOOKUP_SEP
 from django.db.models.deletion import Collector
 from django.db.models.related import RelatedObject
 from django.forms.forms import pretty_name
@@ -280,3 +281,95 @@ def display_for_field(value, field):
         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") -> [
+        <django.db.models.fields.related.ForeignKey object at 0x...>,
+        <django.db.models.fields.related.ManyToManyField object at 0x...>,
+        <django.db.models.fields.CharField object at 0x...>,
+    ]
+    """
+    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

django/contrib/admin/validation.py

@@ -1,7 +1,9 @@
 from django.core.exceptions import ImproperlyConfigured
 from django.db import models
+from django.db.models.fields import FieldDoesNotExist
 from django.forms.models import (BaseModelForm, BaseModelFormSet, fields_for_model,
     _get_foreign_key)
+from django.contrib.admin.util import get_fields_from_path, NotRelationField
 from django.contrib.admin.options import flatten_fieldsets, BaseModelAdmin
 from django.contrib.admin.options import HORIZONTAL, VERTICAL
 
@@ -53,8 +55,15 @@ def validate(cls, model):
     # 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)
+        for idx, fpath in enumerate(cls.list_filter):
+            try:
+                get_fields_from_path(model, fpath)
+            except (NotRelationField, FieldDoesNotExist), e:
+                raise ImproperlyConfigured(
+                    "'%s.list_filter[%d]' refers to '%s' which does not refer to a Field." % (
+                        cls.__name__, idx, fpath
+                    )
+                )
 
     # list_per_page = 100
     if hasattr(cls, 'list_per_page') and not isinstance(cls.list_per_page, int):

django/contrib/admin/views/main.py

@@ -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.utils.encoding import force_unicode, smart_str
@@ -68,9 +68,11 @@ def __init__(self, request, model, list_display, list_display_links, list_filter
     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)

django/db/models/related.py

@@ -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 @@ def __init__(self, parent_model, model, field):
         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,

docs/ref/contrib/admin/index.txt

@@ -458,6 +458,11 @@ how both ``list_display`` and ``list_filter`` work::
         list_display = ('username', 'email', 'first_name', 'last_name', 'is_staff')
         list_filter = ('is_staff', 'is_superuser')
 
+Fields in ``list_filter`` can also span relations using the ``__`` lookup::
+
+    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

docs/releases/1.3.txt

@@ -159,6 +159,8 @@ requests. These include:
       <cache_key_prefixing>` and :ref:`transformation
       <cache_key_transformation>` has been added to the cache API.
 
+    * Support for lookups spanning relations in admin's ``list_filter``.
+
 .. _backwards-incompatible-changes-1.3:
 
 Backwards-incompatible changes in 1.3

tests/regressiontests/admin_views/customadmin.py

@@ -32,3 +32,4 @@ def my_view(self, request):
 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)