Skip to content
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

begin integration docs #1312

Merged
merged 5 commits into from
Dec 3, 2015
Merged

Conversation

NullVoxPopuli
Copy link
Contributor

There are always going to be little quirks here and there when integrating with different technologies, especially newer ones.

Here is a pattern suggested by @bf4 for organizing information on how to prepare other technologies to work along with AMS.

note: this doesn't mean that these docs are only for if the integratee needs to change to work with AMS.
When AMS gets the ability to change the key format (specifically for JSON-api) that will clean up the ember-integration documentation a bit.

…he inability to use query params on a single resource find
@bf4
Copy link
Member

bf4 commented Nov 26, 2015

@bf4
Copy link
Member

bf4 commented Nov 26, 2015

@NullVoxPopuli Let's get this cleaned up and shipped! Great work, just needs some touch-ups, and maybe reference versions of AMS the integrations target, e.g.

Integration Known supported versions Gem name and/or link
Ember.js 0.9.x active-model-adapter
Ember.js 0.10.x docs/integrations/ember-and-json-api.md
Grape 0.10.x #1258
Grape 0.9.x https://github.com/jrhe/grape-active_model_serializers/
Sinatra 0.9.x https://github.com/SauloSilva/sinatra-active-model-serializers/
| Integration | Known supported versions |  Gem name and/or link
|----|-----|----
| Ember.js | 0.9.x | [active-model-adapter](https://github.com/ember-data/active-model-adapter)
| Ember.js | 0.10.x |  [docs/integrations/ember-and-json-api.md](docs/integrations/ember-and-json-api.md)
| Grape | 0.10.x | #1258  |
| Grape | 0.9.x | https://github.com/jrhe/grape-active_model_serializers/ |
| Sinatra | 0.9.x | https://github.com/SauloSilva/sinatra-active-model-serializers/

@NullVoxPopuli
Copy link
Contributor Author

I really like that compatibility table.

I'll push an update shortly

@NullVoxPopuli
Copy link
Contributor Author

This also needs to be mentioned:

#1027 (comment)

@NullVoxPopuli
Copy link
Contributor Author

how's it look, @bf4 ?

)

Mime::Type.unregister :json
Mime::Type.register 'application/json', :json, api_mime_types
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we might want something like https://github.com/cerebris/jsonapi-resources/blob/master/lib/jsonapi/mime_types.rb but I have no way of testing it right now

module JSONAPI
  MEDIA_TYPE = 'application/vnd.api+json'
end

Mime::Type.register JSONAPI::MEDIA_TYPE, :api_json

ActionDispatch::ParamsParser::DEFAULT_PARSERS[Mime::Type.lookup(JSONAPI::MEDIA_TYPE)] = lambda do |body|
  data = JSON.parse(body)
  data = {:_json => data} unless data.is_a?(Hash)
  data.with_indifferent_access
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could certainly add that later - maybe when the JSON API implementation becomes more complete. I think that should be separate from this PR though.

@bf4
Copy link
Member

bf4 commented Dec 3, 2015

Pretty good.

I'm happy to merge, but wanted to bring up in one place

@NullVoxPopuli
Copy link
Contributor Author

re: the links,
I'm wary of including links to other places, as I believe that may appear like we support those links in that they are compatible with AMS -- personally, I haven't tried many of the links -- there may be things forgotten in any number of guides -- or slightly version incompatibilities for whatever reason..

idk. :-\

bf4 added a commit that referenced this pull request Dec 3, 2015
@bf4 bf4 merged commit 5301112 into rails-api:master Dec 3, 2015
@bf4 bf4 deleted the integration-docs branch December 3, 2015 17:26
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.

2 participants