Check type before sending #value message to predicate

[spaghetticode]

Closes #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]

Interestingly, CI is not running on the PR 🤷

[scarroll32]

Interestingly, CI is not running on the PR 🤷

Needs to be approved for the 1st run.

[scarroll32]

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

[spaghetticode]

@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]

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.

[Copilot]

Pull Request Overview

This PR fixes a bug where Ransack was incorrectly assuming all predicates respond to the #value method, which causes issues with certain Arel objects like Arel::SelectManager. The fix reorders the type checking in the casted_array? method to ensure type safety before method calls.

• Reorders boolean logic in casted_array? method to check type before calling #value
• Adds a new ransacker for article_tags that uses Arel::SelectManager
• Adds test coverage for HABTM associations using the new ransacker

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
lib/ransack/nodes/condition.rb	Fixes the type checking order in casted_array? method
spec/support/schema.rb	Adds new article_tags ransacker that returns Arel::SelectManager
spec/ransack/adapters/active_record/base_spec.rb	Adds test case for HABTM associations functionality

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}