From 58d25ebd8368bc68137d522cac2b539bad5626de Mon Sep 17 00:00:00 2001 From: Etienne <45695613+etiennejouan@users.noreply.github.com> Date: Thu, 7 Aug 2025 00:52:28 +0200 Subject: [PATCH] CreateMany optim - .save -> .updateMany + position (#13704) fixes : https://github.com/twentyhq/core-team-issues/issues/1312 --------- Co-authored-by: Charles Bochet --- ...phql-query-create-many-resolver.service.ts | 52 +++-- .../query-runner-args.factory.spec.ts | 65 ++++-- .../factories/query-runner-args.factory.ts | 202 +++++++----------- .../services/record-position.service.ts | 91 +++++++- .../workspace-update-query-builder.ts | 18 +- 5 files changed, 256 insertions(+), 172 deletions(-) diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-create-many-resolver.service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-create-many-resolver.service.ts index bd992d9777..4c713d71ba 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-create-many-resolver.service.ts +++ b/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 & { 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[]> { + ): Promise { 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[], + existingRecords: PartialObjectRecordWithId[], ): { - recordsToUpdate: Partial[]; + recordsToUpdate: PartialObjectRecordWithId[]; recordsToInsert: Partial[]; } { - const recordsToUpdate: Partial[] = []; + const recordsToUpdate: PartialObjectRecordWithId[] = []; const recordsToInsert: Partial[] = []; for (const record of records) { - let existingRecord: Partial | 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[]; + partialRecordsToUpdate: PartialObjectRecordWithId[]; repository: WorkspaceRepository; 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, + record: PartialObjectRecordWithId, objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps, - ) { + ): Omit { let recordWithoutCreatedByUpdate = record; const createdByFieldMetadataId = diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts index 01fbc443c5..a21ec50977 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/query-runner-args.factory.spec.ts +++ b/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 }], diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory.ts index 934f2bff9d..400d23bb29 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory.ts +++ b/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 | undefined, + partialRecordInputs: Partial[] | undefined, options: WorkspaceQueryRunnerOptions, - argPositionBackfillInput: ArgPositionBackfillInput, - ): Promise> { - if (!isDefined(data)) { - return Promise.resolve({}); + shouldBackfillPositionIfUndefined = true, + ): Promise[]> { + if (!isDefined(partialRecordInputs)) { + return []; } + const allOverriddenRecords: Partial[] = []; + 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( diff --git a/packages/twenty-server/src/engine/core-modules/record-position/services/record-position.service.ts b/packages/twenty-server/src/engine/core-modules/record-position/services/record-position.service.ts index 30d2eaedb3..3a9c92cc53 100644 --- a/packages/twenty-server/src/engine/core-modules/record-position/services/record-position.service.ts +++ b/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[]; + workspaceId: string; + objectMetadata: { + isCustom: boolean; + nameSingular: string; + fieldIdByName: Record; + }; + shouldBackfillPositionIfUndefined: boolean; + }): Promise[]> { + const recordsThatNeedFirstPosition: Partial[] = []; + const recordsThatNeedLastPosition: Partial[] = []; + const recordsWithExistingNumberPosition: Partial[] = []; + const recordsThatShouldNotBeUpdated: Partial[] = []; + + 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'); } } diff --git a/packages/twenty-server/src/engine/twenty-orm/repository/workspace-update-query-builder.ts b/packages/twenty-server/src/engine/twenty-orm/repository/workspace-update-query-builder.ts index 67a9129c89..96710abccb 100644 --- a/packages/twenty-server/src/engine/twenty-orm/repository/workspace-update-query-builder.ts +++ b/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( - 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; }[], ): this { + const mainAliasTarget = this.getMainAliasTarget(); + + this.relationNestedConfig = + this.relationNestedQueries.prepareNestedRelationQueries( + inputs.map( + (input) => input.partialEntity, + ) as QueryDeepPartialEntityWithNestedRelationFields[], + mainAliasTarget, + ); this.manyInputs = inputs; return this;