[Django] #34513: Error E108 does not cover some cases
Django
-----------------------------------------+------------------------
Reporter: Baha Sdtbekov | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 4.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
I have two models, `Question` and `Choice`. And if I write `list_display
= ["choice"]` in `QuestionAdmin`, I get no errors.
But when I visit `/admin/polls/question/`, the following trace is
returned:
{{{
Internal Server Error: /admin/polls/question/
Traceback (most recent call last):
File "/some/path/django/contrib/admin/utils.py", line 334, in
label_for_field
field = _get_non_gfk_field(model._meta, name)
File "/some/path/django/contrib/admin/utils.py", line 310, in
_get_non_gfk_field
raise FieldDoesNotExist()
django.core.exceptions.FieldDoesNotExist
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/some/path/django/core/handlers/exception.py", line 55, in inner
response = get_response(request)
File "/some/path/django/core/handlers/base.py", line 220, in
_get_response
response = response.render()
File "/some/path/django/template/response.py", line 111, in render
self.content = self.rendered_content
File "/some/path/django/template/response.py", line 89, in
rendered_content
return template.render(context, self._request)
File "/some/path/django/template/backends/django.py", line 61, in render
return self.template.render(context)
File "/some/path/django/template/base.py", line 175, in render
return self._render(context)
File "/some/path/django/template/base.py", line 167, in _render
return self.nodelist.render(context)
File "/some/path/django/template/base.py", line 1005, in render
return SafeString("".join([node.render_annotated(context) for node in
self]))
File "/some/path/django/template/base.py", line 1005, in <listcomp>
return SafeString("".join([node.render_annotated(context) for node in
self]))
File "/some/path/django/template/base.py", line 966, in render_annotated
return self.render(context)
File "/some/path/django/template/loader_tags.py", line 157, in render
return compiled_parent._render(context)
File "/some/path/django/template/base.py", line 167, in _render
return self.nodelist.render(context)
File "/some/path/django/template/base.py", line 1005, in render
return SafeString("".join([node.render_annotated(context) for node in
self]))
File "/some/path/django/template/base.py", line 1005, in <listcomp>
return SafeString("".join([node.render_annotated(context) for node in
self]))
File "/some/path/django/template/base.py", line 966, in render_annotated
return self.render(context)
File "/some/path/django/template/loader_tags.py", line 157, in render
return compiled_parent._render(context)
File "/some/path/django/template/base.py", line 167, in _render
return self.nodelist.render(context)
File "/some/path/django/template/base.py", line 1005, in render
return SafeString("".join([node.render_annotated(context) for node in
self]))
File "/some/path/django/template/base.py", line 1005, in <listcomp>
return SafeString("".join([node.render_annotated(context) for node in
self]))
File "/some/path/django/template/base.py", line 966, in render_annotated
return self.render(context)
File "/some/path/django/template/loader_tags.py", line 63, in render
result = block.nodelist.render(context)
File "/some/path/django/template/base.py", line 1005, in render
return SafeString("".join([node.render_annotated(context) for node in
self]))
File "/some/path/django/template/base.py", line 1005, in <listcomp>
return SafeString("".join([node.render_annotated(context) for node in
self]))
File "/some/path/django/template/base.py", line 966, in render_annotated
return self.render(context)
File "/some/path/django/template/loader_tags.py", line 63, in render
result = block.nodelist.render(context)
File "/some/path/django/template/base.py", line 1005, in render
return SafeString("".join([node.render_annotated(context) for node in
self]))
File "/some/path/django/template/base.py", line 1005, in <listcomp>
return SafeString("".join([node.render_annotated(context) for node in
self]))
File "/some/path/django/template/base.py", line 966, in render_annotated
return self.render(context)
File "/some/path/django/contrib/admin/templatetags/base.py", line 45, in
render
return super().render(context)
File "/some/path/django/template/library.py", line 258, in render
_dict = self.func(*resolved_args, **resolved_kwargs)
File "/some/path/django/contrib/admin/templatetags/admin_list.py", line
326, in result_list
headers = list(result_headers(cl))
File "/some/path/django/contrib/admin/templatetags/admin_list.py", line
90, in result_headers
text, attr = label_for_field(
File "/some/path/django/contrib/admin/utils.py", line 362, in
label_for_field
raise AttributeError(message)
AttributeError: Unable to lookup 'choice' on Question or QuestionAdmin
[24/Apr/2023 15:43:32] "GET /admin/polls/question/ HTTP/1.1" 500 349913
}}}
I suggest that error E108 be updated to cover this case as well
--
Ticket URL: <https://code.djangoproject.com/ticket/34513>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Baha Sdtbekov):
* owner: nobody => Baha Sdtbekov
* status: new => assigned
Old description:
New description:
For reproduce see [https://github.com/Bakdolot/django_34513 github]
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:1>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by Baha Sdtbekov):
I think I will make a bug fix later if required
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:2>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
* stage: Unreviewed => Accepted
Comment:
Thanks bakdolot đź‘Ť
There's a slight difference between a model instance's attributes and the
model class' meta's fields. Meta stores the reverse relationship as
`choice`, where as this would be setup & named according to whatever the
related_name is declared as.
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:3>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by David Sanders):
fyi potential quick fix, this will cause it to start raising E108 errors.
this is just a demo of where to look. One possibility we could abandon
using `get_field()` and refer to `_meta.fields` instead? 🤔… though that
would mean the E109 check below this would no longer work.
{{{#!diff
--- a/django/contrib/admin/checks.py
+++ b/django/contrib/admin/checks.py
@@ -9,6 +9,7 @@ from django.core.exceptions import FieldDoesNotExist
from django.db import models
from django.db.models.constants import LOOKUP_SEP
from django.db.models.expressions import Combinable
+from django.db.models.fields.reverse_related import ManyToOneRel
from django.forms.models import BaseModelForm, BaseModelFormSet,
_get_foreign_key
from django.template import engines
from django.template.backends.django import DjangoTemplates
@@ -897,6 +898,8 @@ class ModelAdminChecks(BaseModelAdminChecks):
return []
try:
field = obj.model._meta.get_field(item)
+ if isinstance(field, ManyToOneRel):
+ raise FieldDoesNotExist
except FieldDoesNotExist:
try:
field = getattr(obj.model, item)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:4>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by Natalia Bidart):
This is related to the recent work merged for ticket:34481.
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:5>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by David Sanders):
@nessita yup I recognised bakdolot's user username from that patch :D
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:6>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by Baha Sdtbekov):
Oh no they recognized me :D
I apologize very much. I noticed this bug only after merge when I decided
to check again
By the way, I also noticed two bugs related to this
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:7>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by Baha Sdtbekov):
I checked most of the fields and found these fields that are not working
correctly
{{{
class QuestionAdmin(admin.ModelAdmin):
list_display = ["choice", "choice_set", "somem2m",
"SomeM2M_question+", "somem2m_set", "__module__", "__doc__", "objects"]
}}}
Also for reproduce see [https://github.com/Bakdolot/django_34513/ github]
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:8>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:8 Baha Sdtbekov]:
> I checked most of the fields and found these fields that are not working
correctly
>
> {{{
> class QuestionAdmin(admin.ModelAdmin):
> list_display = ["choice", "choice_set", "somem2m",
"SomeM2M_question+", "somem2m_set", "__module__", "__doc__", "objects"]
> }}}
>
> Also for reproduce see [https://github.com/Bakdolot/django_34513/
github]
System checks are helpers that in this case should highlight potentially
reasonable but unsupported options. IMO they don't have to catch all
obviously wrong values that you can find in `__dir__`.
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:9>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by David Sanders):
Yup agreed with felixx if they're putting `__doc__` in there then they
probably need to go back and do a Python tutorial :)
As for `choice_set` & `somem2m` – I thought that's what you fixed up in
the other patch with E109.
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:10>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Comment (by Baha Sdtbekov):
[https://github.com/django/django/pull/16816 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:11>
Django
-------------------------------+-----------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:12>
Django
Reporter: Baha Sdtbekov | Owner: Baha
| Sdtbekov
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Baha Sdtbekov | Owner: Baha
| Sdtbekov
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"c61219a7ae051d2baab53f041e00592011fc550c" c61219a7]:
{{{
#!CommitTicketReference repository=""
revision="c61219a7ae051d2baab53f041e00592011fc550c"
Fixed #34513 -- Added system check for relational fields in
ModelAdmin.list_display.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34513#comment:14>