Opened 6 years ago

Closed 5 years ago

#24316 closed Bug (fixed)

Admin changelist column CSS classes do not use short description, instead using str(list_display_function)

Reported by: JordanBright Owned by: Tim Graham <timograham@…>
Component: contrib.admin Version: 1.7
Severity: Normal Keywords:
Cc: francis@… 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

The column names in a changelist respect the short description but the CSS classes do not. This is particularly bad if you use a lambda as the class name becomes column-&lt; ;function &lt;lambda&gt; at ... in the th and field-<function <lambda> at ... in the td.

I think this is caused by two functions in django/contrib/admin/templatetags/admin_list.py

result_headers uses field_name from for i, field_name in enumerate(cl.list_display) when it should be using text (from text, attr = label_for_field(...)) and doing something to make it a safe css class name, I think

items_for_result also uses field_name from for field_name in cl.list_display and it should probably just use whatever result_headers should be using.

To reproduce simply create a lambda function and put it in a list display then inspect the header and field classes in your browser. I'll attach a test case that reproduces it shortly.

Attachments (2)

24316_test_case.diff (2.1 KB) - added by JordanBright 6 years ago.
24316_test_case_1.diff (2.2 KB) - added by Manu 5 years ago.
New TestCase since previous one was passing 2 asserts that it shouldn't

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by JordanBright

Attachment: 24316_test_case.diff added

comment:1 Changed 6 years ago by JordanBright

Component: Uncategorizedcontrib.admin

comment:2 Changed 6 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:3 Changed 6 years ago by Lorenzo Pascucci

Owner: changed from nobody to Lorenzo Pascucci
Status: newassigned

comment:4 Changed 5 years ago by Lorenzo Pascucci

Owner: Lorenzo Pascucci deleted
Status: assignednew

Changed 5 years ago by Manu

Attachment: 24316_test_case_1.diff added

New TestCase since previous one was passing 2 asserts that it shouldn't

comment:5 Changed 5 years ago by Tim Graham

Has patch: set

I didn't check if this patch fully resolves the ticket, but here's an initial pull request.

I came across this while debugging #23285. A test would occasionally fail because it was checking the ordering of strings like "2000" and "2008" in the response and sometimes they would appear elsewhere in the response such as <td class="field-<function callable_year at 0x7fe02000f230>">.

comment:6 Changed 5 years ago by Tim Graham

I improved the patch to address both cases mention in the ticket description. I chose to use callable.__name__ instead of short_description as I didn't see why the short_description should be considered for a CSS class. In particular, I expect it would be more common to change the short_description than the callable's name, and I think less the CSS class changes, the better. Let me know if I missed something.

comment:7 Changed 5 years ago by Berker Peksag

Triage Stage: AcceptedReady for checkin

comment:8 Changed 5 years ago by Tim Graham <timograham@…>

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In 37f7ef41:

Fixed #24316 -- Made ModelAdmin.list_display callables use an appropriate CSS class name.

Thanks Berker Peksag for the review.

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