-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
RC3 Updates for JSON API #853
Changes from all commits
3ba4386
da86747
83c2854
0f55f21
d82c599
33f3a88
294d066
946d1db
4fcacb0
ef3bfdd
b695180
90c7005
9480b56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -164,6 +164,14 @@ def json_key | |
end | ||
end | ||
|
||
def id | ||
object.id if object | ||
end | ||
|
||
def type | ||
object.class.to_s.demodulize.underscore.pluralize | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I see what you're doing. You are:
Is that it? I'm onboard with that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, easily overriding the type is one of the main reasons I put this here. One can create an I put the id here as well because it made thing more symmetrical, but I can remove it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't be a good idea to add comments or There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea, I was already planning on adding it to the documentation. However, we need to figure out what to do about the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd suggest raising an error when the |
||
end | ||
|
||
def attributes(options = {}) | ||
attributes = | ||
if options[:fields] | ||
|
@@ -172,6 +180,8 @@ def attributes(options = {}) | |
self.class._attributes.dup | ||
end | ||
|
||
attributes += options[:required_fields] if options[:required_fields] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just noticed that this could lead to duplicates, I'll fix it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nvm it's fine |
||
|
||
attributes.each_with_object({}) do |name, hash| | ||
hash[name] = send(name) | ||
end | ||
|
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.
The JSON API spec requires resource
id
to be a string.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.
It's converted to a string in the adapter (https://github.com/rails-api/active_model_serializers/pull/853/files#diff-ae1e38e96a8bf28ae37918a65c448c73R89)
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.
Okay, great 👍