diff --git a/docs/guides.md b/docs/guides.md index 9ac783c97..bf7e84555 100644 --- a/docs/guides.md +++ b/docs/guides.md @@ -6,3 +6,4 @@ title: Guides - [Customising the search](./guides/customising_search) - [Scoping HasMany Relations](./guides/scoping_has_many_relations.md) - [Switching templates with view variants](./guides/switching_templates_with_view_variants.md) +- [Stable sorting](./guides/stable_sorting.md) diff --git a/docs/guides/stable_sorting.md b/docs/guides/stable_sorting.md new file mode 100644 index 000000000..01e0f07ed --- /dev/null +++ b/docs/guides/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; +``` diff --git a/lib/administrate/order.rb b/lib/administrate/order.rb index 5b7c8dbde..8f0172641 100644 --- a/lib/administrate/order.rb +++ b/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) diff --git a/spec/lib/administrate/order_spec.rb b/spec/lib/administrate/order_spec.rb index 15949a5d7..e8d56a44b 100644 --- a/spec/lib/administrate/order_spec.rb +++ b/spec/lib/administrate/order_spec.rb @@ -30,7 +30,7 @@ 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) @@ -38,7 +38,8 @@ 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