`has_many :through` with unscope should affect to through scope

The order of scope evaluation should be from through scope to the
association's own scope. Otherwise the association's scope cannot affect
to through scope.

Fixes #13677.
Closes #28449.

4 files changed, 19 insertions, 23 deletions

diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb
index 0e849c06ef..4220a1499a 100644
--- a/activerecord/lib/active_record/associations/association_scope.rb
+++ b/activerecord/lib/active_record/associations/association_scope.rb
@@ -24,10 +24,10 @@ module ActiveRecord
         scope = klass.unscoped
         owner = association.owner
         alias_tracker = AliasTracker.create(klass.connection, klass.table_name)
-        chain_head, chain_tail = get_chain(reflection, association, alias_tracker)
+        chain = get_chain(reflection, association, alias_tracker)
 
         scope.extending! reflection.extensions
-        add_constraints(scope, owner, chain_head, chain_tail)
+        add_constraints(scope, owner, chain)
       end
 
       def join_type
@@ -98,7 +98,6 @@ module ActiveRecord
         end
 
         class ReflectionProxy < SimpleDelegator # :nodoc:
-          attr_accessor :next
           attr_reader :alias_name
 
           def initialize(reflection, alias_name)
@@ -111,36 +110,32 @@ module ActiveRecord
 
         def get_chain(reflection, association, tracker)
           name = reflection.name
-          runtime_reflection = Reflection::RuntimeReflection.new(reflection, association)
-          previous_reflection = runtime_reflection
+          chain = [Reflection::RuntimeReflection.new(reflection, association)]
           reflection.chain.drop(1).each do |refl|
             alias_name = tracker.aliased_table_for(
               refl.table_name,
               refl.alias_candidate(name),
               refl.klass.type_caster
             )
-            proxy = ReflectionProxy.new(refl, alias_name)
-            previous_reflection.next = proxy
-            previous_reflection = proxy
+            chain << ReflectionProxy.new(refl, alias_name)
           end
-          [runtime_reflection, previous_reflection]
+          chain
         end
 
-        def add_constraints(scope, owner, chain_head, chain_tail)
-          owner_reflection = chain_tail
+        def add_constraints(scope, owner, chain)
+          owner_reflection = chain.last
           table = owner_reflection.alias_name
           scope = last_chain_scope(scope, table, owner_reflection, owner)
 
-          reflection = chain_head
-          while reflection
+          chain.each_cons(2) do |reflection, next_reflection|
             table = reflection.alias_name
-            next_reflection = reflection.next
-
-            unless reflection == chain_tail
-              foreign_table = next_reflection.alias_name
-              scope = next_chain_scope(scope, table, reflection, foreign_table, next_reflection)
-            end
+            foreign_table = next_reflection.alias_name
+            scope = next_chain_scope(scope, table, reflection, foreign_table, next_reflection)
+          end
 
+          chain_head = chain.first
+          chain.reverse_each do |reflection|
+            table = reflection.alias_name
             # Exclude the scope of the association itself, because that
             # was already merged in the #scope method.
             reflection.constraints.each do |scope_chain_item|
@@ -158,8 +153,6 @@ module ActiveRecord
               scope.where_clause += item.where_clause
               scope.order_values |= item.order_values
             end
-
-            reflection = next_reflection
           end
 
           scope
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 49ddcaecdf..a01eb7b242 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -1077,8 +1077,6 @@ module ActiveRecord
     end
 
     class RuntimeReflection < PolymorphicReflection # :nodoc:
-      attr_accessor :next
-
       def initialize(reflection, association)
         @reflection = reflection
         @association = association
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index f8ea51225a..521b388cee 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -1250,6 +1250,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     TenantMembership.current_member = nil
   end
 
+  def test_has_many_through_with_unscope_should_affect_to_through_scope
+    assert_equal [comments(:eager_other_comment1)], authors(:mary).unordered_comments
+  end
+
   def test_has_many_through_with_scope_should_respect_table_alias
     family = Family.create!
     users = 3.times.map { User.create! }
diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb
index 0a52cc5390..3371fcbfcc 100644
--- a/activerecord/test/models/author.rb
+++ b/activerecord/test/models/author.rb
@@ -40,6 +40,7 @@ class Author < ActiveRecord::Base
            class_name: "Post"
 
   has_many :comments_desc, -> { order("comments.id DESC") }, through: :posts, source: :comments
+  has_many :unordered_comments, -> { unscope(:order).distinct }, through: :posts_sorted_by_id_limited, source: :comments
   has_many :funky_comments, through: :posts, source: :comments
   has_many :ordered_uniq_comments, -> { distinct.order("comments.id") }, through: :posts, source: :comments
   has_many :ordered_uniq_comments_desc, -> { distinct.order("comments.id DESC") }, through: :posts, source: :comments