Honour the order of the joining model in a `has_many :through`

association when eager loading.

Previously, eager loading a `has_many :through` association with no
defined order would return the records in the natural order of the
database. Now, these records will be returned in the order that the
joining record is returned, in case there is a defined order there.

2 files changed, 16 insertions, 9 deletions

diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb
index 1dc8bff193..91de52e7e7 100644
--- a/activerecord/lib/active_record/associations/preloader/association.rb
+++ b/activerecord/lib/active_record/associations/preloader/association.rb
@@ -86,7 +86,7 @@ module ActiveRecord
           if owner_keys.any?
             # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000)
             # Make several smaller queries if necessary or make one query if the adapter supports it
-            sliced  = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size)
+            sliced = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size)
 
             records = load_slices sliced
             records.each do |record, owner_key|
diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb
index 56aa23b173..5adb0aac65 100644
--- a/activerecord/lib/active_record/associations/preloader/through_association.rb
+++ b/activerecord/lib/active_record/associations/preloader/through_association.rb
@@ -37,12 +37,7 @@ module ActiveRecord
             }
           end
 
-          record_offset = {}
-          @preloaded_records.each_with_index do |record,i|
-            record_offset[record] = i
-          end
-
-          through_records.each_with_object({}) { |(lhs,center),records_by_owner|
+          through_records.each_with_object({}) do |(lhs,center), records_by_owner|
             pl_to_middle = center.group_by { |record| middle_to_pl[record] }
 
             records_by_owner[lhs] = pl_to_middle.flat_map do |pl, middles|
@@ -52,13 +47,25 @@ module ActiveRecord
                 association.reader
               }.compact
 
-              rhs_records.sort_by { |rhs| record_offset[rhs] }
+              # Respect the order on `reflection_scope` if it exists, else use the natural order.
+              if reflection_scope.values[:order].present?
+                @id_map ||= id_to_index_map @preloaded_records
+                rhs_records.sort_by { |rhs| @id_map[rhs] }
+              else
+                rhs_records
+              end
             end
-          }
+          end
         end
 
         private
 
+        def id_to_index_map(ids)
+          id_map = {}
+          ids.each_with_index { |id, index| id_map[id] = index }
+          id_map
+        end
+
         def reset_association(owners, association_name)
           should_reset = (through_scope != through_reflection.klass.unscoped) ||
              (reflection.options[:source_type] && through_reflection.collection?)