959 api rest startingafter and endingbefore not working properly with orderby (#12012)

Fixes https://github.com/twentyhq/core-team-issues/issues/959

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 } =
+    const {
-      await this.findRecords({
-        request,
-        repository,
-        objectMetadata,
-        objectMetadataNameSingular,
-        objectMetadataItemWithFieldsMaps,
-      });
-
-    return this.formatPaginatedResult(
       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,
+    });
   }
 }

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(
+  formatPaginatedResult({
-    finalRecords: any[],
+    finalRecords,
-    objectMetadataNamePlural: string,
+    objectMetadataNamePlural,
-    isForwardPagination: boolean,
+    isForwardPagination,
-    hasMoreRecords: boolean,
+    hasMoreRecords,
-    totalCount: number,
+    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:
+        startCursor,
-          finalRecords.length > 0
+        endCursor,
-            ? 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,
       },
       totalCount,
     });
@@ -289,7 +289,10 @@ export abstract class RestApiBaseHandler {
     request: Request;
     recordId?: string;
     repository: WorkspaceRepository<ObjectLiteral>;
-    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<ObjectRecord[]>(
+      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<ObjectLiteral[]>(
+      records: finalRecords,
-        records,
-        objectMetadataItemWithFieldsMaps as any,
-        objectMetadata.objectMetadataMaps,
-      ),
       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) {
+    const cursor = inputs.startingAfter || inputs.endingBefore;
-      appliedFilters = {
-        and: [
-          appliedFilters || {},
-          { id: { gt: this.parseCursor(inputs.startingAfter).id } },
-        ],
-      };
-    }
 
-    if (inputs.endingBefore) {
+    if (cursor) {
-      appliedFilters = {
+      const cursorArgFilter = computeCursorArgFilter(
-        and: [
+        this.parseCursor(cursor),
-          appliedFilters || {},
+        inputs.orderBy || [],
-          { id: { lt: this.parseCursor(inputs.endingBefore).id } },
+        objectMetadata.objectMetadataMapItem.fieldsByName,
-        ],
+        isForwardPagination,
-      };
+      );
+
+      appliedFilters = (inputs.filter
+        ? {
+            and: [inputs.filter, { or: cursorArgFilter }],
+          }
+        : { or: cursorArgFilter }) as unknown as ObjectRecordFilter;
     }
 
     return appliedFilters;

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 },
       ]);
     });
 

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 }];
   }
 }

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<T>(
   data: any,
-  objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps,
+  objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps | undefined,
   objectMetadataMaps: ObjectMetadataMaps,
 ): T {
   if (!data) {

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',