Fixed #5833 -- Modified the admin list filters to be easier to customize. Many…

Fixed #5833 -- Modified the admin list filters to be easier to customize. Many thanks to Honza Král, Tom X. Tobin, gerdemb, eandre, sciyoshi, bendavis78 and Julien Phalip for working on this.

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

django/contrib/admin/__init__.py

@@ -4,6 +4,9 @@ from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
 from django.contrib.admin.options import ModelAdmin, HORIZONTAL, VERTICAL
 from django.contrib.admin.options import StackedInline, TabularInline
 from django.contrib.admin.sites import AdminSite, site
+from django.contrib.admin.filters import (ListFilter, SimpleListFilter,
+    FieldListFilter, BooleanFieldListFilter, RelatedFieldListFilter,
+    ChoicesFieldListFilter, DateFieldListFilter, AllValuesFieldListFilter)
 
 
 def autodiscover():

django/contrib/admin/options.py

@@ -1091,7 +1091,7 @@ class ModelAdmin(BaseModelAdmin):
         if (actions and request.method == 'POST' and
                 'index' in request.POST and '_save' not in request.POST):
             if selected:
-                response = self.response_action(request, queryset=cl.get_query_set())
+                response = self.response_action(request, queryset=cl.get_query_set(request))
                 if response:
                     return response
                 else:
@@ -1107,7 +1107,7 @@ class ModelAdmin(BaseModelAdmin):
                 helpers.ACTION_CHECKBOX_NAME in request.POST and
                 'index' not in request.POST and '_save' not in request.POST):
             if selected:
-                response = self.response_action(request, queryset=cl.get_query_set())
+                response = self.response_action(request, queryset=cl.get_query_set(request))
                 if response:
                     return response
                 else:

django/contrib/admin/templatetags/admin_list.py

@@ -319,7 +319,7 @@ def search_form(cl):
 
 @register.inclusion_tag('admin/filter.html')
 def admin_list_filter(cl, spec):
-    return {'title': spec.title(), 'choices' : list(spec.choices(cl))}
+    return {'title': spec.title, 'choices' : list(spec.choices(cl))}
 
 @register.inclusion_tag('admin/actions.html', takes_context=True)
 def admin_actions(context):

django/contrib/admin/validation.py

@@ -3,6 +3,7 @@ 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 import ListFilter, FieldListFilter
 from django.contrib.admin.util import get_fields_from_path, NotRelationField
 from django.contrib.admin.options import (flatten_fieldsets, BaseModelAdmin,
     HORIZONTAL, VERTICAL)
@@ -54,15 +55,43 @@ def validate(cls, model):
     # list_filter
     if hasattr(cls, 'list_filter'):
         check_isseq(cls, 'list_filter', cls.list_filter)
-        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
-                    )
-                )
+        for idx, item in enumerate(cls.list_filter):
+            # There are three options for specifying a filter:
+            #   1: 'field' - a basic field filter, possibly w/ relationships (eg, 'field__rel')
+            #   2: ('field', SomeFieldListFilter) - a field-based list filter class
+            #   3: SomeListFilter - a non-field list filter class
+            if callable(item) and not isinstance(item, models.Field):
+                # If item is option 3, it should be a ListFilter...
+                if not issubclass(item, ListFilter):
+                    raise ImproperlyConfigured("'%s.list_filter[%d]' is '%s'"
+                            " which is not a descendant of ListFilter."
+                            % (cls.__name__, idx, item.__name__))
+                # ...  but not a FieldListFilter.
+                if issubclass(item, FieldListFilter):
+                    raise ImproperlyConfigured("'%s.list_filter[%d]' is '%s'"
+                            " which is of type FieldListFilter but is not"
+                            " associated with a field name."
+                            % (cls.__name__, idx, item.__name__))
+            else:
+                try:
+                    # Check for option #2 (tuple)
+                    field, list_filter_class = item
+                except (TypeError, ValueError):
+                    # item is option #1
+                    field = item
+                else:
+                    # item is option #2
+                    if not issubclass(list_filter_class, FieldListFilter):
+                        raise ImproperlyConfigured("'%s.list_filter[%d][1]'"
+                            " is '%s' which is not of type FieldListFilter."
+                            % (cls.__name__, idx, list_filter_class.__name__))
+                # Validate the field string
+                try:
+                    get_fields_from_path(model, field)
+                except (NotRelationField, FieldDoesNotExist):
+                    raise ImproperlyConfigured("'%s.list_filter[%d]' refers to '%s'"
+                            " which does not refer to a Field."
+                            % (cls.__name__, idx, field))
 
     # list_per_page = 100
     if hasattr(cls, 'list_per_page') and not isinstance(cls.list_per_page, int):

