From aa5da92555c44646b360c4c4b8e953feaa2b6b38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Tue, 1 Apr 2025 15:27:08 +0200 Subject: [PATCH] Improve upsert for spreadsheet import (#11283) Refactor query runner to improve the import method for upserts, we now take into account any unique field and prevent any conflict upfront. Previously, we would only update if an `id` was passed. https://github.com/user-attachments/assets/8087b864-ba42-4b6e-abf2-b9ea66e6c467 This is only a first step, there are other things to fix on the frontend for this to work. --- .../components/MatchColumnSelect.tsx | 8 +- ...phql-query-create-many-resolver.service.ts | 326 ++++++++++++++++-- 2 files changed, 304 insertions(+), 30 deletions(-) diff --git a/packages/twenty-front/src/modules/spreadsheet-import/components/MatchColumnSelect.tsx b/packages/twenty-front/src/modules/spreadsheet-import/components/MatchColumnSelect.tsx index 84f4954b7..5a576030e 100644 --- a/packages/twenty-front/src/modules/spreadsheet-import/components/MatchColumnSelect.tsx +++ b/packages/twenty-front/src/modules/spreadsheet-import/components/MatchColumnSelect.tsx @@ -7,7 +7,7 @@ import { size, useFloating, } from '@floating-ui/react'; -import React, { useCallback, useRef, useState } from 'react'; +import React, { useCallback, useId, useRef, useState } from 'react'; import { createPortal } from 'react-dom'; import { AppTooltip, MenuItem, MenuItemSelect, SelectOption } from 'twenty-ui'; import { ReadonlyDeep } from 'type-fest'; @@ -20,7 +20,6 @@ import { DropdownMenuSeparator } from '@/ui/layout/dropdown/components/DropdownM import { OverlayContainer } from '@/ui/layout/overlay/components/OverlayContainer'; import { useListenClickOutside } from '@/ui/utilities/pointer-event/hooks/useListenClickOutside'; import { useLingui } from '@lingui/react/macro'; -import { v4 as uuidV4 } from 'uuid'; import { useUpdateEffect } from '~/hooks/useUpdateEffect'; const StyledFloatingDropdown = styled.div` @@ -42,6 +41,7 @@ export const MatchColumnSelect = ({ placeholder, }: MatchColumnSelectProps) => { const theme = useTheme(); + const idPrefix = useId(); const dropdownContainerRef = useRef(null); @@ -138,8 +138,8 @@ export const MatchColumnSelect = ({ /> - {options?.map((option) => { - const id = `${uuidV4()}-${option.value}`; + {options?.map((option, index) => { + const id = `${idPrefix}-option-${index}`; return (
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 07a110016..4694775a2 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 @@ -1,6 +1,7 @@ import { Injectable } from '@nestjs/common'; -import { In, InsertResult } from 'typeorm'; +import { capitalize } from 'twenty-shared/utils'; +import { In, InsertResult, ObjectLiteral } from 'typeorm'; import { GraphqlQueryBaseResolverService, @@ -14,7 +15,12 @@ import { QUERY_MAX_RECORDS } from 'src/engine/api/graphql/graphql-query-runner/c import { ObjectRecordsToGraphqlConnectionHelper } from 'src/engine/api/graphql/graphql-query-runner/helpers/object-records-to-graphql-connection.helper'; import { assertIsValidUuid } from 'src/engine/api/graphql/workspace-query-runner/utils/assert-is-valid-uuid.util'; import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum'; +import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types'; import { assertMutationNotOnRemoteObject } from 'src/engine/metadata-modules/object-metadata/utils/assert-mutation-not-on-remote-object.util'; +import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps'; +import { ObjectMetadataMaps } from 'src/engine/metadata-modules/types/object-metadata-maps'; +import { WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository'; +import { formatData } from 'src/engine/twenty-orm/utils/format-data.util'; import { formatResult } from 'src/engine/twenty-orm/utils/format-result.util'; @Injectable() @@ -29,13 +35,270 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol const { authContext, objectMetadataItemWithFieldMaps, objectMetadataMaps } = executionArgs.options; - const objectRecords: InsertResult = !executionArgs.args.upsert - ? await executionArgs.repository.insert(executionArgs.args.data) - : await executionArgs.repository.upsert(executionArgs.args.data, { - conflictPaths: ['id'], - skipUpdateIfNoValuesChanged: true, - }); + const objectRecords = await this.insertOrUpsertRecords(executionArgs); + const upsertedRecords = await this.fetchUpsertedRecords( + executionArgs, + objectRecords, + objectMetadataItemWithFieldMaps, + objectMetadataMaps, + ); + + this.apiEventEmitterService.emitCreateEvents( + upsertedRecords, + authContext, + objectMetadataItemWithFieldMaps, + ); + + await this.processNestedRelationsIfNeeded( + executionArgs, + upsertedRecords, + objectMetadataItemWithFieldMaps, + objectMetadataMaps, + featureFlagsMap, + ); + + return this.formatRecordsForResponse( + upsertedRecords, + objectMetadataItemWithFieldMaps, + objectMetadataMaps, + featureFlagsMap, + ); + } + + private async insertOrUpsertRecords( + executionArgs: GraphqlQueryResolverExecutionArgs, + ): Promise { + if (!executionArgs.args.upsert) { + return await executionArgs.repository.insert(executionArgs.args.data); + } + + return this.performUpsertOperation(executionArgs); + } + + private async performUpsertOperation( + executionArgs: GraphqlQueryResolverExecutionArgs, + ): Promise { + const { objectMetadataItemWithFieldMaps } = executionArgs.options; + const conflictingFields = this.getConflictingFields( + objectMetadataItemWithFieldMaps, + ); + const existingRecords = await this.findExistingRecords( + executionArgs, + conflictingFields, + ); + + const { recordsToUpdate, recordsToInsert } = this.categorizeRecords( + executionArgs.args.data, + conflictingFields, + existingRecords, + ); + + const result: InsertResult = { + identifiers: [], + generatedMaps: [], + raw: [], + }; + + await this.processRecordsToUpdate( + recordsToUpdate, + executionArgs.repository, + objectMetadataItemWithFieldMaps, + result, + ); + + await this.processRecordsToInsert( + recordsToInsert, + executionArgs.repository, + result, + ); + + return result; + } + + private getConflictingFields( + objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps, + ): { + baseField: string; + fullPath: string; + column: string; + }[] { + return objectMetadataItemWithFieldMaps.fields + .filter((field) => field.isUnique || field.name === 'id') + .flatMap((field) => { + const compositeType = compositeTypeDefinitions.get(field.type); + + if (!compositeType) { + return [ + { + baseField: field.name, + fullPath: field.name, + column: field.name, + }, + ]; + } + + const property = compositeType.properties.find( + (prop) => prop.isIncludedInUniqueConstraint, + ); + + return property + ? [ + { + baseField: field.name, + fullPath: `${field.name}.${property.name}`, + column: `${field.name}${capitalize(property.name)}`, + }, + ] + : []; + }); + } + + private async findExistingRecords( + executionArgs: GraphqlQueryResolverExecutionArgs, + conflictingFields: { + baseField: string; + fullPath: string; + column: string; + }[], + ): Promise[]> { + const { objectMetadataItemWithFieldMaps } = executionArgs.options; + const queryBuilder = executionArgs.repository.createQueryBuilder( + objectMetadataItemWithFieldMaps.nameSingular, + ); + + const whereConditions = this.buildWhereConditions( + executionArgs.args.data, + conflictingFields, + ); + + return queryBuilder.orWhere(whereConditions).getMany(); + } + + private getValueFromPath( + record: Partial, + path: string, + ): unknown { + const pathParts = path.split('.'); + + if (pathParts.length === 1) { + return record[path]; + } + + const [parentField, childField] = pathParts; + + return record[parentField]?.[childField]; + } + + private buildWhereConditions( + records: Partial[], + conflictingFields: { + baseField: string; + fullPath: string; + column: string; + }[], + ): Record { + const whereConditions = {}; + + for (const field of conflictingFields) { + const fieldValues = records + .map((record) => this.getValueFromPath(record, field.fullPath)) + .filter(Boolean); + + if (fieldValues.length > 0) { + whereConditions[field.column] = In(fieldValues); + } + } + + return whereConditions; + } + + private categorizeRecords( + records: Partial[], + conflictingFields: { + baseField: string; + fullPath: string; + column: string; + }[], + existingRecords: Partial[], + ): { + recordsToUpdate: Partial[]; + recordsToInsert: Partial[]; + } { + const recordsToUpdate: Partial[] = []; + const recordsToInsert: Partial[] = []; + + for (const record of records) { + let existingRecord: Partial | null = null; + + for (const field of conflictingFields) { + const requestFieldValue = this.getValueFromPath(record, field.fullPath); + + const existingRec = existingRecords.find( + (existingRecord) => + existingRecord[field.column] === requestFieldValue, + ); + + if (existingRec) { + existingRecord = { ...record, id: existingRec.id }; + break; + } + } + + if (existingRecord) { + recordsToUpdate.push({ ...record, id: existingRecord.id }); + } else { + recordsToInsert.push(record); + } + } + + return { recordsToUpdate, recordsToInsert }; + } + + private async processRecordsToUpdate( + recordsToUpdate: Partial[], + repository: WorkspaceRepository, + objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps, + result: InsertResult, + ): Promise { + for (const record of recordsToUpdate) { + const recordId = record.id as string; + + // TODO: we should align update and insert + // For insert, formating is done in the server + // While for update, formatting is done at the resolver level + + const formattedRecord = formatData( + record, + objectMetadataItemWithFieldMaps, + ); + + await repository.update(recordId, formattedRecord); + result.identifiers.push({ id: recordId }); + result.generatedMaps.push({ id: recordId }); + } + } + + private async processRecordsToInsert( + recordsToInsert: Partial[], + repository: WorkspaceRepository, + result: InsertResult, + ): Promise { + if (recordsToInsert.length > 0) { + const insertResult = await repository.insert(recordsToInsert); + + result.identifiers.push(...insertResult.identifiers); + result.generatedMaps.push(...insertResult.generatedMaps); + result.raw.push(...insertResult.raw); + } + } + + private async fetchUpsertedRecords( + executionArgs: GraphqlQueryResolverExecutionArgs, + objectRecords: InsertResult, + objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps, + objectMetadataMaps: ObjectMetadataMaps, + ): Promise { const queryBuilder = executionArgs.repository.createQueryBuilder( objectMetadataItemWithFieldMaps.nameSingular, ); @@ -47,32 +310,43 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol .take(QUERY_MAX_RECORDS) .getMany(); - const upsertedRecords = formatResult( + return formatResult( nonFormattedUpsertedRecords, objectMetadataItemWithFieldMaps, objectMetadataMaps, ); + } - this.apiEventEmitterService.emitCreateEvents( - upsertedRecords, - authContext, - objectMetadataItemWithFieldMaps, - ); - - if (executionArgs.graphqlQuerySelectedFieldsResult.relations) { - await this.processNestedRelationsHelper.processNestedRelations({ - objectMetadataMaps, - parentObjectMetadataItem: objectMetadataItemWithFieldMaps, - parentObjectRecords: upsertedRecords, - relations: executionArgs.graphqlQuerySelectedFieldsResult.relations, - limit: QUERY_MAX_RECORDS, - authContext, - dataSource: executionArgs.dataSource, - isNewRelationEnabled: - featureFlagsMap[FeatureFlagKey.IsNewRelationEnabled], - }); + private async processNestedRelationsIfNeeded( + executionArgs: GraphqlQueryResolverExecutionArgs, + upsertedRecords: ObjectRecord[], + objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps, + objectMetadataMaps: ObjectMetadataMaps, + featureFlagsMap: Record, + ): Promise { + if (!executionArgs.graphqlQuerySelectedFieldsResult.relations) { + return; } + await this.processNestedRelationsHelper.processNestedRelations({ + objectMetadataMaps, + parentObjectMetadataItem: objectMetadataItemWithFieldMaps, + parentObjectRecords: upsertedRecords, + relations: executionArgs.graphqlQuerySelectedFieldsResult.relations, + limit: QUERY_MAX_RECORDS, + authContext: executionArgs.options.authContext, + dataSource: executionArgs.dataSource, + isNewRelationEnabled: + featureFlagsMap[FeatureFlagKey.IsNewRelationEnabled], + }); + } + + private formatRecordsForResponse( + upsertedRecords: ObjectRecord[], + objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps, + objectMetadataMaps: ObjectMetadataMaps, + featureFlagsMap: Record, + ): ObjectRecord[] { const typeORMObjectRecordsParser = new ObjectRecordsToGraphqlConnectionHelper( objectMetadataMaps,