Skip to content

Document include options for Compound Documents #256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
scottfisk opened this issue Jun 23, 2016 · 8 comments
Closed

Document include options for Compound Documents #256

scottfisk opened this issue Jun 23, 2016 · 8 comments

Comments

@scottfisk
Copy link
Contributor

scottfisk commented Jun 23, 2016

With #250 pulled in we now have options to include relations to form compound documents:

  1. Include resources related to the primary data by default.
  2. Include resources related to the primary data through use of a request parameter which customizes which related resources should be returned

Example Current Implementation

  1. default include posts
class UserSerializer(serializers.Serializer):
    name = serializers.CharField(max_length=200)
    # PostSerializer included directly on the serializer
    posts = PostSerializer(many=True, read_only=True)

    class JSONAPIMeta:
        # included_resources specifys resources to include by default
        included_resources = ['posts']
  1. param include posts
class UserSerializer(serializers.Serializer):
    name = serializers.CharField(max_length=200)
    # PostSerializer NOT included directly on the serializer, ResourceRelatedField is 
    # specified as default
    posts = relations.ResourceRelatedField(
            source='post_set', many=True, read_only=True) 

    # specifies serializer to use when passing in the `include` param
    included_serializers = {
        'posts': PostSerializer,
    }

We should probably document this. We can even copy the examples above with a bit of explanation.

@scottfisk
Copy link
Contributor Author

scottfisk commented Jun 23, 2016

Looking at this a couple tweaks might make it more clear what the various options do.

  1. change included_serializers to include_serializers to match the name of the include since that parameter controls the use of these serializers? Maybe I am being overly precise, but I am hoping to make the subtleties easier to follow.
  2. Change included_resources to default_included_resources on JSONAPIMeta?

Any thoughts from people?

@jerel
Copy link
Member

jerel commented Jun 24, 2016

In the spec include is used for the GET parameter and included is used as the key in the returned payload. I still occasionally have to go look at what the GET param is named. However since we already have included_serializers out in the wild maybe we should keep everything in this library named included? (except the code dealing with the GET param obviously)

TLDR; I would lean toward keeping both names the same as they are now.

@scottfisk
Copy link
Contributor Author

scottfisk commented Jun 24, 2016

Yeah forgot that the returned key was included, naming seems fine then. Just need to update documentation to close this issue.

@chrisclark
Copy link
Collaborator

I think it's also worth explicitly documenting how to include relationships more than 1 level deep e.g.

/api/shipment/1/?include=items.product

This is part of the spec, but it wasn't clear to me how it was supported until I walked through IncludedResourcesValidationMixin.

If I have time later this week, I'll try to open a documentation PR (since I'll be documenting this internally for my team anyway).

@jerel
Copy link
Member

jerel commented Aug 9, 2016

@chrisclark did you get around to documenting this internally? If so we'd love to merge a PR 😄

@mblayman
Copy link
Collaborator

mblayman commented Dec 8, 2016

Since this was still open, I took a crack at documenting it in PR #308.

@timmyomahony
Copy link

Thanks for this. I had to find this post to make sense of included records. It would be great to get some official documentation for this. It's a common pattern for sideloading data in Ember for example

@sliverc
Copy link
Member

sliverc commented May 3, 2018

I think #308 has already addressed this issue. Closing it therefore, but if something is still not clear in the documentation please comment.

@sliverc sliverc closed this as completed May 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants