Opened 17 years ago

Closed 16 years ago

#3688 closed (fixed)

[patch] Improve support for mutually-referential models

Reported by: Ben Slavin <benjamin.slavin@…> Owned by: Adrian Holovaty
Component: Core (Other) Version: dev
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Attachments (2)

recursive-related-fields-4692.diff (6.3 KB ) - added by Ben Slavin <benjamin.slavin@…> 17 years ago.
Patch described above.
recursive-related-fields-4692v2.diff (6.5 KB ) - added by Ben Slavin <benjamin.slavin@…> 17 years ago.
Modification based on Ramiro's suggestion. Also fixed a slight typo in the docs (use of : instead of ::)

Download all attachments as: .zip

Change History (12)

by Ben Slavin <benjamin.slavin@…>, 17 years ago

Patch described above.

comment:1 by Ben Slavin <benjamin.slavin@…>, 17 years ago

Summary: Improve support for mutually-referential models[patch] Improve support for mutually-referential models

Modification of title to indicate patch is included.

comment:2 by Ramiro Morales <rm0 _at_ gmx.net>, 17 years ago

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.

by Ben Slavin <benjamin.slavin@…>, 17 years ago

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

comment:3 by Ben Slavin <benjamin.slavin@…>, 17 years ago

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

comment:4 by Russell Keith-Magee, 17 years ago

Resolution: wontfix
Status: newclosed

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).


comment:5 by joaoma@…, 17 years ago

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.

comment:6 by Ben Slavin <benjamin.slavin@…>, 17 years ago

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.

in reply to:  4 comment:7 by Osso, 17 years ago

Resolution: wontfix
Status: closedreopened

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.

comment:8 by Ben Slavin <benjamin.slavin@…>, 17 years ago

Resolution: wontfix
Status: reopenedclosed

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.

comment:9 by Ben Slavin, 16 years ago

Resolution: wontfix
Status: closedreopened

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

comment:10 by Ben Slavin, 16 years ago

Resolution: fixed
Status: reopenedclosed

Fixed in [7158].

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