Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#25723 closed Bug (fixed)

Related field checks should use their model's apps to lookup related models

Reported by: Simon Charette Owned by: nobody
Component: Core (System checks) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

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() == []

Change History (8)

comment:1 by Tim Graham, 8 years ago

Has patch: set
Patch needs improvement: set

comment:2 by Simon Charette, 8 years ago

Patch needs improvement: unset

Comments addressed.

comment:3 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:4 by Simon Charette <charette.s@…>, 8 years ago

Resolution: fixed
Status: newclosed

In c550beb0:

Fixed #25723 -- Made related field checks lookup against their local apps.

comment:5 by Simon Charette <charette.s@…>, 8 years ago

In c049c43e:

[1.9.x] Fixed #25723 -- Made related field checks lookup against their local apps.

Backport of c550beb0ccc8855fde7ff50ada530f7ceff1a595 from master

comment:6 by Collin Anderson, 8 years ago

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. django-taggit is adding Manager.opts, but I'm wondering if we should error on the side of not using self.opts for now.

https://github.com/django/django/pull/5718

Last edited 8 years ago by Collin Anderson (previous) (diff)

comment:7 by Simon Charette, 8 years ago

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.

comment:8 by Collin Anderson, 8 years ago

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.

Note: See TracTickets for help on using tickets.
Back to Top