Opened 18 years ago

Closed 18 years ago

Last modified 18 years ago

#2578 closed defect (fixed)

[patch] Using a non-field in "admin.list_display_links" raises misleading exception

Reported by: robbie <robbie@…> Owned by: Adrian Holovaty
Component: contrib.admin Version: dev
Severity: major Keywords:
Cc: cmlenz@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

A model's "admin.list_display" will happily accept non-fields. If the first listed item is a non-field it is linked to the object change page, as a field would be. However, if a non-field is listed in "admin.list_display_links" a misleading exception is raised.

ex: "admin.list_filter" refers to 'example', which isn't a field.

This message is obviously incorrect (referring to "admin.list_filter", not "admin.list_display_links"), and the check itself seems redundant.

The valid field check can seemingly be removed; at worst, the exception message should be altered actually refer to "admin.list_display_links".

Attachments (2)

list_display_links.diff (879 bytes ) - added by robbie <robbie@…> 18 years ago.
Patch to remove unnecessary field check
django_2578.diff (881 bytes ) - added by Christopher Lenz <cmlenz@…> 18 years ago.
Slightly different patch

Download all attachments as: .zip

Change History (7)

by robbie <robbie@…>, 18 years ago

Attachment: list_display_links.diff added

Patch to remove unnecessary field check

comment:1 by robbie <robbie@…>, 18 years ago

Summary: Using a non-field in "admin.list_display_links" raises misleading exception[patch] Using a non-field in "admin.list_display_links" raises misleading exception

by Christopher Lenz <cmlenz@…>, 18 years ago

Attachment: django_2578.diff added

Slightly different patch

comment:2 by Christopher Lenz <cmlenz@…>, 18 years ago

Cc: cmlenz@… added

I just encountered this issue too, and have attached a simpler/better patch that mirrors the code for list_display.

comment:3 by HolgerSchurig, 18 years ago

If the first listed item is a non-field it is linked to the

... is not true. It happens whether it is a first or n-th element. Here's a sample that fails:

class Item(models.Model):

name = models.CharField('Item', maxlength=20)
contents = models.TextField('Value')

def Contents(self):

if len(self.contents)>60:

return '%s ...' % self.contents[:59]

else:

return self.contents

class Admin:

# Only display up to 60 characters in the list
list_display = ('name', 'Contents')

# Make the whole line a link. Unfortunately, this doesn't work:
list_display_links = list_display

comment:4 by robbie <robbie@…>, 18 years ago

Sorry if I wasn't clear in my original description, but when I said 'first item listed' I was referring to the default behaviour when "list_display_links" isn't set: only the first item in "list_display" is linked.

I was trying to point out that the Exception is being raised is a bug, being as it only occurs if you explicitly define "list_display_links".

However, I agree that the sanity checking in Christopher's patch is a definite improvement on mine simple lobotomy.

comment:5 by Malcolm Tredinnick, 18 years ago

Resolution: fixed
Status: newclosed

(In [3790]) Fixed #2578 -- Give a more accurate error message for admin.list_display_links
at model validation time. Patch from Christopher Lenz.

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