Ticket #3400: 3400.3.diff

django/contrib/admin/filterspecs.py

@@ -11 +11 @@
 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 +62 @@
         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 +94 @@
                    '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 +119 @@
 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 +135 @@
 
         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 +158 @@
 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 +187 @@
 # 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 +1 @@
 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 +281 @@
         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

@@ -53 +53 @@
     # 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):

django/contrib/admin/views/main.py

@@ -1 +1 @@
 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 +68 @@
     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 @@
+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 +21 @@
         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 +458 @@
         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

tests/regressiontests/admin_views/customadmin.py

@@ -32 +32 @@
 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)

new file tests/regressiontests/admin_views/fixtures/admin-views-books.xml

@@ +1 @@
+<?xml version="1.0" encoding="utf-8"?>
+<django-objects version="1.0">
+  <object pk="1" model="admin_views.book">
+    <field type="CharField" name="name">Book 1</field>
+  </object>
+  <object pk="2" model="admin_views.book">
+    <field type="CharField" name="name">Book 2</field>
+  </object>
+  <object pk="1" model="admin_views.promo">
+    <field type="CharField" name="name">Promo 1</field>
+    <field type="ForiegnKey" name="book">1</field>
+  </object>
+  <object pk="2" model="admin_views.promo">
+    <field type="CharField" name="name">Promo 2</field>
+    <field type="ForiegnKey" name="book">2</field>
+  </object>
+  <object pk="1" model="admin_views.chapter">
+    <field type="CharField" name="title">Chapter 1</field>
+    <field type="TextField" name="content">[ insert contents here ]</field>
+    <field type="ForiegnKey" name="book">1</field>
+  </object>
+  <object pk="2" model="admin_views.chapter">
+    <field type="CharField" name="title">Chapter 2</field>
+    <field type="TextField" name="content">[ insert contents here ]</field>
+    <field type="ForiegnKey" name="book">1</field>
+  </object>
+  <object pk="3" model="admin_views.chapter">
+    <field type="CharField" name="title">Chapter 1</field>
+    <field type="TextField" name="content">[ insert contents here ]</field>
+    <field type="ForiegnKey" name="book">2</field>
+  </object>
+  <object pk="4" model="admin_views.chapter">
+    <field type="CharField" name="title">Chapter 2</field>
+    <field type="TextField" name="content">[ insert contents here ]</field>
+    <field type="ForiegnKey" name="book">2</field>
+  </object>
+  <object pk="1" model="admin_views.chapterxtra1">
+    <field type="CharField" name="xtra">ChapterXtra1 1</field>
+    <field type="ForiegnKey" name="chap">1</field>
+  </object>
+  <object pk="2" model="admin_views.chapterxtra1">
+    <field type="CharField" name="xtra">ChapterXtra1 2</field>
+    <field type="ForiegnKey" name="chap">3</field>
+  </object>
+</django-objects>

tests/regressiontests/admin_views/models.py

@@ -90 +90 @@
 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',)
@@ -149 +157 @@
         return self.title
 
 class ThingAdmin(admin.ModelAdmin):
-    list_filter = ('color',)
+    list_filter = ('color', 'color__warm', 'color__value')
 
 class Fabric(models.Model):
     NG_CHOICES = (
@@ -627 +635 @@
 # 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)
 admin.site.register(Album)

tests/regressiontests/admin_views/tests.py

@@ -18 +18 @@
 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 activate, deactivate
 from django.utils import unittest
 
@@ -26 +27 @@
     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
@@ -203 +205 @@
         )
 
     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(
@@ -215 +219 @@
             "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, '<div id="changelist-filter">')
+        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, '<a href="?%s">' % 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'})

tests/regressiontests/modeladmin/tests.py

@@ -58 +58 @@
         # If we specify the fields argument, fieldsets_add and fielsets_change should
         # just stick the fields into a formsets structure and return it.
         class BandAdmin(ModelAdmin):
-             fields = ['name']
+            fields = ['name']
 
         ma = BandAdmin(Band, self.site)
 
@@ -831 +831 @@
         )
 
         class ValidationTestModelAdmin(ModelAdmin):
-            list_filter = ('non_existent_field',)
-
-        self.assertRaisesRegexp(
-            ImproperlyConfigured,
-            "'ValidationTestModelAdmin.list_filter\[0\]' refers to field 'non_existent_field' that is missing from model 'ValidationTestModel'.",
-            validate,
-            ValidationTestModelAdmin,
-            ValidationTestModel,
-        )
-
-        class ValidationTestModelAdmin(ModelAdmin):
             list_filter = ('is_active',)
 
         validate(ValidationTestModelAdmin, ValidationTestModel)