Add tiebreaker by primary key to ensure stable sorting

Author: goosys

lib/administrate/order.rb

@@ -13,10 +13,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/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