Remove number from label identifier list (#12831)

packages/twenty-server/src/engine/metadata-modules/object-metadata/dtos/update-object.input.ts

@@ -1,12 +1,14 @@
 import { Field, InputType } from '@nestjs/graphql';
 
 import { BeforeUpdateOne } from '@ptc-org/nestjs-query-graphql';
+import { Type } from 'class-transformer';
 import {
   IsBoolean,
   IsNotEmpty,
   IsOptional,
   IsString,
   IsUUID,
+  ValidateNested,
 } from 'class-validator';
 
 import { UUIDScalarType } from 'src/engine/api/graphql/workspace-schema-builder/graphql-types/scalars';
@@ -76,6 +78,8 @@ export class UpdateObjectPayload {
 @InputType()
 @BeforeUpdateOne(BeforeUpdateOneObject)
 export class UpdateOneObjectInput {
+  @Type(() => UpdateObjectPayload)
+  @ValidateNested()
   @Field(() => UpdateObjectPayload)
   update: UpdateObjectPayload;
 

packages/twenty-server/src/engine/metadata-modules/object-metadata/hooks/__tests__/before-update-one-object.hook.spec.ts

@@ -656,34 +656,6 @@ describe('BeforeUpdateOneObject', () => {
     expect(result).toEqual(instance);
   });
 
-  it('should throw BadRequestException if label identifier field does not exist', async () => {
-    const labelIdentifierFieldId = 'nonexistent-field-id';
-    const instance: UpdateOneInputType<UpdateObjectPayloadForTest> = {
-      id: mockObjectId,
-      update: {
-        labelIdentifierFieldMetadataId: labelIdentifierFieldId,
-      },
-    };
-
-    const mockObject: Partial<ObjectMetadataEntity> = {
-      id: mockObjectId,
-      isCustom: true,
-    };
-
-    jest
-      .spyOn(objectMetadataService, 'findOneWithinWorkspace')
-      .mockResolvedValue(mockObject as ObjectMetadataEntity);
-
-    jest.spyOn(fieldMetadataRepository, 'findBy').mockResolvedValue([]);
-
-    await expect(
-      hook.run(instance as UpdateOneInputType<UpdateObjectPayload>, {
-        workspaceId: mockWorkspaceId,
-        locale: undefined,
-      }),
-    ).rejects.toThrow('This label identifier does not exist');
-  });
-
   it('should validate image identifier field correctly for custom objects', async () => {
     const imageIdentifierFieldId = 'image-field-id';
     const instance: UpdateOneInputType<UpdateObjectPayloadForTest> = {
@@ -722,32 +694,4 @@ describe('BeforeUpdateOneObject', () => {
 
     expect(result).toEqual(instance);
   });
-
-  it('should throw BadRequestException if image identifier field does not exist', async () => {
-    const imageIdentifierFieldId = 'nonexistent-field-id';
-    const instance: UpdateOneInputType<UpdateObjectPayloadForTest> = {
-      id: mockObjectId,
-      update: {
-        imageIdentifierFieldMetadataId: imageIdentifierFieldId,
-      },
-    };
-
-    const mockObject: Partial<ObjectMetadataEntity> = {
-      id: mockObjectId,
-      isCustom: true,
-    };
-
-    jest
-      .spyOn(objectMetadataService, 'findOneWithinWorkspace')
-      .mockResolvedValue(mockObject as ObjectMetadataEntity);
-
-    jest.spyOn(fieldMetadataRepository, 'findBy').mockResolvedValue([]);
-
-    await expect(
-      hook.run(instance as UpdateOneInputType<UpdateObjectPayload>, {
-        workspaceId: mockWorkspaceId,
-        locale: undefined,
-      }),
-    ).rejects.toThrow('This image identifier does not exist');
-  });
 });

packages/twenty-server/src/engine/metadata-modules/object-metadata/hooks/before-update-one-object.hook.ts

@@ -12,7 +12,7 @@ import {
 } from '@ptc-org/nestjs-query-graphql';
 import { APP_LOCALES, SOURCE_LOCALE } from 'twenty-shared/translations';
 import { isDefined } from 'twenty-shared/utils';
-import { Equal, In, Repository } from 'typeorm';
+import { Repository } from 'typeorm';
 
 import { generateMessageId } from 'src/engine/core-modules/i18n/utils/generateMessageId';
 import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
@@ -57,8 +57,6 @@ export class BeforeUpdateOneObject<T extends UpdateObjectPayload>
       return this.handleStandardObjectUpdate(instance, objectMetadata, locale);
     }
 
-    await this.validateIdentifierFields(instance, workspaceId);
-
     return instance;
   }
 
@@ -437,56 +435,4 @@ export class BeforeUpdateOneObject<T extends UpdateObjectPayload>
       locale,
     );
   }
