CreateMany optim - .save -> .updateMany + position (#13704)

fixes : https://github.com/twentyhq/core-team-issues/issues/1312

---------

Co-authored-by: Charles Bochet <charles@twenty.com>

packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-create-many-resolver.service.ts

@@ -27,6 +27,7 @@ import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/typ
 import { ObjectMetadataMaps } from 'src/engine/metadata-modules/types/object-metadata-maps';
 import { WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository';
 
+type PartialObjectRecordWithId = Partial<ObjectRecord> & { id: string };
 @Injectable()
 export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResolverService<
   CreateManyResolverArgs,
@@ -116,13 +117,15 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol
       objectMetadataItemWithFieldMaps,
     });
 
-    await this.processRecordsToUpdate({
-      partialRecordsToUpdate: recordsToUpdate,
-      repository: executionArgs.repository,
-      objectMetadataItemWithFieldMaps,
-      result,
-      columnsToReturn,
-    });
+    if (recordsToUpdate.length > 0) {
+      await this.processRecordsToUpdate({
+        partialRecordsToUpdate: recordsToUpdate,
+        repository: executionArgs.repository,
+        objectMetadataItemWithFieldMaps,
+        result,
+        columnsToReturn,
+      });
+    }
 
     await this.processRecordsToInsert({
       recordsToInsert,
@@ -181,7 +184,7 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol
       fullPath: string;
       column: string;
     }[],
-  ): Promise<Partial<ObjectRecord>[]> {
+  ): Promise<PartialObjectRecordWithId[]> {
     const { objectMetadataItemWithFieldMaps } = executionArgs.options;
     const queryBuilder = executionArgs.repository.createQueryBuilder(
       objectMetadataItemWithFieldMaps.nameSingular,
@@ -208,12 +211,12 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol
       },
     });
 
-    return await queryBuilder
+    return (await queryBuilder
       .withDeleted()
       .setFindOptions({
         select: selectOptions,
       })
-      .getMany();
+      .getMany()) as PartialObjectRecordWithId[];
   }
 
   private getValueFromPath(
@@ -262,16 +265,16 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol
       fullPath: string;
       column: string;
     }[],
