Opened 3 years ago

Closed 3 years ago

Last modified 10 months ago

#22792 closed Bug (fixed)

Check rule for list_display_links is incorrect

Reported by: Ben Davis Owned by: Greg Chapple
Component: contrib.admin Version: 1.7-beta-2
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

This use case should be allowed according to the documentation:

@admin.register(products.Product)
class ModelAdmin(admin.ModelAdmin):
    list_display = ['brand', 'product', 'category']
    list_editable = list_display
    list_display_links = None

However, checks will fail saying:

<class 'products.admin.ProductAdmin'>: (admin.E124) The value of 'list_editable[0]' refers to the first field in 'list_display' ('brand'), which cannot be used unless 'list_display_links' is set.

I think this check is a mistake. What it should be checking for is this: If the first values of list_editable and list_display are the same, list_display_links must be either set to None, or must be set to a field other than the first field in list_display.

Change History (8)

comment:1 Changed 3 years ago by Tim Graham

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Another issue we should also fix is in the previous check admin.E123 -- it should be elif cls.list_display_links and field_name in cls.list_display_links so that if list_display_links is None there isn't a TypeError: argument of type 'NoneType' is not iterable error.

comment:2 Changed 3 years ago by Greg Chapple

Owner: changed from nobody to Greg Chapple
Status: newassigned

comment:3 Changed 3 years ago by Greg Chapple

Has patch: set

Created a PR here: https://github.com/django/django/pull/2786

The use case given above now works, and admin.E123 has also been fixed.

comment:4 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In d8f19bb3b6f858bef499fdab41948a5a5e8d55aa:

Fixed #22792 -- Updated checks for list_display_links in model admin

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

In d232a5f93f2c0be93b6aa5669e9cba0f328a4a9b:

[1.7.x] Fixed #22792 -- Updated checks for list_display_links in model admin

Backport of d8f19bb3b6 from master

comment:6 Changed 21 months ago by Tim Graham <timograham@…>

In 65bd053f:

Fixed #26229 -- Improved check for model admin check admin.E124

Refs #22792

comment:7 in reply to:  6 Changed 10 months ago by matsaman

Replying to Tim Graham <timograham@…>:

65bd053f

Can we get this backported to 1.8?

-                  cls.list_display_links is not None):
+                  not cls.list_display_links and cls.list_display_links is not None):

comment:8 Changed 10 months ago by Tim Graham

Per our supported versions policy, 1.8 is only receiving fixes for security and data loss issues.

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