-
-  private async validateIdentifierFields(
-    instance: UpdateOneInputType<T>,
-    workspaceId: string,
-  ): Promise<void> {
-    if (
-      !instance.update.labelIdentifierFieldMetadataId &&
-      !instance.update.imageIdentifierFieldMetadataId
-    ) {
-      return;
-    }
-
-    const fields = await this.fieldMetadataRepository.findBy({
-      workspaceId: Equal(workspaceId),
-      objectMetadataId: Equal(instance.id.toString()),
-      id: In(
-        [
-          instance.update.labelIdentifierFieldMetadataId,
-          instance.update.imageIdentifierFieldMetadataId,
-        ].filter((id) => id !== null),
-      ),
-    });
-
-    const fieldIds = fields.map((field) => field.id);
-
-    this.validateLabelIdentifier(instance, fieldIds);
-    this.validateImageIdentifier(instance, fieldIds);
-  }
-
-  private validateLabelIdentifier(
-    instance: UpdateOneInputType<T>,
-    fieldIds: string[],
-  ): void {
-    if (
-      instance.update.labelIdentifierFieldMetadataId &&
-      !fieldIds.includes(instance.update.labelIdentifierFieldMetadataId)
-    ) {
-      throw new BadRequestException('This label identifier does not exist');
-    }
-  }
-
-  private validateImageIdentifier(
-    instance: UpdateOneInputType<T>,
-    fieldIds: string[],
-  ): void {
-    if (
-      instance.update.imageIdentifierFieldMetadataId &&
-      !fieldIds.includes(instance.update.imageIdentifierFieldMetadataId)
-    ) {
-      throw new BadRequestException('This image identifier does not exist');
-    }
-  }
 }

packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts

@@ -34,6 +34,7 @@ import {
 import { RemoteTableRelationsService } from 'src/engine/metadata-modules/remote-server/remote-table/remote-table-relations/remote-table-relations.service';
 import { SearchVectorService } from 'src/engine/metadata-modules/search-vector/search-vector.service';
 import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps';
+import { validateMetadataIdentifierFieldMetadataIds } from 'src/engine/metadata-modules/utils/validate-metadata-identifier-field-metadata-id.utils';
 import { validateNameAndLabelAreSyncOrThrow } from 'src/engine/metadata-modules/utils/validate-name-and-label-are-sync-or-throw.util';
 import { validatesNoOtherObjectWithSameNameExistsOrThrows } from 'src/engine/metadata-modules/utils/validate-no-other-object-with-same-name-exists-or-throw.util';
 import { WorkspaceMetadataCacheService } from 'src/engine/metadata-modules/workspace-metadata-cache/services/workspace-metadata-cache.service';
@@ -300,6 +301,14 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
       });
     }
 
