Opened 8 years ago

Closed 8 years ago

#26229 closed Bug (fixed)

Missing check for list_editable in admin

Reported by: Alasdair Nicol Owned by: nobody
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Arising from http://stackoverflow.com/questions/35456634/

If list_display and list_editable are both lists with a single item, and list_display_links is not set, then the list_editable won't have any effect, because the field will be displayed as a link rather than a form field. We already have admin checks for usage of list_editable, but this case does not appear to be covered.

class CustomAdmin(admin.ModelAdmin):
    list_display = ('status', )
    list_editable = ('status',)  

#22792 is a related ticket.

Change History (7)

comment:1 by Alasdair Nicol, 8 years ago

When I get a chance, I will have a closer look at the code and try to work out what the problem is .

At a first glance, ListDisplayEditableTests should have a test case similar to test_list_display_same_as_list_editable, but with list_display_links undefined. In that case, there should be a warning if the first item in list_display is in list_editable.

comment:2 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted

comment:3 by Alasdair Nicol, 8 years ago

Has patch: set

I've opened a pull request https://github.com/django/django/pull/6156/files

The two old unit tests from #22792 still pass following my changes, and I have added an additional test. However, I found it a tricky problem to get my head around, so I'm not 100% certain I got it correct. I've added comments to the pull request to explain my reasoning.

Last edited 8 years ago by Alasdair Nicol (previous) (diff)

comment:4 by Tim Graham, 8 years ago

Patch needs improvement: set

Left a few comments for improvement.

comment:5 by Alasdair Nicol, 8 years ago

Patch needs improvement: unset

Pull request updated as suggested.

comment:7 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: newclosed

In 65bd053f:

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

Refs #22792

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