-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add docs for relationship links #1909
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
@vasilakisfil, thanks for your PR! By analyzing the annotation information on this pull request, we identified @joaomdmoura, @bf4 and @NullVoxPopuli to be potential reviewers |
b6c7570
to
3343cc0
Compare
Thanks so much for this! |
3343cc0
to
ca5455c
Compare
``` | ||
|
||
|
||
### Links as an property of the resource definiton |
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.
should read: "Links as a property"
looks good, just needs some grammatical updates |
|
||
AMS offers you many ways to add links in your JSON, depending on your needs. | ||
|
||
The following examples are in Rails but the logic is the same for any framework |
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.
Is this true? none of the controllers (or even the serializers) have a superclass.
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.
Yes I always use a BaseXXX class. I will just add the ApplicationController
and ActiveModel::Serializer
in the code examples.
8223a25
to
f83df9c
Compare
Fixed what you told me, rebased + squashed. |
ActiveModelSerializers offers you many ways to add links in your JSON, depending on your needs. | ||
Usually the most common use case for links is supporting nested resources. | ||
|
||
The following examples are in Rails but the logic is the same for any framework |
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.
Having two sentences start with the same phrase kinda feels awkward. :-\ idk
b059268
to
56b3dab
Compare
Fixed what you told me, rebased + squashed. |
@NullVoxPopuli sure do how you like :) let me know if you need any help! |
@vasilakisfil I submitted a PR to your repo. once that's merged, I'll feel pretty good about merging this. |
aad0b28
to
e5858bd
Compare
Merged, rebased and squashed! |
can you add a changelog entry to give yourself credit? :-) |
Add docs for links Add docs for links Add docs for links Add docs for links Add docs for links Add controller info Grammar fixing Improve docs some small wording changes Add pr to changelog
e5858bd
to
f80ebe6
Compare
Done! Rebased from master + squashed! |
Add docs for links Add docs for links Add docs for links Add docs for links Add docs for links Add controller info Grammar fixing Improve docs some small wording changes Add pr to changelog
Purpose
Simple doc for relationship links. I, personally had issues with relationship links, took me 4 hours to figure out how to add them in JSONAPI and I am not a newcomer in this project. I think we need this.
Changes
Just a markdown file
Caveats
I also add a note about empty relationship but WITH links, taken from here #1325 and here #1720
Related GitHub issues
#1325
#1720
Additional helpful information