#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: 0 |
-------------------------------------+-------------------------------------
Comment (by fas):
I have a few comments:
- 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.
- 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 get_query_set method should be called no matter what is in the
request parameters. See next point.
- 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.
On a side note, I have been using filters exactly like this (including all
my points) for years and it works extremely well.
What do you think?
--
Ticket URL: <http://code.djangoproject.com/ticket/5833#comment:121>
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.
