-
Notifications
You must be signed in to change notification settings - Fork 178
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
Fix #where for hashes with string keys #292
Fix #where for hashes with string keys #292
Conversation
Signed-off-by: Stanislav Dobrovolschii <[email protected]>
Signed-off-by: Stanislav Dobrovolschii <[email protected]>
Thank you for the bug request. I'm thinking we should move the conversion lower in the stack. Think moving the |
Signed-off-by: Stanislav Dobrovolschii <[email protected]>
@kbrock Cool idea, thanks for suggesting it, it definitely makes things consistent. Conversions are inside the like: def _read_attribute(key)
attributes[key.to_s]
end
def read_attribute(key)
attributes[key.to_sym]
end But didn't find a reason to keep them both, as |
@usernam3 thanks for the change. vm = ActiveRecordModel.first
vm.read_attribute(:name)
# => "abc"
vm.read_attribute("name")
# => "abc"
vm._read_attribute(:name)
# => nil
vm._read_attribute("name")
# => "abc" Yes, technically |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good
- Support `has_many :through` associations active-hash#296 @flavorjones - Rails 7.1 support active-hash#291 @y-yagi - Rails 7.1: fix sqlite3 issue active-hash#303 @flavorjones - Rails 7.1.3: add missing `has_query_constraints?` active-hash#300 @flavorjones - `Array#pluck` supports methods active-hash#299 @iberianpig - Prefer `safe_constantize` over `constantize` active-hash#297 @flavorjones - Treat `nil` and `blank?` as different values active-hash#295 @kbrock - Fix `#where` for string keys active-hash#292 @usernam3
- Ruby 3.3 support active-hash#298 @m-nakamura145 - Support `has_many :through` associations active-hash#296 @flavorjones - Rails 7.1 support active-hash#291 @y-yagi - Rails 7.1: fix sqlite3 issue active-hash#303 @flavorjones - Rails 7.1.3: add missing `has_query_constraints?` active-hash#300 @flavorjones - `Array#pluck` supports methods active-hash#299 @iberianpig - Prefer `safe_constantize` over `constantize` active-hash#297 @flavorjones - Treat `nil` and `blank?` as different values active-hash#295 @kbrock - Fix `#where` for string keys active-hash#292 @usernam3
✌️ Hey
active_hash
maintainers ✌️ Thank you for such great gem that makes development with ruby even more productive 🙇I noticed that the latest version (
v3.2.1
) has undocumented side-effect which affect the behaviour of one of the core features - search. To be specific the#where
for hash with string keys stopped working (example:ActiveHashModel.where("id" => 1)
).It seems that transition from
Object#public_send
toActiveHash::Base#read_attribute
(in #281) isn't backward-compatible as those methods have a bit different expectation towards arguments. Object#public_send - converts arguments to symbols,read_attribute
- not (whileattributes
internally are stored with symbolized keys).Small demo