Opened 14 months ago

Closed 14 months ago

Last modified 14 months ago

#29636 closed Bug (invalid)

Failing admin.E108 system check in 2.1

Reported by: Petr Boros Owned by: nobody
Component: contrib.admin Version: 2.1
Severity: Normal Keywords: SystemCheckError
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Petr Boros)

After upgrading to Django 2.1, we have system checks failing due to admin.E108 SystemCheckError. This behavior was not present in Django 2.0.8. I don't see any relevant API change documented in the 2.1 Release notes, therefore I'm filing this as a bug.

The error:

<class 'automation.admin.TriggerAdmin'>: (admin.E108) The value of 'list_display[3]' refers to 'position', which is not a callable, an attribute of 'TriggerAdmin', or an attribute or method on 'automation.Trigger'.

The automation.admin.TriggerAdmin class:

from django.contrib import admin
from automation.models import Trigger

@admin.register(Trigger)
class TriggerAdmin(admin.ModelAdmin):
    model = Trigger
    list_display = ('company', 'trigger_type', 'action_type', 'position')
    list_filter = ('company', 'trigger_type', 'action_type')

The automation.models.Trigger model (abbreviated):

from django.db import models
from core.fields import PositionField

class Trigger(models.Model):
    position = PositionField(
        verbose_name=_('Priority'),
        collection='company'
    )

    (...)

The core.fields.PositionField field (abbreviated, originally taken from the django-positions package):

from django.db import models

class PositionField(models.IntegerField):
    (...)

Attachments (1)

position_field.py (10.6 KB) - added by Petr Boros 14 months ago.
PositionField source

Download all attachments as: .zip

Change History (9)

comment:1 Changed 14 months ago by Petr Boros

Description: modified (diff)

comment:2 Changed 14 months ago by Petr Boros

Summary: Failing admin.E108 system check after upgrading to Django 2.1Failing admin.E108 system check in 2.1

comment:3 Changed 14 months ago by Petr Boros

I investigated Django source code and believe this is a regression introduced in commit 47016ad (https://github.com/django/django/commit/47016adbf54b54143d4cf052eeb29fc72d27e6b1), specifically in the else branch starting on line 673.

This is the current code present in 2.1:

        else:
            return [
                checks.Error(
                    "The value of '%s' refers to '%s', which is not a callable, "
                    "an attribute of '%s', or an attribute or method on '%s.%s'." % (
                        label, item, obj.__class__.__name__,
                        model._meta.app_label, model._meta.object_name,
                    ),
                    obj=obj.__class__,
                    id='admin.E108',
                )
            ]

I modified it to match the original code before the alleged regression, that is:

        else:
            try:
                model._meta.get_field(item)
            except FieldDoesNotExist:

                return [
                    checks.Error(
                        "The value of '%s' refers to '%s', which is not a callable, "
                        "an attribute of '%s', or an attribute or method on '%s.%s'." % (
                            label, item, obj.__class__.__name__,
                            model._meta.app_label, model._meta.object_name,
                        ),
                        obj=obj.__class__,
                        id='admin.E108',
                    )
                ]
            else:
                return []

and the checks are not failing.

If this is indeed an unintended regression, I'd be happy to submit a PR.

comment:4 Changed 14 months ago by Petr Boros

Component: Core (System checks)contrib.admin

comment:5 Changed 14 months ago by Tim Graham

I think the report is missing some detail to reproduce the issue (perhaps some implementation detail of PositionField). For your case, it looks like hasattr(Trigger, 'position') is failing which isn't the case for normal model fields.

comment:6 in reply to:  5 Changed 14 months ago by Petr Boros

Replying to Tim Graham:

I think the report is missing some detail to reproduce the issue (perhaps some implementation detail of PositionField). For your case, it looks like hasattr(Trigger, 'position') is failing which isn't the case for normal model fields.

You are right, hasattr(Trigger, 'position') == False. However, at the same time, 'position' in Trigger.__dict__.keys() == True.

Why is that?

I am attaching the full source code of PositionField to this ticket for reference. Note that the original source code comes from django-positions package.

Changed 14 months ago by Petr Boros

Attachment: position_field.py added

PositionField source

comment:7 Changed 14 months ago by Simon Charette

Resolution: invalid
Status: newclosed

Hey Petr,

The issue is that PositionField.__get__ raises an attribute error when instance=None, that is when accessed at the model class level. That makes hasattr(Trigger, 'position') return False.

You'll want to fix that to return self I believe.

comment:8 in reply to:  7 Changed 14 months ago by Petr Boros

Replying to Simon Charette:

Hey Simon,

you're right. Thank you very much.

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