[Django] #24146: (admin.E116) The value of 'list_filter[0]' refers to 'through__field', which does not refer to a Field.
Django
which does not refer to a Field.
---------------------------------+--------------------
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+--------------------
{{{
class PersonAdmin(admin.ModelAdmin):
list_filter = ['studenthousehold__primary']
class Student(models.Model):
name = models.CharField(max_length=255)
class Household(models.Model):
students = models.ManyToManyField(Student, through='StudentHousehold')
class StudentHousehold(models.Model):
household = models.ForeignKey(Household)
student = models.ForeignKey(Student)
primary = models.BooleanField(default=False)
}}}
Works fine on 1.7, but on 1.8:
{{{
./manage.py check
SystemCheckError: System check identified some issues:
ERRORS:
<class 'app.admin.PersonAdmin'>: (admin.E116) The value of
'list_filter[0]' refers to 'studenthousehold__primary', which does not
refer to a Field.
System check identified 1 issue (0 silenced).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24146>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
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 collinanderson):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
To reproduce:
git clone https://github.com/collinanderson/24146
cd 24146
./manage check
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:1>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
The actual underlying exception is:
{{{
File "django/contrib/admin/checks.py", line 715, in
_check_list_filter_item
get_fields_from_path(model, field)
File "django/contrib/admin/utils.py", line 473, in get_fields_from_path
fields.append(parent._meta.get_field(piece))
File "django/db/models/options.py", line 540, in get_field
"be available yet." % (self.object_name, field_name)
django.core.exceptions.FieldDoesNotExist: Household has no field named
'studenthousehold'. The app cache isn't ready yet, so if this is a forward
field, it won't be available yet.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:2>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:3>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
Here's a simple patch, but it modifies the API.
https://github.com/django/django/pull/3912
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:4>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by collinanderson):
* version: master => 1.8alpha1
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:5>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by berkerpeksag):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:6>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* version: 1.8alpha1 => master
* needs_tests: 0 => 1
Comment:
Does a test in `admin_checks` not reproduce the issue because of app
loading differences when running the tests?
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:7>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
Exactly, because it's only an issue while apps are loading.
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:8>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:9>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"e8171daf0cd7f0e070395cb4c850c17fea32f11d"]:
{{{
#!CommitTicketReference repository=""
revision="e8171daf0cd7f0e070395cb4c850c17fea32f11d"
Fixed #24146 -- Fixed a missing fields regression in admin checks.
This allows using get_field() early in the app loading process.
Thanks to PirosB3 and Tim Graham.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:10>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
I'm not convinced this is the right fix. Exposing reverse fields before
the app-cache is ready is a recipe for unpredictable behavior, which is
exactly what app-loading and the meta-refactor were supposed to fix. Why
do admin checks run before the app cache is ready? That seems wrong -
can't we fix that instead?
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:11>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
The admin checks run in `ready()`, and `apps.ready` isn't `True` until all
`ready()`s are done.
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:12>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
It seems we already have two stages of app-registry "readyness" -- there's
`models_ready`, which is set to `True` before the app-config `ready()`
methods run, and then there's `ready`, which isn't set until afterwards.
It seems to me that it's really `models_ready` which should be the basis
for the meta API deciding that the model graph is ready for use; in
general, app-config `ready()` methods should be able to introspect models
in a deterministic way.
So I don't think we should introduce the possibility of non-deterministic
introspection into the core of the meta API, as this commit does. Instead,
we should switch the meta API to require `models_ready` rather than
require `ready` before it caches the full model graph.
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:13>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
Anyway, I'm not attached to any particular solution, as long as it works
:)
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:15>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
We're kind of repeating the discussion on the PR. :)
https://github.com/django/django/pull/3912
In my head I though Daniel was opposed to it, but apparently on the PR he
said "I think models_ready is a good place.".
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:14>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
I don't have time in the next couple days, but towards the end of this
week I can put together an alternative PR that maintains deterministic
behavior but uses `models_ready` instead of `ready`. I think I'd feel
better if the existing commit were rolled back sooner so we don't get
anything else that depends on the non-deterministic early calling of
`get_field()`, but I can also just roll it back in my alternate PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:16>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"0e489c19f1554ecfd9825daacfbac73be8ce723e"]:
{{{
#!CommitTicketReference repository=""
revision="0e489c19f1554ecfd9825daacfbac73be8ce723e"
Reverted "Fixed #24146 -- Fixed a missing fields regression in admin
checks."
This reverts commit e8171daf0cd7f0e070395cb4c850c17fea32f11d.
A new solution is forthcoming.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:17>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"92d5bedc56155e4c91c7644200415b3e42cdd31f"]:
{{{
#!CommitTicketReference repository=""
revision="92d5bedc56155e4c91c7644200415b3e42cdd31f"
[1.8.x] Reverted "Fixed #24146 -- Fixed a missing fields regression in
admin checks."
This reverts commit e8171daf0cd7f0e070395cb4c850c17fea32f11d.
A new solution is forthcoming.
Backport of 0e489c19f1554ecfd9825daacfbac73be8ce723e from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:18>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 1 => 0
* version: master => 1.8alpha1
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:19>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: closed => new
* resolution: fixed =>
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:20>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by pirosb3):
* cc: pirosb3 (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:21>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by pirosb3):
Hi all,
Yes, I am happy for the check to go on {{{ models_ready }}}. This should
resolve the current issue and also another one carljm reported earlier. We
should make sure that reverse models are always accessible on anything
that is public API, so this includes any hooks or ModelAdmin
registrations. Can anyone think of other similar parts that should be
tested?
If we are going in this direction (given that what I said above is
correct), I have made a PR with a few changes:
https://github.com/django/django/pull/4053. After we assure Jenkins
passes, I will add a few other tests admin-specific.
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:22>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by pirosb3):
* has_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:23>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"19188826b4aa989475418f1ea9bf8631b04da1e8"]:
{{{
#!CommitTicketReference repository=""
revision="19188826b4aa989475418f1ea9bf8631b04da1e8"
Fixed #24146 -- Allowed model._meta.get_field() to be used after
apps.models_ready
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:24>
Django
which does not refer to a Field.
Reporter: collinanderson | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"fdcc9c47d51289576ce17849900c96c161273037"]:
{{{
#!CommitTicketReference repository=""
revision="fdcc9c47d51289576ce17849900c96c161273037"
[1.8.x] Fixed #24146 -- Allowed model._meta.get_field() to be used after
apps.models_ready
Backport of 19188826b4aa989475418f1ea9bf8631b04da1e8 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24146#comment:25>