Backfill position when not input (#5696)

- refactor record position factory and record position query factory
- override position if not present during createMany

To avoid overriding the same positions for all data in createMany, the
logic is:
- if inserted last, use last position + arg index + 1
- if inserted first, use first position - arg index - 1

packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/__tests__/record-position-query.factory.spec.ts

@@ -16,74 +16,66 @@ describe('RecordPositionQueryFactory', () => {
   });
 
   describe('create', () => {
-    describe('createForGet', () => {
+    it('should return query and params for FIND_BY_POSITION', async () => {
-      it('should return a string with the position when positionValue is first', async () => {
-        const positionValue = 'first';
-
-        const result = await factory.create(
-          RecordPositionQueryType.GET,
-          positionValue,
-          objectMetadataItem,
-          dataSourceSchema,
-        );
-
-        expect(result).toEqual(
-          `SELECT position FROM workspace_test."company"
-            WHERE "position" IS NOT NULL ORDER BY "position" ASC LIMIT 1`,
-        );
-      });
-
-      it('should return a string with the position when positionValue is last', async () => {
-        const positionValue = 'last';
-
-        const result = await factory.create(
-          RecordPositionQueryType.GET,
-          positionValue,
-          objectMetadataItem,
-          dataSourceSchema,
-        );
-
-        expect(result).toEqual(
-          `SELECT position FROM workspace_test."company"
-            WHERE "position" IS NOT NULL ORDER BY "position" DESC LIMIT 1`,
-        );
-      });
-    });
-
-    it('should return a string with the position when positionValue is a number', async () => {
       const positionValue = 1;
-
+      const queryType = RecordPositionQueryType.FIND_BY_POSITION;
-      try {
+      const [query, params] = await factory.create(
-        await factory.create(
+        { positionValue, recordPositionQueryType: queryType },
-          RecordPositionQueryType.GET,
-          positionValue,
-          objectMetadataItem,
-          dataSourceSchema,
-        );
-      } catch (error) {
-        expect(error.message).toEqual(
-          'RecordPositionQueryType.GET requires positionValue to be a number',
-        );
-      }
-    });
-  });
-
-  describe('createForUpdate', () => {
-    it('should return a string when RecordPositionQueryType is UPDATE', async () => {
-      const positionValue = 1;
-
-      const result = await factory.create(
-        RecordPositionQueryType.UPDATE,
-        positionValue,
         objectMetadataItem,
         dataSourceSchema,
       );
 
-      expect(result).toEqual(
+      expect(query).toEqual(
-        `UPDATE workspace_test."company"
+        `SELECT position FROM ${dataSourceSchema}."${objectMetadataItem.nameSingular}"
+            WHERE "position" = $1`,
+      );
+      expect(params).toEqual([positionValue]);
+    });
+
+    it('should return query and params for FIND_MIN_POSITION', async () => {
+      const queryType = RecordPositionQueryType.FIND_MIN_POSITION;
+      const [query, params] = await factory.create(
+        { recordPositionQueryType: queryType },
+        objectMetadataItem,
+        dataSourceSchema,
+      );
+
+      expect(query).toEqual(
+        `SELECT MIN(position) as position FROM ${dataSourceSchema}."${objectMetadataItem.nameSingular}"`,
+      );
+      expect(params).toEqual([]);
+    });
+
+    it('should return query and params for FIND_MAX_POSITION', async () => {
+      const queryType = RecordPositionQueryType.FIND_MAX_POSITION;
+      const [query, params] = await factory.create(
+        { recordPositionQueryType: queryType },
+        objectMetadataItem,
+        dataSourceSchema,
+      );
+
+      expect(query).toEqual(
+        `SELECT MAX(position) as position FROM ${dataSourceSchema}."${objectMetadataItem.nameSingular}"`,
+      );
+      expect(params).toEqual([]);
+    });
+
+    it('should return query and params for UPDATE_POSITION', async () => {
+      const positionValue = 1;
+      const recordId = '1';
+      const queryType = RecordPositionQueryType.UPDATE_POSITION;
+      const [query, params] = await factory.create(
+        { positionValue, recordId, recordPositionQueryType: queryType },
+        objectMetadataItem,
+        dataSourceSchema,
+      );
+
+      expect(query).toEqual(
+        `UPDATE ${dataSourceSchema}."${objectMetadataItem.nameSingular}"
             SET "position" = $1
             WHERE "id" = $2`,
       );
+      expect(params).toEqual([positionValue, recordId]);
     });
   });
 });

packages/twenty-server/src/engine/api/graphql/workspace-query-builder/factories/record-position-query.factory.ts

@@ -1,54 +1,119 @@
 import { Injectable } from '@nestjs/common';
 
+import { computeTableName } from 'src/engine/utils/compute-table-name.util';
+
 export enum RecordPositionQueryType {
-  GET = 'GET',
+  FIND_MIN_POSITION = 'FIND_MIN_POSITION',
-  UPDATE = 'UPDATE',
+  FIND_MAX_POSITION = 'FIND_MAX_POSITION',
+  FIND_BY_POSITION = 'FIND_BY_POSITION',
+  UPDATE_POSITION = 'UPDATE_POSITION',
 }
 
+type FindByPositionQueryArgs = {
+  positionValue: number;
+  recordPositionQueryType: RecordPositionQueryType.FIND_BY_POSITION;
+};
+
+type FindMinPositionQueryArgs = {
+  recordPositionQueryType: RecordPositionQueryType.FIND_MIN_POSITION;
+};
+
+type FindMaxPositionQueryArgs = {
+  recordPositionQueryType: RecordPositionQueryType.FIND_MAX_POSITION;
+};
+
+type UpdatePositionQueryArgs = {
+  recordId: string;
+  positionValue: number;
+  recordPositionQueryType: RecordPositionQueryType.UPDATE_POSITION;
+};
+
+type RecordPositionQuery = string;
+
+type RecordPositionQueryParams = any[];
+
+export type RecordPositionQueryArgs =
+  | FindByPositionQueryArgs
+  | FindMinPositionQueryArgs
+  | FindMaxPositionQueryArgs
+  | UpdatePositionQueryArgs;
+
 @Injectable()
 export class RecordPositionQueryFactory {
-  async create(
+  create(
-    recordPositionQueryType: RecordPositionQueryType,
+    recordPositionQueryArgs: RecordPositionQueryArgs,
-    positionValue: 'first' | 'last' | number,
     objectMetadata: { isCustom: boolean; nameSingular: string },
     dataSourceSchema: string,
-  ): Promise<string> {
+  ): [RecordPositionQuery, RecordPositionQueryParams] {
-    const name =
+    const name = computeTableName(
-      (objectMetadata.isCustom ? '_' : '') + objectMetadata.nameSingular;
+      objectMetadata.nameSingular,
+      objectMetadata.isCustom,
+    );
 
-    switch (recordPositionQueryType) {
+    switch (recordPositionQueryArgs.recordPositionQueryType) {
-      case RecordPositionQueryType.GET:
+      case RecordPositionQueryType.FIND_BY_POSITION:
-        if (typeof positionValue === 'number') {
+        return this.buildFindByPositionQuery(
-          throw new Error(
+          recordPositionQueryArgs satisfies FindByPositionQueryArgs,
-            'RecordPositionQueryType.GET requires positionValue to be a number',
+          name,
-          );
+          dataSourceSchema,
-        }
+        );
-
+      case RecordPositionQueryType.FIND_MIN_POSITION:
-        return this.createForGet(positionValue, name, dataSourceSchema);
+        return this.buildFindMinPositionQuery(name, dataSourceSchema);
-      case RecordPositionQueryType.UPDATE:
+      case RecordPositionQueryType.FIND_MAX_POSITION:
-        return this.createForUpdate(name, dataSourceSchema);
+        return this.buildFindMaxPositionQuery(name, dataSourceSchema);
+      case RecordPositionQueryType.UPDATE_POSITION:
+        return this.buildUpdatePositionQuery(
+          recordPositionQueryArgs satisfies UpdatePositionQueryArgs,
+          name,
+          dataSourceSchema,
+        );
       default:
         throw new Error('Invalid RecordPositionQueryType');
     }
   }
 
-  private async createForGet(
+  private buildFindByPositionQuery(
-    positionValue: 'first' | 'last',
+    { positionValue }: FindByPositionQueryArgs,
     name: string,
     dataSourceSchema: string,
-  ): Promise<string> {
+  ): [RecordPositionQuery, RecordPositionQueryParams] {
-    const orderByDirection = positionValue === 'first' ? 'ASC' : 'DESC';
+    return [
-
+      `SELECT position FROM ${dataSourceSchema}."${name}"
-    return `SELECT position FROM ${dataSourceSchema}."${name}"
+            WHERE "position" = $1`,
-            WHERE "position" IS NOT NULL ORDER BY "position" ${orderByDirection} LIMIT 1`;
+      [positionValue],
+    ];
   }
 
-  private async createForUpdate(
+  private buildFindMaxPositionQuery(
     name: string,
     dataSourceSchema: string,
-  ): Promise<string> {
+  ): [RecordPositionQuery, RecordPositionQueryParams] {
-    return `UPDATE ${dataSourceSchema}."${name}"
+    return [
+      `SELECT MAX(position) as position FROM ${dataSourceSchema}."${name}"`,
+      [],
+    ];
+  }
+
+  private buildFindMinPositionQuery(
+    name: string,
+    dataSourceSchema: string,
+  ): [RecordPositionQuery, RecordPositionQueryParams] {
+    return [
+      `SELECT MIN(position) as position FROM ${dataSourceSchema}."${name}"`,
+      [],
+    ];
+  }
+
+  private buildUpdatePositionQuery(
+    { recordId, positionValue }: UpdatePositionQueryArgs,
+    name: string,
+    dataSourceSchema: string,
+  ): [RecordPositionQuery, RecordPositionQueryParams] {
+    return [
+      `UPDATE ${dataSourceSchema}."${name}"
             SET "position" = $1
-            WHERE "id" = $2`;
+            WHERE "id" = $2`,
+      [positionValue, recordId],
+    ];
   }
 }

packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts

@@ -12,12 +12,14 @@ describe('QueryRunnerArgsFactory', () => {
   const recordPositionFactory = {
     create: jest.fn().mockResolvedValue(2),
   };
+  const workspaceId = 'workspaceId';
   const options = {
     fieldMetadataCollection: [
       { name: 'position', type: FieldMetadataType.POSITION },
       { name: 'testNumber', type: FieldMetadataType.NUMBER },
     ] as FieldMetadataInterface[],
     objectMetadataItem: { isCustom: true, nameSingular: 'test' },
+    workspaceId,
   } as WorkspaceQueryRunnerOptions;
 
   let factory: QueryRunnerArgsFactory;
@@ -68,6 +70,36 @@ describe('QueryRunnerArgsFactory', () => {
         ResolverArgsType.CreateMany,
       );
 
+      expect(recordPositionFactory.create).toHaveBeenCalledWith(
+        'last',
+        { isCustom: true, nameSingular: 'test' },
+        workspaceId,
+        0,
+      );
+      expect(result).toEqual({
+        id: 'uuid',
+        data: [{ position: 2, testNumber: 1 }],
+      });
+    });
+
+    it('createMany type should override position if not present', async () => {
+      const args = {
+        id: 'uuid',
+        data: [{ testNumber: '1' }],
+      };
+
+      const result = await factory.create(
+        args,
+        options,
+        ResolverArgsType.CreateMany,
+      );
+
+      expect(recordPositionFactory.create).toHaveBeenCalledWith(
+        'first',
+        { isCustom: true, nameSingular: 'test' },
+        workspaceId,
+        0,
+      );
       expect(result).toEqual({
         id: 'uuid',
         data: [{ position: 2, testNumber: 1 }],

packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/record-position.factory.spec.ts

@@ -9,14 +9,15 @@ describe('RecordPositionFactory', () => {
     create: jest.fn().mockResolvedValue('query'),
   };
 
-  const workspaceDataSourceService = {
+  let workspaceDataSourceService;
-    getSchemaName: jest.fn().mockReturnValue('schemaName'),
-    executeRawQuery: jest.fn().mockResolvedValue([{ position: 1 }]),
-  };
 
   let factory: RecordPositionFactory;
 
   beforeEach(async () => {
+    workspaceDataSourceService = {
+      getSchemaName: jest.fn().mockReturnValue('schemaName'),
+      executeRawQuery: jest.fn().mockResolvedValue([{ position: 1 }]),
+    };
     const module: TestingModule = await Test.createTestingModule({
       providers: [
         RecordPositionFactory,
@@ -44,10 +45,20 @@ describe('RecordPositionFactory', () => {
 
     it('should return the value when value is a number', async () => {
       const value = 1;
+
+      workspaceDataSourceService.executeRawQuery.mockResolvedValue([]);
+
       const result = await factory.create(value, objectMetadata, workspaceId);
 
       expect(result).toEqual(value);
     });
+    it('should throw an error when position is not unique', async () => {
+      const value = 1;
+
+      await expect(
+        factory.create(value, objectMetadata, workspaceId),
+      ).rejects.toThrow('Position is not unique');
+    });
     it('should return the existing position -1 when value is first', async () => {
       const value = 'first';
       const result = await factory.create(value, objectMetadata, workspaceId);

packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory.ts

@@ -16,6 +16,11 @@ import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/fi
 
 import { RecordPositionFactory } from './record-position.factory';
 
+type ArgPositionBackfillInput = {
+  argIndex?: number;
+  shouldBackfillPosition: boolean;
+};
+
 @Injectable()
 export class QueryRunnerArgsFactory {
   constructor(private readonly recordPositionFactory: RecordPositionFactory) {}
@@ -39,8 +44,11 @@ export class QueryRunnerArgsFactory {
         return {
           ...args,
           data: await Promise.all(
-            (args as CreateManyResolverArgs).data.map((arg) =>
+            (args as CreateManyResolverArgs).data.map((arg, index) =>
-              this.overrideDataByFieldMetadata(arg, options, fieldMetadataMap),
+              this.overrideDataByFieldMetadata(arg, options, fieldMetadataMap, {
+                argIndex: index,
+                shouldBackfillPosition: true,
+              }),
             ),
           ),
         } satisfies CreateManyResolverArgs;
@@ -73,6 +81,7 @@ export class QueryRunnerArgsFactory {
             (args as FindDuplicatesResolverArgs).data,
             options,
             fieldMetadataMap,
+            { shouldBackfillPosition: false },
           ),
         };
       default:
@@ -84,11 +93,14 @@ export class QueryRunnerArgsFactory {
     data: Record<string, any> | undefined,
     options: WorkspaceQueryRunnerOptions,
     fieldMetadataMap: Map<string, FieldMetadataInterface>,
+    argPositionBackfillInput: ArgPositionBackfillInput,
   ) {
     if (!data) {
       return;
     }
 
+    let isFieldPositionPresent = false;
+
     const createArgPromiseByArgKey = Object.entries(data).map(
       async ([key, value]) => {
         const fieldMetadata = fieldMetadataMap.get(key);
@@ -99,6 +111,8 @@ export class QueryRunnerArgsFactory {
 
         switch (fieldMetadata.type) {
           case FieldMetadataType.POSITION:
+            isFieldPositionPresent = true;
+
             return [
               key,
               await this.recordPositionFactory.create(
@@ -108,6 +122,7 @@ export class QueryRunnerArgsFactory {
                   nameSingular: options.objectMetadataItem.nameSingular,
                 },
                 options.workspaceId,
+                argPositionBackfillInput.argIndex,
               ),
             ];
           case FieldMetadataType.NUMBER:
@@ -120,6 +135,27 @@ export class QueryRunnerArgsFactory {
 
     const newArgEntries = await Promise.all(createArgPromiseByArgKey);
 
+    if (
+      !isFieldPositionPresent &&
+      argPositionBackfillInput.shouldBackfillPosition
+    ) {
+      return Object.fromEntries([
+        ...newArgEntries,
+        [
+          'position',
+          await this.recordPositionFactory.create(
+            'first',
+            {
+              isCustom: options.objectMetadataItem.isCustom,
+              nameSingular: options.objectMetadataItem.nameSingular,
+            },
+            options.workspaceId,
+            argPositionBackfillInput.argIndex,
+          ),
+        ],
+      ]);
+    }
+
     return Object.fromEntries(newArgEntries);
   }
 

packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/record-position.factory.ts

@@ -4,6 +4,7 @@ import { isDefined } from 'class-validator';
 
 import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service';
 import {
+  RecordPositionQueryArgs,
   RecordPositionQueryFactory,
   RecordPositionQueryType,
 } from 'src/engine/api/graphql/workspace-query-builder/factories/record-position-query.factory';
@@ -19,40 +20,76 @@ export class RecordPositionFactory {
     value: number | 'first' | 'last',
     objectMetadata: { isCustom: boolean; nameSingular: string },
     workspaceId: string,
+    index = 0,
   ): Promise<number> {
-    if (typeof value === 'number') {
-      return value;
-    }
-
     const dataSourceSchema =
       this.workspaceDataSourceService.getSchemaName(workspaceId);
 
-    const query = await this.recordPositionQueryFactory.create(
+    if (typeof value === 'number') {
-      RecordPositionQueryType.GET,
+      const recordWithSamePosition = await this.findRecordPosition(
-      value,
+        {
+          recordPositionQueryType: RecordPositionQueryType.FIND_BY_POSITION,
+          positionValue: value,
+        },
+        objectMetadata,
+        dataSourceSchema,
+        workspaceId,
+      );
+
+      if (recordWithSamePosition) {
+        throw new Error('Position is not unique');
+      }
+
+      return value;
+    }
+
+    if (value === 'first') {
+      const recordWithMinPosition = await this.findRecordPosition(
+        {
+          recordPositionQueryType: RecordPositionQueryType.FIND_MIN_POSITION,
+        },
+        objectMetadata,
+        dataSourceSchema,
+        workspaceId,
+      );
+
+      return isDefined(recordWithMinPosition?.position)
+        ? recordWithMinPosition.position - index - 1
+        : 1;
+    }
+
+    const recordWithMaxPosition = await this.findRecordPosition(
+      {
+        recordPositionQueryType: RecordPositionQueryType.FIND_MAX_POSITION,
+      },
+      objectMetadata,
+      dataSourceSchema,
+      workspaceId,
+    );
+
+    return isDefined(recordWithMaxPosition?.position)
+      ? recordWithMaxPosition.position + index + 1
+      : 1;
+  }
+
+  private async findRecordPosition(
+    recordPositionQueryArgs: RecordPositionQueryArgs,
+    objectMetadata: { isCustom: boolean; nameSingular: string },
+    dataSourceSchema: string,
+    workspaceId: string,
+  ) {
+    const [query, params] = await this.recordPositionQueryFactory.create(
+      recordPositionQueryArgs,
       objectMetadata,
       dataSourceSchema,
     );
 
-    // If the value was 'first', the first record will be the one with the lowest position
-    // If the value was 'last', the first record will be the one with the highest position
     const records = await this.workspaceDataSourceService.executeRawQuery(
       query,
-      [],
+      params,
       workspaceId,
-      undefined,
     );
 
-    if (
+    return records?.[0];
-      !isDefined(records) ||
-      records.length === 0 ||
-      !isDefined(records[0]?.position)
-    ) {
-      return 1;
-    }
-
-    return value === 'first'
-      ? records[0].position - 1
-      : records[0].position + 1;
   }
 }

packages/twenty-server/src/engine/api/graphql/workspace-query-runner/services/record-position-backfill-service.ts

@@ -31,16 +31,19 @@ export class RecordPositionBackfillService {
     const dataSourceSchema =
       this.workspaceDataSourceService.getSchemaName(workspaceId);
 
-    const query = await this.recordPositionQueryFactory.create(
+    const [query, params] = await this.recordPositionQueryFactory.create(
-      RecordPositionQueryType.UPDATE,
+      {
-      position,
+        recordPositionQueryType: RecordPositionQueryType.UPDATE_POSITION,
+        recordId,
+        positionValue: position,
+      },
       objectMetadata as ObjectMetadataInterface,
       dataSourceSchema,
     );
 
     this.workspaceDataSourceService.executeRawQuery(
       query,
-      [position, recordId],
+      params,
       workspaceId,
       undefined,
     );