Fix preloading association with scope including joins

6 files changed, 32 insertions, 49 deletions

diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb
index 208d1b2670..a18994cec4 100644
--- a/activerecord/lib/active_record/associations/preloader.rb
+++ b/activerecord/lib/active_record/associations/preloader.rb
@@ -54,8 +54,6 @@ module ActiveRecord
         autoload :BelongsTo,           "active_record/associations/preloader/belongs_to"
       end
 
-      NULL_RELATION = Struct.new(:values, :where_clause, :joins_values).new({}, Relation::WhereClause.empty, [])
-
       # Eager loads the named associations for the given Active Record record(s).
       #
       # In this description, 'association name' shall refer to the name passed
@@ -93,7 +91,6 @@ module ActiveRecord
       def preload(records, associations, preload_scope = nil)
         records       = Array.wrap(records).compact.uniq
         associations  = Array.wrap(associations)
-        preload_scope = preload_scope || NULL_RELATION
 
         if records.empty?
           []
diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb
index 4513dce15e..85343040db 100644
--- a/activerecord/lib/active_record/associations/preloader/association.rb
+++ b/activerecord/lib/active_record/associations/preloader/association.rb
@@ -11,7 +11,6 @@ module ActiveRecord
           @reflection    = reflection
           @preload_scope = preload_scope
           @model         = owners.first && owners.first.class
-          @scope         = nil
           @preloaded_records = []
         end
 
@@ -23,14 +22,6 @@ module ActiveRecord
           raise NotImplementedError
         end
 
-        def scope
-          @scope ||= build_scope
-        end
-
-        def records_for(ids)
-          scope.where(association_key_name => ids)
-        end
-
         # The name of the key on the associated records
         def association_key_name
           raise NotImplementedError
@@ -104,54 +95,35 @@ module ActiveRecord
             # Make several smaller queries if necessary or make one query if the adapter supports it
             slices = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size)
             @preloaded_records = slices.flat_map do |slice|
-              records_for(slice).load(&block)
+              records_for(slice, &block)
             end
             @preloaded_records.group_by do |record|
               convert_key(record[association_key_name])
             end
           end
 
+          def records_for(ids, &block)
+            scope.where(association_key_name => ids).load(&block)
+          end
+
+          def scope
+            @scope ||= build_scope
+          end
+
           def reflection_scope
             @reflection_scope ||= reflection.scope_for(klass)
           end
 
           def build_scope
-            scope = klass.unscoped
-
-            values = reflection_scope.values
-            preload_values = preload_scope.values
-
-            scope.where_clause = reflection_scope.where_clause + preload_scope.where_clause
-            scope.references_values = Array(values[:references]) + Array(preload_values[:references])
-
-            if preload_values[:select] || values[:select]
-              scope._select!(preload_values[:select] || values[:select])
-            end
-            scope.includes! preload_values[:includes] || values[:includes]
-            if preload_scope.joins_values.any?
-              scope.joins!(preload_scope.joins_values)
-            else
-              scope.joins!(reflection_scope.joins_values)
-            end
-
-            if order_values = preload_values[:order] || values[:order]
-              scope.order!(order_values)
-            end
-
-            if preload_values[:reordering] || values[:reordering]
-              scope.reordering_value = true
-            end
-
-            if preload_values[:readonly] || values[:readonly]
-              scope.readonly!
-            end
+            scope = klass.default_scoped
 
-            if options[:as]
-              scope.where!(klass.table_name => { reflection.type => model.base_class.sti_name })
+            if reflection.type
+              scope.where!(reflection.type => model.base_class.sti_name)
             end
 
-            scope.unscope_values = Array(values[:unscope]) + Array(preload_values[:unscope])
-            klass.default_scoped.merge(scope)
+            scope.merge!(reflection_scope)
+            scope.merge!(preload_scope) if preload_scope
+            scope
           end
       end
     end
diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb
index 34587fd3f5..0999746cd5 100644
--- a/activerecord/lib/active_record/associations/preloader/through_association.rb
+++ b/activerecord/lib/active_record/associations/preloader/through_association.rb
@@ -79,17 +79,24 @@ module ActiveRecord
 
           def through_scope
             scope = through_reflection.klass.unscoped
+            values = reflection_scope.values
 
             if options[:source_type]
               scope.where! reflection.foreign_type => options[:source_type]
             else
               unless reflection_scope.where_clause.empty?
-                scope.includes_values = Array(reflection_scope.values[:includes] || options[:source])
+                scope.includes_values = Array(values[:includes] || options[:source])
                 scope.where_clause = reflection_scope.where_clause
+                if joins = values[:joins]
+                  scope.joins!(source_reflection.name => joins)
+                end
+                if left_outer_joins = values[:left_outer_joins]
+                  scope.left_outer_joins!(source_reflection.name => left_outer_joins)
+                end
               end
 
-              scope.references! reflection_scope.values[:references]
-              if scope.eager_loading? && order_values = reflection_scope.values[:order]
+              scope.references! values[:references]
+              if scope.eager_loading? && order_values = values[:order]
                 scope = scope.order(order_values)
               end
             end
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 55b294cfaa..db8248dd99 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -68,6 +68,10 @@ class EagerAssociationTest < ActiveRecord::TestCase
       "expected to find only david's posts"
   end
 
+  def test_loading_with_scope_including_joins
+    assert_equal clubs(:boring_club), Member.preload(:general_club).find(1).general_club
+  end
+
   def test_with_ordering
     list = Post.all.merge!(includes: :comments, order: "posts.id DESC").to_a
     [:other_by_mary, :other_by_bob, :misc_by_mary, :misc_by_bob, :eager_other,
diff --git a/activerecord/test/models/club.rb b/activerecord/test/models/club.rb
index 49d7b24a3b..3d441b1d48 100644
--- a/activerecord/test/models/club.rb
+++ b/activerecord/test/models/club.rb
@@ -8,6 +8,8 @@ class Club < ActiveRecord::Base
 
   has_many :favourites, -> { where(memberships: { favourite: true }) }, through: :memberships, source: :member
 
+  scope :general, -> { left_joins(:category).where(categories: { name: "General" }) }
+
   private
 
     def private_method
diff --git a/activerecord/test/models/member.rb b/activerecord/test/models/member.rb
index 36f2461b84..b9597c6b9a 100644
--- a/activerecord/test/models/member.rb
+++ b/activerecord/test/models/member.rb
@@ -22,6 +22,7 @@ class Member < ActiveRecord::Base
   has_many :organization_member_details_2, through: :organization, source: :member_details
 
   has_one :club_category, through: :club, source: :category
+  has_one :general_club, -> { general }, through: :current_membership, source: :club
 
   has_many :current_memberships, -> { where favourite: true }
   has_many :clubs, through: :current_memberships