#5833: Custom FilterSpecs
-------------------------------------+-------------------------------------
Reporter: | Owner: jkocherhans
Honza_Kral | Status: assigned
Type: New | Component: contrib.admin
feature | Severity: Normal
Milestone: | Keywords: nfa-someday
Version: SVN | list_filter filterspec nfa-
Resolution: | changelist ep2008
Triage Stage: Accepted | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 1 | Easy pickings: 0
-------------------------------------+-------------------------------------
Changes (by julien):
* needs_better_patch: 0 => 1
* easy: => 0
Comment:
Thanks for reviewing the patch!
Replying to [comment:121 fas]:
> - get_query_set should have no request parameter. It is passed in the
__init__, you can set self.request there to be available for all the
methods of the filter. has_output, title, ... could all depend on request.
It is arbitrary that only get_query_set gets passed the request,
especially if it can be accessed with self.request.
I would prefer passing the request as a parameter for the sake of being
explicit. This would also be more consistent with the way things work in
other public APIs in Django, for example in `ModelAdmin`.
> - there is no reason why _choices() should have an underscore (as
introduced in the latest patch). It is not a private method and intended
to be overridden in inheriting filters.
The two main reasons I've added an underscore is because I haven't come up
with a clean and simple API for `FieldListFilter` yet and also to avoid
the developer accidentally overriding it instead of the `get_choices()`
method which is part of the suggested new public API. If we're to remove
the underscore then I'd like at least to find a more distinct name for it.
> - I think the whole used_params and query_parameter_name thing is too
restricting. Who says I use only one parameter?
> The better implementation is to provide get_value with the parameter key
and a default value, like this:
>
> {{{
> def get_value(self, name, default):
> return self.params.get(name, default)
> }}}
>
> You would then usually do something like this:
>
> {{{
> class MyCustomFilter(FilterSpec):
> ALL = 'all'
> MY_VALUE1 = 'some-param-value'
> MY_VALUE2 = 'some-other-param-value'
> # ...
> DEFAULT_VALUE = MY_VALUE1
>
> def __init__(self, ...):
> super(...)...
> self.arg = 'my-parameter-key'
> self.value = self.get_value(self.arg, self.DEFAULT_VALUE)
>
> def get_query_set(self, qs):
> if self.value == self.ALL:
> return ....
> if self.value == self.MY_VALUE1:
> return ...
>
>
> }}}
>
> The get_query_set method should be called no matter what is in the
parameters (as there should be no query_parameter_name in the first
place).
>
> - When applying the get_query_set method, if it returns None, it should
not be used. The problem of having to check if to call the method based on
request parameters disappears, as do custom checks in the method.
This all seems interesting. Could you elaborate on how the developer would
specify the choices/options available in the filter?
Your approach seems valid but it still feels quite complex. I have been
trying to come up with a very simple and easy API to cover most use cases.
I believe that in most use cases one would just need one parameter. This
is why I originally called the class `SimpleFilterSpec` (and in a later
patch `SimpleListFilter`). But Jacob didn't like the split between
`SimpleListFilter` and `FieldListFilter` (see his comment in
http://groups.google.com/group/django-
developers/browse_thread/thread/ec7c4124e7317145). I tend to agree with
Jacob in that this split didn't seem ideal, but I also tend to agree with
you that the current implementation of `ListFilter` is a bit too
restrictive. Currently my preference would be to reintroduce
`SimpleListFilter` to offer this dead-simple (yet restrictive) approach,
and to modify `ListFilterBase` to accommodate for a less restrictive API
(and also let `FieldListFilter` use that API to validate that it works).
I'll continue thinking this through, but please feel free to provide any
other criticisms or insights.
--
Ticket URL: <http://code.djangoproject.com/ticket/5833#comment:122>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
