search

activerecord-hackery / ransack

Object-based searching.
https://activerecord-hackery.github.io/ransack/
MIT License
5.66k stars 799 forks source link

Check type before sending `#value` message to predicate #1468

Open spaghetticode opened 9 months ago

spaghetticode commented 9 months ago

Closes https://github.com/activerecord-hackery/ransack/issues/1467

The previous implementation was giving for granted that every predicate respond to #value, which doesn't seem to be the case at least when having a Arel::SelectManager.

by simply inverting the terms of the existing AND check we can fix the issue without introducing unknown side effects.

spaghetticode commented 9 months ago

Interestingly, CI is not running on the PR 🤷

scarroll32 commented 9 months ago

Interestingly, CI is not running on the PR 🤷

Needs to be approved for the 1st run.

scarroll32 commented 9 months ago

@spaghetticode could you please add a test for this change?

spaghetticode commented 8 months ago

@scarroll32 I've just added a test that fails without the change.

The method I modified is private, so it needs to be tested indirectly. I followed the existing examples, but I had to add a new ransacker to the Person model, I hope it makes sense. Please let me know if it works for you, thanks!

MadelineCollier commented 1 month ago

I am also having this issue, will there be any movement on this PR @scarroll32 ?

NoMethodError:
       undefined method `value' for #<Arel::SelectManager:0x000000010b0fb3b8 @ast=#<Arel::Nodes::SelectStatement:0x000000010b0fb390 @cores.........

               predicate.value.is_a?(Array) && predicate.is_a?(Arel::Nodes::Casted)
                        ^^^^^^
     # ./spec/models/spree/product_spec.rb:667:in `block (3 levels) in <top (required)>'

Finished in 28 minutes 11 seconds (files took 4.77 seconds to load)
1 example, 1 failure

For now I have locked my gemfile to ransack 4.1.1 and specs pass on that version.