Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#25858 closed Bug (fixed)

Deriving from abstract model with foreign key to model in same app broken on Django 1.9

Reported by: Karl Hobley Owned by: Simon Charette
Component: Database layer (models, ORM) Version: 1.9
Severity: Release blocker Keywords:
Cc: Karl Hobley, Simon Charette 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

As of Django 1.9, it is no longer possible to derive a new model from abstract model in another app if that abstract model has a local foreign key to a model in the same app.

See example project here: https://github.com/kaedroho/djangobugtest

For example, I have two apps: app1 and app2

app1 contains a concrete model and an abstract model. The abstract model has a foreign key to the concrete one (note the Foreign key is defined as a string and doesn't include the app label):

class ConcreteModel(models.Model):
    pass


class AbstractModel(models.Model):
    foreign_key = models.ForeignKey('ConcreteModel')

    class Meta:
        abstract = True

In app2, we have another model that derives from the abstract model in app 1

class DerivedModel(app1.AbstractModel):
    pass

When running makemigrations, the following system check error occurs:

app2.DerivedModel.foreign_key: (fields.E300) Field defines a relation with model 'ConcreteModel', which is either not installed, or is abstract.

For apps upgrading to Django 1.9 and already have migrations, the following error occurs when running tests (after the database has been initialised but before the first test runs. It looks like it's happening while serialising the database):

ValueError: Related model u'ConcreteModel' cannot be resolved

Traceback from an actual site: https://gist.github.com/kaedroho/01b29332308018abfa29

This issue only occurs when the ForeignKey is defined with a string that doesn't contain the app label. So referencing the model class directly or using "app_label.ModelName" works.

Change History (14)

comment:1 by Karl Hobley, 9 years ago

Cc: Karl Hobley added

comment:2 by Karl Hobley, 9 years ago

Version 0, edited 9 years ago by Karl Hobley (next)

comment:3 by Tim Graham, 9 years ago

Cc: Simon Charette added
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Not sure if the issue can/should be fixed, but we could at least document the requirement and backwards incompatibility.

comment:4 by Simon Charette, 9 years ago

Owner: changed from nobody to Simon Charette
Status: newassigned

I think we should try fixing this. It can lead to confusing errors if a second ConcreteModel is defined in app2 for example. I'll see if I can come up with a non-invasive patch.

comment:5 by Tim Graham, 9 years ago

I guess there's a small design decision about whether or not it could possibly be a useful "feature" to have the option of using the model name only in the ForeignKey to be able to refer to a local model in the app in which the abstract model is actually used and to require an app_label in the ForeignKey otherwise?

comment:6 by Simon Charette, 9 years ago

Indeed, I've been trying to reach Carl to talk about it but I guess I should simply write to @developpers.

If we were to consider this change as a feature I guess we should go through a deprecation period.

comment:8 by Simon Charette, 9 years ago

Has patch: set

PR based on input from Markus, Shai and Aymeric.

comment:9 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by Simon Charette <charette.s@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In bc7d201b:

Fixed #25858 -- Bound abstract model application relative relationships.

Thanks to Karl Hobley for the report and Markus, Shai, Aymeric for their input
and Tim for the review.

comment:11 by Simon Charette <charette.s@…>, 9 years ago

In 27ef6403:

[1.9.x] Fixed #25858 -- Bound abstract model application relative relationships.

Thanks to Karl Hobley for the report and Markus, Shai, Aymeric for their input
and Tim for the review.

Backport of bc7d201bdbaeac14a49f51a9ef292d6312b4c45e from master

comment:12 by Simon Charette, 9 years ago

In order to fix #26186 the patch for this ticket is planed to be reverted in Django 1.9.3.

The suggested approach for the issue reported here is to bound your app relative reference to an application (app_label.ConcreteModel in your reported case) because what we initially thought as a new behavior introduced in Django 1.9 has been possible since at least Django 1.2 given the appropriate setup.

In order to support both use cases it was decided that app relative lazy relationships should be resolved against their concrete model's app_label and an explicit app_label should be added to the lazy relationship if the abstract model was meant to be used in a foreign application while maintaining a relationship to a specific application's model.

comment:13 by Simon Charette <charette.s@…>, 9 years ago

In 0223e213:

Fixed #26186 -- Documented how app relative relationships of abstract models behave.

This partially reverts commit bc7d201bdbaeac14a49f51a9ef292d6312b4c45e.

Thanks Tim for the review.

Refs #25858.

comment:14 by Simon Charette <charette.s@…>, 9 years ago

In 48cf7516:

[1.9.x] Fixed #26186 -- Documented how app relative relationships of abstract models behave.

This partially reverts commit bc7d201bdbaeac14a49f51a9ef292d6312b4c45e.

Thanks Tim for the review.

Refs #25858.

Backport of 0223e213dd690b6b6e0669f836a20efb10998c83 from master

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