From 191bbb9e129d6e5009a52c6d3b61989531112b4a Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Fri, 18 Jul 2025 21:38:36 +0200 Subject: [PATCH] Prevent field name conflicts (#13280) Fixes https://github.com/twentyhq/twenty/issues/13184 --- .../SettingsObjectNewFieldConfigure.tsx | 9 +- .../field-metadata-morph-relation.service.ts | 5 +- .../field-metadata-relation.service.ts | 36 ++- .../services/field-metadata.service.ts | 5 +- ...data-graphql-api-exception-handler.util.ts | 2 +- .../validate-field-name-availability.utils.ts | 10 +- ...relation-creation.integration-spec.ts.snap | 16 +- ...relation-creation.integration-spec.ts.snap | 128 ++++++++-- ...data-relation-creation.integration-spec.ts | 223 ++++++++++++++---- ...ate-one-field-metadata.integration-spec.ts | 56 +++++ 10 files changed, 402 insertions(+), 88 deletions(-) diff --git a/packages/twenty-front/src/pages/settings/data-model/new-field/SettingsObjectNewFieldConfigure.tsx b/packages/twenty-front/src/pages/settings/data-model/new-field/SettingsObjectNewFieldConfigure.tsx index ff4ee020c..9ba8cbcc1 100644 --- a/packages/twenty-front/src/pages/settings/data-model/new-field/SettingsObjectNewFieldConfigure.tsx +++ b/packages/twenty-front/src/pages/settings/data-model/new-field/SettingsObjectNewFieldConfigure.tsx @@ -17,6 +17,7 @@ import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar'; import { SubMenuTopBarContainer } from '@/ui/layout/page/components/SubMenuTopBarContainer'; import { View } from '@/views/types/View'; import { ViewType } from '@/views/types/ViewType'; +import { ApolloError } from '@apollo/client'; import { zodResolver } from '@hookform/resolvers/zod'; import { useLingui } from '@lingui/react/macro'; import { useEffect, useState } from 'react'; @@ -149,14 +150,8 @@ export const SettingsObjectNewFieldConfigure = () => { setIsSaving(false); } catch (error) { setIsSaving(false); - const isDuplicateFieldNameInObject = (error as Error).message.includes( - 'duplicate key value violates unique constraint "IndexOnNameObjectMetadataIdAndWorkspaceIdUnique"', - ); - enqueueErrorSnackBar({ - message: isDuplicateFieldNameInObject - ? t`Please use different names for your source and destination fields` - : undefined, + apolloError: error instanceof ApolloError ? error : undefined, }); } }; diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-morph-relation.service.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-morph-relation.service.ts index a6fce0f5f..daffd496b 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-morph-relation.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-morph-relation.service.ts @@ -77,7 +77,7 @@ export class FieldMetadataMorphRelationService { } const relationFieldMetadataForCreate = - await this.fieldMetadataRelationService.addCustomRelationFieldMetadataForCreation( + this.fieldMetadataRelationService.computeCustomRelationFieldMetadataForCreation( { fieldMetadataInput: fieldMetadataForCreate, relationCreationPayload: relation, @@ -94,6 +94,7 @@ export class FieldMetadataMorphRelationService { fieldMetadataInput: relationFieldMetadataForCreate, fieldMetadataType: relationFieldMetadataForCreate.type, objectMetadataMaps, + objectMetadata, }, ); @@ -118,7 +119,7 @@ export class FieldMetadataMorphRelationService { ); const targetFieldMetadataToCreateWithRelation = - await this.fieldMetadataRelationService.addCustomRelationFieldMetadataForCreation( + this.fieldMetadataRelationService.computeCustomRelationFieldMetadataForCreation( { fieldMetadataInput: targetFieldMetadataToCreate, relationCreationPayload: { diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-relation.service.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-relation.service.ts index 06b59adc5..991bd7b69 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-relation.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-relation.service.ts @@ -1,5 +1,6 @@ import { Injectable, ValidationError } from '@nestjs/common'; +import { t } from '@lingui/core/macro'; import { plainToInstance } from 'class-transformer'; import { IsEnum, IsString, IsUUID, validateOrReject } from 'class-validator'; import { FieldMetadataType } from 'twenty-shared/types'; @@ -17,6 +18,7 @@ import { FieldMetadataException, FieldMetadataExceptionCode, } from 'src/engine/metadata-modules/field-metadata/field-metadata.exception'; +import { computeRelationFieldJoinColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-relation-field-join-column-name.util'; import { prepareCustomFieldMetadataForCreation } from 'src/engine/metadata-modules/field-metadata/utils/prepare-field-metadata-for-creation.util'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; import { RelationOnDeleteAction } from 'src/engine/metadata-modules/relation-metadata/relation-on-delete-action.type'; @@ -28,7 +30,6 @@ import { validateMetadataNameOrThrow } from 'src/engine/metadata-modules/utils/v import { computeMetadataNameFromLabel } from 'src/engine/metadata-modules/utils/validate-name-and-label-are-sync-or-throw.util'; import { isFieldMetadataInterfaceOfType } from 'src/engine/utils/is-field-metadata-of-type.util'; import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; -import { computeRelationFieldJoinColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-relation-field-join-column-name.util'; export class RelationCreationPayloadValidation { @IsUUID() @@ -93,7 +94,7 @@ export class FieldMetadataRelationService { }); const targetFieldMetadataToCreateWithRelation = - await this.addCustomRelationFieldMetadataForCreation({ + this.computeCustomRelationFieldMetadataForCreation({ fieldMetadataInput: targetFieldMetadataToCreate, relationCreationPayload: { targetObjectMetadataId: objectMetadata.id, @@ -134,9 +135,13 @@ export class FieldMetadataRelationService { fieldMetadataInput, fieldMetadataType, objectMetadataMaps, + objectMetadata, }: Pick< ValidateFieldMetadataArgs, - 'fieldMetadataInput' | 'fieldMetadataType' | 'objectMetadataMaps' + | 'fieldMetadataInput' + | 'fieldMetadataType' + | 'objectMetadataMaps' + | 'objectMetadata' >): Promise { // TODO: clean typings, we should try to validate both update and create inputs in the same function const isRelation = @@ -150,6 +155,11 @@ export class FieldMetadataRelationService { .relationCreationPayload, ) ) { + validateFieldNameAvailabilityOrThrow( + `${fieldMetadataInput.name}Id`, + objectMetadata, + ); + const relationCreationPayload = ( fieldMetadataInput as unknown as CreateFieldInput ).relationCreationPayload; @@ -180,6 +190,24 @@ export class FieldMetadataRelationService { computedMetadataNameFromLabel, objectMetadataTarget, ); + + validateFieldNameAvailabilityOrThrow( + `${computedMetadataNameFromLabel}Id`, + objectMetadataTarget, + ); + + if ( + computedMetadataNameFromLabel === fieldMetadataInput.name && + objectMetadata.id === objectMetadataTarget.id + ) { + throw new FieldMetadataException( + `Name "${computedMetadataNameFromLabel}" cannot be the same on both side of the relation`, + FieldMetadataExceptionCode.INVALID_FIELD_INPUT, + { + userFriendlyMessage: t`Name "${computedMetadataNameFromLabel}" cannot be the same on both side of the relation`, + }, + ); + } } } @@ -285,7 +313,7 @@ export class FieldMetadataRelationService { }); } - addCustomRelationFieldMetadataForCreation({ + computeCustomRelationFieldMetadataForCreation({ fieldMetadataInput, relationCreationPayload, joinColumnName, diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata.service.ts index 28b07a3e3..c31ae175c 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata.service.ts @@ -32,6 +32,7 @@ import { computeColumnName, computeCompositeColumnName, } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util'; +import { computeRelationFieldJoinColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-relation-field-join-column-name.util'; import { createMigrationActions } from 'src/engine/metadata-modules/field-metadata/utils/create-migration-actions.util'; import { generateRatingOptions } from 'src/engine/metadata-modules/field-metadata/utils/generate-rating-optionts.util'; import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; @@ -57,7 +58,6 @@ import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global. import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util'; import { WorkspaceMigrationRunnerService } from 'src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service'; import { ViewService } from 'src/modules/view/services/view.service'; -import { computeRelationFieldJoinColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-relation-field-join-column-name.util'; @Injectable() export class FieldMetadataService extends TypeOrmQueryService { @@ -671,7 +671,7 @@ export class FieldMetadataService extends TypeOrmQueryService { if (error instanceof InvalidMetadataException) { - throw new UserInputError(error.message); + throw new UserInputError(error); } if (error instanceof FieldMetadataException) { diff --git a/packages/twenty-server/src/engine/metadata-modules/utils/validate-field-name-availability.utils.ts b/packages/twenty-server/src/engine/metadata-modules/utils/validate-field-name-availability.utils.ts index 779aeac02..5c95ec61d 100644 --- a/packages/twenty-server/src/engine/metadata-modules/utils/validate-field-name-availability.utils.ts +++ b/packages/twenty-server/src/engine/metadata-modules/utils/validate-field-name-availability.utils.ts @@ -1,4 +1,5 @@ import { t } from '@lingui/core/macro'; +import { FieldMetadataType } from 'twenty-shared/types'; import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types'; import { computeCompositeColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util'; @@ -39,14 +40,17 @@ export const validateFieldNameAvailabilityOrThrow = ( if ( Object.values(objectMetadata.fieldsById).some( - (field) => field.name === name, + (field) => + field.name === name || + (field.type === FieldMetadataType.RELATION && + `${field.name}Id` === name), ) ) { throw new InvalidMetadataException( - `Name "${name}" is not available`, + `Name "${name}" is not available as it is already used by another field`, InvalidMetadataExceptionCode.NOT_AVAILABLE, { - userFriendlyMessage: t`This name is not available.`, + userFriendlyMessage: t`This name is not available as it is already used by another field`, }, ); } diff --git a/packages/twenty-server/test/integration/metadata/suites/field-metadata/morph-relation/__snapshots__/failing-field-metadata-morph-relation-creation.integration-spec.ts.snap b/packages/twenty-server/test/integration/metadata/suites/field-metadata/morph-relation/__snapshots__/failing-field-metadata-morph-relation-creation.integration-spec.ts.snap index 40a37e558..6d27f0e0c 100644 --- a/packages/twenty-server/test/integration/metadata/suites/field-metadata/morph-relation/__snapshots__/failing-field-metadata-morph-relation-creation.integration-spec.ts.snap +++ b/packages/twenty-server/test/integration/metadata/suites/field-metadata/morph-relation/__snapshots__/failing-field-metadata-morph-relation-creation.integration-spec.ts.snap @@ -5,9 +5,10 @@ exports[`Field metadata morph relation creation should fail relation MANY_TO_ONE { "extensions": { "code": "BAD_USER_INPUT", - "userFriendlyMessage": "An error occurred.", + "subCode": "Name not available", + "userFriendlyMessage": "This name is not available as it is already used by another field", }, - "message": "Name "collisionfieldlabel" is not available", + "message": "Name "collisionfieldlabel" is not available as it is already used by another field", "name": "UserInputError", }, ] @@ -18,6 +19,7 @@ exports[`Field metadata morph relation creation should fail relation MANY_TO_ONE { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Invalid label", "userFriendlyMessage": "An error occurred.", }, "message": "Invalid label: " "", @@ -31,6 +33,7 @@ exports[`Field metadata morph relation creation should fail relation MANY_TO_ONE { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Exceeds max length", "userFriendlyMessage": "An error occurred.", }, "message": "Name is too long: it exceeds the 63 characters limit.", @@ -44,6 +47,7 @@ exports[`Field metadata morph relation creation should fail relation MANY_TO_ONE { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Input too short", "userFriendlyMessage": "An error occurred.", }, "message": "Input is too short: """, @@ -98,9 +102,10 @@ exports[`Field metadata morph relation creation should fail relation ONE_TO_MANY { "extensions": { "code": "BAD_USER_INPUT", - "userFriendlyMessage": "An error occurred.", + "subCode": "Name not available", + "userFriendlyMessage": "This name is not available as it is already used by another field", }, - "message": "Name "collisionfieldlabel" is not available", + "message": "Name "collisionfieldlabel" is not available as it is already used by another field", "name": "UserInputError", }, ] @@ -111,6 +116,7 @@ exports[`Field metadata morph relation creation should fail relation ONE_TO_MANY { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Invalid label", "userFriendlyMessage": "An error occurred.", }, "message": "Invalid label: " "", @@ -124,6 +130,7 @@ exports[`Field metadata morph relation creation should fail relation ONE_TO_MANY { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Exceeds max length", "userFriendlyMessage": "An error occurred.", }, "message": "Name is too long: it exceeds the 63 characters limit.", @@ -137,6 +144,7 @@ exports[`Field metadata morph relation creation should fail relation ONE_TO_MANY { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Input too short", "userFriendlyMessage": "An error occurred.", }, "message": "Input is too short: """, diff --git a/packages/twenty-server/test/integration/metadata/suites/field-metadata/relation/__snapshots__/failing-field-metadata-relation-creation.integration-spec.ts.snap b/packages/twenty-server/test/integration/metadata/suites/field-metadata/relation/__snapshots__/failing-field-metadata-relation-creation.integration-spec.ts.snap index d0c1e7f4a..88c69194b 100644 --- a/packages/twenty-server/test/integration/metadata/suites/field-metadata/relation/__snapshots__/failing-field-metadata-relation-creation.integration-spec.ts.snap +++ b/packages/twenty-server/test/integration/metadata/suites/field-metadata/relation/__snapshots__/failing-field-metadata-relation-creation.integration-spec.ts.snap @@ -1,23 +1,39 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Field metadata relation creation should fail relation MANY_TO_ONE when targetFieldLabel conflicts with an existing field on target object metadata id 1`] = ` +exports[`Field metadata relation creation should fail relation MANY_TO_ONE (relationCreationPayload) when targetFieldLabel conflicts with an existing {name}Id on target object metadata id 1`] = ` [ { "extensions": { "code": "BAD_USER_INPUT", - "userFriendlyMessage": "An error occurred.", + "subCode": "Name not available", + "userFriendlyMessage": "This name is not available as it is already used by another field", }, - "message": "Name "collisionfieldlabel" is not available", + "message": "Name "fieldNameBisId" is not available as it is already used by another field", "name": "UserInputError", }, ] `; -exports[`Field metadata relation creation should fail relation MANY_TO_ONE when targetFieldLabel contains only whitespace 1`] = ` +exports[`Field metadata relation creation should fail relation MANY_TO_ONE (relationCreationPayload) when targetFieldLabel conflicts with an existing field on target object metadata id 1`] = ` [ { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Name not available", + "userFriendlyMessage": "This name is not available as it is already used by another field", + }, + "message": "Name "fieldName" is not available as it is already used by another field", + "name": "UserInputError", + }, +] +`; + +exports[`Field metadata relation creation should fail relation MANY_TO_ONE (relationCreationPayload) when targetFieldLabel contains only whitespace 1`] = ` +[ + { + "extensions": { + "code": "BAD_USER_INPUT", + "subCode": "Invalid label", "userFriendlyMessage": "An error occurred.", }, "message": "Invalid label: " "", @@ -26,11 +42,12 @@ exports[`Field metadata relation creation should fail relation MANY_TO_ONE when ] `; -exports[`Field metadata relation creation should fail relation MANY_TO_ONE when targetFieldLabel exceeds maximum length 1`] = ` +exports[`Field metadata relation creation should fail relation MANY_TO_ONE (relationCreationPayload) when targetFieldLabel exceeds maximum length 1`] = ` [ { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Exceeds max length", "userFriendlyMessage": "An error occurred.", }, "message": "Name is too long: it exceeds the 63 characters limit.", @@ -39,11 +56,12 @@ exports[`Field metadata relation creation should fail relation MANY_TO_ONE when ] `; -exports[`Field metadata relation creation should fail relation MANY_TO_ONE when targetFieldLabel is empty 1`] = ` +exports[`Field metadata relation creation should fail relation MANY_TO_ONE (relationCreationPayload) when targetFieldLabel is empty 1`] = ` [ { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Input too short", "userFriendlyMessage": "An error occurred.", }, "message": "Input is too short: """, @@ -52,7 +70,7 @@ exports[`Field metadata relation creation should fail relation MANY_TO_ONE when ] `; -exports[`Field metadata relation creation should fail relation MANY_TO_ONE when targetObjectMetadataId is unknown 1`] = ` +exports[`Field metadata relation creation should fail relation MANY_TO_ONE (relationCreationPayload) when targetObjectMetadataId is unknown 1`] = ` [ { "extensions": { @@ -65,7 +83,7 @@ exports[`Field metadata relation creation should fail relation MANY_TO_ONE when ] `; -exports[`Field metadata relation creation should fail relation MANY_TO_ONE when type is a wrong value 1`] = ` +exports[`Field metadata relation creation should fail relation MANY_TO_ONE (relationCreationPayload) when type is a wrong value 1`] = ` [ { "extensions": { @@ -79,7 +97,7 @@ exports[`Field metadata relation creation should fail relation MANY_TO_ONE when ] `; -exports[`Field metadata relation creation should fail relation MANY_TO_ONE when type is not provided 1`] = ` +exports[`Field metadata relation creation should fail relation MANY_TO_ONE (relationCreationPayload) when type is not provided 1`] = ` [ { "extensions": { @@ -93,24 +111,68 @@ exports[`Field metadata relation creation should fail relation MANY_TO_ONE when ] `; -exports[`Field metadata relation creation should fail relation ONE_TO_MANY when targetFieldLabel conflicts with an existing field on target object metadata id 1`] = ` +exports[`Field metadata relation creation should fail relation MANY_TO_ONE when {name}Id is already used 1`] = ` [ { "extensions": { "code": "BAD_USER_INPUT", - "userFriendlyMessage": "An error occurred.", + "subCode": "INVALID_FIELD_INPUT", + "userFriendlyMessage": "Name is not available, it may be duplicating another field's name.", }, - "message": "Name "collisionfieldlabel" is not available", + "message": "Name "fieldNameBisId" is not available, check that it is not duplicating another field's name.", "name": "UserInputError", }, ] `; -exports[`Field metadata relation creation should fail relation ONE_TO_MANY when targetFieldLabel contains only whitespace 1`] = ` +exports[`Field metadata relation creation should fail relation MANY_TO_ONE when target and source are the same object and name are the same 1`] = ` [ { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "INVALID_FIELD_INPUT", + "userFriendlyMessage": "An error occurred.", + }, + "message": "Relation creation payload is invalid: targetObjectMetadataId must be a UUID", + "name": "UserInputError", + }, +] +`; + +exports[`Field metadata relation creation should fail relation ONE_TO_MANY (relationCreationPayload) when targetFieldLabel conflicts with an existing {name}Id on target object metadata id 1`] = ` +[ + { + "extensions": { + "code": "BAD_USER_INPUT", + "subCode": "Name not available", + "userFriendlyMessage": "This name is not available as it is already used by another field", + }, + "message": "Name "fieldNameBisId" is not available as it is already used by another field", + "name": "UserInputError", + }, +] +`; + +exports[`Field metadata relation creation should fail relation ONE_TO_MANY (relationCreationPayload) when targetFieldLabel conflicts with an existing field on target object metadata id 1`] = ` +[ + { + "extensions": { + "code": "BAD_USER_INPUT", + "subCode": "Name not available", + "userFriendlyMessage": "This name is not available as it is already used by another field", + }, + "message": "Name "fieldName" is not available as it is already used by another field", + "name": "UserInputError", + }, +] +`; + +exports[`Field metadata relation creation should fail relation ONE_TO_MANY (relationCreationPayload) when targetFieldLabel contains only whitespace 1`] = ` +[ + { + "extensions": { + "code": "BAD_USER_INPUT", + "subCode": "Invalid label", "userFriendlyMessage": "An error occurred.", }, "message": "Invalid label: " "", @@ -119,11 +181,12 @@ exports[`Field metadata relation creation should fail relation ONE_TO_MANY when ] `; -exports[`Field metadata relation creation should fail relation ONE_TO_MANY when targetFieldLabel exceeds maximum length 1`] = ` +exports[`Field metadata relation creation should fail relation ONE_TO_MANY (relationCreationPayload) when targetFieldLabel exceeds maximum length 1`] = ` [ { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Exceeds max length", "userFriendlyMessage": "An error occurred.", }, "message": "Name is too long: it exceeds the 63 characters limit.", @@ -132,11 +195,12 @@ exports[`Field metadata relation creation should fail relation ONE_TO_MANY when ] `; -exports[`Field metadata relation creation should fail relation ONE_TO_MANY when targetFieldLabel is empty 1`] = ` +exports[`Field metadata relation creation should fail relation ONE_TO_MANY (relationCreationPayload) when targetFieldLabel is empty 1`] = ` [ { "extensions": { "code": "BAD_USER_INPUT", + "subCode": "Input too short", "userFriendlyMessage": "An error occurred.", }, "message": "Input is too short: """, @@ -145,7 +209,7 @@ exports[`Field metadata relation creation should fail relation ONE_TO_MANY when ] `; -exports[`Field metadata relation creation should fail relation ONE_TO_MANY when targetObjectMetadataId is unknown 1`] = ` +exports[`Field metadata relation creation should fail relation ONE_TO_MANY (relationCreationPayload) when targetObjectMetadataId is unknown 1`] = ` [ { "extensions": { @@ -158,7 +222,7 @@ exports[`Field metadata relation creation should fail relation ONE_TO_MANY when ] `; -exports[`Field metadata relation creation should fail relation ONE_TO_MANY when type is a wrong value 1`] = ` +exports[`Field metadata relation creation should fail relation ONE_TO_MANY (relationCreationPayload) when type is a wrong value 1`] = ` [ { "extensions": { @@ -172,7 +236,7 @@ exports[`Field metadata relation creation should fail relation ONE_TO_MANY when ] `; -exports[`Field metadata relation creation should fail relation ONE_TO_MANY when type is not provided 1`] = ` +exports[`Field metadata relation creation should fail relation ONE_TO_MANY (relationCreationPayload) when type is not provided 1`] = ` [ { "extensions": { @@ -185,3 +249,31 @@ exports[`Field metadata relation creation should fail relation ONE_TO_MANY when }, ] `; + +exports[`Field metadata relation creation should fail relation ONE_TO_MANY when {name}Id is already used 1`] = ` +[ + { + "extensions": { + "code": "BAD_USER_INPUT", + "subCode": "INVALID_FIELD_INPUT", + "userFriendlyMessage": "Name is not available, it may be duplicating another field's name.", + }, + "message": "Name "fieldNameBisId" is not available, check that it is not duplicating another field's name.", + "name": "UserInputError", + }, +] +`; + +exports[`Field metadata relation creation should fail relation ONE_TO_MANY when target and source are the same object and name are the same 1`] = ` +[ + { + "extensions": { + "code": "BAD_USER_INPUT", + "subCode": "INVALID_FIELD_INPUT", + "userFriendlyMessage": "An error occurred.", + }, + "message": "Relation creation payload is invalid: targetObjectMetadataId must be a UUID", + "name": "UserInputError", + }, +] +`; diff --git a/packages/twenty-server/test/integration/metadata/suites/field-metadata/relation/failing-field-metadata-relation-creation.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/field-metadata/relation/failing-field-metadata-relation-creation.integration-spec.ts index 8562cc718..918c0c1f1 100644 --- a/packages/twenty-server/test/integration/metadata/suites/field-metadata/relation/failing-field-metadata-relation-creation.integration-spec.ts +++ b/packages/twenty-server/test/integration/metadata/suites/field-metadata/relation/failing-field-metadata-relation-creation.integration-spec.ts @@ -15,60 +15,138 @@ type GlobalTestContext = { targetObjectId: string; sourceObjectId: string; }; + collisionFieldName: string; + collisionFieldNameWithId: string; collisionFieldLabel: string; + collisionFieldLabelWithId: string; }; const globalTestContext: GlobalTestContext = { objectMetadataIds: { targetObjectId: '', sourceObjectId: '', }, - collisionFieldLabel: 'collisionfieldlabel', + collisionFieldLabel: 'Field Name', + collisionFieldName: 'fieldName', + collisionFieldNameWithId: 'fieldNameBisId', + collisionFieldLabelWithId: 'Field Name Bis Id', }; type TestedRelationCreationPayload = Partial< NonNullable >; +type TestedContext = { + input: { + name?: string; + relationCreationPayload?: TestedRelationCreationPayload; + }; +}; + type CreateOneObjectMetadataItemTestingContext = EachTestingContext< - | TestedRelationCreationPayload - | ((context: GlobalTestContext) => TestedRelationCreationPayload) + TestedContext | ((context: GlobalTestContext) => TestedContext) >[]; describe('Field metadata relation creation should fail', () => { const failingLabelsCreationTestsUseCase: CreateOneObjectMetadataItemTestingContext = [ // TODO @prastoin add coverage other fields such as the Type, icon etc etc ( using edge cases fuzzing etc ) { - title: 'when targetFieldLabel is empty', - context: { targetFieldLabel: '' }, - }, - { - title: 'when targetFieldLabel exceeds maximum length', - context: { targetFieldLabel: 'A'.repeat(64) }, - }, - { - // Not handled gracefully should be refactored - title: 'when targetObjectMetadataId is unknown', - context: { targetObjectMetadataId: faker.string.uuid() }, - }, - { - title: 'when targetFieldLabel contains only whitespace', - context: { targetFieldLabel: ' ' }, + title: '(relationCreationPayload) when targetFieldLabel is empty', + context: { + input: { relationCreationPayload: { targetFieldLabel: '' } }, + }, }, { title: - 'when targetFieldLabel conflicts with an existing field on target object metadata id', + '(relationCreationPayload) when targetFieldLabel exceeds maximum length', + context: { + input: { + relationCreationPayload: { targetFieldLabel: 'A'.repeat(64) }, + }, + }, + }, + { + // Not handled gracefully should be refactored + title: + '(relationCreationPayload) when targetObjectMetadataId is unknown', + context: { + input: { + relationCreationPayload: { + targetObjectMetadataId: faker.string.uuid(), + }, + }, + }, + }, + { + title: + '(relationCreationPayload) when targetFieldLabel contains only whitespace', + context: { + input: { + relationCreationPayload: { targetFieldLabel: ' ' }, + }, + }, + }, + { + title: + '(relationCreationPayload) when targetFieldLabel conflicts with an existing field on target object metadata id', context: ({ collisionFieldLabel, objectMetadataIds }) => ({ - targetObjectMetadataId: objectMetadataIds.targetObjectId, - targetFieldLabel: collisionFieldLabel, + input: { + relationCreationPayload: { + targetObjectMetadataId: objectMetadataIds.targetObjectId, + targetFieldLabel: collisionFieldLabel, + }, + }, }), }, { - title: 'when type is not provided', - context: { type: undefined }, + title: + '(relationCreationPayload) when targetFieldLabel conflicts with an existing {name}Id on target object metadata id', + context: ({ collisionFieldLabelWithId, objectMetadataIds }) => ({ + input: { + relationCreationPayload: { + targetObjectMetadataId: objectMetadataIds.targetObjectId, + targetFieldLabel: collisionFieldLabelWithId, + }, + }, + }), }, { - title: 'when type is a wrong value', - context: { type: 'wrong' as RelationType }, + title: '(relationCreationPayload) when type is not provided', + context: { + input: { + relationCreationPayload: { type: undefined }, + }, + }, + }, + { + title: '(relationCreationPayload) when type is a wrong value', + context: { + input: { + relationCreationPayload: { type: 'wrong' as RelationType }, + }, + }, + }, + { + title: 'when {name}Id is already used', + context: ({ collisionFieldNameWithId }) => ({ + input: { + name: collisionFieldNameWithId, + relationCreationPayload: { targetFieldIcon: '' }, + }, + }), + }, + { + title: + 'when target and source are the same object and name are the same', + context: { + input: { + name: 'relationName', + relationCreationPayload: { + targetObjectMetadataId: + globalTestContext.objectMetadataIds.sourceObjectId, + targetFieldLabel: 'Relation Name', + }, + }, + }, }, ]; @@ -79,8 +157,8 @@ describe('Field metadata relation creation should fail', () => { }, } = await createOneObjectMetadata({ input: getMockCreateObjectInput({ - namePlural: 'collisionRelations', - nameSingular: 'collisionRelation', + namePlural: 'sourceObjects', + nameSingular: 'sourceObject', }), }); @@ -90,8 +168,8 @@ describe('Field metadata relation creation should fail', () => { }, } = await createOneObjectMetadata({ input: getMockCreateObjectInput({ - namePlural: 'collisionRelationTargets', - nameSingular: 'collisionRelationTarget', + namePlural: 'targetObjects', + nameSingular: 'targetObject', }), }); @@ -100,17 +178,54 @@ describe('Field metadata relation creation should fail', () => { targetObjectId, }; - const { data } = await createOneFieldMetadata({ - input: { - objectMetadataId: targetObjectId, - name: globalTestContext.collisionFieldLabel, - label: 'LabelThatCouldBeAnything', - isLabelSyncedWithName: false, - type: FieldMetadataType.TEXT, - }, - }); + const { data: collisionFieldWithLabelTargetData } = + await createOneFieldMetadata({ + input: { + objectMetadataId: targetObjectId, + name: globalTestContext.collisionFieldName, + label: 'LabelThatCouldBeAnything', + isLabelSyncedWithName: false, + type: FieldMetadataType.TEXT, + }, + }); - expect(data).toBeDefined(); + const { data: collisionFieldWithIdTargetData } = + await createOneFieldMetadata({ + input: { + objectMetadataId: targetObjectId, + name: globalTestContext.collisionFieldNameWithId, + label: 'LabelThatCouldBeAnything', + isLabelSyncedWithName: false, + type: FieldMetadataType.TEXT, + }, + }); + + const { data: collisionFieldWithLabelSourceData } = + await createOneFieldMetadata({ + input: { + objectMetadataId: sourceObjectId, + name: globalTestContext.collisionFieldName, + label: 'LabelThatCouldBeAnything', + isLabelSyncedWithName: false, + type: FieldMetadataType.TEXT, + }, + }); + + const { data: collisionFieldWithIdSourceData } = + await createOneFieldMetadata({ + input: { + objectMetadataId: sourceObjectId, + name: globalTestContext.collisionFieldNameWithId, + label: 'LabelThatCouldBeAnything', + isLabelSyncedWithName: false, + type: FieldMetadataType.TEXT, + }, + }); + + expect(collisionFieldWithLabelTargetData).toBeDefined(); + expect(collisionFieldWithIdTargetData).toBeDefined(); + expect(collisionFieldWithLabelSourceData).toBeDefined(); + expect(collisionFieldWithIdSourceData).toBeDefined(); }); afterAll(async () => { @@ -128,14 +243,21 @@ describe('Field metadata relation creation should fail', () => { it.each(failingLabelsCreationTestsUseCase)( 'relation ONE_TO_MANY $title', async ({ context }) => { - const computedContext = - typeof context === 'function' ? context(globalTestContext) : context; + const computedRelationCreationPayload = + typeof context === 'function' + ? context(globalTestContext).input.relationCreationPayload + : context.input.relationCreationPayload; + + const computedName = + typeof context === 'function' + ? context(globalTestContext).input.name + : context.input.name; const { errors } = await createOneFieldMetadata({ expectToFail: true, input: { objectMetadataId: globalTestContext.objectMetadataIds.sourceObjectId, - name: 'fieldname', + name: computedName ?? 'fieldname', label: 'Relation field', isLabelSyncedWithName: false, type: FieldMetadataType.RELATION, @@ -145,7 +267,7 @@ describe('Field metadata relation creation should fail', () => { targetObjectMetadataId: globalTestContext.objectMetadataIds.targetObjectId, targetFieldIcon: 'IconBuildingSkyscraper', - ...computedContext, + ...computedRelationCreationPayload, }, }, }); @@ -158,14 +280,21 @@ describe('Field metadata relation creation should fail', () => { it.each(failingLabelsCreationTestsUseCase)( 'relation MANY_TO_ONE $title', async ({ context }) => { - const computedContext = - typeof context === 'function' ? context(globalTestContext) : context; + const computedRelationCreationPayload = + typeof context === 'function' + ? context(globalTestContext).input.relationCreationPayload + : context.input.relationCreationPayload; + + const computedName = + typeof context === 'function' + ? context(globalTestContext).input.name + : context.input.name; const { errors } = await createOneFieldMetadata({ expectToFail: true, input: { objectMetadataId: globalTestContext.objectMetadataIds.sourceObjectId, - name: 'fieldname', + name: computedName ?? 'fieldname', label: 'Relation field', isLabelSyncedWithName: false, type: FieldMetadataType.RELATION, @@ -175,7 +304,7 @@ describe('Field metadata relation creation should fail', () => { targetObjectMetadataId: globalTestContext.objectMetadataIds.targetObjectId, targetFieldIcon: 'IconBuildingSkyscraper', - ...computedContext, + ...computedRelationCreationPayload, }, }, }); diff --git a/packages/twenty-server/test/integration/metadata/suites/field-metadata/update-one-field-metadata.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/field-metadata/update-one-field-metadata.integration-spec.ts index 34ae3b08d..223164720 100644 --- a/packages/twenty-server/test/integration/metadata/suites/field-metadata/update-one-field-metadata.integration-spec.ts +++ b/packages/twenty-server/test/integration/metadata/suites/field-metadata/update-one-field-metadata.integration-spec.ts @@ -8,6 +8,8 @@ import { createOneObjectMetadata } from 'test/integration/metadata/suites/object import { deleteOneObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/delete-one-object-metadata.util'; import { FieldMetadataType } from 'twenty-shared/types'; +import { RelationType } from 'src/engine/metadata-modules/field-metadata/interfaces/relation-type.interface'; + describe('updateOne', () => { describe('FieldMetadataService name/label sync', () => { let listingObjectId = ''; @@ -115,5 +117,59 @@ describe('updateOne', () => { 'Name is not synced with label. Expected name: "testName", got newName', ); }); + + it('should throw if the field name is not available because of other field with the same name', async () => { + await createOneFieldMetadata({ + input: { + objectMetadataId: listingObjectId, + type: FieldMetadataType.TEXT, + name: 'otherTestName', + label: 'Test name', + }, + }); + + const { errors } = await updateOneFieldMetadata({ + input: { + idToUpdate: testFieldId, + updatePayload: { name: 'testName' }, + }, + }); + + // Assert + expect(errors[0].message).toBe( + 'Name "testName" is not available, check that it is not duplicating another field\'s name.', + ); + }); + + it('should throw if the field name is not available because of other relation field using the same {name}Id', async () => { + // Arrange + await createOneFieldMetadata({ + input: { + objectMetadataId: listingObjectId, + type: FieldMetadataType.RELATION, + name: 'children', + label: 'Children', + relationCreationPayload: { + targetObjectMetadataId: listingObjectId, + targetFieldLabel: 'parent', + targetFieldIcon: 'IconBuildingSkyscraper', + type: RelationType.ONE_TO_MANY, + }, + }, + }); + + // Act + const { errors } = await updateOneFieldMetadata({ + input: { + idToUpdate: testFieldId, + updatePayload: { name: 'parentId' }, + }, + }); + + // Assert + expect(errors[0].message).toBe( + 'Name "parentId" is not available, check that it is not duplicating another field\'s name.', + ); + }); }); });