ransack
ransack copied to clipboard
Support nested children in `Arel::Nodes::And`
Fixes #925. Fixes #1240.
This looks great but do we have test coverage?
Nope, also I tried to reproduce the issue and couldn't.
@ciihla thank you for this contribution, is it possible to add a test (that would fail first without your PR) ?
@ciihla thank you for this contribution, is it possible to add a test (that would fail first without your PR) ?
The test is very specific, maybe it has to do something with combination of multitenancy(acts_as_tenant
) + postgres_ext-serializers
+ our specific usage (hierarchy sql condition). but basically our test fails within this:
let!(:column) { create(:column, space: space) }
let!(:column2) { create(:column, space: space) }
let!(:column_value) { create(:column_value, space: space, feature: feature_1, column: column) }
let!(:column_value2) { create(:column_value, space: space, feature: feature_2, column: column2) }
let(:params) { { q: { column_values_column_id_not_eq: column.id, id_in: [feature_1.id, feature_2.id] } } }
it 'returns all data' do
expect(space.features
.select('features.*, case when count(childs.id) > 0 then true else false end as has_children')
.joins('left join features childs on childs.parent_id = features.id')
.group('features.id').ransack(params[:q]).result.to_a.count).to eq(1)
end
error:
NoMethodError:
undefined method `children' for nil:NilClass
# /Users/jan.uhlar/.rvm/gems/ruby-3.0.3@pb-backend/gems/ransack-2.5.0/lib/ransack/adapters/active_record/context.rb:146:in `block in remove_association'
# /Users/jan.uhlar/.rvm/gems/ruby-3.0.3@pb-backend/gems/ransack-2.5.0/lib/ransack/adapters/active_record/context.rb:145:in `delete_if'
# /Users/jan.uhlar/.rvm/gems/ruby-3.0.3@pb-backend/gems/ransack-2.5.0/lib/ransack/adapters/active_record/context.rb:145:in `remove_association'
# /Users/jan.uhlar/.rvm/gems/ruby-3.0.3@pb-backend/gems/ransack-2.5.0/lib/ransack/adapters/active_record/ransack/nodes/condition.rb:10:in `block in arel_predicate'
Sorry I dont have much time to elaborate more :(
Thanks @ciihla, it definitely makes sense to me that a third gem is involved here. If you find time to isolate this to a sample app, it'd be great. Even temporarily removing either of these gems from your application to see if the issue still reproduces and further isolate the culprit would be great.
We encountered another use case that this patch seems to fix: querying a scoped association where the association model also has a default scope (multiple conditions). Here is a standalone test that exposes the error, and a copy with the patch applied that executes without error.
Here is another example reproducing the failure with a different set of conditions.
@shannondoah I just ran into this same issue, have you been running this patch in production?