Adds a :links option to the relationship macros

[nikz]

This allows specifying a :links option to a has_many/has_one relationship, which means you can specify self or related links as per the JSON API spec (these are often useful for not loading all associated objects in a single payload)

A few people have chatted about this option or various parts of it in issues (#253, #252, partially in #94).

I'm not 100% if this is the syntax you folks would like, but it's impossible to copy ActiveModel::Serializers as the block to the relationship macros is already in use for sparse recordsets. For that reason, I used a :links option so the syntax ends up looking like:

        has_many :actors, links: {
          self:    :actors_relationship_url,
          related: -> (object, params = {}) {
            "http://movies.com/movies/#{object.name.parameterize}/actors/"
          }
        }

This may not be what you folks want, but I thought I'd open a quick PR and then that way we have something to discuss! I'm absolutely fine if y'all want to do this a different way, or you'd like me to change something. Cheers! ✨ 👍

[nikz]

Hey folks, would love to get this one in or otherwise discuss it with y'all - is there any other way for me to get in touch?

Thanks!

[deivinsontejeda]

👍

[rowan]

👍

[shishirmk]

@nikz I can take a look into this soon. I would have liked to use a convention to define the related block rather than specify it per relationship.

If it's a special case then it makes sense to define such a block.

How is this feature implemented in AMS?

[nikz]

@shishirmk Convention sounds great - I guess we could just add an option that assumes you have a _url method on your model? Because the problem is that we're in the model context so there's not necessarily a URL generator available.

How about something like:

  has_many :actors, related_link: :actors_url

Which would call the actors_url method?

The syntax in AMS looks something like:

  has_many :actors do
    link :related do
      actors_url
    end
    include_data false
  end

So I think it would require a breaking API change to support a similar syntax (and I'm not necessarily sure it's nicer).

More than happy to chat about making the API better!

[christophersansone]

@nikz Thanks for getting the ball rolling here. And @shishirmk thanks for being cautious about how the feature should be presented in FJ. I'm (finally) starting the migration of my largest app from AMS to FJ and we use this feature, happy to lend a hand here.

The JSON API spec allows for data and links to be available (individually and together), so whatever we expose in FJ should support what the spec allows.

Here is an example from the spec document:

{
  "type": "articles",
  "id": "1",
  "attributes": {
    "title": "Rails is Omakase"
  },
  "relationships": {
    "author": {
      "links": {
        "self": "/articles/1/relationships/author",
        "related": "/articles/1/author"
      },
      "data": { "type": "people", "id": "9" }
    }
  }
}

I would think a reasonable implementation could look like this:

has_many :actors, include_data: true, links: {
  related: -> { |object| `http://movies.com/movies/#{object.id}/actors }
}

Or possibly:

has_many :actors, include_data: true, links: -> { |object, params|
  { related: `http://movies.com/movies/#{object.id}/actors` }
}

I think I prefer the latter, but either way, I feel this follows the JSON API spec pretty well. A few key points:

• include_data could be true, false, or a Proc that contains the object and params. It would always default to true, so that way the behavior will work as it does today. And it would work independently of the links.

• There can be several links, just like the API spec.

• The URLs will often need to be generated dynamically, since it goes against Rails conventions to have URLs in your model. It's more of a controller / view thing. Therefore we will need to expose a Proc. In the Rails world, I don't think it would be very common for the URL to be available anywhere else, so I personally think we would only need to support a Proc, not a method name.

Let me know what you all think here.

[fusion2004]

I really like how it was implemented in jsonapi-rb, where you can specify how to get the data for the relationship, as many links as you want, as well as whatever metadata you want.

Their default is to only serialize the relationship links unless the relationship is included, in which case the data is then also serialized. This prevents additional SQL queries to get relationship ID(s) when you are not including some relationships in a request. (There is also an option to always include the linkage data.)

Along those lines, I think it would make sense to implement the serialization of the linkage data in a similar fashion, and perhaps name the option key always_include_data instead of include_data. include_data: false indicates to me that it would never serialize the linkage data, which is not the functionality I want or that I imagine most users would want either.

[christophersansone]

I like the explicitness of the jsonapi-rb example syntax. I think it might be hard to do that in FJ currently because it is using the block differently. Currently it is used to determine the object that should be referenced as the data. We may be able to handle both situations... but we don't use that style of syntax anywhere else now. It's currently all just hashes and Procs.

…

On Aug 22, 2018, at 6:05 PM, Mark Oleson ***@***.***> wrote: I really like how it was implemented in jsonapi-rb, where you can specify how to get the data for the relationship, as many links as you want, as well as whatever metadata you want. Their default is to only serialize the relationship links unless the relationship is included, in which case the data is then serialized. This prevents additional SQL queries to get relationship IDs when you are not including some relationships in a request. (There is also an option to always include the linkage data.) — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[shishirmk]

@christophersansone Thank you for jumping in with your input. I like the syntax you have brought up. in the above comment.

has_many :actors,  links: {
  related: -> { |object| `http://movies.com/movies/#{object.id}/actors }
}

@nikz any objections to this syntax? If this looks ok. lets go ahead with this syntax.

[christophersansone]

@shishirmk Thanks for the quick response. I like the syntax you mention. If that's the case, my only questions would be surrounding how to handle the relationship's data key. Do we have a separate parameter (e.g. include_data) to specify whether or not to include the data key? Do we want to allow a data parameter as a Proc as well? Does the presence of links or data change the default behavior.

A bit off topic, but along the lines of @fusion2004's comments, I do think it would be a performance win if we offered greater control to how the data key of the relationship is determined. For instance, the typical belongs_to :author in Rails will touch that relationship and therefore load the model, even though all we need is the ID which is already present. Maybe we have the links as described above, then also have a data key that can customize its behavior.

[YoranBrondsema]

We are currently migrating from AMS to FJ and need this functionality as well. Is there anything blocking the merge of this PR?

As to the data/include_data discussion (which I think can be handled separately from the links, in another PR), I think the approach taken by jsonapi-rb is more flexible. How about having something like:

# equivalent to include_data: false
has_many :actors, data: nil

has_many :actors, data: Proc.new { |record|
  record.actors.some_filter
}

[YoranBrondsema]

I don't think the &block argument is used anywhere in the body?

[YoranBrondsema]

The , is redundant.

[nikz]

@shishirmk No objections, but I'm not sure that's valid Ruby? We could do:

has_many :actors,  links: {
  related: lambda { |object| `http://movies.com/movies/#{object.id}/actors` }
}

But that seems equivalent to:

has_many :actors,  links: {
  related: -> (object, params = {}) { `http://movies.com/movies/#{object.id}/actors` }
}

Which is what I have (unless I'm completely mistaken?)

[nikz]

@christophersansone IMHO having a data key would be much more explicit - at the moment the passed block is simply used, which is why I avoided doing anything with that as it would be a breaking API change.

Once this PR is sorted (and assuming it passes muster 😄) I'd be more than happy to create a separate PR adding the data key and deprecating the implicit-block-syntax?

[nikz]

@YoranBrondsema thanks for the review, I've submitted fixes for those things now.

(Sorry for the slow replies on this everyone - I was on holiday!)

[nikz]

@shishirmk heya! I've just updated this PR to the latest upstream master. I've also added a lazy_load_data option that means this has everything you need to lazily load JSON API relationships.

Is this a feature that we think would be useful for the gem? Would love to get it merged, and no worries if you'd like me to make changes. I think it's a fairly common thing to want to do (e.g if you have hundreds of posts by each author, you don't necessarily want to load them all on the author listing page)

Thanks!

[mfly]

@nikz nice work on that one! It definitely looks like a common use case.
@shishirmk can we expect this code to reach the master any soon?

[shishirmk]

@nikz thank you for all the work you have put into this.

[nikz]

@shishirmk no problems, happy to help - let me know if there are any bugs or anything that crops up 👍