-
Notifications
You must be signed in to change notification settings - Fork 89
Fix compound relationships includes #116
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
Conversation
83aca8d to
e566217
Compare
e566217 to
3f5c1ea
Compare
|
Hi @fotinakis, any chance of having it merged soon? This would be a super helpful addition for us. Thanks. |
fotinakis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Exelord thanks for this, sorry for the delay reviewing.
Can you check out the one test comment here and see what the correct behavior for that should be? After that LGTM
| ], | ||
| } | ||
| # Also test that it handles string include arguments. | ||
| includes = 'long-comments, long-comments.post.author' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes the behavior of the test, it's specifically testing the behavior of "overlapping include paths" but this no longer does that.
| if includes | ||
| direct_children_includes = includes.reject { |key| key.include?('.') } | ||
| passthrough_options[:include_linkages] = direct_children_includes | ||
| include_linkages = includes.map { |key| key.to_s.split('.').first } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
|
@Exelord friendly ping here |
|
@fotinakis Done. |
|
Published in jsonapi-serializers v1.0.1. Thanks for contributing! |
Hey,
I noticed that including compound relationships of the resource doesn't add correct linkage data.
Resources model
Current behavior
calling a url
GET /author/4?include=posts.commentsdoesn't return linkage forpostsrelationship ofauthorresourceExpected standard
According to JSONAPI spec: http://jsonapi.org/format/#fetching-includes
Proposed behavior
calling a url
GET /author/4?include=posts.commentsdoes return linkage forpostsrelationship ofauthorresource