Code

Opened 8 months ago

Closed 8 months ago

#21361 closed Bug (fixed)

admin.SimpleListFilter should fill used_parameters before doing lookups

Reported by: ronnie@… Owned by: akaariai
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

Attachments (0)

Change History (9)

comment:1 Changed 8 months ago by chrismedrela

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

@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 8 months ago by vajrasky

  • Owner changed from nobody to vajrasky
  • Status changed from new to 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 Changed 8 months ago by vajrasky

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 8 months ago by vajrasky

  • Has patch set

comment:5 Changed 8 months 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 8 months ago by vajrasky

  • Patch needs improvement unset

Updated PR based on Chris Medrela's review.

comment:7 Changed 8 months ago by chrismedrela

  • Triage Stage changed from Accepted to Ready for checkin

comment:8 Changed 8 months ago by akaariai

  • Owner changed from vajrasky to akaariai

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 8 months ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 68b540c97751f052a7622a7ab80a32a7d89d2aa2:

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

Reviewed by Chris Medrela.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.