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

Ignore belongs_to relations when parsing params #284

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kindrowboat
Copy link
Contributor

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.

@balvig
Copy link

balvig commented Sep 16, 2014

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...?

@dturnerTS
Copy link
Contributor

@aauthor 👍
@balvig Take a look at #261 I think that will explain the problem.

@kindrowboat
Copy link
Contributor Author

@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.

@balvig
Copy link

balvig commented Sep 17, 2014

@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! 😓

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

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

balvig added a commit to cooksnaps/her that referenced this pull request Sep 17, 2014
…params

Ignore belongs_to relations when parsing params
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.

None yet

3 participants