django/contrib/admin/views/main.py

-from django.contrib.admin.filterspecs import FilterSpec
-from django.contrib.admin.options import IncorrectLookupParameters
-from django.contrib.admin.util import quote, get_fields_from_path
+import operator
+
 from django.core.exceptions import SuspiciousOperation
 from django.core.paginator import InvalidPage
 from django.db import models
 from django.utils.encoding import force_unicode, smart_str
 from django.utils.translation import ugettext, ugettext_lazy
 from django.utils.http import urlencode
-import operator
+
+from django.contrib.admin import FieldListFilter
+from django.contrib.admin.options import IncorrectLookupParameters
+from django.contrib.admin.util import quote, get_fields_from_path
 
 # The system will display a "Show all" link on the change list only if the
 # total result count is less than or equal to this setting.
@@ -23,6 +25,9 @@ TO_FIELD_VAR = 't'
 IS_POPUP_VAR = 'pop'
 ERROR_FLAG = 'e'
 
+IGNORED_PARAMS = (
+    ALL_VAR, ORDER_VAR, ORDER_TYPE_VAR, SEARCH_VAR, IS_POPUP_VAR, TO_FIELD_VAR)
+
 # Text to display within change-list table cells if the value is blank.
 EMPTY_CHANGELIST_VALUE = ugettext_lazy('(None)')
 
@@ -36,7 +41,9 @@ def field_needs_distinct(field):
 
 
 class ChangeList(object):
-    def __init__(self, request, model, list_display, list_display_links, list_filter, date_hierarchy, search_fields, list_select_related, list_per_page, list_editable, model_admin):
+    def __init__(self, request, model, list_display, list_display_links,
+            list_filter, date_hierarchy, search_fields, list_select_related,
+            list_per_page, list_editable, model_admin):
         self.model = model
         self.opts = model._meta
         self.lookup_opts = self.opts
@@ -70,20 +77,39 @@ class ChangeList(object):
             self.list_editable = list_editable
         self.order_field, self.order_type = self.get_ordering()
         self.query = request.GET.get(SEARCH_VAR, '')
-        self.query_set = self.get_query_set()
+        self.query_set = self.get_query_set(request)
         self.get_results(request)
-        self.title = (self.is_popup and ugettext('Select %s') % force_unicode(self.opts.verbose_name) or ugettext('Select %s to change') % force_unicode(self.opts.verbose_name))
-        self.filter_specs, self.has_filters = self.get_filters(request)
+        if self.is_popup:
+            title = ugettext('Select %s')
+        else:
+            title = ugettext('Select %s to change')
+        self.title = title % force_unicode(self.opts.verbose_name)
         self.pk_attname = self.lookup_opts.pk.attname
 
-    def get_filters(self, request):
+    def get_filters(self, request, use_distinct=False):
         filter_specs = []
+        cleaned_params, use_distinct = self.get_lookup_params(use_distinct)
         if self.list_filter:
-            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)
+            for list_filer in self.list_filter:
+                if callable(list_filer):
+                    # This is simply a custom list filter class.
+                    spec = list_filer(request, cleaned_params,
+                        self.model, self.model_admin)
+                else:
+                    field_path = None
+                    try:
+                        # This is custom FieldListFilter class for a given field.
+                        field, field_list_filter_class = list_filer
+                    except (TypeError, ValueError):
+                        # This is simply a field name, so use the default
+                        # FieldListFilter class that has been registered for
+                        # the type of the given field.
+                        field, field_list_filter_class = list_filer, FieldListFilter.create
+                    if not isinstance(field, models.Field):
+                        field_path = field
+                        field = get_fields_from_path(self.model, field_path)[-1]
+                    spec = field_list_filter_class(field, request, cleaned_params,
+                        self.model, self.model_admin, field_path=field_path)
                 if spec and spec.has_output():
                     filter_specs.append(spec)
         return filter_specs, bool(filter_specs)
@@ -175,14 +201,13 @@ class ChangeList(object):
             order_type = params[ORDER_TYPE_VAR]
         return order_field, order_type
 
-    def get_query_set(self):
-        use_distinct = False
-
-        qs = self.root_query_set
+    def get_lookup_params(self, use_distinct=False):
         lookup_params = self.params.copy() # a dictionary of the query string
-        for i in (ALL_VAR, ORDER_VAR, ORDER_TYPE_VAR, SEARCH_VAR, IS_POPUP_VAR, TO_FIELD_VAR):
-            if i in lookup_params:
-                del lookup_params[i]
+
+        for ignored in IGNORED_PARAMS:
+            if ignored in lookup_params:
+                del lookup_params[ignored]
+
         for key, value in lookup_params.items():
             if not isinstance(key, str):
                 # 'key' will be used as a keyword argument later, so Python
