Skip to content

Posting to nested routes #25

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

Open
klobuczek opened this issue May 8, 2017 · 5 comments
Open

Posting to nested routes #25

klobuczek opened this issue May 8, 2017 · 5 comments

Comments

@klobuczek
Copy link

In a case of /posts/1/comments and a payload

{data: {type: 'comments', attributes: { content: 'Some comment'}}}

I would like to be able to add the relationship to posts without the client having to specify the relationship in the payload. I really hope this is compliant with the spec.
How would I implement this in json_api compliable? jsonapi_create does not take any params, but it would be helpful if it did.

@richmolj
Copy link
Contributor

richmolj commented May 8, 2017

Hey @klobuczek, I do have an option for you, but would also like to better understand the use case. I've personally found these relationship endpoints to be unnecessary. Since these libs honor "sideposting", I either "sidepost" the comment using the /posts endpoint or specify the relationship in the /comments endpoint. I know you mentioned avoiding the relationship in the payload, what's the reasoning for this?

Just as an example, using the jsorm client makes this easy:

Post.find('1').then((response) => {
  let post = response.data;
  post.comments.push(new Comment({ content: 'Some comment' }));
  post.save({ with: ['comments'] }).then((response) => { ... });
});

That said, you could do something like this to get the desired behavior, I think:

before_action :merge_foreign_key

def merge_foreign_key
  params[:data][:attributes].merge!(post_id: params[:id])
  # or
  deserialized_params.attributes.merge!(post_id: params[:id])
end

@klobuczek
Copy link
Author

@richmolj the way you are doing this in the jsorm snippet requires that the client has the entire list comments and PATCHES the Post back with all the comments plus the new one. That's what the specification says. This could be a lot of back and forth and it is prone to mistakes.
POSTing to /comments endpoint would be an option, but it feels unintuitive to POST comments to different place than from where to GET them. Assuming you would do GET /post/1/comments.
There is another problem with having an id in the url and the payload, which would have to be double checked for equality. This is even an issue with non nested PATCH e.g. PATCH /post/1 requires the id: 1 being present in the url and the payload. I think this is not checked anywhere at the moment.

@richmolj
Copy link
Contributor

richmolj commented May 8, 2017

requires that the client has the entire list comments and PATCHES the Post back with all the comments plus the new one. That's what the specification says.

Yeah. Unfortunately jsonapi has punted on nested relationships despite tons of need/issues on github. If this had been addressed, I feel pretty confident the specification would not require the POST to include every comment. It's a rather silly requirement - how often are you trying to fully replace the relationship versus add/update/destroy/disassociate a single entity?

So, because I - and many others - believe this should be part of the spec, we deviate and do not require the full comments collection.

I recognize deviating from the spec is very much less than ideal. I wish jsonapi would address this issue so we would not have to. FWIW, I now run only non-nested URLs with sideposting - no nested routes or relationship routes - and the only thing that changed was everything became a lot easier.

POSTing to /comments endpoint would be an option, but it feels unintuitive to POST comments to different place than from where to GET them. Assuming you would do GET /post/1/comments.

I avoid nested routes altogether and I haven't had problems so far. This appears to be the 'ember way' - it's rather hard to use ember-data with nested URLs, and you can see lots of discussion around this issue in the ember community. I would POST to /comments and GET /comments?filter[post_id]=1

Regarding the URL in the payload and the URL - I hear you, I'd be open to a PR to the Deserializer

@klobuczek
Copy link
Author

Hmm, I have heard that opinion before. jsonapi by itself is already hard for humans. Getting rid of nested routes is like removing the last part about the domain model which is still graspable to humans.
I bet flat routes are much easier in gem implementation and usage by libraries like ember. I was told that if the payload has all links it is easy to do any routes in ember.
In our case nested routes help as well efficient navigation of the graph database as they define canonical traversal which is usually the most performant rather than just finding resources by a collection of various foreign keys.

@richmolj
Copy link
Contributor

richmolj commented May 9, 2017

@klobuczek I'm not unsympathetic to nested routes, but I'm currently leaning against out-of-the-box support.

The root of the issue is, I dislike two ways to do the same thing. Once you have sideposting - which is a must have in my opinion, since sometimes things need to run within a transaction - the nested routes become another way to do the same thing.

Regarding human readability - I think it's mostly personal preference and what use case you are optimizing for. I personally think nested routes / jsonapi relationship routes are unnecessarily confusing and are inherently less performant (due to multiple http requests needed). The sideposting payloads are hard to read for humans, completely agree there...but this is mostly mitigated by the client you are using.

This follows the same tradeoff as included/sideloading IMO - less human-readable, but this is mitigated by the client and technically superior. I think this can be further mitigated with tooling (I talked to someone at RailsConf who has a CLI for easy introspection, for instance). Now that the core libraries are done, enhanced tooling like this (including an auto-documenting swagger replacement) is a key goal of jsonapi-suite in the near future.

If you're looking for canonical traversal, I suggest links is the best place for this. If anything I would expect these routes to have better performance - if performance is the issue let's definitely try to address that.

All that said, I would be in favor of a separately maintained library.

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

No branches or pull requests

2 participants