Fix wrong table alias when using nested join

activerecord-hackery · Apr 15, 2019 · 0d736a1 · 0d736a1
1 parent dbd3eab
commit 0d736a1
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+* Fix wrong table alias when using nested join. for ActiveRecord >= 5.2
+  PR [374](https://github.com/activerecord-hackery/ransack/pull/374)
+
+  *hiichan*
+
 ## Version 2.1.1 - 2018-12-05
 
 * Add `arabic` translation

diff --git a/lib/ransack/adapters/active_record/context.rb b/lib/ransack/adapters/active_record/context.rb
@@ -312,15 +312,15 @@ def build_association(name, parent = @base, klass = nil)
             alias_tracker = ::ActiveRecord::Associations::AliasTracker.create(self.klass.connection, parent.table.name, [])
             jd = Polyamorous::JoinDependency.new(
               parent.base_klass,
-              parent.base_klass.arel_table,
+              parent.table,
               Polyamorous::Join.new(name, @join_type, klass),
               alias_tracker
             )
             found_association = jd.instance_variable_get(:@join_root).children.last
           else
             jd = Polyamorous::JoinDependency.new(
               parent.base_klass,
-              parent.base_klass.arel_table,
+              parent.table,
               Polyamorous::Join.new(name, @join_type, klass),
             )
             found_association = jd.instance_variable_get(:@join_root).children.last

diff --git a/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb b/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb
@@ -47,7 +47,7 @@ def join_constraints(outer_joins, join_type)
       }
 
       joins.concat outer_joins.flat_map { |oj|
-        if join_root.match? oj.join_root
+        if join_root.match?(oj.join_root) && join_root.table.name == oj.join_root.table.name
           walk(join_root, oj.join_root)
         else
           oj.join_root.children.flat_map { |child|

diff --git a/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb b/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb
@@ -29,6 +29,22 @@ def build(associations, base_klass)
       end
     end
 
+    def join_constraints(joins_to_add, join_type, alias_tracker)
+      @alias_tracker = alias_tracker
+
+      construct_tables!(join_root)
+      joins = make_join_constraints(join_root, join_type)
+
+      joins.concat joins_to_add.flat_map { |oj|
+        construct_tables!(oj.join_root)
+        if join_root.match?(oj.join_root) && join_root.table.name == oj.join_root.table.name
+          walk join_root, oj.join_root
+        else
+          make_join_constraints(oj.join_root, join_type)
+        end
+      }
+    end
+
     private
       def make_constraints(parent, child, join_type = Arel::Nodes::OuterJoin)
         foreign_table = parent.table

diff --git a/spec/ransack/search_spec.rb b/spec/ransack/search_spec.rb
@@ -227,13 +227,37 @@ module Ransack
           children_people_name_field} = 'Ernie'/
       end
 
+      it 'use appropriate table alias' do
+        skip "Make this spec pass for Rails <5.2" if ::ActiveRecord::VERSION::STRING < '5.2.0'
+        s = Search.new(Person, {
+          name_eq: "person_name_query",
+          articles_title_eq: "person_article_title_query",
+          parent_name_eq: "parent_name_query",
+          parent_articles_title_eq: 'parents_article_title_query'
+        }).result
+        real_query = remove_quotes_and_backticks(s.to_sql)
+
+        expect(real_query)
+          .to include "LEFT OUTER JOIN articles ON articles.person_id = people.id"
+        expect(real_query)
+          .to include "LEFT OUTER JOIN articles articles_people ON articles_people.person_id = parents_people.id"
+        expect(real_query)
+          .to include "people.name = 'person_name_query'"
+        expect(real_query)
+          .to include "articles.title = 'person_article_title_query'"
+        expect(real_query)
+          .to include "parents_people.name = 'parent_name_query'"
+        expect(real_query)
+          .to include "articles_people.title = 'parents_article_title_query'"
+      end
+
       # FIXME: Make this spec pass for Rails 4.1 / 4.2 / 5.0 and not just 4.0 by
       # commenting out lines 221 and 242 to run the test. Addresses issue #374.
       # https://github.com/activerecord-hackery/ransack/issues/374
       #
       it 'evaluates conditions for multiple `belongs_to` associations to the
       same table contextually' do
-        skip "Make this spec pass for Rails >5.0"
+        skip "Make this spec pass for Rails <5.2" if ::ActiveRecord::VERSION::STRING < '5.2.0'
         s = Search.new(
           Recommendation,
           person_name_eq: 'Ernie',
@@ -248,7 +272,7 @@ module Ransack
             ON target_people_recommendations.id = recommendations.target_person_id
           LEFT OUTER JOIN people parents_people
             ON parents_people.id = target_people_recommendations.parent_id
-          WHERE ((people.name = 'Ernie' AND parents_people.name = 'Test'))
+          WHERE (people.name = 'Ernie' AND parents_people.name = 'Test')
         SQL
         .squish
         expect(real_query).to eq expected_query