Failing admin.E108 system check in 2.1

[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):
    (...)

[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.

[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.

[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.

[Petr Boros]

PositionField source

[Simon Charette]

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.

[Petr Boros]

Replying to Simon Charette:

Hey Simon,

you're right. Thank you very much.