@@ -195,10 +220,11 @@ class ChangeList(object):
                 # instance
                 field_name = key.split('__', 1)[0]
                 try:
-                    f = self.lookup_opts.get_field_by_name(field_name)[0]
+                    field = self.lookup_opts.get_field_by_name(field_name)[0]
+                    use_distinct = field_needs_distinct(field)
                 except models.FieldDoesNotExist:
-                    raise IncorrectLookupParameters
-                use_distinct = field_needs_distinct(f)
+                    # It might be a custom NonFieldFilter
+                    pass
 
             # if key ends with __in, split parameter into separate values
             if key.endswith('__in'):
@@ -214,11 +240,28 @@ class ChangeList(object):
                 lookup_params[key] = value
 
             if not self.model_admin.lookup_allowed(key, value):
-                raise SuspiciousOperation(
-                    "Filtering by %s not allowed" % key
-                )
+                raise SuspiciousOperation("Filtering by %s not allowed" % key)
+
+        return lookup_params, use_distinct
+
+    def get_query_set(self, request):
+        lookup_params, use_distinct = self.get_lookup_params(use_distinct=False)
+        self.filter_specs, self.has_filters = self.get_filters(request, use_distinct)
+
+        # Let every list filter modify the qs and params to its liking
+        qs = self.root_query_set
+        for filter_spec in self.filter_specs:
+            new_qs = filter_spec.queryset(request, qs)
+            if new_qs is not None:
+                qs = new_qs
+                for param in filter_spec.used_params():
+                    try:
+                        del lookup_params[param]
+                    except KeyError:
+                        pass
 
-        # Apply lookup parameters from the query string.
+        # Apply the remaining lookup parameters from the query string (i.e.
+        # those that haven't already been processed by the filters).
         try:
             qs = qs.filter(**lookup_params)
         # Naked except! Because we don't have any other way of validating "params".
@@ -226,8 +269,8 @@ class ChangeList(object):
         # values are not in the correct type, so we might get FieldError, ValueError,
         # ValicationError, or ? from a custom field that raises yet something else
         # when handed impossible data.
-        except:
-            raise IncorrectLookupParameters
+        except Exception, e:
+            raise IncorrectLookupParameters(e)
 
         # Use select_related() if one of the list_display options is a field
         # with a relationship and the provided queryset doesn't already have
@@ -238,11 +281,11 @@ class ChangeList(object):
             else:
                 for field_name in self.list_display:
                     try:
-                        f = self.lookup_opts.get_field(field_name)
+                        field = self.lookup_opts.get_field(field_name)
                     except models.FieldDoesNotExist:
                         pass
                     else:
-                        if isinstance(f.rel, models.ManyToOneRel):
+                        if isinstance(field.rel, models.ManyToOneRel):
                             qs = qs.select_related()
                             break
 

django/db/models/related.py

@@ -27,7 +27,7 @@ class RelatedObject(object):
         as SelectField choices for this field.
 
         Analogue of django.db.models.fields.Field.get_choices, provided
