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.

Sign up for GitHub

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

Jump to bottom

Treat all Hash-like input as a Hash #181

Merged
merged 1 commit into from
Aug 17, 2024
Merged

Treat all Hash-like input as a Hash #181

merged 1 commit into from
Aug 17, 2024

Conversation

ql
Copy link
Contributor

@ql ql commented Aug 15, 2024 •
edited
Loading

issue #120 - first try an easy way

@ql ql changed the title Treat all input which responds to to_h as Hash Treat all Hash-like input as a Hash Aug 15, 2024
@DmitryTsepelev DmitryTsepelev marked this pull request as ready for review August 17, 2024 14:13
@DmitryTsepelev DmitryTsepelev merged commit 27d4a62 into DmitryTsepelev:master Aug 17, 2024
16 checks passed
@Morozzzko
Copy link

oh my goodness I just found that in the changelog and THANK YOU FOR THAT

I can now drop a lot of unnecessary .to_h's from my code

THANK YOU

@23tux
Copy link
Contributor

23tux commented Jan 9, 2025 •
edited
Loading

I just updated my code and found out, that this breaks something. When you assign an instance of a store model, and this instance implements it's own to_h method, this instance get's (needlessly IMO) converted to a Hash, because it responds to #to_h, but it already is an instance of store model.

It seems that in one.rb this case is handled:

store_model/lib/store_model/types/one.rb

Lines 36 to 37 in 126a98c

elsif value.is_a?(@model_klass)
value

But in one_polymorphic.rb it is not handled

store_model/lib/store_model/types/one_polymorphic.rb

Lines 36 to 43 in 126a98c

if value.is_a?(String)
decode_and_initialize(value)
elsif value.respond_to?(:to_h) # Hash itself included
extract_model_klass(value).new(value.to_h)
else
raise_cast_error(value) unless value.class.ancestors.include?(StoreModel::Model)
value

I'm gonna try for a PR

@23tux 23tux mentioned this pull request Jan 9, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ql @Morozzzko @23tux @DmitryTsepelev