-    existingRecords: Partial<ObjectRecord>[],
+    existingRecords: PartialObjectRecordWithId[],
   ): {
-    recordsToUpdate: Partial<ObjectRecord>[];
+    recordsToUpdate: PartialObjectRecordWithId[];
     recordsToInsert: Partial<ObjectRecord>[];
   } {
-    const recordsToUpdate: Partial<ObjectRecord>[] = [];
+    const recordsToUpdate: PartialObjectRecordWithId[] = [];
     const recordsToInsert: Partial<ObjectRecord>[] = [];
 
     for (const record of records) {
-      let existingRecord: Partial<ObjectRecord> | null = null;
+      let existingRecord: PartialObjectRecordWithId | null = null;
 
       for (const field of conflictingFields) {
         const requestFieldValue = this.getValueFromPath(record, field.fullPath);
@@ -309,9 +312,9 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol
     repository,
     objectMetadataItemWithFieldMaps,
     result,
-    columnsToReturn: _,
+    columnsToReturn,
   }: {
-    partialRecordsToUpdate: Partial<ObjectRecord>[];
+    partialRecordsToUpdate: PartialObjectRecordWithId[];
     repository: WorkspaceRepository<ObjectLiteral>;
     objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps;
     result: InsertResult;
@@ -322,15 +325,20 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol
         this.getRecordWithoutCreatedBy(record, objectMetadataItemWithFieldMaps),
       );
 
-    const savedRecords = await repository.save(
-      partialRecordsToUpdateWithoutCreatedByUpdate,
+    const savedRecords = await repository.updateMany(
+      partialRecordsToUpdateWithoutCreatedByUpdate.map((record) => ({
+        criteria: record.id,
+        partialEntity: record,
+      })),
+      undefined,
+      columnsToReturn,
     );
 
     result.identifiers.push(
-      ...savedRecords.map((record) => ({ id: record.id })),
+      ...savedRecords.generatedMaps.map((record) => ({ id: record.id })),
     );
     result.generatedMaps.push(
-      ...savedRecords.map((record) => ({ id: record.id })),
+      ...savedRecords.generatedMaps.map((record) => ({ id: record.id })),
     );
   }
 
@@ -436,9 +444,9 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol
   }
 
   private getRecordWithoutCreatedBy(
-    record: Partial<ObjectRecord>,
+    record: PartialObjectRecordWithId,
     objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps,
-  ) {
+  ): Omit<PartialObjectRecordWithId, 'createdBy'> {
     let recordWithoutCreatedByUpdate = record;
 
     const createdByFieldMetadataId =

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

@@ -6,16 +6,27 @@ import { WorkspaceQueryRunnerOptions } from 'src/engine/api/graphql/workspace-qu
 import { ResolverArgsType } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface';
 
 import { QueryRunnerArgsFactory } from 'src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory';
-import {
-  RecordPositionService,
-  RecordPositionServiceCreateArgs,
-} from 'src/engine/core-modules/record-position/services/record-position.service';
+import { RecordPositionService } from 'src/engine/core-modules/record-position/services/record-position.service';
 import { RecordInputTransformerService } from 'src/engine/core-modules/record-transformer/services/record-input-transformer.service';
 import { FieldMetadataMap } from 'src/engine/metadata-modules/types/field-metadata-map';
 
 describe('QueryRunnerArgsFactory', () => {
   const recordPositionService = {
-    buildRecordPosition: jest.fn().mockResolvedValue(2),
+    overridePositionOnRecords: jest
+      .fn()
+      .mockImplementation(
+        ({ partialRecordInputs }: { partialRecordInputs: any[] }) => {
+          return Promise.resolve(
+            partialRecordInputs.map((record: any) => ({
+              ...record,
+              position:
+                record.position === 'last' || !record.position
+                  ? 2
+                  : record.position,
+            })),
+          );
+        },
+      ),
   };
   const workspaceId = 'workspaceId';
   const options = {
@@ -96,16 +107,24 @@ describe('QueryRunnerArgsFactory', () => {
         ResolverArgsType.CreateMany,
       );
 
-      const expectedArgs: RecordPositionServiceCreateArgs = {
-        value: 'last',
-        objectMetadata: { isCustom: true, nameSingular: 'testNumber' },
+      const expectedArgs = {
+        partialRecordInputs: [{ position: 'last', testNumber: 1 }],
+        objectMetadata: {
+          isCustom: true,
+          nameSingular: 'testNumber',
+          fieldIdByName: {
+            position: 'position-id',
+            testNumber: 'testNumber-id',
+            otherField: 'otherField-id',
+          },
+        },
         workspaceId,
-        index: 0,
+        shouldBackfillPositionIfUndefined: true,
       };
 
-      expect(recordPositionService.buildRecordPosition).toHaveBeenCalledWith(
-        expectedArgs,
-      );
+      expect(
+        recordPositionService.overridePositionOnRecords,
+      ).toHaveBeenCalledWith(expectedArgs);
       expect(result).toEqual({
         id: 'uuid',
         data: [{ position: 2, testNumber: 1 }],
@@ -124,16 +143,24 @@ describe('QueryRunnerArgsFactory', () => {
         ResolverArgsType.CreateMany,
       );
 
-      const expectedArgs: RecordPositionServiceCreateArgs = {
-        value: 'first',
-        objectMetadata: { isCustom: true, nameSingular: 'testNumber' },
+      const expectedArgs = {
+        partialRecordInputs: [{ testNumber: 1 }],
+        objectMetadata: {
+          isCustom: true,
+          nameSingular: 'testNumber',
+          fieldIdByName: {
+            position: 'position-id',
+            testNumber: 'testNumber-id',
+            otherField: 'otherField-id',
+          },
+        },
         workspaceId,
-        index: 0,
+        shouldBackfillPositionIfUndefined: true,
       };
 
-      expect(recordPositionService.buildRecordPosition).toHaveBeenCalledWith(
-        expectedArgs,
-      );
+      expect(
+        recordPositionService.overridePositionOnRecords,
+      ).toHaveBeenCalledWith(expectedArgs);
       expect(result).toEqual({
         id: 'uuid',
         data: [{ position: 2, testNumber: 1 }],

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

@@ -27,11 +27,6 @@ import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace.
 import { FieldMetadataMap } from 'src/engine/metadata-modules/types/field-metadata-map';
 import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps';
 
-type ArgPositionBackfillInput = {
-  argIndex?: number;
-  shouldBackfillPosition: boolean;
-};
-
 @Injectable()
 export class QueryRunnerArgsFactory {
   constructor(
@@ -47,50 +42,36 @@ export class QueryRunnerArgsFactory {
     const fieldMetadataMapByNameByName =
       options.objectMetadataItemWithFieldMaps.fieldsById;
 
-    const shouldBackfillPosition = Object.values(
-      options.objectMetadataItemWithFieldMaps.fieldsById,
-    ).some(
-      (field) =>
-        field.type === FieldMetadataType.POSITION && field.name === 'position',
-    );
-
     switch (resolverArgsType) {
       case ResolverArgsType.CreateOne:
         return {
           ...args,
-          data: await this.overrideDataByFieldMetadata(
-            (args as CreateOneResolverArgs).data,
-            options,
-            {
-              argIndex: 0,
-              shouldBackfillPosition,
-            },
-          ),
+          data: (
+            await this.overrideDataByFieldMetadata(
+              [(args as CreateOneResolverArgs).data],
+              options,
+            )
+          )[0],
         } satisfies CreateOneResolverArgs;
       case ResolverArgsType.CreateMany:
         return {
           ...args,
-          data: await Promise.all(
-            (args as CreateManyResolverArgs).data?.map((arg, index) =>
-              this.overrideDataByFieldMetadata(arg, options, {
-                argIndex: index,
-                shouldBackfillPosition,
-              }),
-            ) ?? [],
+          data: await this.overrideDataByFieldMetadata(
+            (args as CreateManyResolverArgs).data,
+            options,
           ),
         } satisfies CreateManyResolverArgs;
       case ResolverArgsType.UpdateOne:
         return {
           ...args,
           id: (args as UpdateOneResolverArgs).id,
-          data: await this.overrideDataByFieldMetadata(
-            (args as UpdateOneResolverArgs).data,
-            options,
-            {
-              argIndex: 0,
-              shouldBackfillPosition: false,
-            },
-          ),
+          data: (
+            await this.overrideDataByFieldMetadata(
+              [(args as UpdateOneResolverArgs).data],
+              options,
+              false,
+            )
+          )[0],
         } satisfies UpdateOneResolverArgs;
       case ResolverArgsType.UpdateMany:
         return {
@@ -99,14 +80,13 @@ export class QueryRunnerArgsFactory {
             (args as UpdateManyResolverArgs).filter,
             options.objectMetadataItemWithFieldMaps,
           ),
-          data: await this.overrideDataByFieldMetadata(
-            (args as UpdateManyResolverArgs).data,
-            options,
-            {
-              argIndex: 0,
-              shouldBackfillPosition: false,
-            },
-          ),
+          data: (
+            await this.overrideDataByFieldMetadata(
+              [(args as UpdateManyResolverArgs).data],
+              options,
+              false,
+            )
+          )[0],
         } satisfies UpdateManyResolverArgs;
       case ResolverArgsType.FindOne:
         return {
@@ -137,13 +117,10 @@ export class QueryRunnerArgsFactory {
               ),
             ) ?? [],
           )) as string[],
-          data: await Promise.all(
-            (args as FindDuplicatesResolverArgs).data?.map((arg, index) =>
-              this.overrideDataByFieldMetadata(arg, options, {
-                argIndex: index,
-                shouldBackfillPosition,
-              }),
-            ) ?? [],
+          data: await this.overrideDataByFieldMetadata(
+            (args as FindDuplicatesResolverArgs).data,
+            options,
+            false,
           ),
         } satisfies FindDuplicatesResolverArgs;
       case ResolverArgsType.MergeMany:
@@ -168,97 +145,70 @@ export class QueryRunnerArgsFactory {
   }
 
   private async overrideDataByFieldMetadata(
-    data: Partial<ObjectRecord> | undefined,
+    partialRecordInputs: Partial<ObjectRecord>[] | undefined,
     options: WorkspaceQueryRunnerOptions,
-    argPositionBackfillInput: ArgPositionBackfillInput,
-  ): Promise<Partial<ObjectRecord>> {
-    if (!isDefined(data)) {
-      return Promise.resolve({});
+    shouldBackfillPositionIfUndefined = true,
+  ): Promise<Partial<ObjectRecord>[]> {
+    if (!isDefined(partialRecordInputs)) {
+      return [];
     }
 
+    const allOverriddenRecords: Partial<ObjectRecord>[] = [];
+
     const workspace = options.authContext.workspace;
 
     workspaceValidator.assertIsDefinedOrThrow(workspace);
 
-    let isFieldPositionPresent = false;
+    const overriddenPositionRecords =
+      await this.recordPositionService.overridePositionOnRecords({
+        partialRecordInputs,
+        workspaceId: workspace.id,
+        objectMetadata: {
+          isCustom: options.objectMetadataItemWithFieldMaps.isCustom,
+          nameSingular: options.objectMetadataItemWithFieldMaps.nameSingular,
+          fieldIdByName: options.objectMetadataItemWithFieldMaps.fieldIdByName,
+        },
+        shouldBackfillPositionIfUndefined,
+      });
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    const createArgByArgKeyPromises: Promise<[string, any]>[] = Object.entries(
-      data,
+    for (const record of overriddenPositionRecords) {
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    ).map(async ([key, value]): Promise<[string, any]> => {
-      const fieldMetadataId =
-        options.objectMetadataItemWithFieldMaps.fieldIdByName[key];
-      const fieldMetadata =
-        options.objectMetadataItemWithFieldMaps.fieldsById[fieldMetadataId];
+      const createArgByArgKey: [string, any][] = await Promise.all(
+        Object.entries(record).map(async ([key, value]) => {
+          const fieldMetadataId =
+            options.objectMetadataItemWithFieldMaps.fieldIdByName[key];
+          const fieldMetadata =
+            options.objectMetadataItemWithFieldMaps.fieldsById[fieldMetadataId];
 
-      if (!fieldMetadata) {
-        return [key, value];
-      }
+          if (!fieldMetadata) {
+            return [key, value];
+          }
 
-      switch (fieldMetadata.type) {
-        case FieldMetadataType.POSITION: {
-          isFieldPositionPresent = true;
+          switch (fieldMetadata.type) {
+            case FieldMetadataType.NUMBER:
+            case FieldMetadataType.RICH_TEXT:
+            case FieldMetadataType.PHONES:
+            case FieldMetadataType.RICH_TEXT_V2:
+            case FieldMetadataType.LINKS:
+            case FieldMetadataType.EMAILS: {
+              const transformedValue =
+                await this.recordInputTransformerService.transformFieldValue(
+                  fieldMetadata.type,
+                  value,
+                );
 
-          const newValue = await this.recordPositionService.buildRecordPosition(
-            {
-              value,
-              workspaceId: workspace.id,
-              objectMetadata: {
-                isCustom: options.objectMetadataItemWithFieldMaps.isCustom,
-                nameSingular:
-                  options.objectMetadataItemWithFieldMaps.nameSingular,
-              },
-              index: argPositionBackfillInput.argIndex,
-            },
-          );
+              return [key, transformedValue];
+            }
+            default:
+              return [key, value];
+          }
+        }),
+      );
 
-          return [key, newValue];
-        }
-        case FieldMetadataType.NUMBER:
-        case FieldMetadataType.RICH_TEXT:
-        case FieldMetadataType.PHONES:
-        case FieldMetadataType.RICH_TEXT_V2:
-        case FieldMetadataType.LINKS:
-        case FieldMetadataType.EMAILS: {
-          const transformedValue =
-            await this.recordInputTransformerService.transformFieldValue(
-              fieldMetadata.type,
-              value,
-            );
-
-          return [key, transformedValue];
-        }
-        default:
-          return [key, value];
-      }
-    });
-
-    const newArgEntries = await Promise.all(createArgByArgKeyPromises);
-
-    if (
-      !isFieldPositionPresent &&
-      argPositionBackfillInput.shouldBackfillPosition
-    ) {
-      return Object.fromEntries([
-        ...newArgEntries,
-        [
-          'position',
-          await this.recordPositionService.buildRecordPosition({
-            value: 'first',
-            workspaceId: workspace.id,
-            objectMetadata: {
-              isCustom: options.objectMetadataItemWithFieldMaps.isCustom,
-              nameSingular:
-                options.objectMetadataItemWithFieldMaps.nameSingular,
-            },
-            index: argPositionBackfillInput.argIndex,
-          }),
-        ],
-      ]);
+      allOverriddenRecords.push(Object.fromEntries(createArgByArgKey));
     }
 
-    return Object.fromEntries(newArgEntries);
+    return allOverriddenRecords;
   }
 
   private overrideFilterByFieldMetadata(

packages/twenty-server/src/engine/core-modules/record-position/services/record-position.service.ts

@@ -1,5 +1,9 @@
 import { Injectable } from '@nestjs/common';
 
+import { isDefined } from 'twenty-shared/utils';
+
+import { ObjectRecord } from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface';
+
 import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
 
 export type RecordPositionServiceCreateArgs = {
@@ -46,6 +50,89 @@ export class RecordPositionService {
       : 1;
   }
 
+  async overridePositionOnRecords({
+    partialRecordInputs,
+    workspaceId,
+    objectMetadata,
+    shouldBackfillPositionIfUndefined,
+  }: {
+    partialRecordInputs: Partial<ObjectRecord>[];
+    workspaceId: string;
+    objectMetadata: {
+      isCustom: boolean;
+      nameSingular: string;
+      fieldIdByName: Record<string, string>;
+    };
+    shouldBackfillPositionIfUndefined: boolean;
+  }): Promise<Partial<ObjectRecord>[]> {
+    const recordsThatNeedFirstPosition: Partial<ObjectRecord>[] = [];
+    const recordsThatNeedLastPosition: Partial<ObjectRecord>[] = [];
+    const recordsWithExistingNumberPosition: Partial<ObjectRecord>[] = [];
+    const recordsThatShouldNotBeUpdated: Partial<ObjectRecord>[] = [];
+
+    const positionFieldId = objectMetadata.fieldIdByName['position'];
+
+    if (!isDefined(positionFieldId)) {
+      return partialRecordInputs;
+    }
+
+    for (const partialRecordInput of partialRecordInputs) {
+      if (partialRecordInput.position === 'last') {
+        recordsThatNeedLastPosition.push(partialRecordInput);
+      } else if (typeof partialRecordInput.position === 'number') {
+        recordsWithExistingNumberPosition.push(partialRecordInput);
+      } else if (partialRecordInput.position === 'first') {
+        recordsThatNeedFirstPosition.push(partialRecordInput);
+      } else if (
+        partialRecordInput.position === undefined &&
+        shouldBackfillPositionIfUndefined
+      ) {
+        recordsThatNeedFirstPosition.push(partialRecordInput);
+      } else {
+        recordsThatShouldNotBeUpdated.push(partialRecordInput);
+      }
+    }
+
+    if (recordsThatNeedFirstPosition.length > 0) {
+      const existingRecordMinPosition = await this.findMinPosition(
+        objectMetadata,
+        workspaceId,
+      );
+
+      const minPosition = Math.min(
+        ...recordsWithExistingNumberPosition.map((record) => record.position),
+        isDefined(existingRecordMinPosition) ? existingRecordMinPosition : 1,
+      );
+
+      for (const [index, record] of recordsThatNeedFirstPosition.entries()) {
+        record.position = minPosition - index - 1;
+      }
+    }
+
+    if (recordsThatNeedLastPosition.length > 0) {
+      const existingRecordMaxPosition = await this.findMaxPosition(
+        objectMetadata,
+        workspaceId,
+      );
+
+      const maxPosition = Math.max(
+        ...recordsThatNeedLastPosition.map((record) => record.position),
+        isDefined(existingRecordMaxPosition) ? existingRecordMaxPosition : 1,
+      );
+
+      for (const [index, record] of recordsThatNeedLastPosition.entries()) {
+        record.position = maxPosition + index + 1;
+      }
+    }
+
+    return [
+      ...recordsThatNeedFirstPosition,
+      ...recordsThatNeedLastPosition,
+      ...recordsWithExistingNumberPosition,
+      ...recordsThatShouldNotBeUpdated,
+    ];
+  }
+
   async findByPosition(
     positionValue: number | null,
     objectMetadata: { isCustom: boolean; nameSingular: string },
@@ -100,7 +187,7 @@ export class RecordPositionService {
         },
       );
 
-    return repository.minimum('position');
+    return await repository.minimum('position');
   }
 
   private async findMaxPosition(
@@ -116,6 +203,6 @@ export class RecordPositionService {
         },
       );
 
-    return repository.maximum('position');
+    return await repository.maximum('position');
   }
 }

packages/twenty-server/src/engine/twenty-orm/repository/workspace-update-query-builder.ts

@@ -236,7 +236,10 @@ export class WorkspaceUpdateQueryBuilder<
       const results: UpdateResult[] = [];
 
       for (const input of this.manyInputs) {
-        this.expressionMap.valuesSet = input.partialEntity;
+        this.expressionMap.valuesSet = formatData(
+          input.partialEntity,
+          objectMetadata,
+        );
         this.where({ id: input.criteria });
 
         const nestedRelationQueryBuilder = new WorkspaceSelectQueryBuilder(
@@ -284,13 +287,13 @@ export class WorkspaceUpdateQueryBuilder<
       });
 
       const formattedResults = formatResult<T[]>(
-        results.map((result) => result.raw),
+        results.flatMap((result) => result.raw),
         objectMetadata,
         this.internalContext.objectMetadataMaps,
       );
 
       return {
-        raw: results.map((result) => result.raw),
+        raw: results.flatMap((result) => result.raw),
         generatedMaps: formattedResults,
         affected: results.length,
       };
@@ -383,6 +386,15 @@ export class WorkspaceUpdateQueryBuilder<
       partialEntity: QueryDeepPartialEntity<T>;
     }[],
   ): this {
+    const mainAliasTarget = this.getMainAliasTarget();
+
+    this.relationNestedConfig =
+      this.relationNestedQueries.prepareNestedRelationQueries(
+        inputs.map(
+          (input) => input.partialEntity,
+        ) as QueryDeepPartialEntityWithNestedRelationFields<T>[],
+        mainAliasTarget,
+      );
     this.manyInputs = inputs;
 
     return this;