Skip to content
This repository has been archived by the owner on Oct 19, 2021. It is now read-only.

Supporting the include query parameter for JSON-API #83

Closed
janko opened this issue May 6, 2015 · 7 comments · Fixed by #95
Closed

Supporting the include query parameter for JSON-API #83

janko opened this issue May 6, 2015 · 7 comments · Fixed by #95

Comments

@janko
Copy link
Collaborator

janko commented May 6, 2015

One other thing I was thinking about was somehow adding the support for automatic include recognition. This is also part of JSON-API specification; it is under MAY (so servers don't have to implement it), but I think many people will want to use it, and IMO that's for me one of main advantages of JSON-API, because it really makes the API flexible.

The nice thing is that Yaks already supports passing in env, in which case we have the "QUERY_STRING" available. This is what I did in my application:

require "rack/utils"

class BaseMapper < Yaks::Mapper
  def self.has_one(name, **options)
    super name, **options, if: -> { include_association?(name) }
  end

  def self.has_many(name, **options)
    super name, **options, if: -> { include_association?(name) }
  end

  private

  def include_association?(name)
    includes.any? do |relationship|
      relationship.split(".")[mapper_stack.size] == name.to_s
    end
  end

  def includes
    query = Rack::Utils.parse_query(env["QUERY_STRING"])
    query["include"].to_s.split(",")
  end
end

You see that this is relatively easy to implement. This now nicely supports e.g. /posts/1?include=author,comments.author (mapper_stack was a life saver, maybe we could document it somewhere). Naturally, it should be opt-in, and possible to set per-association (sometimes user may want one association to always be included).

What I was most impressed about Yaks was its already great support for JSON-API, and that everything just worked without me having to do anything. So I want to continue that line, what do you think?

@plexus
Copy link
Owner

plexus commented May 6, 2015

That's pretty cool stuff. At this point I would say to just stick it in the COOKBOOK. Yaks is deliberately unopinionated about how you structure your API. In other words we stay away at this point from request handling, which this comes close to.

Pagination and error handling are similar. I haven't found a good sweet spot yet about how to build it into Yaks without dictating how people should structure their API.

I can see a space for other projects to sit on top of Yaks but with stronger conventions, specific framework integrations, etc.

What we definitely should do though is make things easy, and provide good docs and examples. There's still a lot that could be documented better. So what I suggest

  • Add this code snippet to the COOKBOOK, with a clear explanation of what it achieves and how it works
  • Mention in the JSON-API section of the README that people can find an example of how to implement ?include in the COOKBOOK

@plexus
Copy link
Owner

plexus commented May 6, 2015

Also cool that mapper_stack was useful for this. The main reason we have it is to detect if we're rendering a top-level resource or a nested resource. Documenting it would obviously be a good thing. Maybe we need an "Advanced Mapper Writing" section in the README?

@plexus
Copy link
Owner

plexus commented May 9, 2015

I've given this some more thought, and I think there could be a place for having these kind of "behaviors" as modules that you can include in your mappers.

include Yaks::Behavior::OptionalIncludes

@janko
Copy link
Collaborator Author

janko commented May 10, 2015

Cool, it'd be great ship this with Yaks, because it's my favourite "feature" of JSON-API (and I guess it could be general for any spec). I was intending to add it to the cookbook anytime now, but I will work on the behaviour idea now, I will have time on tuesday (need to help clean my own issue/PR tickets :P).

@danelowe
Copy link
Collaborator

Hey @janko-m I'm pinching your code here. I was doing something very similar, but had forgotten about the ability to include sub-sub-resources e.g. comments.author 🙇

@janko
Copy link
Collaborator Author

janko commented May 14, 2015

@plexus and I agreed we could put it in a separate module and ship with Yaks. I will try to do this today or tomorrow.

@danelowe
Copy link
Collaborator

🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants