Skip to content

fix resource identifier objects#637

Open
acid wants to merge 2 commits intoJSONAPI-Resources:masterfrom
POSpulse:fix_resource_identifier_objects
Open

fix resource identifier objects#637
acid wants to merge 2 commits intoJSONAPI-Resources:masterfrom
POSpulse:fix_resource_identifier_objects

Conversation

@acid
Copy link
Copy Markdown
Contributor

@acid acid commented Feb 26, 2016

According to
http://jsonapi.org/format/#document-resource-identifier-objects
rescource identifier objects can contain an additional meta attribute.
This fix adds supports for the full specification.

Comment thread lib/jsonapi/request.rb Outdated
type: unformat_key(raw['type']).to_s,
id: raw['id']
id: raw['id'],
meta: raw['meta']
Copy link
Copy Markdown
Contributor

@beauby beauby Apr 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does add a (possibly nil) meta attribute to each resource identifier object, right?
Maybe the following would make more sense:

hash = {
  type: unformat_key(raw['type']).to_s,
  id: raw['id']
}

hash[:meta] = raw[:meta] if raw.key?(:meta)

hash

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that, thanks! :)

@acid acid force-pushed the fix_resource_identifier_objects branch 3 times, most recently from 2386c29 to b4f9fb9 Compare April 20, 2016 21:23
@lgebhardt
Copy link
Copy Markdown
Contributor

@acid Looks like the meta will get dropped in parse_to_many_links_object, though I haven't tested it.

According to
http://jsonapi.org/format/#document-resource-identifier-objects
rescource identifier objects can contain an additional meta attribute.
This fix adds supports for the full specification.
@acid acid force-pushed the fix_resource_identifier_objects branch from b4f9fb9 to da9783f Compare May 30, 2016 08:54
@acid acid force-pushed the fix_resource_identifier_objects branch from da9783f to 7ce6c3e Compare May 30, 2016 09:04
@acid
Copy link
Copy Markdown
Contributor Author

acid commented May 30, 2016

@lgebhardt you're right. Fixed it, thanks!

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

Successfully merging this pull request may close these issues.

3 participants