Opened 11 years ago
Closed 11 years ago
#21361 closed Bug (fixed)
admin.SimpleListFilter should fill used_parameters before doing lookups
Reported by: | Owned by: | Anssi Kääriäinen | |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | SimpleListFilter, used_parameters |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
I had a case where i needed the self.value() in the lookup function. But the value is empty because the init function first does the lookup and then fills the used_parameters.
I have overwritten the init function in my superclass, but i think they could be switched safely
Change History (9)
comment:1 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Let's use the example from Django documentation (https://docs.djangoproject.com/en/dev/ref/contrib/admin/). We have two filters (80's and 90's). I imagine Ronnie had this scenario:
When you click 80's, the filter would be transformed to:
80-85
86-89
90's
When you click 90's, the filter would be transformed to:
80's
90-95
96-99
The default filters are of course:
80's
90's
comment:3 by , 11 years ago
Here is the PR: https://github.com/django/django/pull/1896
This feature is not convincing. But the change is trivial. So I leave to core developers whether they want to incorporate this feature or not.
comment:4 by , 11 years ago
Has patch: | set |
---|
comment:5 by , 11 years ago
Patch needs improvement: | set |
---|
In my opinion the patch looks really good. There are only minor issues like PEP8 -- see my comments on the PR.
comment:6 by , 11 years ago
Patch needs improvement: | unset |
---|
Updated PR based on Chris Medrela's review.
comment:7 by , 11 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:8 by , 11 years ago
Owner: | changed from | to
---|
It seems like the change is safe to do. While I don't see huge need for this change, this change is useful to at least the ticket's reporter & reviewer.
I'll do final polish & commit.
comment:9 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
@Ronnie, thank you for your input. Could you create a pull request? BTW What is your case that you need
self.value()
in the lookup function?