+    validateMetadataIdentifierFieldMetadataIds({
+      fieldMetadataItems: Object.values(existingObjectMetadata.fieldsById),
+      labelIdentifierFieldMetadataId:
+        inputPayload.labelIdentifierFieldMetadataId,
+      imageIdentifierFieldMetadataId:
+        inputPayload.imageIdentifierFieldMetadataId,
+    });
+
     const updatedObject = await super.updateOne(inputId, inputPayload);
 
     await this.handleObjectNameAndLabelUpdates(

packages/twenty-server/src/engine/metadata-modules/utils/validate-metadata-identifier-field-metadata-id.utils.ts

@@ -0,0 +1,94 @@
+import {
+  isDefined,
+  isLabelIdentifierFieldMetadataTypes,
+} from 'twenty-shared/utils';
+
+import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface';
+
+import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
+import {
+  ObjectMetadataException,
+  ObjectMetadataExceptionCode,
+} from 'src/engine/metadata-modules/object-metadata/object-metadata.exception';
+
+type Validator = {
+  validator: (args: {
+    fieldMetadataId: string;
+    matchingFieldMetadata?: FieldMetadataEntity | FieldMetadataInterface;
+  }) => boolean;
+  label: string;
+};
+
+type ValidateMetadataIdentifierFieldMetadataIdOrThrowArgs = {
+  fieldMetadataId: string;
+  fieldMetadataItems: FieldMetadataEntity[] | FieldMetadataInterface[];
+  validators: Validator[];
+};
+const validatorRunner = ({
+  fieldMetadataId,
+  fieldMetadataItems,
+  validators,
+}: ValidateMetadataIdentifierFieldMetadataIdOrThrowArgs): void => {
+  const matchingFieldMetadata = fieldMetadataItems.find(
+    (fieldMetadata) => fieldMetadata.id === fieldMetadataId,
+  );
+
+  validators.forEach(({ label, validator }) => {
+    if (validator({ fieldMetadataId, matchingFieldMetadata })) {
+      throw new ObjectMetadataException(
+        label,
+        ObjectMetadataExceptionCode.INVALID_OBJECT_INPUT,
+      );
+    }
+  });
+};
+
+type ValidateMetadataIdentifierFieldMetadataIdsArgs = {
+  labelIdentifierFieldMetadataId: string | undefined;
+  imageIdentifierFieldMetadataId: string | undefined;
+  fieldMetadataItems: FieldMetadataEntity[] | FieldMetadataInterface[];
+};
+export const validateMetadataIdentifierFieldMetadataIds = ({
+  imageIdentifierFieldMetadataId,
+  labelIdentifierFieldMetadataId,
+  fieldMetadataItems,
+}: ValidateMetadataIdentifierFieldMetadataIdsArgs) => {
+  const isMatchingFieldMetadataDefined: Validator['validator'] = ({
+    matchingFieldMetadata,
+  }) => !isDefined(matchingFieldMetadata);
+
+  if (isDefined(labelIdentifierFieldMetadataId)) {
+    validatorRunner({
+      fieldMetadataId: labelIdentifierFieldMetadataId,
+      fieldMetadataItems,
+      validators: [
+        {
+          validator: isMatchingFieldMetadataDefined,
+          label:
+            'labelIdentifierFieldMetadataId validation failed: related field metadata not found',
+        },
+        {
+          validator: ({ matchingFieldMetadata }) =>
+            isDefined(matchingFieldMetadata) &&
+            !isLabelIdentifierFieldMetadataTypes(matchingFieldMetadata.type),
+          label:
+            'labelIdentifierFieldMetadataId validation failed: it must be a TEXT or FULL_NAME field metadata type id',
+        },
+      ],
+    });
+  }
+
+  if (isDefined(imageIdentifierFieldMetadataId)) {
+    validatorRunner({
+      fieldMetadataId: imageIdentifierFieldMetadataId,
+      fieldMetadataItems,
+      validators: [
+        {
+          validator: isMatchingFieldMetadataDefined,
+          label:
+            'imageIdentifierFieldMetadataId validation failed: related field metadata not found',
+        },
+      ],
+    });
+  }
+};

packages/twenty-server/test/integration/metadata/suites/object-metadata/__snapshots__/failing-update-one-object-metadata.integration-spec.ts.snap

@@ -0,0 +1,34 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Object metadata update should fail when labelIdentifier is not a TEXT or NAME field 1`] = `
+[
+  {
+    "extensions": {
+      "code": "BAD_USER_INPUT",
+    },
+    "message": "labelIdentifierFieldMetadataId validation failed: it must be a TEXT or FULL_NAME field metadata type id",
+  },
+]
+`;
+
+exports[`Object metadata update should fail when labelIdentifier is not a known field metadata id 1`] = `
+[
+  {
+    "extensions": {
+      "code": "BAD_USER_INPUT",
+    },
+    "message": "labelIdentifierFieldMetadataId validation failed: related field metadata not found",
+  },
+]
+`;
+
+exports[`Object metadata update should fail when labelIdentifier is not a uuid 1`] = `
+[
+  {
+    "extensions": {
+      "code": "BAD_USER_INPUT",
+    },
+    "message": "labelIdentifierFieldMetadataId must be a UUID",
+  },
+]
+`;

packages/twenty-server/test/integration/metadata/suites/object-metadata/failing-update-one-object-metadata.integration-spec.ts

@@ -0,0 +1,96 @@
+import { createOneFieldMetadata } from 'test/integration/metadata/suites/field-metadata/utils/create-one-field-metadata.util';
+import { createOneObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/create-one-object-metadata.util';
+import { deleteOneObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/delete-one-object-metadata.util';
+import { getMockCreateObjectInput } from 'test/integration/metadata/suites/object-metadata/utils/generate-mock-create-object-metadata-input';
+import { updateOneObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/update-one-object-metadata.util';
+import { EachTestingContext } from 'twenty-shared/testing';
+import { FieldMetadataType } from 'twenty-shared/types';
+
+import { UpdateObjectPayload } from 'src/engine/metadata-modules/object-metadata/dtos/update-object.input';
+
+type TestingRuntimeContext = {
+  objectMetadataId: string;
+  numberFieldMetadataId: string;
+};
+
+type CreateOneObjectMetadataItemTestingContext = EachTestingContext<
+  | ((args: TestingRuntimeContext) => Partial<UpdateObjectPayload>)
+  | Partial<UpdateObjectPayload>
+>[];
+
+const labelIdentifierFailingTestsUseCase: CreateOneObjectMetadataItemTestingContext =
+  [
+    {
+      title: 'when labelIdentifier is not a uuid',
+      context: {
+        labelIdentifierFieldMetadataId: 'not-a-uuid',
+      },
+    },
+    {
+      title: 'when labelIdentifier is not a known field metadata id',
+      context: {
+        labelIdentifierFieldMetadataId: '42422020-f49c-4159-8751-76a24f47b360',
+      },
+    },
+    {
+      title: 'when labelIdentifier is not a TEXT or NAME field',
+      context: ({ numberFieldMetadataId }) => ({
+        labelIdentifierFieldMetadataId: numberFieldMetadataId,
+      }),
+    },
+  ];
+
+const allTestsUseCases = [...labelIdentifierFailingTestsUseCase];
+
+describe('Object metadata update should fail', () => {
+  let objectMetadataId: string;
+  let numberFieldMetadataId: string;
+
+  beforeAll(async () => {
+    const { data } = await createOneObjectMetadata({
+      input: getMockCreateObjectInput(),
+    });
+
+    objectMetadataId = data.createOneObject.id;
+
+    const {
+      data: { createOneField },
+    } = await createOneFieldMetadata({
+      input: {
+        objectMetadataId: objectMetadataId,
+        name: 'testName',
+        label: 'Test name',
+        isLabelSyncedWithName: true,
+        type: FieldMetadataType.NUMBER,
+      },
+    });
+
+    numberFieldMetadataId = createOneField.id;
+  });
+
+  afterAll(async () => {
+    await deleteOneObjectMetadata({
+      input: {
+        idToDelete: objectMetadataId,
+      },
+    });
+  });
+
+  it.each(allTestsUseCases)('$title', async ({ context }) => {
+    const updatePayload =
+      typeof context === 'function'
+        ? context({ numberFieldMetadataId, objectMetadataId })
+        : context;
+
+    const { errors } = await updateOneObjectMetadata({
+      input: {
+        idToUpdate: objectMetadataId,
+        updatePayload,
+      },
+      expectToFail: true,
+    });
+
+    expect(errors).toBeDefined();
+    expect(errors).toMatchSnapshot();
+  });
+});