From 8fea98d338c33d3046efc8c7166226f9c4e14403 Mon Sep 17 00:00:00 2001 From: Alexey Vasiliev Date: Sat, 14 Sep 2024 10:22:33 +0300 Subject: [PATCH] Fix inverts search, when have additional conditions in scope (like default_scope) --- lib/ransack/adapters/active_record/context.rb | 35 ++++++++++++++ .../adapters/active_record/context_spec.rb | 48 +++++++++++-------- spec/ransack/search_spec.rb | 2 - 3 files changed, 62 insertions(+), 23 deletions(-) diff --git a/lib/ransack/adapters/active_record/context.rb b/lib/ransack/adapters/active_record/context.rb index e28fd8122..7666ed6cc 100644 --- a/lib/ransack/adapters/active_record/context.rb +++ b/lib/ransack/adapters/active_record/context.rb @@ -165,9 +165,11 @@ def build_correlated_subquery(association) join_constraints = extract_joins(association) join_root = join_constraints.shift correlated_key = extract_correlated_key(join_root) + join_conditions = extract_join_conditions(join_root, correlated_key) subquery = Arel::SelectManager.new(association.base_klass) subquery.from(join_root.left) subquery.project(correlated_key) + subquery.where(join_conditions) if join_conditions join_constraints.each do |j| subquery.join_sources << Arel::Nodes::InnerJoin.new(j.left, j.right) end @@ -209,6 +211,39 @@ def extract_correlated_key(join_root) end end + def extract_join_conditions(join_root, correlated_key) + case join_root + when Arel::Nodes::OuterJoin + # one of join_root.right/join_root.left is expected to be Arel::Nodes::On + if join_root.right.is_a?(Arel::Nodes::On) + join_root.right.expr = extract_join_conditions(join_root.right.expr, correlated_key) + elsif join_root.left.is_a?(Arel::Nodes::On) + join_root.right.expr = extract_join_conditions(join_root.left.expr, correlated_key) + else + raise 'Ransack encountered an unexpected arel structure' + end + when Arel::Nodes::Grouping + join_root.expr = extract_join_conditions(join_root.expr, correlated_key) + when Arel::Nodes::Equality + pk = primary_key + + if (join_root.left == pk && join_root.right == correlated_key) || + (join_root.right == pk && join_root.left == correlated_key) + return nil # skip correlated_key node + end + + join_root + when Arel::Nodes::And + filtered_children = join_root.children.filter_map do |child| + extract_join_conditions(child, correlated_key) + end + + Arel::Nodes::And.new(filtered_children) + else + join_root + end + end + def get_parent_and_attribute_name(str, parent = @base) attr_name = nil diff --git a/spec/ransack/adapters/active_record/context_spec.rb b/spec/ransack/adapters/active_record/context_spec.rb index 893b9be33..d7865a77f 100644 --- a/spec/ransack/adapters/active_record/context_spec.rb +++ b/spec/ransack/adapters/active_record/context_spec.rb @@ -44,39 +44,45 @@ module ActiveRecord search = Search.new(Person, { articles_title_not_eq: 'some_title' }, context: subject) attribute = search.conditions.first.attributes.first constraints = subject.build_correlated_subquery(attribute.parent).constraints - constraint = constraints.first - - expect(constraints.length).to eql 1 - expect(constraint.left.name).to eql 'person_id' - expect(constraint.left.relation.name).to eql 'articles' - expect(constraint.right.name).to eql 'id' - expect(constraint.right.relation.name).to eql 'people' + equality_constraint = constraints.detect { |c| c.is_a?(Arel::Nodes::Equality) } + and_constraints = constraints.detect { |c| c.is_a?(Arel::Nodes::And) } + + expect(constraints.length).to eql 2 + expect(equality_constraint.left.name).to eql 'person_id' + expect(equality_constraint.left.relation.name).to eql 'articles' + expect(equality_constraint.right.name).to eql 'id' + expect(equality_constraint.right.relation.name).to eql 'people' + expect(and_constraints.to_sql).to include('default_scope') end it 'build correlated subquery for Child STI model when predicate is not_eq' do search = Search.new(Person, { story_articles_title_not_eq: 'some_title' }, context: subject) attribute = search.conditions.first.attributes.first constraints = subject.build_correlated_subquery(attribute.parent).constraints - constraint = constraints.first - - expect(constraints.length).to eql 1 - expect(constraint.left.relation.name).to eql 'articles' - expect(constraint.left.name).to eql 'person_id' - expect(constraint.right.relation.name).to eql 'people' - expect(constraint.right.name).to eql 'id' + equality_constraint = constraints.detect { |c| c.is_a?(Arel::Nodes::Equality) } + and_constraints = constraints.detect { |c| c.is_a?(Arel::Nodes::And) } + + expect(constraints.length).to eql 2 + expect(equality_constraint.left.relation.name).to eql 'articles' + expect(equality_constraint.left.name).to eql 'person_id' + expect(equality_constraint.right.relation.name).to eql 'people' + expect(equality_constraint.right.name).to eql 'id' + expect(and_constraints.to_sql).to include('default_scope') end it 'build correlated subquery for Child STI model when predicate is eq' do search = Search.new(Person, { story_articles_title_not_eq: 'some_title' }, context: subject) attribute = search.conditions.first.attributes.first constraints = subject.build_correlated_subquery(attribute.parent).constraints - constraint = constraints.first - - expect(constraints.length).to eql 1 - expect(constraint.left.relation.name).to eql 'articles' - expect(constraint.left.name).to eql 'person_id' - expect(constraint.right.relation.name).to eql 'people' - expect(constraint.right.name).to eql 'id' + equality_constraint = constraints.detect { |c| c.is_a?(Arel::Nodes::Equality) } + and_constraints = constraints.detect { |c| c.is_a?(Arel::Nodes::And) } + + expect(constraints.length).to eql 2 + expect(equality_constraint.left.relation.name).to eql 'articles' + expect(equality_constraint.left.name).to eql 'person_id' + expect(equality_constraint.right.relation.name).to eql 'people' + expect(equality_constraint.right.name).to eql 'id' + expect(and_constraints.to_sql).to include('default_scope') end it 'build correlated subquery for multiple conditions (default scope)' do diff --git a/spec/ransack/search_spec.rb b/spec/ransack/search_spec.rb index 6426a20ad..d039b2ee3 100644 --- a/spec/ransack/search_spec.rb +++ b/spec/ransack/search_spec.rb @@ -147,7 +147,6 @@ module Ransack expect(s.result.to_sql).to include 'published' end - # The failure/oversight in Ransack::Nodes::Condition#arel_predicate or deeper is beyond my understanding of the structures it 'preserves (inverts) default scope and conditions for negative subqueries' do # the positive case (published_articles_title_eq) is # SELECT "people".* FROM "people" @@ -178,7 +177,6 @@ module Ransack # AND "articles"."title" = 'Test' AND "articles"."published" = 't' AND ('default_scope' = 'default_scope') # ) ORDER BY "people"."id" DESC - pending("spec should pass, but I do not know how/where to fix lib code") s = Search.new(Person, published_articles_title_not_eq: 'Test') expect(s.result.to_sql).to include 'default_scope' expect(s.result.to_sql).to include 'published'