Merge pull request #35247 from bogdan/fix-source-reflection-reset-code

Fix reset of the source association when through association is loaded

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

@@ -115,7 +115,7 @@ module ActiveRecord
           def build_scope
             scope = klass.scope_for_association
 
-            if reflection.type
+            if reflection.type && !reflection.through_reflection?
               scope.where!(reflection.type => model.polymorphic_name)
             end
 

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

@@ -7,10 +7,9 @@ module ActiveRecord
         def run(preloader)
           already_loaded     = owners.first.association(through_reflection.name).loaded?
           through_scope      = through_scope()
-          reflection_scope   = target_reflection_scope
           through_preloaders = preloader.preload(owners, through_reflection.name, through_scope)
           middle_records     = through_preloaders.flat_map(&:preloaded_records)
-          preloaders         = preloader.preload(middle_records, source_reflection.name, reflection_scope)
+          preloaders         = preloader.preload(middle_records, source_reflection.name, scope)
           @preloaded_records = preloaders.flat_map(&:preloaded_records)
 
           owners.each do |owner|
@@ -25,18 +24,18 @@ module ActiveRecord
               owner.association(through_reflection.name).reset if through_scope
             end
             result = through_records.flat_map do |record|
-              association = record.association(source_reflection.name)
+              record.association(source_reflection.name).target
-              target = association.target
-              association.reset if preload_scope
-              target
             end
             result.compact!
-            if reflection_scope
+            result.sort_by! { |rhs| preload_index[rhs] } if scope.order_values.any?
-              result.sort_by! { |rhs| preload_index[rhs] } if reflection_scope.order_values.any?
+            result.uniq! if scope.distinct_value
-              result.uniq! if reflection_scope.distinct_value
-            end
             associate_records_to_owner(owner, result)
           end
+          unless scope.empty_scope?
+            middle_records.each do |owner|
+              owner.association(source_reflection.name).reset
+            end
+          end
         end
 
         private
@@ -91,16 +90,6 @@ module ActiveRecord
 
             scope unless scope.empty_scope?
           end
-
-          def target_reflection_scope
-            if preload_scope
-              reflection_scope.merge(preload_scope)
-            elsif reflection.scope
-              reflection_scope
-            else
-              nil
-            end
-          end
       end
     end
   end

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -2073,10 +2073,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_associations_order_should_be_priority_over_throughs_order
-    david = authors(:david)
+    original = authors(:david)
     expected = [12, 10, 9, 8, 7, 6, 5, 3, 2, 1]
-    assert_equal expected, david.comments_desc.map(&:id)
+    assert_equal expected, original.comments_desc.map(&:id)
-    assert_equal expected, Author.includes(:comments_desc).find(david.id).comments_desc.map(&:id)
+    preloaded = Author.includes(:comments_desc).find(original.id)
+    assert_equal expected, preloaded.comments_desc.map(&:id)
+    assert_equal original.posts_sorted_by_id.first.comments.map(&:id), preloaded.posts_sorted_by_id.first.comments.map(&:id)
   end
 
   def test_dynamic_find_should_respect_association_order_for_through

activerecord/test/cases/associations/nested_through_associations_test.rb

@@ -610,6 +610,12 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase
     assert_equal hotel, Hotel.joins(:cake_designers, :drink_designers).take
   end
 
+  def test_has_many_through_reset_source_reflection_after_loading_is_complete
+    preloaded = Category.preload(:ordered_post_comments).find(1, 2).last
+    original = Category.find(2)
+    assert_equal original.ordered_post_comments.ids, preloaded.ordered_post_comments.ids
+  end
+
   private
 
     def assert_includes_and_joins_equal(query, expected, association)

activerecord/test/models/category.rb

@@ -26,6 +26,7 @@ class Category < ActiveRecord::Base
   has_many :categorizations
   has_many :special_categorizations
   has_many :post_comments, through: :posts, source: :comments
+  has_many :ordered_post_comments, -> { order(id: :desc) }, through: :posts, source: :comments
 
   has_many :authors, through: :categorizations
   has_many :authors_with_select, -> { select "authors.*, categorizations.post_id" }, through: :categorizations, source: :author