#10743 closed New feature (fixed)
Support lookup separators in ModelAdmin.list_display
Reported by: | mrts | Owned by: | Tom Carrick |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | efficient-admin, list_display |
Cc: | ales.zoulek@…, zachborboa@…, olivier.dalang@…, Ofer Nave, Adam Johnson, Egor R, Nina Menezes, Tom Carrick | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | 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 theModelAdmin.queryset()
method correspondingly, selecting only these fields withonly()
, - 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)
Change History (37)
comment:1 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 15 years ago
Has patch: | set |
---|---|
Keywords: | list_display added |
by , 15 years ago
Attachment: | patch.2.diff added |
---|
by , 15 years ago
Attachment: | patch.diff added |
---|
comment:3 by , 15 years ago
Cc: | added |
---|
comment:4 by , 15 years ago
milestone: | 1.2 |
---|
1.2 is feature-frozen, moving this feature request off the milestone.
comment:5 by , 14 years ago
milestone: | → 1.4 |
---|
comment:6 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:7 by , 14 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
comment:2 by , 13 years ago
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.
comment:4 by , 10 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
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 by , 9 years ago
Resolution: | wontfix |
---|---|
Status: | closed → new |
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 by , 9 years ago
Cc: | added |
---|
comment:7 by , 8 years ago
Cc: | added |
---|
comment:8 by , 3 years ago
I'm eager to have this, and am surprised that it hasn't happened after 13 years.
In the meantime, I'm relying on this very small package which implements a version of this feature:
comment:9 by , 3 years ago
Cc: | added |
---|
comment:10 by , 3 years ago
Cc: | added |
---|
comment:11 by , 3 years ago
Cc: | added |
---|
comment:13 by , 20 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:14 by , 20 months ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | set |
comment:16 by , 14 months ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Patch needs improvement: | unset |
Resetting patch flags so it gets shown in the review queue.
comment:17 by , 14 months ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
comment:18 by , 13 months ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
comment:19 by , 13 months ago
Patch needs improvement: | set |
---|
Marking as needs improvement per Natalia's comment.
comment:20 by , 13 months ago
Patch needs improvement: | unset |
---|
comment:21 by , 10 months ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Setting as patch needs improvement following this PR comment stating that selenium test would be needed for the list display column ordering.
comment:22 by , 10 months ago
Selenium test was added and it looks great, I just suggested two unit tests for the changes in the admin's utils module. Leaving flags as they are.
comment:23 by , 10 months ago
Cc: | added |
---|---|
Needs tests: | unset |
Patch needs improvement: | unset |
comment:24 by , 10 months ago
Cc: | added |
---|
comment:25 by , 10 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
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.