Merge pull request #40056 from kamipo/fix_preloader_associate_by_default

Fix preloader to associate preloaded records by default

activerecord/lib/active_record/associations/preloader.rb

@@ -96,6 +96,10 @@ def preload(records, associations, preload_scope = nil)
         end
       end
 
+      def initialize(associate_by_default: true)
+        @associate_by_default = associate_by_default
+      end
+
       private
         # Loads all the given data into +records+ for the +association+.
         def preloaders_on(association, records, scope, polymorphic_parent = false)
@@ -144,7 +148,7 @@ def preloaders_for_one(association, records, scope, polymorphic_parent)
 
         def preloaders_for_reflection(reflection, records, scope)
           records.group_by { |record| record.association(reflection.name).klass }.map do |rhs_klass, rs|
-            preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope).run
+            preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope, @associate_by_default).run
           end
         end
 
@@ -159,7 +163,7 @@ def grouped_records(association, records, polymorphic_parent)
         end
 
         class AlreadyLoaded # :nodoc:
-          def initialize(klass, owners, reflection, preload_scope)
+          def initialize(klass, owners, reflection, preload_scope, associate_by_default = true)
             @owners = owners
             @reflection = reflection
           end

activerecord/lib/active_record/associations/preloader/association.rb

@@ -4,25 +4,22 @@ module ActiveRecord
   module Associations
     class Preloader
       class Association #:nodoc:
-        def initialize(klass, owners, reflection, preload_scope)
+        def initialize(klass, owners, reflection, preload_scope, associate_by_default = true)
           @klass         = klass
           @owners        = owners.uniq(&:__id__)
           @reflection    = reflection
           @preload_scope = preload_scope
+          @associate     = associate_by_default || !preload_scope || preload_scope.empty_scope?
           @model         = owners.first && owners.first.class
         end
 
         def run
-          if !preload_scope || preload_scope.empty_scope?
-            owners.each do |owner|
-              associate_records_to_owner(owner, records_by_owner[owner] || [])
-            end
-          else
-            # Custom preload scope is used and
-            # the association cannot be marked as loaded
-            # Loading into a Hash instead
-            records_by_owner
-          end
+          records = records_by_owner
+
+          owners.each do |owner|
+            associate_records_to_owner(owner, records[owner] || [])
+          end if @associate
+
           self
         end
 

activerecord/lib/active_record/associations/preloader/through_association.rb

@@ -4,7 +4,7 @@ module ActiveRecord
   module Associations
     class Preloader
       class ThroughAssociation < Association # :nodoc:
-        PRELOADER = ActiveRecord::Associations::Preloader.new
+        PRELOADER = ActiveRecord::Associations::Preloader.new(associate_by_default: false)
 
         def initialize(*)
           super

activerecord/test/cases/associations_test.rb

@@ -354,8 +354,23 @@ def test_requires_symbol_argument
   end
 end
 
+class PreloaderTest < ActiveRecord::TestCase
+  fixtures :posts, :comments
+
+  def test_preload_with_scope
+    post = posts(:welcome)
+
+    preloader = ActiveRecord::Associations::Preloader.new
+    preloader.preload([post], :comments, Comment.where(body: "Thank you for the welcome"))
+
+    assert_predicate post.comments, :loaded?
+    assert_equal [comments(:greetings)], post.comments
+  end
+end
+
 class GeneratedMethodsTest < ActiveRecord::TestCase
   fixtures :developers, :computers, :posts, :comments
+
   def test_association_methods_override_attribute_methods_of_same_name
     assert_equal(developers(:david), computers(:workstation).developer)
     # this next line will fail if the attribute methods module is generated lazily