Opened 10 years ago

Last modified 4 weeks ago

#24947 assigned New feature

Move admin changelist filters into a separate class (Mixin)

Reported by: Emmanuelle Delescolle Owned by: Antoliny
Component: contrib.admin Version: dev
Severity: Normal Keywords: admin changelist filter
Cc: Marc Tamlyn, Antoliny Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As suggested by @mjtamlyn at DjangoConEurope, move admin changelist filters into a separate class, so that it is usable both by the admin and CBV's.

I am thinking of creating first 2, then 3 mixins:

  • a BaseListFilter mixin holding most of the logic of filtering
  • an AdminFilter mixin (extending BaseListFilter) with admin changelist specifics
  • finally a FilteredList Mixin (extending BaseListFilter) to use with CBV's

According to the ticket's flags, the next step(s) to move this issue forward are:

  • To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is: [https://github.com/django/django/pull/#### PR].

Change History (6)

comment:1 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Merijn Bertels, 9 years ago

Maybe we should make this ticket a subticket of a bigger feature "Give Class Based Views (CBV) more admin functionality" or "Re-use admin code so that users can use it in CBV - like actions, filters or inlines". I think the advance of is this approach is that when we give the class based views (CBV) more functionality, 3th party projects are going to build "better" admin apps and Django can re-use the code to slowly transfer the admin to the new views.

One thing that I all ready found is that ListFilter need a ChangList to render the url for each option. So you can make a ListView that uses a one or more ListFilters in the CBV only when you want to render it you suddenly need a ChangList.

Maybe it's a better idea to start a Group Discussion for this subject.

comment:3 by Antoliny, 5 weeks ago

Cc: Antoliny added
Owner: changed from Emmanuelle Delescolle to Antoliny
Status: newassigned

comment:4 by Emma Delescolle, 5 weeks ago

*hides under a rock*

I had absolutely no recollection this ticket had ever been accepted. I'm very glad it was. And now that you have taken it under your wing, I have no intention of stealing your thunder. But if you need help or a review or anything, feel free to ping me here or on github!

in reply to:  4 comment:5 by Antoliny, 5 weeks ago

Replying to Emma Delescolle:

*hides under a rock*

I had absolutely no recollection this ticket had ever been accepted. I'm very glad it was. And now that you have taken it under your wing, I have no intention of stealing your thunder. But if you need help or a review or anything, feel free to ping me here or on github!

Hello Emma Delescolle :)
I am currently working on a similar issue (Issue 36175).
While working on this, I felt that not only Pagination but also Filter should be separated. While searching to see if a similar issue existed, I came across this ticket.

I agree that the ChangeList class has too many responsibilities, and I believe a structural improvement is necessary. However, I think this issue can only proceed successfully after completing the refactoring of Pagination.
(It will probably depend on my skills..)

Thank you for submitting this ticket Emma Delescolle!
I will do my best to improve the Admin page. Since separating Pagination is a similar task, having your review would be incredibly helpful and greatly appreciated 💪

comment:6 by Emma Delescolle, 4 weeks ago

I'll give it a look some time tomorrow!

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