Opened 9 months ago

Last modified 4 days ago

#28687 assigned Cleanup/optimization

Add a 'Not Empty' option to admin's related filter

Reported by: Brillgen Developers Owned by: Jake Barber
Component: contrib.admin Version: 2.0
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Currently the admin related filter has an 'All' and a 'Empty Value' option ...however it does not have an equally easy to implement and performant NOT 'Empty' option for those cases where we want to filter like that. It maps cleanly onto isnull = True/False.

Would this be accepted if a patch was added since its rather easy to do but the core needs to be ok with it first.

Change History (8)

comment:1 Changed 9 months ago by Jake Barber

Owner: changed from nobody to Jake Barber
Status: newassigned

comment:2 Changed 6 months ago by Tim Graham

Easy pickings: unset
Summary: Not 'Empty' Related Filter optionAdd a 'Not Empty' option to admin's related filter
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Seems okay at first glance.

comment:3 Changed 2 months ago by Carlton Gibson

Has patch: set
Patch needs improvement: set

comment:4 Changed 8 weeks ago by Carlton Gibson

Patch needs improvement: unset

PR is functionally OK.

It hardcodes a (translatable) "Not Empty" string. I'm not sure if we need to do something more sophisticated there (or take a different approach, such as using a filter subclass if you want this behaviour) or if it's OK as it is.

comment:5 Changed 8 weeks ago by Claude Paroz

As for the display value, it should mimic the model_admin.get_empty_value_display() behavior (get_not_empty_value_display()).

comment:6 Changed 7 weeks ago by Carlton Gibson

Patch needs improvement: set

comment:7 Changed 2 weeks ago by Carlton Gibson

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

OK, patch looks good.

I have a small reservation about whether we want yet more API on ModelAdmin to add the not_empty_value_display option, rather than steering users to subclassing FieldListFilter themselves, but bar that...

comment:8 Changed 4 days ago by Tim Graham

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

I agree -- get_not_empty_value_display() looks heavy handed for this use case. get_empty_value_display() is used elsewhere besides the RelatedFieldListFilter and it corresponds to the values displayed in the changelist -- that's not the case for the "not empty" case.

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