fix(agent): add primary key tiebreaker to paginated sorts

packages/agent/src/utils/context-filter-factory.ts

@@ -1,7 +1,13 @@
 import type { Collection, ConditionTree } from '@forestadmin/datasource-toolkit';
 import type { Context } from 'koa';
 
-import { ConditionTreeFactory, Filter, PaginatedFilter } from '@forestadmin/datasource-toolkit';
+import {
+  ConditionTreeFactory,
+  Filter,
+  PaginatedFilter,
+  Sort,
+  SortFactory,
+} from '@forestadmin/datasource-toolkit';
 
 import QueryStringParser from './query-string';
 
@@ -13,7 +19,10 @@ export default class ContextFilterFactory {
     partialFilter?: Partial<PaginatedFilter>,
   ): PaginatedFilter {
     return new PaginatedFilter({
-      sort: QueryStringParser.parseSort(collection, context),
+      sort: ContextFilterFactory.withPrimaryKeyTiebreaker(
+        collection,
+        QueryStringParser.parseSort(collection, context),
+      ),
       page: QueryStringParser.parsePagination(context),
       ...ContextFilterFactory.build(collection, context, scope),
       ...partialFilter,
@@ -38,4 +47,15 @@ export default class ContextFilterFactory {
       ...partialFilter,
     });
   }
+
+  private static withPrimaryKeyTiebreaker(collection: Collection, sort: Sort): Sort {
+    if (sort.length === 0) return sort;
+
+    const { ascending } = sort[sort.length - 1];
+    const missingPrimaryKeys = SortFactory.byPrimaryKeys(collection)
+      .filter(({ field }) => !sort.some(clause => clause.field === field))
+      .map(({ field }) => ({ field, ascending }));
+
+    return new Sort(...sort, ...missingPrimaryKeys);
+  }
 }

packages/agent/test/routes/access/csv.test.ts

@@ -192,6 +192,33 @@ describe('CsvRoute', () => {
       const csvResult = await readCsv(context.response.body as AsyncGenerator<string>);
       expect(csvResult).toEqual(['name,id\n', 'a,1\nab,2\nabc,3\n']);
     });
+
+    it('should page the export with the primary key as sort tiebreaker', async () => {
+      jest.restoreAllMocks();
+      const { options, services, dataSource } = setup();
+      const csvRoute = new CsvRoute(services, options, dataSource, 'books');
+      const context = createMockContext({
+        customProperties: {
+          query: { 'fields[books]': 'id,name', header: 'id,name', sort: '-name', timezone: 'UTC' },
+        },
+      });
+      const list = jest.fn().mockReturnValue([]);
+      dataSource.getCollection('books').list = list;
+
+      await csvRoute.handleCsv(context);
+      await readCsv(context.response.body as AsyncGenerator<string>);
+
+      expect(list).toHaveBeenCalledWith(
+        expect.anything(),
+        expect.objectContaining({
+          sort: [
+            { field: 'name', ascending: false },
+            { field: 'id', ascending: false },
+          ],
+        }),
+        ['id', 'name'],
+      );
+    });
   });
 
   describe('with special characters in names', () => {

packages/agent/test/security/related-read-permissions.test.ts

@@ -903,7 +903,10 @@ describe('read permissions on related collections', () => {
         buildContext({ query: { sort: '-account.balance' } }),
       );
 
-      expect(list.mock.calls[0][1].sort).toEqual([{ field: 'account:balance', ascending: false }]);
+      expect(list.mock.calls[0][1].sort).toEqual([
+        { field: 'account:balance', ascending: false },
+        { field: 'id', ascending: false },
+      ]);
     });
 
     it('should serve an extended search the stack cannot describe', async () => {

packages/agent/test/utils/context-filter-factory.test.ts

@@ -1,3 +1,5 @@
+import type { Collection, ColumnSchema } from '@forestadmin/datasource-toolkit';
+
 import {
   ConditionTreeBranch,
   ConditionTreeLeaf,
@@ -92,5 +94,104 @@ describe('FilterFactory', () => {
         }),
       );
     });
+
+    describe('primary key tiebreaker', () => {
+      const buildCollection = (fields: Record<string, ColumnSchema>) =>
+        factories.collection.build({
+          schema: factories.collectionSchema.build({ fields }),
+        });
+
+      const buildSort = (collection: Collection, sort?: string) =>
+        ContextFilterFactory.buildPaginated(
+          collection,
+          createMockContext({ customProperties: { query: { sort, timezone: 'Europe/Paris' } } }),
+          null,
+        ).sort;
+
+      test('should append the primary key in the direction of a descending sort', () => {
+        const collection = buildCollection({
+          id: factories.columnSchema.numericPrimaryKey().build(),
+          createdDate: factories.columnSchema.text().build(),
+        });
+
+        expect(buildSort(collection, '-createdDate')).toEqual([
+          { field: 'createdDate', ascending: false },
+          { field: 'id', ascending: false },
+        ]);
+      });
+
+      test('should append the primary key in the direction of the last sort clause', () => {
+        const collection = buildCollection({
+          id: factories.columnSchema.numericPrimaryKey().build(),
+          name: factories.columnSchema.text().build(),
+          createdDate: factories.columnSchema.text().build(),
+        });
+
+        expect(buildSort(collection, '-name,createdDate')).toEqual([
+          { field: 'name', ascending: false },
+          { field: 'createdDate', ascending: true },
+          { field: 'id', ascending: true },
+        ]);
+      });
+
+      test('should keep the sort unchanged when it already contains the primary key', () => {
+        const collection = buildCollection({
+          id: factories.columnSchema.numericPrimaryKey().build(),
+          createdDate: factories.columnSchema.text().build(),
+        });
+
+        expect(buildSort(collection, 'createdDate,-id')).toEqual([
+          { field: 'createdDate', ascending: true },
+          { field: 'id', ascending: false },
+        ]);
+      });
+
+      test('should append only the missing fields of a composite primary key', () => {
+        const collection = buildCollection({
+          tenantId: factories.columnSchema.numericPrimaryKey().build(),
+          id: factories.columnSchema.numericPrimaryKey().build(),
+          createdDate: factories.columnSchema.text().build(),
+        });
+
+        expect(buildSort(collection, '-createdDate,tenantId')).toEqual([
+          { field: 'createdDate', ascending: false },
+          { field: 'tenantId', ascending: true },
+          { field: 'id', ascending: true },
+        ]);
+      });
+
+      test('should append every field of a composite primary key', () => {
+        const collection = buildCollection({
+          tenantId: factories.columnSchema.numericPrimaryKey().build(),
+          id: factories.columnSchema.numericPrimaryKey().build(),
+          createdDate: factories.columnSchema.text().build(),
+        });
+
+        expect(buildSort(collection, '-createdDate')).toEqual([
+          { field: 'createdDate', ascending: false },
+          { field: 'tenantId', ascending: false },
+          { field: 'id', ascending: false },
+        ]);
+      });
+
+      test('should skip a primary key that is not sortable', () => {
+        const collection = buildCollection({
+          id: factories.columnSchema.numericPrimaryKey().build({ isSortable: false }),
+          createdDate: factories.columnSchema.text().build(),
+        });
+
+        expect(buildSort(collection, '-createdDate')).toEqual([
+          { field: 'createdDate', ascending: false },
+        ]);
+      });
+
+      test('should leave an empty sort empty', () => {
+        const collection = buildCollection({
+          id: factories.columnSchema.numericPrimaryKey().build(),
+        });
+
+        expect(buildSort(collection, undefined)).toEqual(new Sort());
+      });
+    });
   });
 });