her icon indicating copy to clipboard operation
her copied to clipboard

Ignore belongs_to relations when parsing params

Open kindrowboat opened this issue 11 years ago • 4 comments

By convention, belongs_to relations are not parsed into the params; however in some contexts, the belongs_to :data_key might be set in attributes. The #to_params method does not attempt to parse these attributes, and so, one ends up with a JSON body containing:

{
  ...
  "some_belongs_to_key":"#<SomeBelongsToKlass:0x00000005dfd388>",
  ...
}

This is extra noise through the pipe at best, and in our use, cause the resource server to complain and 400/500.

This fix, strips out any belongs_to data_key in the attributes when parsing params.

kindrowboat avatar Sep 15 '14 20:09 kindrowboat

Great to see this! Has been bugging me as well :) For me the belongs_to classes still seem to slip through in nested associations even when using this fix...ie

class Recipe
  include Her::Model
  has_many :steps
  accepts_nested_attributes_for :steps
end

class Step
  include Her::Model
  belongs_to :recipe
end

results in something like

{
  "recipe"=>
  {
    "steps"=> [{"id"=>"1","recipe"=>"#<Recipe:0x007fc2e5deebd0>"}]
  }
}

Haven't found the cause myself yet, maybe something deeper going on...?

balvig avatar Sep 16 '14 10:09 balvig

@aauthor :+1: @balvig Take a look at https://github.com/remiprev/her/pull/261 I think that will explain the problem.

dturnerTS avatar Sep 16 '14 16:09 dturnerTS

@dturnerTS, unless I misunderstood, this is not the same situation as #261, because Step does define a belongs_to relationship. @balvig if you'd like to create a failing spec for this case in a branch and share it, I'd be happy look into it more.

kindrowboat avatar Sep 16 '14 18:09 kindrowboat

@aauthor, @dturnerTS Sorry about that, while writing the failing spec I actually realised the problem was in fact that Step doesn't define a belongs_to relationship...I somehow added that while formatting the example for github! :sweat:

The problem is indeed #261 ! Thanks for pointing me in the right direction :grin:

For reference, here is the failing spec showing that particular problem: https://github.com/balvig/her/commit/be7ffd77e7585cb8ce018b836a9465f9c73af382

balvig avatar Sep 17 '14 11:09 balvig