Merge pull request #10164 from neerajdotname/3002-final

While merging relations preserve context for joins

activerecord/CHANGELOG.md

@@ -1,5 +1,37 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Preserve context while merging relations with join information.
+
+        class Comment < ActiveRecord::Base
+          belongs_to :post
+        end
+
+        class Author < ActiveRecord::Base
+          has_many :posts
+        end
+
+        class Post < ActiveRecord::Base
+          belongs_to :author
+          has_many :comments
+        end
+
+    `Comment.joins(:post).merge(Post.joins(:author).merge(Author.where(:name => "Joe Blogs"))).all`
+    would fail with
+    `ActiveRecord::ConfigurationError: Association named 'author' was not found on Comment`.
+
+    It is failing because `all` is being called on relation which looks like this after all
+    the merging: `{:joins=>[:post, :author], :where=>[#<Arel::Nodes::Equality: ....}`. In this
+    relation all the context that `Post` was joined with `Author` is lost and hence the error
+    that `author` was not found on `Comment`.
+
+    Ths solution is to build JoinAssociation when two relations with join information are being
+    merged. And later while building the arel use the  previously built `JoinAssociation` record
+    in `JoinDependency#graft` to build the right from clause.
+
+    Fixes #3002.
+
+    *Jared Armstrong and Neeraj Singh*
+
 *   `default_scopes?` is deprecated. Check for `default_scopes.empty?` instead.
 
     *Agis Anastasopoulos*

activerecord/lib/active_record/associations/join_dependency/join_association.rb

@@ -55,7 +55,12 @@ def ==(other)
 
         def find_parent_in(other_join_dependency)
           other_join_dependency.join_parts.detect do |join_part|
-            parent == join_part
+            case parent
+            when JoinBase
+              parent.base_klass == join_part.base_klass
+            else
+              parent == join_part
+            end
           end
         end
 

activerecord/lib/active_record/relation/merger.rb

@@ -39,7 +39,7 @@ def other
     end
 
     class Merger # :nodoc:
-      attr_reader :relation, :values
+      attr_reader :relation, :values, :other
 
       def initialize(relation, other)
         if other.default_scoped? && other.klass != relation.klass
@@ -48,11 +48,12 @@ def initialize(relation, other)
 
         @relation = relation
         @values   = other.values
+        @other    = other
       end
 
       NORMAL_VALUES = Relation::SINGLE_VALUE_METHODS +
                       Relation::MULTI_VALUE_METHODS -
-                      [:where, :order, :bind, :reverse_order, :lock, :create_with, :reordering, :from] # :nodoc:
+                      [:joins, :where, :order, :bind, :reverse_order, :lock, :create_with, :reordering, :from] # :nodoc:
 
       def normal_values
         NORMAL_VALUES
@@ -66,12 +67,39 @@ def merge
 
         merge_multi_values
         merge_single_values
+        merge_joins
 
         relation
       end
 
       private
 
+      def merge_joins
+        return if values[:joins].blank?
+
+        if other.klass == relation.klass
+          relation.joins! *values[:joins]
+        else
+          joins_dependency, rest = values[:joins].partition do |join|
+            case join
+            when Hash, Symbol, Array
+              true
+            else
+              false
+            end
+          end
+
+          join_dependency = ActiveRecord::Associations::JoinDependency.new(other.klass, 
+                                                                           joins_dependency,
+                                                                           [])
+          relation.joins! rest
+
+          join_dependency.join_associations.each do |association|
+            @relation = association.join_relation(relation)
+          end
+        end
+      end
+
       def merge_multi_values
         relation.where_values = merged_wheres
         relation.bind_values  = merged_binds

activerecord/test/cases/relation_test.rb

@@ -1,10 +1,12 @@
 require "cases/helper"
 require 'models/post'
 require 'models/comment'
+require 'models/author'
+require 'models/rating'
 
 module ActiveRecord
   class RelationTest < ActiveRecord::TestCase
-    fixtures :posts, :comments
+    fixtures :posts, :comments, :authors
 
     class FakeKlass < Struct.new(:table_name, :name)
     end
@@ -176,6 +178,13 @@ def test_references_values_dont_duplicate
       relation.merge!(where: ['foo = ?', 'bar'])
       assert_equal ['foo = bar'], relation.where_values
     end
+
+    def test_relation_merging_with_merged_joins
+      special_comments_with_ratings = SpecialComment.joins(:ratings)
+      posts_with_special_comments_with_ratings = Post.group("posts.id").joins(:special_comments).merge(special_comments_with_ratings)
+      assert_equal 3, authors(:david).posts.merge(posts_with_special_comments_with_ratings).to_a.length
+    end
+
   end
 
   class RelationMutationTest < ActiveSupport::TestCase