rails-api / active_model_serializers

ActiveModel::Serializer implementation and Rails hooks
MIT License
5.33k stars 1.39k forks source link

JSON API adapter's "include" option not translating dasherized keys to underscored #1725

Open chrisdpeters opened 8 years ago

chrisdpeters commented 8 years ago

Expected behavior vs actual behavior

If I run a URL like this against my API with a dasherized key in the include param:

https://api.example.com/themes/1234?include=content-templates

I get no included section in the JSON API payload.

However, I do get the included section if I use the conventional Ruby underscored naming:

https://api.example.com/themes/1234?include=content_templates

The related code within the Rails controller is simple enough:

theme = current_site.themes.find(params[:id])
render json: theme, include: params[:include]

It seems to me like it should be translating the dasherized key to underscored under the hood.

Environment

ActiveModelSerializers Version (commit ref if not on tag): 0.10.0.rc5

Output of ruby -e "puts RUBY_DESCRIPTION": ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-linux]

OS Type & Version: Dockerized Debian 8.4

Integrated application and version (e.g., Rails, Grape, etc): Rails 4.2.6

Backtrace

(e.g., provide any applicable backtraces from your application)

Additonal helpful information

(e.g., Gemfile.lock, configurations, PR containing a failing test, git bisect results)

I started poking around with this commit but felt like I was a tad out of my element. I think what I did with line #21 was appropriate (but now realize I should have been using #tr instead of #gsub). But now I suspect that my change for line #52 may not even make sense if you're starting with a hash instead of a string.

If you can help me clear that up, I'm sure I could clean it up and submit a PR. Let me know.

beauby commented 8 years ago

Closing this as the parsing of include parameters are now delegated to the jsonapi gem. Moreover, the name of the relationships in the include parameter should match those of the relationships in the generated JSON.

chrisdpeters commented 8 years ago

@beauby Would you provide me with an example of how to use the jsonapi gem to tell AMS which serializers to include when it's rendering my queried records?

For example, this is the controller code:

theme = current_site.themes.find(params[:id])
render json: theme, include: params[:include]

What do I do with that 2nd line to get AMS to load the correct association name(s) from params[:include]? Right now, it's passing in content-templates, which matches what the JSONAPI payload defines for the relationship. But AMS seems to be expecting content_templates instead.

chrisdpeters commented 8 years ago

I'll have to give it a shot tomorrow, but I suppose I need to write the equivalent of this? (Perhaps just always parsing in a before_action.)

render json: theme, include: jsonapi::IncludeDirective.new(params[:include])

It just feels incomplete that AMS wouldn't do this for me automatically like it does for everything else in rendering the payload. I've already told it that I want dasherized keys, so it feels like it should be able to handle that along with the include directive passed in to it.

beauby commented 8 years ago

Reopening this, as it is actually a short-coming of AMS, since the key transformation is done automatically under the hood. Will work on a PR in the coming days.