[Django] #25723: Related field checks should use their model's apps to lookup related models
Django
------------------------------------------------+------------------------
Reporter: charettes | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
Related model fields are using the global apps registry to determine
whether or not their related model is registered (`to`, `through`).
This prevents the creation of checks tests that define model using an
isolated `Apps()` instance.
e.g.
{{{#!python
isolated_apps = Apps()
class Foo(models.Model):
class Meta:
apps = isolated_apps
class Bar(models.Model):
foo = models.ForeignKey(Foo)
class Meta:
apps = isolated_apps
# The following assertion will fail since the `to` check
# will detect a `fields.E300` issue.
asset Bar._meta.get_field('foo').check() == []
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25723>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: charettes | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/5618 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/25723#comment:1>
Django
Reporter: charettes | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* needs_better_patch: 1 => 0
Comment:
Comments addressed.
--
Ticket URL: <https://code.djangoproject.com/ticket/25723#comment:2>
Django
Reporter: charettes | Owner: nobody
Type: Bug | Status: new
checks) |
Severity: Normal | Resolution:
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
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/25723#comment:3>
Django
Reporter: charettes | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: master
checks) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette <charette.s@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"c550beb0ccc8855fde7ff50ada530f7ceff1a595" c550beb0]:
{{{
#!CommitTicketReference repository=""
revision="c550beb0ccc8855fde7ff50ada530f7ceff1a595"
Fixed #25723 -- Made related field checks lookup against their local apps.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25723#comment:4>
Django
Reporter: charettes | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: master
checks) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette <charette.s@…>):
In [changeset:"c049c43e28a3e46eaed13bb8e018b221250a0145" c049c43e]:
{{{
#!CommitTicketReference repository=""
revision="c049c43e28a3e46eaed13bb8e018b221250a0145"
[1.9.x] Fixed #25723 -- Made related field checks lookup against their
local apps.
Backport of c550beb0ccc8855fde7ff50ada530f7ceff1a595 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25723#comment:5>
Django
Reporter: charettes | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: master
checks) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
This seems to have broken django-taggit. https://github.com/alex/django-
taggit/issues/360
Manager.opts was just added in 1.9. django-taggit seems to have worked
fine without it up to this point. I'm wondering if we should error on the
side of not using self.opts for now.
https://github.com/django/django/pull/5718
--
Ticket URL: <https://code.djangoproject.com/ticket/25723#comment:6>
Django
Reporter: charettes | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: master
checks) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
Not sure about what should be done here. I don't mind if we use
`self.model._meta` like proposed to allow older version of taggit to work
with 1.9 out of the box but on the other hand this whole issue is caused
by the fact taggit override `contribute_to_class()` without calling
`super()`.
Does django-taggit also override `related_query_name()` and
`resolve_related_fields()`? These methods also use `self.opts`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25723#comment:7>
Django
Reporter: charettes | Owner: nobody
Type: Bug | Status: closed
Component: Core (System | Version: master
checks) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
Ahh, yeah, the more I think about it, it's probably best to have the error
thrown in checks right away rather than have a possible issue when the
code is actually running.
--
Ticket URL: <https://code.djangoproject.com/ticket/25723#comment:8>