Check type before sending #value message to predicate
#1468
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spaghetticode
added a commit
to peterberkenbosch/solidus
that referenced
this pull request
Dec 15, 2023
Issue: activerecord-hackery/ransack#1467 PR: activerecord-hackery/ransack#1468 Hopefully the PR will be merged soon so we can remove this patch.
|
Interestingly, CI is not running on the PR 🤷 |
elia
pushed a commit
to peterberkenbosch/solidus
that referenced
this pull request
Dec 20, 2023
Issue: activerecord-hackery/ransack#1467 PR: activerecord-hackery/ransack#1468 Hopefully the PR will be merged soon so we can remove this patch.
elia
pushed a commit
to peterberkenbosch/solidus
that referenced
this pull request
Dec 20, 2023
Issue: activerecord-hackery/ransack#1467 PR: activerecord-hackery/ransack#1468 Hopefully the PR will be merged soon so we can remove this patch.
Needs to be approved for the 1st run. |
|
@spaghetticode could you please add a test for this change? |
The previous implementation was giving for granted that every predicate respond to `#value`, but that 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. In order to test the change, a new ransacker has been added to the Person model.
spaghetticode
force-pushed
the
fix-casted_array-method
branch
from
e72e954
to
e7c8250
Compare
|
@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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 aArel::SelectManager.by simply inverting the terms of the existing AND check we can fix the issue without introducing unknown side effects.