Add primary key tiebreaker for stable sorting (#2886)

This pull request improves the stability of sorting in Administrate by introducing a tiebreaker using the primary key. When ordering by a column, the primary key is now used as a secondary sort key. If the relation does not have a primary key, the tiebreaker is skipped.

Without a tiebreaker, sorting by a non-unique column can result in unpredictable order for records with the same value. This can cause issues such as the order changing on every page load, or the same record appearing on multiple pages when paginating. By using the primary key as a tiebreaker, we guarantee a stable sort order and prevent these problems.

Changes:

- Add the primary key as a tiebreaker when ordering by a column
- Skip the tiebreaker if the relation has no primary key
- Update and add specs to cover these scenarios

Author: goosys

docs/using_administrate.md

@@ -0,0 +1,5 @@
+---
+title: Using Administrate
+---
+
+- [Stable sorting](./using_administrate/stable_sorting.md)

docs/using_administrate/stable_sorting.md

@@ -0,0 +1,26 @@
+---
+title: Stable Sorting
+---
+
+The display order of `index` pages and `HasMany` fields is controlled by `Administrate::Order`.
+By default, both are set to `nil`, so the model's default sort order is applied.
+
+You can toggle sorting by clicking on a table header attribute. When sorting by a specific attribute, a tiebreaker is used to ensure stable sorting.
+
+The tiebreaker uses the table's primary key.
+The generated SQL looks like this:
+
+```sql
+-- When toggling the name attribute
+SELECT * FROM users ORDER BY name DESC, id DESC;
+
+-- When toggling the name attribute again
+SELECT * FROM users ORDER BY name ASC, id ASC;
+```
+
+If there is no primary key (such as in a join table), the tiebreaker is not used.
+
+```sql
+-- When toggling the name attribute
+SELECT * FROM users ORDER BY name DESC;
+```

lib/administrate/order.rb

@@ -12,10 +12,18 @@ def apply(relation)
 
       order = relation.arel_table[sorting_column].public_send(direction)
 
-      return relation.reorder(order) if
-        column_exist?(relation, sorting_column)
-
-      relation
+      tiebreak_key = relation.primary_key
+      tiebreak_order = relation.arel_table[tiebreak_key].public_send(direction)
+
+      if column_exist?(relation, sorting_column)
+        if column_exist?(relation, tiebreak_key) && sorting_column.to_s != tiebreak_key.to_s
+          relation.reorder(order, tiebreak_order)
+        else
+          relation.reorder(order)
+        end
+      else
+        relation
+      end
     end
 
     def ordered_by?(attr)

spec/example_app/app/views/layouts/docs.html.erb

@@ -37,6 +37,7 @@
         <li><a href="/rails_api">Rails API</a></li>
         <li><a href="/extending_administrate">Extending Administrate</a></li>
         <li><a href="/guides">Guides</a></li>
+        <li><a href="/using_administrate">Using Administrate</a></li>
       </ul>
     </div>
 

spec/lib/administrate/order_spec.rb

@@ -30,15 +30,16 @@
     end
 
     context "when `order` argument is valid" do
-      it "orders by the column" do
+      it "orders by the column and tiebreaks by the primary key" do
         order = Administrate::Order.new(:name, :asc)
         relation = relation_with_column(:name)
         allow(relation).to receive(:reorder).and_return(relation)
 
         ordered = order.apply(relation)
 
         expect(relation).to have_received(:reorder).with(
-          to_sql('"table_name"."name" ASC')
+          to_sql('"table_name"."name" ASC'),
+          to_sql('"table_name"."id" ASC')
         )
         expect(ordered).to eq(relation)
       end
@@ -51,7 +52,8 @@
         ordered = order.apply(relation)
 
         expect(relation).to have_received(:reorder).with(
-          to_sql('"table_name"."name" DESC')
+          to_sql('"table_name"."name" DESC'),
+          to_sql('"table_name"."id" DESC')
         )
         expect(ordered).to eq(relation)
       end
@@ -64,10 +66,47 @@
         ordered = order.apply(relation)
 
         expect(relation).to have_received(:reorder).with(
-          to_sql('"table_name"."name" ASC')
+          to_sql('"table_name"."name" ASC'),
+          to_sql('"table_name"."id" ASC')
         )
         expect(ordered).to eq(relation)
       end
+
+      context "and same with own primary key" do
+        it "orders by the primary key" do
+          order = Administrate::Order.new(:id, :asc)
+          relation = relation_with_column(:name)
+          allow(relation).to receive(:reorder).and_return(relation)
+
+          ordered = order.apply(relation)
+
+          expect(relation).to have_received(:reorder).with(
+            to_sql('"table_name"."id" ASC')
+          )
+          expect(ordered).to eq(relation)
+        end
+      end
+
+      context "when the relation has no primary key" do
+        it "orders by the column without tiebreaks" do
+          order = Administrate::Order.new(:name, :asc)
+          relation = double(
+            klass: double(reflect_on_association: nil),
+            columns_hash: {"name" => :column_info},
+            table_name: "table_name",
+            arel_table: Arel::Table.new("table_name"),
+            primary_key: nil
+          )
+          allow(relation).to receive(:reorder).and_return(relation)
+
+          ordered = order.apply(relation)
+
+          expect(relation).to have_received(:reorder).with(
+            to_sql('"table_name"."name" ASC')
+          )
+          expect(ordered).to eq(relation)
+        end
+      end
     end
 
     context "when relation has_many association" do
@@ -329,9 +368,10 @@
   def relation_with_column(column)
     double(
       klass: double(reflect_on_association: nil),
-      columns_hash: {column.to_s => :column_info},
+      columns_hash: {column.to_s => :column_info, "id" => :column_info},
       table_name: "table_name",
-      arel_table: Arel::Table.new("table_name")
+      arel_table: Arel::Table.new("table_name"),
+      primary_key: "id"
     )
   end