Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Initializer can take stringified hashes #9

Merged
merged 1 commit into from
Mar 18, 2017

Conversation

tatey
Copy link
Contributor

@tatey tatey commented Mar 17, 2017

This patch makes ShallowAttributes work out of the box with ActionController::StrongParameters. The motivations for this patch are:

  1. ShallowAttributes expects to work with symbolized hashes, but parameters in Rails are stringified hashes.
  2. The initializer was mutating the hash. When I passed it to another object, it was missing key-value pairs. This tripped me up for a good 10 minutes.

We were working around 2 by calling params.to_h.symbolize_keys before passing it to the initializer. This made the API feel a little rough and gave me the motivation to write this patch.

You'll notice I used #each_pair over #each_with_object or #reduce. This is the only public method that is available on both Hash and ActionController::StrongParameters which lets us symbolize the hash.

I'm aware you may be opposed to the patch because it creates a new object and uses #each_pair. If you are opposed then I am open to exploring other options to make a great out of the box Rails experience.

Either way let me know. All feedback welcome.

Cheers,
Tate

@coveralls
Copy link

coveralls commented Mar 17, 2017

Coverage Status

Coverage increased (+0.02%) to 99.098% when pulling a9e28c7 on tatey:stringified-hash into 3740a54 on davydovanton:master.

Copy link
Owner

@davydovanton davydovanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good for me 👍

@davydovanton
Copy link
Owner

Thanks for contribution!

@davydovanton davydovanton merged commit 3c9b1a3 into davydovanton:master Mar 18, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants