Opened 8 years ago

Closed 8 years ago

#26632 closed Bug (fixed)

System check for list_display_links ignores value of ModelAdmin.get_list_display()

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

Description

The list_display_links system check should also look at the values returned by ModelAdmin.get_list_display() before displaying error (admin.E111).

# models
from django.db import models

class Question(models.Model):
    question_text = models.CharField(max_length=200)
    pub_date = models.DateTimeField('date published')
# admin
from django.contrib import admin
from .models import Question

class QuestionAdmin(admin.ModelAdmin):
    # list_display = ['id', 'pub_date', 'question_text']
    list_display_links = ['id', 'pub_date', 'question_text',]

    def get_list_display(self, request):
        return ['id', 'pub_date', 'question_text']

admin.site.register(Question, QuestionAdmin)
$ python manage.py check
SystemCheckError: System check identified some issues:

ERRORS:
<class 'myapp.admin.QuestionAdmin'>: (admin.E111) The value of 'list_display_links[0]' refers to 'id', which is not defined in 'list_display'.
<class 'myapp.admin.QuestionAdmin'>: (admin.E111) The value of 'list_display_links[1]' refers to 'pub_date', which is not defined in 'list_display'.
<class 'myapp.admin.QuestionAdmin'>: (admin.E111) The value of 'list_display_links[2]' refers to 'question_text', which is not defined in 'list_display'.

Change History (7)

comment:1 by Tim Graham, 8 years ago

Component: Uncategorizedcontrib.admin
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

I don't think we can check these methods because we'd need an HttpRequest.

As Luke said, "I think the problems with the false negatives for the admin checks may need to be solved another way - namely by removing them. That's unfortunate, but I found them tricky in the past - they may prevent common errors, but they also prevent more advanced ways that you might want to use the admin."

Maybe we could try disabling each check if the method version of check's attribute is defined.

comment:2 by Anton Samarchyan, 8 years ago

Cc: desecho@… added
Has patch: set
Version: 1.9master

Added PR.

comment:3 by Simon Charette, 8 years ago

What about adding a hint to the error that you should override get_list_display_link() to return dynamic results if get_list_display() was overrode?

comment:4 by Tim Graham, 8 years ago

Needs tests: set

A test is also needed.

in reply to:  3 comment:5 by Anton Samarchyan, 8 years ago

Replying to Simon Charette:

What about adding a hint to the error that you should override get_list_display_link() to return dynamic results if get_list_display() was overrode?

Currently, I silence all errors if get_list_display() is overridden. Do you want to bring the errors back or create a new error or else?

comment:6 by Anton Samarchyan, 8 years ago

Needs tests: unset

Added test PR.

Last edited 8 years ago by Anton Samarchyan (previous) (diff)

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

Resolution: fixed
Status: newclosed

In da792400:

Fixed #26632 -- Skipped admin.E111 list_display_links check if get_list_display() is overridden.

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