active_model_serializers icon indicating copy to clipboard operation
active_model_serializers copied to clipboard

Underscore incoming params before filtering during Deserialization

Open aliaporci opened this issue 9 years ago • 8 comments

Purpose

TL;DR Ensure general use case of using snake case for permitted params i.e. jsonapi_parse(params, only: [:first_name]) even when deserializing dasherized or camel cased params.

Long version: In Rails-land, we generally default to snake case when describing variables, symbols, etc. Right now, if we have incoming params with kebab or camel cased fields, we have to match the case when deserializing in the controller.

E.g.

If we send dasherized parameters such as

{
  'data' => {
    'type' => 'users',
    'id' => 'blah',
    'attributes' => {
      'first-name' => 'Freddie',
      'last-name' => 'Mercury'
    }
  }
}

to a UserController containing this (fairly standard) method

def user_params
  ActiveModelSerializers::Deserialization.jsonapi_parse(params,
    only: [:first_name, :last_name])
end

the result will be empty user_params as both 'first-name' and 'last-name' are filtered out by the only option before the params are ever parsed.

The current (ugly and unsatisfying) workaround is to list out the kebabed or camel cased parameters as strings (i.e. only: ['first-name', 'last-name'] or to convince whoever is building a front end for your API to patch their Ember adapter or whatever to match whatever you're using (snake case FTW).

I'd much rather be able to use snake case for permitted params consistently across all of my Rails apps and not worry about remembering how one particular front end is configured to send params over (and not worry about having to change the casing of all of my permitted params in all of my controllers in the event the status quo changes!).

Changes

Call existing underscore function on incoming parameter field keys when filtering against options passed into the parse method

Caveats

Might be overkill.

Related GitHub issues

Possible made obsolete by #1927 if there's ever any movement on it.

Additional helpful information

Here's the blog post that let me know that I and the developers I talked to about this weren't alone: http://kyleshevlin.com/all-the-steps-needed-to-get-active-model-serializers-to-work-with-jsonapiadapter-and-jsonapiserializer-in-ember/

aliaporci avatar Nov 22 '16 04:11 aliaporci

@aliaporci, thanks for your PR! By analyzing the history of the files in this pull request, we identified @domitian, @lawitschka and @remear to be potential reviewers.

mention-bot avatar Nov 22 '16 04:11 mention-bot

@aliaporci Thanks so much for the writeup and link!

I'm kind of surprised this issue hasn't come up before. This must have been a problem for all sorts of people. (I've generally been of the opinion that Ember does a better job at the adapter layer of handling Rails snake case than Rails does in producing and consuming kebab-case to avoid these issues :))

bf4 avatar Nov 27 '16 02:11 bf4

we'll need a before/after benchmark of this too. Do you feel comfortable running bin/bench locally on master and on your branch?

bf4 avatar Nov 27 '16 02:11 bf4

As an Ember developer, this needs to be implemented!

axsuul avatar Mar 16 '17 07:03 axsuul

Needs tests

bf4 avatar Mar 16 '17 14:03 bf4

using this piece of codes for now.

props = ActiveModelSerializers::Deserialization.jsonapi_parse params
props = ActionController::Parameters.new(**props)
props.permit :attr1, :attr2

Hope the pull request #1928 get merged soon.

hardywu avatar Apr 13 '17 18:04 hardywu

any news?

hardywu avatar Aug 21 '19 05:08 hardywu

@hardywu I guess not :(

bf4 avatar Aug 21 '19 15:08 bf4