-        initially for utilisation by RelatedFilterSpec.
+        initially for utilisation by RelatedFieldListFilter.
         """
         first_choice = include_blank and blank_choice or []
         queryset = self.model._default_manager.all()

docs/ref/contrib/admin/index.txt

@@ -525,30 +525,113 @@ subclass::
 
 .. attribute:: ModelAdmin.list_filter
 
-    Set ``list_filter`` to activate filters in the right sidebar of the change
-    list page of the admin. This should be a list of field names, and each
-    specified field should be either a ``BooleanField``, ``CharField``,
-    ``DateField``, ``DateTimeField``, ``IntegerField`` or ``ForeignKey``.
-
-    This example, taken from the ``django.contrib.auth.models.User`` model,
-    shows how both ``list_display`` and ``list_filter`` work::
+    .. versionchanged:: 1.4
 
-        class UserAdmin(admin.ModelAdmin):
-            list_display = ('username', 'email', 'first_name', 'last_name', 'is_staff')
-            list_filter = ('is_staff', 'is_superuser')
-
-    The above code results in an admin change list page that looks like this:
+    Set ``list_filter`` to activate filters in the right sidebar of the change
+    list page of the admin, as illustrated in the following screenshot:
 
         .. image:: _images/users_changelist.png
 
-    (This example also has ``search_fields`` defined. See below.)
-
-    .. versionadded:: 1.3
-
-    Fields in ``list_filter`` can also span relations using the ``__`` lookup::
-
-        class UserAdminWithLookup(UserAdmin):
-            list_filter = ('groups__name')
+    ``list_filter`` should be a list of elements, where each element should be
+    of one of the following types:
+
+        * a field name, where the specified field should be either a
+          ``BooleanField``, ``CharField``, ``DateField``, ``DateTimeField``,
+          ``IntegerField``, ``ForeignKey`` or ``ManyToManyField``, for example::
+
+              class PersonAdmin(ModelAdmin):
+                  list_filter = ('is_staff', 'company')
+
+          .. versionadded:: 1.3
+
+          Field names in ``list_filter`` can also span relations
+          using the ``__`` lookup, for example::
+
+              class PersonAdmin(UserAdmin):
+                  list_filter = ('company__name',)
+
+        * a class inheriting from :mod:`django.contrib.admin.SimpleListFilter`,
+          which you need to provide the ``title`` and ``parameter_name``
+          attributes to and override the ``lookups`` and ``queryset`` methods,
+          e.g.::
+
+               from django.db.models import Q
+               from django.utils.translation import ugettext_lazy as _
+
+               from django.contrib.admin import SimpleListFilter
+
+               class DecadeBornListFilter(SimpleListFilter):
+                   # Human-readable title which will be displayed in the
+                   # right admin sidebar just above the filter options.
+                   title = _('decade born')
+
+                   # Parameter for the filter that will be used in the URL query.
+                   parameter_name = 'decade'
+
+                   def lookups(self, request):
+                       """
+                       Returns a list of tuples. The first element in each
+                       tuple is the coded value for the option that will
+                       appear in the URL query. The second element is the
+                       human-readable name for the option that will appear
+                       in the right sidebar.
+                       """
+                       return (
+                           ('80s', 'in the eighties'),
+                           ('other', 'other'),
+                       )
+
+                   def queryset(self, request, queryset):
+                       """
+                       Returns the filtered queryset based on the value
+                       provided in the query string and retrievable via
+                       ``value()``.
+                       """
+                       # Compare the requested value (either '80s' or 'other')
+                       # to decide how to filter the queryset.
+                       if self.value() == '80s':
+                           return queryset.filter(birthday__year__gte=1980,
+                                                   birthday__year__lte=1989)
+                       if self.value() == 'other':
+                           return queryset.filter(Q(year__lte=1979) |
+                                                   Q(year__gte=1990))
+
+               class PersonAdmin(ModelAdmin):
+                   list_filter = (DecadeBornListFilter,)
+
+          .. note::
+
+              As a convenience, the ``HttpRequest`` object is passed to the
+              filter's methods, for example::
+
+                  class AuthDecadeBornListFilter(DecadeBornListFilter):
+
+                      def lookups(self, request):
+                          if request.user.is_authenticated():
+                              return (
+                                  ('80s', 'in the eighties'),
+                                  ('other', 'other'),
+                              )
+                          else:
+                              return (
+                                  ('90s', 'in the nineties'),
+                              )
+
+        * a tuple, where the first element is a field name and the second
+          element is a class inheriting from
+          :mod:`django.contrib.admin.FieldListFilter`, for example::
+
+              from django.contrib.admin import BooleanFieldListFilter
+
+              class PersonAdmin(ModelAdmin):
+                  list_filter = (
+                      ('is_staff', BooleanFieldListFilter),
+                  )
+
+          .. note::
+
+              The ``FieldListFilter`` API is currently considered internal
+              and prone to refactoring.
 
 .. attribute:: ModelAdmin.list_per_page
 

docs/releases/1.4.txt

@@ -37,6 +37,15 @@ compatibility with old browsers, this change means that you can use any HTML5
 features you need in admin pages without having to lose HTML validity or
 override the provided templates to change the doctype.
 
+List filters in admin interface
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Prior to Django 1.4, the Django admin app allowed specifying change list
+filters by specifying a field lookup (including spanning relations), and
+not custom filters. This has been rectified with a simple API previously
+known as "FilterSpec" which was used internally. For more details, see the
+documentation for :attr:`~django.contrib.admin.ModelAdmin.list_filter`.
+
 ``reverse_lazy``
 ~~~~~~~~~~~~~~~~
 

tests/regressiontests/admin_filterspecs/models.py → tests/regressiontests/admin_filters/models.py

@@ -2,22 +2,12 @@ from django.db import models
 from django.contrib.auth.models import User
 
 class Book(models.Model):
-    title = models.CharField(max_length=25)
+    title = models.CharField(max_length=50)
     year = models.PositiveIntegerField(null=True, blank=True)
     author = models.ForeignKey(User, related_name='books_authored', blank=True, null=True)
     contributors = models.ManyToManyField(User, related_name='books_contributed', blank=True, null=True)
+    is_best_seller = models.NullBooleanField(default=0)
+    date_registered = models.DateField(null=True)
 
     def __unicode__(self):
         return self.title
-
-class BoolTest(models.Model):
-    NO = False
-    YES = True
-    YES_NO_CHOICES = (
-        (NO, 'no'),
-        (YES, 'yes')
-    )
-    completed = models.BooleanField(
-        default=NO,
-        choices=YES_NO_CHOICES
-    )

tests/regressiontests/admin_views/models.py

@@ -611,7 +611,7 @@ class Gadget(models.Model):
         return self.name
 
 class CustomChangeList(ChangeList):
-    def get_query_set(self):
+    def get_query_set(self, request):
         return self.root_query_set.filter(pk=9999) # Does not exist
 
 class GadgetAdmin(admin.ModelAdmin):

tests/regressiontests/modeladmin/tests.py

@@ -2,19 +2,21 @@ from datetime import date
 
 from django import forms
 from django.conf import settings
-from django.contrib.admin.options import ModelAdmin, TabularInline, \
-    HORIZONTAL, VERTICAL
+from django.contrib.admin.options import (ModelAdmin, TabularInline,
+    HORIZONTAL, VERTICAL)
 from django.contrib.admin.sites import AdminSite
 from django.contrib.admin.validation import validate
 from django.contrib.admin.widgets import AdminDateWidget, AdminRadioSelect
+from django.contrib.admin import (SimpleListFilter,
+     BooleanFieldListFilter)
 from django.core.exceptions import ImproperlyConfigured
 from django.forms.models import BaseModelFormSet
 from django.forms.widgets import Select
 from django.test import TestCase
 from django.utils import unittest
 
-from models import Band, Concert, ValidationTestModel, \
-    ValidationTestInlineModel
+from models import (Band, Concert, ValidationTestModel,
+    ValidationTestInlineModel)
 
 
 # None of the following tests really depend on the content of the request,
@@ -851,8 +853,65 @@ class ValidationTests(unittest.TestCase):
             ValidationTestModel,
         )
 
+        class RandomClass(object):
+            pass
+
+        class ValidationTestModelAdmin(ModelAdmin):
+            list_filter = (RandomClass,)
+
+        self.assertRaisesRegexp(
+            ImproperlyConfigured,
+            "'ValidationTestModelAdmin.list_filter\[0\]' is 'RandomClass' which is not a descendant of ListFilter.",
+            validate,
+            ValidationTestModelAdmin,
+            ValidationTestModel,
+        )
+
+        class ValidationTestModelAdmin(ModelAdmin):
+            list_filter = (('is_active', RandomClass),)
+
+        self.assertRaisesRegexp(
+            ImproperlyConfigured,
+            "'ValidationTestModelAdmin.list_filter\[0\]\[1\]' is 'RandomClass' which is not of type FieldListFilter.",
+            validate,
+            ValidationTestModelAdmin,
+            ValidationTestModel,
+        )
+
+        class AwesomeFilter(SimpleListFilter):
+            def get_title(self):
+                return 'awesomeness'
+            def get_choices(self, request):
+                return (('bit', 'A bit awesome'), ('very', 'Very awesome'), )
+            def get_query_set(self, cl, qs):
+                return qs
+
+        class ValidationTestModelAdmin(ModelAdmin):
+            list_filter = (('is_active', AwesomeFilter),)
+
+        self.assertRaisesRegexp(
+            ImproperlyConfigured,
+            "'ValidationTestModelAdmin.list_filter\[0\]\[1\]' is 'AwesomeFilter' which is not of type FieldListFilter.",
+            validate,
+            ValidationTestModelAdmin,
+            ValidationTestModel,
+        )
+
+        class ValidationTestModelAdmin(ModelAdmin):
+            list_filter = (BooleanFieldListFilter,)
+
+        self.assertRaisesRegexp(
+            ImproperlyConfigured,
+            "'ValidationTestModelAdmin.list_filter\[0\]' is 'BooleanFieldListFilter' which is of type FieldListFilter but is not associated with a field name.",
+            validate,
+            ValidationTestModelAdmin,
+            ValidationTestModel,
+        )
+
+        # Valid declarations below -----------
+
         class ValidationTestModelAdmin(ModelAdmin):
-            list_filter = ('is_active',)
+            list_filter = ('is_active', AwesomeFilter, ('is_active', BooleanFieldListFilter))
 
         validate(ValidationTestModelAdmin, ValidationTestModel)