ransack icon indicating copy to clipboard operation
ransack copied to clipboard

Support nested children in `Arel::Nodes::And`

Open ciihla opened this issue 2 years ago • 8 comments

Fixes #925. Fixes #1240.

ciihla avatar Mar 08 '22 09:03 ciihla

This looks great but do we have test coverage?

scarroll32 avatar Mar 10 '22 15:03 scarroll32

Nope, also I tried to reproduce the issue and couldn't.

deivid-rodriguez avatar Mar 10 '22 15:03 deivid-rodriguez

@ciihla thank you for this contribution, is it possible to add a test (that would fail first without your PR) ?

scarroll32 avatar Mar 13 '22 16:03 scarroll32

@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 :(

ciihla avatar Mar 14 '22 08:03 ciihla

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.

deivid-rodriguez avatar Mar 14 '22 09:03 deivid-rodriguez

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 avatar Jan 19 '24 22:01 shannondoah

@shannondoah I just ran into this same issue, have you been running this patch in production?

tappleby avatar May 16 '24 18:05 tappleby