diff --git a/packages/agent/src/utils/context-filter-factory.ts b/packages/agent/src/utils/context-filter-factory.ts index 30939c55b5..868220618a 100644 --- a/packages/agent/src/utils/context-filter-factory.ts +++ b/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 { 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); + } } diff --git a/packages/agent/test/routes/access/csv.test.ts b/packages/agent/test/routes/access/csv.test.ts index 0fc4463d30..0d6393b164 100644 --- a/packages/agent/test/routes/access/csv.test.ts +++ b/packages/agent/test/routes/access/csv.test.ts @@ -192,6 +192,33 @@ describe('CsvRoute', () => { const csvResult = await readCsv(context.response.body as AsyncGenerator); 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); + + expect(list).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + sort: [ + { field: 'name', ascending: false }, + { field: 'id', ascending: false }, + ], + }), + ['id', 'name'], + ); + }); }); describe('with special characters in names', () => { diff --git a/packages/agent/test/security/related-read-permissions.test.ts b/packages/agent/test/security/related-read-permissions.test.ts index 14adb86b4d..5dbb9c5c82 100644 --- a/packages/agent/test/security/related-read-permissions.test.ts +++ b/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 () => { diff --git a/packages/agent/test/utils/context-filter-factory.test.ts b/packages/agent/test/utils/context-filter-factory.test.ts index 3e929f21fc..a43dd292dc 100644 --- a/packages/agent/test/utils/context-filter-factory.test.ts +++ b/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) => + 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()); + }); + }); }); });