diff --git a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-get-many.handler.ts b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-get-many.handler.ts index 2432a4d14..b0be1302e 100644 --- a/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-get-many.handler.ts +++ b/packages/twenty-server/src/engine/api/rest/core/handlers/rest-api-get-many.handler.ts @@ -15,21 +15,29 @@ export class RestApiGetManyHandler extends RestApiBaseHandler { objectMetadataItemWithFieldsMaps, } = await this.getRepositoryAndMetadataOrFail(request); - const { records, isForwardPagination, hasMoreRecords, totalCount } = - await this.findRecords({ - request, - repository, - objectMetadata, - objectMetadataNameSingular, - objectMetadataItemWithFieldsMaps, - }); - - return this.formatPaginatedResult( + const { records, + isForwardPagination, + hasMoreRecords, + totalCount, + startCursor, + endCursor, + } = await this.findRecords({ + request, + repository, + objectMetadata, + objectMetadataNameSingular, + objectMetadataItemWithFieldsMaps, + }); + + return this.formatPaginatedResult({ + finalRecords: records, objectMetadataNamePlural, isForwardPagination, hasMoreRecords, totalCount, - ); + startCursor, + endCursor, + }); } } diff --git a/packages/twenty-server/src/engine/api/rest/core/interfaces/rest-api-base.handler.ts b/packages/twenty-server/src/engine/api/rest/core/interfaces/rest-api-base.handler.ts index 0f9cef1f1..941abbb7d 100644 --- a/packages/twenty-server/src/engine/api/rest/core/interfaces/rest-api-base.handler.ts +++ b/packages/twenty-server/src/engine/api/rest/core/interfaces/rest-api-base.handler.ts @@ -7,7 +7,7 @@ import { In, ObjectLiteral, SelectQueryBuilder } from 'typeorm'; import { ObjectRecord, - OrderByDirection, + ObjectRecordFilter, } from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface'; import { GraphqlQueryParser } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query.parser'; @@ -30,6 +30,8 @@ import { WorkspacePermissionsCacheService } from 'src/engine/metadata-modules/wo import { WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository'; import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; import { formatResult as formatGetManyData } from 'src/engine/twenty-orm/utils/format-result.util'; +import { encodeCursor } from 'src/engine/api/graphql/graphql-query-runner/utils/cursors.util'; +import { computeCursorArgFilter } from 'src/engine/api/graphql/graphql-query-runner/utils/compute-cursor-arg-filter'; export interface PageInfo { hasNextPage?: boolean; @@ -243,13 +245,23 @@ export abstract class RestApiBaseHandler { }; } - formatPaginatedResult( - finalRecords: any[], - objectMetadataNamePlural: string, - isForwardPagination: boolean, - hasMoreRecords: boolean, - totalCount: number, - ) { + formatPaginatedResult({ + finalRecords, + objectMetadataNamePlural, + isForwardPagination, + hasMoreRecords, + totalCount, + startCursor, + endCursor, + }: { + finalRecords: any[]; + objectMetadataNamePlural: string; + isForwardPagination: boolean; + hasMoreRecords: boolean; + totalCount: number; + startCursor: string | null; + endCursor: string | null; + }) { const hasPreviousPage = !isForwardPagination && hasMoreRecords; return this.formatResult({ @@ -259,20 +271,8 @@ export abstract class RestApiBaseHandler { pageInfo: { hasNextPage: isForwardPagination && hasMoreRecords, ...(hasPreviousPage ? { hasPreviousPage } : {}), - startCursor: - finalRecords.length > 0 - ? Buffer.from(JSON.stringify({ id: finalRecords[0].id })).toString( - 'base64', - ) - : null, - endCursor: - finalRecords.length > 0 - ? Buffer.from( - JSON.stringify({ - id: finalRecords[finalRecords.length - 1].id, - }), - ).toString('base64') - : null, + startCursor, + endCursor, }, totalCount, }); @@ -289,7 +289,10 @@ export abstract class RestApiBaseHandler { request: Request; recordId?: string; repository: WorkspaceRepository; - objectMetadata: any; + objectMetadata: { + objectMetadataMaps: ObjectMetadataMaps; + objectMetadataMapItem: ObjectMetadataItemWithFieldMaps; + }; objectMetadataNameSingular: string; objectMetadataItemWithFieldsMaps: | ObjectMetadataItemWithFieldMaps @@ -305,16 +308,23 @@ export abstract class RestApiBaseHandler { const fieldMetadataMapByName = objectMetadataItemWithFieldsMaps?.fieldsByName || {}; + const fieldMetadataMapByJoinColumnName = objectMetadataItemWithFieldsMaps?.fieldsByJoinColumnName || {}; + const isForwardPagination = !inputs.endingBefore; + const graphqlQueryParser = new GraphqlQueryParser( fieldMetadataMapByName, fieldMetadataMapByJoinColumnName, objectMetadata.objectMetadataMaps, ); - const filters = this.computeFilters(inputs); + const filters = this.computeFilters({ + inputs, + objectMetadata, + isForwardPagination, + }); let selectQueryBuilder = isDefined(filters) ? graphqlQueryParser.applyFilterToBuilder( @@ -326,11 +336,9 @@ export abstract class RestApiBaseHandler { const totalCount = await this.getTotalCount(selectQueryBuilder); - const isForwardPagination = !inputs.endingBefore; - selectQueryBuilder = graphqlQueryParser.applyOrderToBuilder( selectQueryBuilder, - [...(inputs.orderBy || []), { id: OrderByDirection.AscNullsFirst }], + inputs.orderBy || [], objectMetadataNameSingular, isForwardPagination, ); @@ -356,15 +364,29 @@ export abstract class RestApiBaseHandler { const hasMoreRecords = records.length < totalCount; + const finalRecords = formatGetManyData( + records, + objectMetadataItemWithFieldsMaps, + objectMetadata.objectMetadataMaps, + ); + + const startCursor = + finalRecords.length > 0 + ? encodeCursor(finalRecords[0], inputs.orderBy) + : null; + + const endCursor = + finalRecords.length > 0 + ? encodeCursor(finalRecords[finalRecords.length - 1], inputs.orderBy) + : null; + return { - records: formatGetManyData( - records, - objectMetadataItemWithFieldsMaps as any, - objectMetadata.objectMetadataMaps, - ), + records: finalRecords, totalCount, hasMoreRecords, isForwardPagination, + startCursor, + endCursor, }; } @@ -376,25 +398,35 @@ export abstract class RestApiBaseHandler { return await countQuery.getCount(); } - computeFilters(inputs: QueryVariables) { + computeFilters({ + inputs, + objectMetadata, + isForwardPagination, + }: { + inputs: QueryVariables; + objectMetadata: { + objectMetadataMaps: ObjectMetadataMaps; + objectMetadataMapItem: ObjectMetadataItemWithFieldMaps; + }; + isForwardPagination: boolean; + }) { let appliedFilters = inputs.filter; - if (inputs.startingAfter) { - appliedFilters = { - and: [ - appliedFilters || {}, - { id: { gt: this.parseCursor(inputs.startingAfter).id } }, - ], - }; - } + const cursor = inputs.startingAfter || inputs.endingBefore; - if (inputs.endingBefore) { - appliedFilters = { - and: [ - appliedFilters || {}, - { id: { lt: this.parseCursor(inputs.endingBefore).id } }, - ], - }; + if (cursor) { + const cursorArgFilter = computeCursorArgFilter( + this.parseCursor(cursor), + inputs.orderBy || [], + objectMetadata.objectMetadataMapItem.fieldsByName, + isForwardPagination, + ); + + appliedFilters = (inputs.filter + ? { + and: [inputs.filter, { or: cursorArgFilter }], + } + : { or: cursorArgFilter }) as unknown as ObjectRecordFilter; } return appliedFilters; diff --git a/packages/twenty-server/src/engine/api/rest/input-factories/__tests__/order-by-input.factory.spec.ts b/packages/twenty-server/src/engine/api/rest/input-factories/__tests__/order-by-input.factory.spec.ts index 674c40352..18af04d20 100644 --- a/packages/twenty-server/src/engine/api/rest/input-factories/__tests__/order-by-input.factory.spec.ts +++ b/packages/twenty-server/src/engine/api/rest/input-factories/__tests__/order-by-input.factory.spec.ts @@ -37,7 +37,10 @@ describe('OrderByInputFactory', () => { it('should return default if order by missing', () => { const request: any = { query: {} }; - expect(service.create(request, objectMetadata)).toEqual([{}]); + expect(service.create(request, objectMetadata)).toEqual([ + {}, + { id: OrderByDirection.AscNullsFirst }, + ]); }); it('should create order by parser properly', () => { @@ -50,6 +53,7 @@ describe('OrderByInputFactory', () => { expect(service.create(request, objectMetadata)).toEqual([ { fieldNumber: OrderByDirection.AscNullsFirst }, { fieldText: OrderByDirection.DescNullsLast }, + { id: OrderByDirection.AscNullsFirst }, ]); }); @@ -62,6 +66,7 @@ describe('OrderByInputFactory', () => { expect(service.create(request, objectMetadata)).toEqual([ { fieldNumber: OrderByDirection.AscNullsFirst }, + { id: OrderByDirection.AscNullsFirst }, ]); }); @@ -74,6 +79,7 @@ describe('OrderByInputFactory', () => { expect(service.create(request, objectMetadata)).toEqual([ { fieldCurrency: { amountMicros: OrderByDirection.AscNullsFirst } }, + { id: OrderByDirection.AscNullsFirst }, ]); }); @@ -86,6 +92,7 @@ describe('OrderByInputFactory', () => { expect(service.create(request, objectMetadata)).toEqual([ { fieldCurrency: { amountMicros: OrderByDirection.DescNullsLast } }, + { id: OrderByDirection.AscNullsFirst }, ]); }); @@ -100,6 +107,7 @@ describe('OrderByInputFactory', () => { expect(service.create(request, objectMetadata)).toEqual([ { fieldCurrency: { amountMicros: OrderByDirection.DescNullsLast } }, { fieldText: { label: OrderByDirection.AscNullsLast } }, + { id: OrderByDirection.AscNullsFirst }, ]); }); diff --git a/packages/twenty-server/src/engine/api/rest/input-factories/order-by-input.factory.ts b/packages/twenty-server/src/engine/api/rest/input-factories/order-by-input.factory.ts index 554d171c0..a3a942578 100644 --- a/packages/twenty-server/src/engine/api/rest/input-factories/order-by-input.factory.ts +++ b/packages/twenty-server/src/engine/api/rest/input-factories/order-by-input.factory.ts @@ -25,7 +25,7 @@ export class OrderByInputFactory { const orderByQuery = request.query.order_by; if (typeof orderByQuery !== 'string') { - return [{}]; + return this.addDefaultOrderById([{}]); } //orderByQuery = field_1[AscNullsFirst],field_2[DescNullsLast],field_3 @@ -82,6 +82,14 @@ export class OrderByInputFactory { checkArrayFields(objectMetadata.objectMetadataMapItem, result); - return result; + return this.addDefaultOrderById(result); + } + + addDefaultOrderById(orderBy: ObjectRecordOrderBy) { + const hasIdOrder = orderBy.some((o) => Object.keys(o).includes('id')); + + return hasIdOrder + ? orderBy + : [...orderBy, { id: OrderByDirection.AscNullsFirst }]; } } diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/format-result.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/format-result.util.ts index f66b014e8..986310311 100644 --- a/packages/twenty-server/src/engine/twenty-orm/utils/format-result.util.ts +++ b/packages/twenty-server/src/engine/twenty-orm/utils/format-result.util.ts @@ -17,7 +17,7 @@ import { isValidDate } from 'src/utils/date/isValidDate'; export function formatResult( data: any, - objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps, + objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps | undefined, objectMetadataMaps: ObjectMetadataMaps, ): T { if (!data) { diff --git a/packages/twenty-server/test/integration/rest/suites/rest-api-core-find-many.integration-spec.ts b/packages/twenty-server/test/integration/rest/suites/rest-api-core-find-many.integration-spec.ts index d63c9ccd1..712994e18 100644 --- a/packages/twenty-server/test/integration/rest/suites/rest-api-core-find-many.integration-spec.ts +++ b/packages/twenty-server/test/integration/rest/suites/rest-api-core-find-many.integration-spec.ts @@ -206,6 +206,32 @@ describe('Core REST API Find Many endpoint', () => { ); }); + it('should support pagination with ordering', async () => { + const descResponse = await makeRestAPIRequest({ + method: 'get', + path: '/people?order_by=position[DescNullsLast]&limit=2', + }).expect(200); + + const descPeople = descResponse.body.data.people; + const endingBefore = descResponse.body.pageInfo.endCursor; + const lastPosition = descPeople[descPeople.length - 1].position; + + expect(descResponse.body.pageInfo.hasNextPage).toBe(true); + expect(descPeople.length).toEqual(2); + expect(lastPosition).toEqual(2); + + const descResponseWithPaginationResponse = await makeRestAPIRequest({ + method: 'get', + path: `/people?order_by=position[DescNullsLast]&limit=2&starting_after=${endingBefore}`, + }).expect(200); + + const descResponseWithPagination = + descResponseWithPaginationResponse.body.data.people; + + expect(descResponseWithPagination.length).toEqual(2); + expect(descResponseWithPagination[0].position).toEqual(lastPosition - 1); + }); + it('should handle invalid cursor gracefully', async () => { await makeRestAPIRequest({ method: 'get',