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

MONGOID-5328: [WIP] Hash and localized fields assignment should work with ActionController::Parameters objects #5237

Conversation

johnnyshields
Copy link
Contributor

@johnnyshields johnnyshields commented Apr 20, 2022

TODO

  • Tests

This PR makes Object, Hash and :localized field assignment work with ActionController::Parameters objects, which are Hash-like but (as of Rails 5.x) do not inherit from Hash class. It is necessary to call #to_h on the ActionController::Parametersobject, and Hash.evolve looks like the right place to do it.

As a side note, to err on the side of security, I think it is better to call #to_h rather than #to_unsafe_h on the ActionController::Parameters, which will filter out any unpermitted parameters. I need to test how this works in various scenarios (#permit!) etc.

…er parameters. It is necessary to call `#to_h` on the ActiveController::Parameters object, and Hash.evolve looks like the right place to do it.

As a side note, to err on the side of security, I think it is better to call `#to_h` rather than `#to_unsafe_h` on the ActiveController::Parameters, which will filter out any unpermitted parameters. I need to test how this works in various scenarios (#permit!) etc.
@johnnyshields johnnyshields changed the title WIP: Hash and localized fields assignment should work with ActionController::Parameters objects MONGOID-5308: [WIP] Hash and localized fields assignment should work with ActionController::Parameters objects Apr 20, 2022
@johnnyshields johnnyshields changed the title MONGOID-5308: [WIP] Hash and localized fields assignment should work with ActionController::Parameters objects MONGOID-5328: [WIP] Hash and localized fields assignment should work with ActionController::Parameters objects Apr 24, 2022
@johnnyshields johnnyshields marked this pull request as draft April 25, 2022 14:59
@Neilshweky
Copy link
Contributor

@johnnyshields how do you feel about reworking this PR to take into account all of the new code that went into the mongoization epic?

@johnnyshields
Copy link
Contributor Author

@Neilshweky, sure, will be glad to do it.

@johnnyshields
Copy link
Contributor Author

(FYI will wait until dust settles on the mongoize/evolve refactoring before handling)

@Neilshweky
Copy link
Contributor

Is there an update on this?

@johnnyshields
Copy link
Contributor Author

johnnyshields commented Jul 29, 2022

@Neilshweky no update unfortunately, I've ended up not needing this for my app (I think we just called .to_h everywhere as a workaround) so I may not look at it for awhile. If you'd like to pick it up by all means please do so 🙇‍♂️

@Neilshweky Neilshweky closed this Jul 29, 2022
@Neilshweky
Copy link
Contributor

I've opened #5417 to continue this work

@johnnyshields
Copy link
Contributor Author

Thank you! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants