Request for comments: custom admin filters

46 views
Skip to first unread message

Julien Phalip

unread,
Apr 13, 2011, 10:00:02 AM4/13/11
to Django developers
Hello there,

I've just posted a patch with a suggested implementation of custom
admin filters: http://code.djangoproject.com/attachment/ticket/5833/5833.custom-filterspecs.3.diff

I've written some pretty comprehensive tests and have also had a stab
at writing some documentation. You should find all the details there
but I thought I'd give a little teaser here first. Here's an example
of what you could do with it:

The model:

class Book(Model):
title = models.CharField()
pages = models.PositiveIntegerField()

The admin:

from django.contrib import admin

class BookSizeFilter(admin.SimpleListFilter):

def get_title(self):
return u'size'

def get_choices(self, request):
return (
(u'small', u'Small'),
(u'medium', u'Medium'),
(u'large', u'Large'),
)

def get_query_set(self, changelist, queryset):
size = self.get_value()
if size == u'small':
return queryset.filter(pages__gte=1, pages__lte=100)
if size == u'medium':
return queryset.filter(pages__gte=101, pages__lte=200)
if size == u'large':
return queryset.filter(pages__gte=201)
return queryset

class BookAdmin(admin.ModelAdmin):
list_filter = [BookSizeFilter,]

Custom filters is a feature that has been requested by many people and
for a long time. If you're one of those eager people, it would be
awesome if you could take a look at the patch and, if you like the
proposed API, try it in your own real projects to see if all the edge
cases are covered.

I'm also really keen to hear your thoughts on the various sub-features
that ship with that patch, on the suggested API, and on the various
implementation details.

The patch is pretty hefty so I'd really like to take this to a close
ASAP before it grows stale and becomes harder to maintain.

Many thanks!

Julien :-)

Jacob Kaplan-Moss

unread,
Apr 13, 2011, 1:11:07 PM4/13/11
to django-developers
Hi Julien --

Thanks for your work on this! I'm working my way through the patch,
and it's looking good. I'm pretty happy with the internals, though I
do have some questions about the public API:

* I'm rather unhappy with the `SimpleListFilter`/`FieldListFilter`
breakdown, and especially the way `FieldListFilter` is documented.
This isn't friendly:

Note that this method is far more complex than simply using a field
name or a ``SimpleListFilter`` class, as there currently is no simple
way available to manipulate a ``FieldListFilter``. You may, however,
find some useful examples with the built-in filters defined in
:mod:`django.contrib.admin.filterspecs`.

Ugh.

It seems to me that `FieldListFilter` is something of an internal
detail required to maintain the existence of a bunch of pre-refactor
stuff, right? If so, I think I'd like to see `SimpleListFilter`
renamed to something more obvious (maybe call it `ListFilter` and call
the parent class `BaseListFilter` or collapse it into a single class).
Then the docs can explain that a list filter could be a
`FieldListFilter` but that that API is considered internal and prone
to change/refactoring. Make sense?

* There's a weird discrepancy (to me) between the signatures of
`get_choices(self, request)` and `get_query_set(self, changelist,
queryset)`. I'd expect to have the `request` available to both
methods, I think, and I don't really know what `changelist` is doing
there or what I'd use it for. Can you talk a bit about why those
signatures work that way?

* `def get_title(self): return "size"` seems like overkill -- why not
just `title="size"`?

Thanks again!

Jacob

Mikhail Korobov

unread,
Apr 13, 2011, 2:56:33 PM4/13/11
to Django developers
Hi Julien,

Thanks for working on this!

Will your patch simplify creating filters not based on choices? E.g.
min/max filters where exact minimun and maximum values can be entered
(using form with method=GET and input fields for values)? I want a
pony ;)

It would also be good to make FilterSpec.output using template, not
the hardcoded python.

Julien Phalip

unread,
Apr 13, 2011, 5:04:51 PM4/13/11
to Django developers
Hi Mikhail,

On Apr 14, 4:56 am, Mikhail Korobov <kmik...@googlemail.com> wrote:
> Will your patch simplify creating filters not based on choices? E.g.
> min/max filters where exact minimun and maximum values can be entered
> (using form with method=GET and input fields for values)? I want a
> pony ;)

No, this patch does not allow this. This would be a new separate
feature. Feel free to open a ticket ;)

> It would also be good to make FilterSpec.output using template, not
> the hardcoded python.

I totally agree that using a template would be better, but I'd like to
keep this patch focused on what has been requested in #5833. Again,
feel free to open a specific ticket for that ;)

Julien

Julien Phalip

unread,
Apr 13, 2011, 6:46:50 PM4/13/11
to Django developers
Hi Jacob,

Thanks a lot for your feedback. I agree with all your points and have
made the corresponding amendments in a new patch:
http://code.djangoproject.com/attachment/ticket/5833/5833.custom-filterspecs.4.diff

Let me know if you've got any further feedback on this.

Cheers!

Julien

Julien Phalip

unread,
Apr 13, 2011, 8:26:30 PM4/13/11
to Django developers
On Apr 14, 8:46 am, Julien Phalip <jpha...@gmail.com> wrote:
> Thanks a lot for your feedback. I agree with all your points and have
> made the corresponding amendments in a new patch:http://code.djangoproject.com/attachment/ticket/5833/5833.custom-filt...

Sorry, just in case you've already started reviewing the patch (but no
rush!), I've just realised I had missed one small thing in it, which
is now fixed: the get_query_set() method now receives the request as
parameter too.

Everything is now ready for review in the latest patch:
http://code.djangoproject.com/attachment/ticket/5833/5833.custom-filterspecs.5.diff

Thank you!

Julien
Reply all
Reply to author
Forward
0 new messages