Merge pull request #1004 from gregmolnar/fix-polymorphic-joins

fix polymorphic joins with rails > 5.2

.gitignore

@@ -4,3 +4,4 @@ Gemfile.lock
 pkg/*
 coverage/*
 .DS_Store
+.byebug_history

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

Gemfile

@@ -3,7 +3,7 @@ gemspec
 
 gem 'rake'
 
-rails = ENV['RAILS'] || '5-0-stable'
+rails = ENV['RAILS'] || '5-2-stable'
 
 gem 'pry'
 
@@ -22,23 +22,15 @@ when /^v/ # A tagged version
     gem 'activerecord', require: false
     gem 'actionpack'
   end
-  if rails >= 'v5.2.0'
-    gem 'mysql2', '~> 0.4.4'
-  end
 else
   git 'git://github.com/rails/rails.git', :branch => rails do
     gem 'activesupport'
     gem 'activemodel'
     gem 'activerecord', require: false
     gem 'actionpack'
   end
-  if rails == '3-0-stable'
-    gem 'mysql2', '< 0.3'
-  end
-  if rails == '5-2-stable'
-    gem 'mysql2', '~> 0.4.4'
-  end
 end
+gem 'mysql2', '~> 0.4.4'
 
 group :test do
   # TestUnit was removed from Ruby 2.2 but still needed for testing Rails 3.x.

lib/ransack/adapters/active_record/context.rb

@@ -312,21 +312,20 @@ 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
           end
 
-
           @associations_pot[found_association] = parent
 
           # TODO maybe we dont need to push associations here, we could loop
@@ -337,14 +336,11 @@ def build_association(name, parent = @base, klass = nil)
           if ::ActiveRecord::VERSION::STRING > Constants::RAILS_5_2_0
             @join_dependency.send(:construct_tables!, jd.instance_variable_get(:@join_root))
           else
-            @join_dependency.send(
-              :construct_tables!, jd.instance_variable_get(:@join_root), found_association
-            )
+            @join_dependency.send(:construct_tables!, jd.instance_variable_get(:@join_root), found_association)
           end
 
           # Leverage the stashed association functionality in AR
           @object = @object.joins(jd)
-
           found_association
         end
 

polyamorous/lib/polyamorous.rb

@@ -19,6 +19,11 @@ module Polyamorous
     require "polyamorous/activerecord_#{ar_version}_ruby_2/#{file}"
   end
 
+  if ar_version >= "5.2.0"
+    require "polyamorous/activerecord_#{ar_version}_ruby_2/reflection.rb"
+    ::ActiveRecord::Reflection::AbstractReflection.send(:prepend, Polyamorous::ReflectionExtensions)
+  end
+
   Polyamorous::JoinDependency.send(:prepend, Polyamorous::JoinDependencyExtensions)
   Polyamorous::JoinDependency.singleton_class.send(:prepend, Polyamorous::JoinDependencyExtensions::ClassMethods)
   Polyamorous::JoinAssociation.send(:prepend, Polyamorous::JoinAssociationExtensions)

polyamorous/lib/polyamorous/activerecord_5.1_ruby_2/join_association.rb

@@ -7,8 +7,7 @@ def self.prepended(base)
       base.class_eval { attr_reader :join_type }
     end
 
-    def initialize(reflection, children, polymorphic_class = nil,
-                   join_type = Arel::Nodes::InnerJoin)
+    def initialize(reflection, children, polymorphic_class = nil, join_type = Arel::Nodes::InnerJoin)
       @join_type = join_type
       if polymorphic_class && ::ActiveRecord::Base > polymorphic_class
         swapping_reflection_klass(reflection, polymorphic_class) do |reflection|

polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_association.rb

@@ -7,8 +7,7 @@ def self.prepended(base)
       base.class_eval { attr_reader :join_type }
     end
 
-    def initialize(reflection, children, alias_tracker, polymorphic_class = nil,
-      join_type = Arel::Nodes::InnerJoin)
+    def initialize(reflection, children, alias_tracker, polymorphic_class = nil, join_type = Arel::Nodes::InnerJoin)
       @join_type = join_type
       if polymorphic_class && ::ActiveRecord::Base > polymorphic_class
         swapping_reflection_klass(reflection, polymorphic_class) do |reflection|
@@ -23,7 +22,7 @@ def initialize(reflection, children, alias_tracker, polymorphic_class = nil,
     def build_constraint(klass, table, key, foreign_table, foreign_key)
       if reflection.polymorphic?
         super(klass, table, key, foreign_table, foreign_key)
-        .and(foreign_table[reflection.foreign_type].eq(reflection.klass.name))
+          .and(foreign_table[reflection.foreign_type].eq(reflection.klass.name))
       else
         super(klass, table, key, foreign_table, foreign_key)
       end

polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb

@@ -38,7 +38,6 @@ def build(associations, base_klass)
     # passing an additional argument, `join_type`, to #join_constraints.
     #
     def join_constraints(outer_joins, join_type)
-      @alias_tracker = alias_tracker
       joins = join_root.children.flat_map { |child|
         if join_type == Arel::Nodes::OuterJoin
           make_polyamorous_left_outer_joins join_root, child
@@ -48,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|

polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/reflection.rb

@@ -0,0 +1,12 @@
+module Polyamorous
+  module ReflectionExtensions
+    def build_join_constraint(table, foreign_table)
+      if polymorphic?
+        super(table, foreign_table)
+        .and(foreign_table[foreign_type].eq(klass.name))
+      else
+        super(table, foreign_table)
+      end
+    end
+  end
+end

polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_association.rb

@@ -18,14 +18,5 @@ def initialize(reflection, children, polymorphic_class = nil, join_type = Arel::
         super(reflection, children)
       end
     end
-
-    def build_constraint(klass, table, key, foreign_table, foreign_key)
-      if reflection.polymorphic?
-        super(klass, table, key, foreign_table, foreign_key)
-        .and(foreign_table[reflection.foreign_type].eq(reflection.klass.name))
-      else
-        super(klass, table, key, foreign_table, foreign_key)
-      end
-    end
   end
 end

polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb

@@ -3,7 +3,6 @@
 module Polyamorous
   module JoinDependencyExtensions
     # Replaces ActiveRecord::Associations::JoinDependency#build
-    #
     def build(associations, base_klass)
       associations.map do |name, right|
         if name.is_a? Join
@@ -30,6 +29,31 @@ 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
+        foreign_klass = parent.base_klass
+        join_type = child.join_type || join_type if join_type == Arel::Nodes::InnerJoin
+        joins = child.join_constraints(foreign_table, foreign_klass, join_type, alias_tracker)
+        joins.concat child.children.flat_map { |c| make_constraints(child, c, join_type) }
+      end
+
     module ClassMethods
       # Prepended before ActiveRecord::Associations::JoinDependency#walk_tree
       #

polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/reflection.rb

@@ -0,0 +1,2 @@
+# active_record_5.2.1_ruby_2/reflection.rb
+require 'polyamorous/activerecord_5.2.0_ruby_2/reflection'