Skip to content

Conversation

@vinc
Copy link

@vinc vinc commented May 31, 2017

In ruby we use underscore for symbols, but dash are prefered for keys with JSON API.

By default, Active Model Serializers transforms underscore into dash when JSON API is used, otherwise it leaves the keys unaltered: https://github.com/rails-api/active_model_serializers/blob/v0.10.6/docs/general/key_transforms.md

This PR is a request for comments regarding the transformation of keys during serialization and deserialization.

If we have a JSON API exposing a user resource with a first-name attribute, this allow us to access it by user.first_name in Rails if we add key_transform :dash to the model.

So far this PR is quite restricted:

  • Only JSON API
  • Only :dash option
  • No recursion on the keys of nested attributes

key_transform could also be applied to regular APIs, otherwise it should maybe be renommed json_api_key_transform?

@vinc
Copy link
Author

vinc commented May 31, 2017

Oups, some tests are failing with older versions of ruby, I'll rewrite the code to avoid using Hash#transform_keys.

@edtjones
Copy link
Collaborator

Thanks for this @vinc - can we deprecate but not remove :underscore ?

@zacharywelch
Copy link
Collaborator

zacharywelch commented Jun 1, 2017

See this sorta thing being useful in a variety of ways 😎 As you mentioned apis may send json properties as camelCase, snake_case, PascalCase, or dash-cased. Since we only ever want snake_case once it reaches our ruby code what are your thoughts on handling this in a separate Faraday middleware instead? Or do you think the greater need is to transform keys on a per-model basis like parse_root_in_json?

@vinc
Copy link
Author

vinc commented Jun 1, 2017

@edtjones by keeping but deprecating :underscore, do you mean keeping the current behaviour of not transforming the attribute keys but making it deprecated? I think the current behaviour should stay the default, it's the least surprising, and the transformations should be done only if key_transform :dash is explicitely added to the model. I coded it that way in this PR, the transformation is opt-in.

@zacharywelch I can indeed see this code as a separate middleware, it'd keep the main code lean, especially if I add all the transformations and make it recursive for nested attributes! But would it be possible to use the same middleware for doing the inverse operation when we serialize the data to be transmitted to the API?

@edtjones
Copy link
Collaborator

edtjones commented Jun 2, 2017

@vinc yeah - that's what I meant. That's fine. I only skimmed the PR on a mobile so I didn't clock that you were introducing an optional method call :-)

Re middleware - that would be the best way, I agree with @zacharywelch - and you can do transformations in both directions. Various examples here https://github.com/lostisland/faraday_middleware

@edtjones
Copy link
Collaborator

edtjones commented Jun 2, 2017

(@vinc just to say I won't merge this for now, pending refactoring into a middleware - hope ok)

@zacharywelch
Copy link
Collaborator

zacharywelch commented Jun 3, 2017

@vinc another idea would be to package your own faraday middleware so it's available to anyone using Faraday. There's a bunch of great examples on github

@vinc
Copy link
Author

vinc commented Jun 8, 2017

Hi @edtjones and @zacharywelch, thanks for your suggestions! I'll look into writting a middleware to handle this.

@zacharywelch
Copy link
Collaborator

I'm going to close this pull request since you'll be creating a separate middleware. Good luck and feel free to provide a link here once it's complete 😺 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