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

Collection#find_by does not filter on all options passed #88

Open
ibrahima opened this issue Mar 15, 2022 · 1 comment
Open

Collection#find_by does not filter on all options passed #88

ibrahima opened this issue Mar 15, 2022 · 1 comment

Comments

@ibrahima
Copy link

ibrahima commented Mar 15, 2022

Hi! I don't know if this is intended, but at the very least it might be misleading. It turns out that if you pass a hash of options to Collection#find_by, it does not actually filter the returned records by all the options, but rather only looks at the first key/value pair of options. This led to a (potential and possibly unimportant) bug in some code someone on my team had written, because the find_by method looks like it should behave like ActiveRecord's method, but it does not.

https://github.com/apotonick/disposable/blob/610308800376510344cdba59174fb32a59d5b095/lib/disposable/twin/collection.rb#L17..L20

It might be good to at least document this behavior, if it's not desirable to change it right now. Thanks!

@yogeshjain999
Copy link
Collaborator

Hi @ibrahima, thanks for reporting this issue.

I think this is a valid point and find_by implementation should be changed to consider all given options, something like below.

def find_by(options)
  find do |item|
    options.collect do |field, value|                                                                                       
      item.send(field).to_s == value.to_s
    end.all?(true)
  end 
end

cc @apotonick

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

No branches or pull requests

2 participants