Opened 3 years ago

Closed 3 years ago

#21361 closed Bug (fixed)

admin.SimpleListFilter should fill used_parameters before doing lookups

Reported by: ronnie@… Owned by: Anssi Kääriäinen
Component: contrib.admin Version: master
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 Changed 3 years ago by chrismedrela

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

@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?

comment:2 Changed 3 years ago by Vajrasky Kok

Owner: changed from nobody to Vajrasky Kok
Status: newassigned

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 Changed 3 years ago by Vajrasky Kok

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 Changed 3 years ago by Vajrasky Kok

Has patch: set

comment:5 Changed 3 years ago by chrismedrela

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 Changed 3 years ago by Vajrasky Kok

Patch needs improvement: unset

Updated PR based on Chris Medrela's review.

comment:7 Changed 3 years ago by chrismedrela

Triage Stage: AcceptedReady for checkin

comment:8 Changed 3 years ago by Anssi Kääriäinen

Owner: changed from Vajrasky Kok to Anssi Kääriäinen

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 Changed 3 years ago by Anssi Kääriäinen <akaariai@…>

Resolution: fixed
Status: assignedclosed

In 68b540c97751f052a7622a7ab80a32a7d89d2aa2:

Fixed #21361 -- allowed access self.value() from SimpleListFilter lookup

Reviewed by Chris Medrela.

Note: See TracTickets for help on using tickets.
Back to Top