disposable icon indicating copy to clipboard operation
disposable copied to clipboard

Collection#find_by does not filter on all options passed

Open ibrahima opened this issue 2 years ago • 1 comments

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!

ibrahima avatar Mar 15 '22 07:03 ibrahima

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

yogeshjain999 avatar Mar 15 '22 10:03 yogeshjain999