Opened 7 years ago

Last modified 4 months ago

#10743 new New feature

Support lookup separators in ModelAdmin.list_display

Reported by: mrts Owned by: nobody
Component: contrib.admin Version: master
Severity: Normal Keywords: efficient-admin, list_display
Cc: ales.zoulek@…, zachborboa@…, olivier.dalang@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As with #3400, supporting the __ lookup separator in ModelAdmin.list_display is useful as it avoids calling __unicode__ bluntly, giving users more control over the output:

  • display several fields from the same related model,
  • as listed in #3400, traverse several levels of relations.

One could argue that most of this can be achieved with __unicode__ by cramming the information to its output.

However, there's a larger problem with deferred fields (deferring is exemplified in this comment) -- namely, that couples admin code to the implementation of __unicode__ in related models. That is, knowledge of what fields are referred to in __unicode__ is required to write proper deferred statements.

A case to illustrate how changes in __unicode__ can have dramatic, unintentional impact:

  • assume the related objects are large and complex,
  • to keep the admin changelist view snappy, the developer looks which fields are referred to in their __unicode__ and writes the ModelAdmin.queryset() method correspondingly, selecting only these fields with only(),
  • a single query is all that is needed to display the data in changelist view, admin is snappy, there is much rejoicing,
  • a second person keeps hacking at the related models, unknowing the impact changes to __unicode__ could have to admin, adds another field to it,
  • suddenly rendering the changelist results in n queries, each of which pulls in the whole object.

All that can be avoided with explicit __ support.

Attachments (2)

patch.2.diff (4.9 KB) - added by Rilt 7 years ago.
patch.diff (4.9 KB) - added by Rilt 7 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 7 years ago by Alex Gaynor

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

comment:2 Changed 7 years ago by Rilt

Has patch: set
Keywords: list_display added

Added a patch, but would definitely like someone to look at it and comment. The basic jist of what I did was add an additional check for any strings with __ in them, indicating a field spanning a relationship, and then looping through each relationship until the proper attribute was grabbed. Then in the admin validation, there is a recursive function which accurately checks that the field exists and is not a ManyToMany.

Last edited 9 months ago by Aymeric Augustin (previous) (diff)

Changed 7 years ago by Rilt

Attachment: patch.2.diff added

Changed 7 years ago by Rilt

Attachment: patch.diff added

comment:3 Changed 7 years ago by anonymous

Cc: ales.zoulek@… added

comment:4 Changed 7 years ago by James Bennett

milestone: 1.2

1.2 is feature-frozen, moving this feature request off the milestone.

comment:5 Changed 6 years ago by anonymous

milestone: 1.4

comment:6 Changed 5 years ago by Chris Beaven

Severity: Normal
Type: New feature

comment:7 Changed 5 years ago by Julien Phalip

Needs documentation: set
Needs tests: set

comment:8 Changed 5 years ago by Jacob

milestone: 1.4

Milestone 1.4 deleted

comment:2 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Added a patch, but would definitely like someone to look at it and comment. The basic jist of what I did was add an additional check for any strings with __ in them, indicating a field spanning a relationship, and then looping through each relationship until the proper attribute was grabbed. Then in the admin validation, there is a recursive function which accurately checks that the field exists and is not a ManyToMany.

Last edited 9 months ago by Aymeric Augustin (previous) (diff)

comment:3 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:4 Changed 2 years ago by Aymeric Augustin

Resolution: wontfix
Status: newclosed

If I understand correctly, the goal of this ticket is to enable Django to automatically optimize the queryset for building the admin changelist by adding appropriate fields to select_related() and only().

But that can't be done automatically as soon as someone puts a callable in display_list. So we're back to square 1, except Django pretended that it could handle the optimization automatically, and failed at that.

In my opinion that would be worse that just telling the user to write an appropriate get_queryset. For this reason (and also considering limited demand for this feature over the last five years) I'm going to reject this ticket.

comment:5 Changed 9 months ago by Aymeric Augustin

Resolution: wontfix
Status: closednew

I'm going to reopen this ticket because I disagree with my 19-month-ago-former-self.

My present-day-self keeps trying to write:

    list_display = ['foo__bar']

and being mad at Django that it doesn't work.

We could limit that syntax to relation fields and not support calling methods on the target model if that helps. It would still solve 95% of the problem.

comment:6 Changed 6 months ago by Zach Borboa

Cc: zachborboa@… added

comment:7 Changed 4 months ago by Olivier

Cc: olivier.dalang@… added
Note: See TracTickets for help on using tickets.
Back to Top