Skip to content

Commit

Permalink
Fixed #28490 -- Removed unused code in admin.E108 check.
Browse files Browse the repository at this point in the history
  • Loading branch information
@hrichards @timgraham
hrichards authored and timgraham committed Oct 2, 2017
1 parent 5d9b736 commit 47016ad
Showing 1 changed file with 20 additions and 42 deletions.
62 changes: 20 additions & 42 deletions django/contrib/admin/checks.py
View file
Expand Up @@ -643,54 +643,32 @@ def _check_list_display_item(self, obj, model, item, label):
elif hasattr(obj, item):
return []
elif hasattr(model, item):
# getattr(model, item) could be an X_RelatedObjectsDescriptor
try:
field = model._meta.get_field(item)
except FieldDoesNotExist:
try:
field = getattr(model, item)
except AttributeError:
field = None

if field is None:
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',
)
]
elif isinstance(field, models.ManyToManyField):
return [
checks.Error(
"The value of '%s' must not be a ManyToManyField." % label,
obj=obj.__class__,
id='admin.E109',
)
]
else:
return []
else:
try:
model._meta.get_field(item)
except FieldDoesNotExist:
return [
# This is a deliberate repeat of E108; there's more than one path
# required to test this condition.
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:
if isinstance(field, models.ManyToManyField):
return [
checks.Error(
"The value of '%s' must not be a ManyToManyField." % label,
obj=obj.__class__,
id='admin.E109',
)
]
return []
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',
)
]

def _check_list_display_links(self, obj):
""" Check that list_display_links is a unique subset of list_display.
Expand Down

3 comments on commit 47016ad

@goblinJoel
Copy link

@goblinJoel goblinJoel commented on 47016ad Aug 9, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While upgrading from Django 1.11 to 2.2, I found this change causes one of our custom field types to fail system checks when used in an admin's list_display. The dev who made the field had overridden contribute_to_class() to assign a descriptor class that raises an AttributeError on __get__() if no instance is supplied, making the field attribute only accessible from instances and not from the class itself.

Previously, this still worked, as model._meta.get_field(item) returned true. Now, hasattr() must also be true.

I'm pretty sure I can change our __get__() to just return the descriptor in that case, as ImageField's descriptor does, but I thought I'd note this in case there was some reason the behavior I described should be supported.

@charettes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW all descriptors where changed to return self if instance is None in 5ef0c03 and 9f6b704 as raising an attribute error broke introspection (help(), IDEs).

There's a few other places in Django that expects Model.field_name to return something for a known defined field.

@goblinJoel
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the response! That's reassuring. I see the FileDescriptor was updated in 1.10, and our field was made before that. I bet the prior dev was following that example.

I also found there is a bug thread for my problem, so I'll copy my comment to there: https://code.djangoproject.com/ticket/30543

Please sign in to comment.