Merge pull request rails#54607 from joshuay03/respect-order-columns-in-reverse-order

Ensure `#reverse_order` respects `implicit_order_column`

Author: byroot

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Respect `implicit_order_column` in `ActiveRecord::Relation#reverse_order`.
+
+    *Joshua Young*
+
 *   Add column types to `ActiveRecord::Result` for SQLite3.
 
     *Andrew Kane*

activerecord/lib/active_record/relation/finder_methods.rb

@@ -638,7 +638,7 @@ def find_last(limit)
       end
 
       def ordered_relation
-        if order_values.empty? && (model.implicit_order_column || !model.query_constraints_list.nil? || primary_key)
+        if order_values.empty? && !_order_columns.empty?
           order(_order_columns.map { |column| table[column].asc })
         else
           self

activerecord/lib/active_record/relation/query_methods.rb

@@ -2009,9 +2009,12 @@ def table_name_matches?(from)
 
       def reverse_sql_order(order_query)
         if order_query.empty?
-          return [table[primary_key].desc] if primary_key
+          if !_reverse_order_columns.empty?
+            return _reverse_order_columns.map { |column| table[column].desc }
+          end
+
           raise IrreversibleOrderError,
-            "Relation has no current order and table has no primary key to be used as default order"
+            "Relation has no current order and table has no order columns to be used as default order"
         end
 
         order_query.flat_map do |o|
@@ -2036,6 +2039,13 @@ def reverse_sql_order(order_query)
         end
       end
 
+      def _reverse_order_columns
+        roc = []
+        roc << model.implicit_order_column if model.implicit_order_column
+        roc << model.primary_key if model.primary_key
+        roc.flatten.uniq.compact
+      end
+
       def does_not_support_reverse?(order)
         # Account for String subclasses like Arel::Nodes::SqlLiteral that
         # override methods like #count.

activerecord/test/cases/relations_test.rb

@@ -370,6 +370,19 @@ def test_default_reverse_order_on_table_without_primary_key
     end
   end
 
+  def test_default_reverse_order_on_table_without_primary_key_with_implicit_order_column
+    ordered_edge = Class.new(Edge) do
+      self.implicit_order_column = "source_id"
+    end
+
+    ordered_edge.delete_all
+
+    edge_1 = ordered_edge.create!(source_id: 1, sink_id: 2)
+    edge_2 = ordered_edge.create!(source_id: 2, sink_id: 3)
+    assert_equal edge_1.source_id, ordered_edge.all.first.source_id
+    assert_equal edge_2.source_id, ordered_edge.all.reverse_order.first.source_id
+  end
+
   def test_order_with_hash_and_symbol_generates_the_same_sql
     assert_equal Topic.order(:id).to_sql, Topic.order(id: :asc).to_sql
   end