[patch] Improve support for mutually-referential models

[Ben Slavin <benjamin.slavin@…>]

ForeignKey and other related model fields allow recursive relationships; however, there can be problems when dealing with mutually referential models.

For example, consider a user class and a series of articles:

class User(models.Model):
    saved_articles = models.ManyToManyField('Article')
    # ...

class Article(models.Model):
    author = models.ForeignKey(User)
    # ...

It doesn't make much sense for Users and Articles to be stored in the same
models.py file.

By using dotted-notation (app_name.model_name), it should be
possible to reference across models.py files when recursive import
statements would otherwise cause problems.

my_project/accounts/models.py

class User(models.Model):
    saved_articles = models.ManyToManyField('articles.Article')
    # ...

my_project/articles/models.py

class Article(models.Model):
    author = models.ForeignKey('accounts.User')
    # ...

Attached patch adds this functionality and provides documentation.

[Ben Slavin <benjamin.slavin@…>]

Patch described above.

[Ben Slavin <benjamin.slavin@…>]

Modification of title to indicate patch is included.

[Ramiro Morales <rm0 _at_ gmx.net>]

Ben,

I'm not a Django guru so I can't comment on the code changes your patch implements. But I 'd like to point out that IMHO the changes to model-api.txt eliminate the paragraphs and examples that explicitely explain ManyToManyField and OneToOneField relationship field can be on 'self' but the new Recursive relationships section it adds doesn't make it clear with the same emphasis (it just talks about the ForeignKey case).

So, from the point of view an user implementing such a common use case (that obviously doesn't involves relationships to external models at all) the documentation is slightly less helpful than before.

Just my 2 cents.

[Ben Slavin <benjamin.slavin@…>]

Modification based on Ramiro's suggestion. Also fixed a slight typo in the docs (use of : instead of ::)

[Ben Slavin <benjamin.slavin@…>]

Ramiro,

You're right about removing some of the clarity. I've uploaded a new version of the patch to address your concern... what do you think?

It's tough to articulate complex relationships clearly... to terse and it's confusing, too verbose and it's intimidating. If you have any suggestions to make it more understandable, please let me know.

In terms of OneToOneField, I don't think that self-referential OneToOneFields actually work. Because of the OneToOneField internals, they must share the same ID (last I checked)... so it's literally a reference to 'self' and therefore somewhat silly. I may be mistaken here, though.

• Ben

[Russell Keith-Magee]

The current behaviour is by design. Text-style references are permitted within a single application, but not across applications. If you have two models in different apps that are self referential, then you don't really have two apps - you have one app (or, two apps that are so closely related that they probably should be one app).

[joaoma@…]

What if I have an amazingly large number of models, and I want to split the models across different files, and there are mutually-referential models in different files? This is a problem I've run into once, and I had to put all the models back in one (giant) models.py file.

[Ben Slavin <benjamin.slavin@…>]

joaoma:

You might find this helpful: CookBookSplitModelsToFiles

Then, using the already-existing 'modelname' reference style (string) will work since they're still in the same application.

[Osso]

Replying to russellm:

The current behaviour is by design. Text-style references are permitted within a single application, but not across applications. If you have two models in different apps that are self referential, then you don't really have two apps - you have one app (or, two apps that are so closely related that they probably should be one app).

This means you have to put the articles and the users in the same app which doesn't make sense.

[Ben Slavin <benjamin.slavin@…>]

Replying to Osso:

This means you have to put the articles and the users in the same app which doesn't make sense.

I assure you that Russell understood the implications you describe when he marked the ticket as wontfix. This is a design decision on the dev team's part... I don't completely agree with the choice, but I respect it.

The actual number of users impacted is very small (this doesn't tend to pop-up with the built-in user model). If you need the functionality this ticket describes, just apply the attached patch... it works. :-)

If you can come up with an argument that goes beyond what's already been discussed, please bring it up... but don't reopen the ticket without adding anything new to the discussion.

[Ben Slavin]

(Reopening to change resolution. Sorry if there's a way to do it in one step -- I don't see it.)

[Ben Slavin]

Fixed in [7158].