Better support generic relationships #606

Closed
bennullgraham opened this Issue Jan 23, 2013 · 7 comments

Comments

Projects
None yet
3 participants

Edit: Updated title from "Support for Hyperlinked Generic Relationships", to better represent the remaining work to be done - @tomchristie 12th Feb 2012


Allow for generic foreign relationships to be represented with hyperlinks.

Some background discussion at django.reddit.com

I naively hoped to make a pull request along with this issue, but realised I'm some time from understanding the REST framework code well enough to write non-embarrassing patches for it.

Collaborator

tomchristie commented Jan 23, 2013

Thanks Ben. An alternative if you up for spending a little time on it would be a pull req with a failing test, which is always appreciated. Even if there's some thing your not sure about with the test its really useful as a more concrete guide to exactly what behaviour you'd expect to see. (But no problem if not)

I think at least for a first pass there's likely to be a limitation that GFK hyperlinks are read-only. (Problem is how do you map an arbitrary URL back to an arbitrary Model instance) Would be useful to know if eg tastypie supports writable hyperlinked GFKs.

On 23 Jan 2013, at 05:57, Ben Graham notifications@github.com wrote:

Allow for generic foreign relationships to be represented with hyperlinks.

Some background discussion at django.reddit.com

I naively hoped to make a pull request along with this issue, but realised I'm some time from understanding the REST framework code well enough to write non-embarrassing patches for it.


Reply to this email directly or view it on GitHub.

Collaborator

tomchristie commented Jan 23, 2013

Also, noting this in the meantime for anyone else finding this:

The right way at the moment to approach this is to subclass RelatedField/ManyRelatedField, and override .to_native to provide exactly the behavior you want. This ticket is to add HyperlinkRelatedField support to GFKs out-of-the-box, so that you don't always need a custom implementation.

Collaborator

tomchristie commented Jan 26, 2013

Closing related pull req #607. Hopefully the test module cleanup should make the issue a bit more obvious...

Related fields that span a GenericRelation (Reverse GFK) work just fine. GenericRelation only ever points at a single model class as the target, so there's no problem.

Related fields that span a GenericForeignKey are the issue, because it can point to multiple types of model. At the moment there's no way of specifying how you'd expect the related field to behave. (eg in the test module, if you wanted to use a hyperlinked relationship you'd need some way of specifying that Note objects should use a URL named note-detail, and Bookmark objects should use a URL named bookmark-detail, or whatever) Likewise for nested relationships you'd need some way of mapping the object type to the serializer that should be used for that type.

In order to support this we'd probably want something like...

tagged_item = serializers.GenericRelatedField({
    Note: serializers.HyperlinkedRelatedField(view_name='note-detail'),
    Bookmark: : serializers.HyperlinkedRelatedField(view_name='bookmark-detail')
})

That would allow you to express generic FKs in a flexible and explicit way.

Sound right to you?

Apologies Tom, I did get the test case arse-about.

Your description is spot-on: how do you map the related models to URL names? I like the explicitness of your suggestion, and it's concise without relying on magical introspection of user code.

Collaborator

tomchristie commented Feb 12, 2013

Note: there's now some much better documentation on generic relations, that'll go up soon: https://github.com/tomchristie/django-rest-framework/blob/master/docs/api-guide/relations.md#generic-relationships

If anyone's interested in taking it on I think the GenericRelatedField idea would probably still be worth doing.

My pull request seems to lack some travis CI love. I'm going to take care of this as soon as possible. But I'm still interested in your verdict of the general design, Tom. I don't consider it bulletproof but I'd love to have something generally approved going on when it comes down to GenericForeignKeys.

Regards Lukas

Collaborator

tomchristie commented Aug 18, 2014

We've got some decent documentation around this. At this point any further support should almost certainly be in the form of a third-party package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment