Opened 9 years ago

Closed 9 years ago

Last modified 9 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
Component: contrib.admin Version: master
Severity: major Keywords:
Cc: cmlenz@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

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@…> 9 years ago.
Patch to remove unnecessary field check
django_2578.diff (881 bytes) - added by Christopher Lenz <cmlenz@…> 9 years ago.
Slightly different patch

Download all attachments as: .zip

Change History (7)

Changed 9 years ago by robbie <robbie@…>

Patch to remove unnecessary field check

comment:1 Changed 9 years ago by robbie <robbie@…>

  • Summary changed from Using a non-field in "admin.list_display_links" raises misleading exception to [patch] Using a non-field in "admin.list_display_links" raises misleading exception

Changed 9 years ago by Christopher Lenz <cmlenz@…>

Slightly different patch

comment:2 Changed 9 years ago by Christopher Lenz <cmlenz@…>

  • 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 Changed 9 years ago by HolgerSchurig

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 Changed 9 years ago by robbie <robbie@…>

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 Changed 9 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(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