Fix and refactor relation field name collision validation (#15920)

# Introduction Fixes https://github.com/twentyhq/private-issues/issues/371 We weren't strictly validating relation field collision on join column name availability of the target field object ## Refactor Extracted morph or relation specific condition out of the common flat field metadata name validate availability to be located in the dedicated morph or relation flat field validator ## Tests Added two tests, ONE_TO_MANY and MANY_TO_ONE in order to cover the use case
2025-11-20 16:10:35 +01:00
parent 9c6d8330df
commit e5255df1a1
11 changed files with 484 additions and 86 deletions
@@ -1,20 +1,15 @@
 import { msg } from '@lingui/core/macro';
-import {
-  FieldMetadataType,
-  compositeTypeDefinitions,
-} from 'twenty-shared/types';
+import { compositeTypeDefinitions } from 'twenty-shared/types';
 import { isDefined } from 'twenty-shared/utils';

 import { FieldMetadataExceptionCode } from 'src/engine/metadata-modules/field-metadata/field-metadata.exception';
 import { computeCompositeColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util';
 import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
 import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type';
-import { findFlatEntityByIdInFlatEntityMapsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps-or-throw.util';
 import { findManyFlatEntityByIdInFlatEntityMapsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/find-many-flat-entity-by-id-in-flat-entity-maps-or-throw.util';
 import { type FlatFieldMetadataValidationError } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata-validation-error.type';
 import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type';
-import { isFlatFieldMetadataOfType } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-flat-field-metadata-of-type.util';
-import { isMorphOrRelationFlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-morph-or-relation-flat-field-metadata.util';
+import { getObjectFieldNamesAndJoinColumnNames } from 'src/engine/metadata-modules/flat-field-metadata/utils/get-object-field-names-and-join-column-names.util';
 import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type';

 const getReservedCompositeFieldNames = (
@@ -41,15 +36,13 @@ const getReservedCompositeFieldNames = (
 };

 export const validateFlatFieldMetadataNameAvailability = ({
-  flatFieldMetadata,
+  name,
  flatFieldMetadataMaps,
-  remainingFlatEntityMapsToValidate,
  flatObjectMetadata,
 }: {
-  flatFieldMetadata: FlatFieldMetadata;
+  name: string;
  flatObjectMetadata: FlatObjectMetadata;
  flatFieldMetadataMaps: FlatEntityMaps<FlatFieldMetadata>;
-  remainingFlatEntityMapsToValidate?: FlatEntityMaps<FlatFieldMetadata>;
 }): FlatFieldMetadataValidationError[] => {
  const errors: FlatFieldMetadataValidationError[] = [];
  const objectFlatFieldMetadatas =
@@ -60,63 +53,37 @@ export const validateFlatFieldMetadataNameAvailability = ({
  const reservedCompositeFieldsNames = getReservedCompositeFieldNames(
    objectFlatFieldMetadatas,
  );
-  const flatFieldMetadataName = flatFieldMetadata.name;

-  if (
-    !isFlatFieldMetadataOfType(
-      // CHALLENGE Could remove the is morph relation assertion
-      flatFieldMetadata,
-      FieldMetadataType.MORPH_RELATION,
-    ) &&
-    objectFlatFieldMetadatas.some((existingFlatFieldMetadata) => {
-      const firstDegreeCollision =
-        existingFlatFieldMetadata.name === flatFieldMetadataName;
-
-      if (firstDegreeCollision) {
-        return true;
-      }
-
-      if (!isMorphOrRelationFlatFieldMetadata(existingFlatFieldMetadata)) {
-        return false;
-      }
-
-      const targetFlatFieldMetadata =
-        flatFieldMetadata.id ===
-        existingFlatFieldMetadata.relationTargetFieldMetadataId
-          ? flatFieldMetadata
-          : (remainingFlatEntityMapsToValidate?.byId[
-              existingFlatFieldMetadata.relationTargetFieldMetadataId
-            ] ??
-            findFlatEntityByIdInFlatEntityMapsOrThrow({
-              flatEntityId:
-                existingFlatFieldMetadata.relationTargetFieldMetadataId,
-              flatEntityMaps: flatFieldMetadataMaps,
-            }));
-
-      if (!isMorphOrRelationFlatFieldMetadata(targetFlatFieldMetadata)) {
-        return false;
-      }
-
-      return (
-        targetFlatFieldMetadata.settings.joinColumnName ===
-        flatFieldMetadataName
-      );
-    })
-  ) {
+  if (reservedCompositeFieldsNames.includes(name)) {
    errors.push({
-      code: FieldMetadataExceptionCode.NOT_AVAILABLE,
-      value: flatFieldMetadataName,
-      message: `Name "${flatFieldMetadataName}" is not available as it is already used by another field`,
-      userFriendlyMessage: msg`Name "${flatFieldMetadataName}" is not available as it is already used by another field`,
+      code: FieldMetadataExceptionCode.RESERVED_KEYWORD,
+      message: `Name "${name}" is reserved composite field name`,
+      value: name,
+      userFriendlyMessage: msg`Name "${name}" is not available`,
    });
  }

-  if (reservedCompositeFieldsNames.includes(flatFieldMetadataName)) {
+  const { objectFieldNamesAndJoinColumnNames } =
+    getObjectFieldNamesAndJoinColumnNames({
+      flatFieldMetadataMaps,
+      flatObjectMetadata,
+    });
+
+  if (objectFieldNamesAndJoinColumnNames.fieldNames.includes(name)) {
    errors.push({
-      code: FieldMetadataExceptionCode.RESERVED_KEYWORD,
-      message: `Name "${flatFieldMetadataName}" is reserved composite field name`,
-      value: flatFieldMetadataName,
-      userFriendlyMessage: msg`Name "${flatFieldMetadataName}" is not available`,
+      code: FieldMetadataExceptionCode.NOT_AVAILABLE,
+      value: name,
+      message: `Name "${name}" is not available as it is already used by another field`,
+      userFriendlyMessage: msg`Name "${name}" is not available as it is already used by another field`,
+    });
+  }
+
+  if (objectFieldNamesAndJoinColumnNames.joinColumnNames.includes(name)) {
+    errors.push({
+      code: FieldMetadataExceptionCode.NOT_AVAILABLE,
+      value: name,
+      message: `Name "${name}" is not available as it is already used by another join column name`,
+      userFriendlyMessage: msg`Name "${name}" is not available as it is already used by join column name`,
    });
  }

@@ -0,0 +1,79 @@
+import { msg } from '@lingui/core/macro';
+import { RelationType } from 'twenty-shared/types';
+import { assertUnreachable, isDefined } from 'twenty-shared/utils';
+
+import { FieldMetadataExceptionCode } from 'src/engine/metadata-modules/field-metadata/field-metadata.exception';
+import { type MorphOrRelationFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/types/morph-or-relation-field-metadata-type.type';
+import { type MetadataFlatEntityAndRelatedFlatEntityMapsForValidation } from 'src/engine/metadata-modules/flat-entity/types/metadata-flat-entity-and-related-flat-entity-maps-for-validation.type';
+import { findFlatEntityByIdInFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps.util';
+import { type FlatFieldMetadataValidationError } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata-validation-error.type';
+import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type';
+import { validateFlatFieldMetadataNameAvailability } from 'src/engine/metadata-modules/flat-field-metadata/validators/utils/validate-flat-field-metadata-name-availability.util';
+
+export const validateMorphOrRelationFlatFieldJoinColumName = ({
+  flatFieldMetadata,
+  optimisticFlatEntityMapsAndRelatedFlatEntityMaps: {
+    flatFieldMetadataMaps,
+    flatObjectMetadataMaps,
+  },
+}: {
+  flatFieldMetadata: FlatFieldMetadata<MorphOrRelationFieldMetadataType>;
+  optimisticFlatEntityMapsAndRelatedFlatEntityMaps: MetadataFlatEntityAndRelatedFlatEntityMapsForValidation<'fieldMetadata'>;
+}): FlatFieldMetadataValidationError[] => {
+  const errors: FlatFieldMetadataValidationError[] = [];
+
+  switch (flatFieldMetadata.settings.relationType) {
+    case RelationType.MANY_TO_ONE: {
+      if (!isDefined(flatFieldMetadata.settings.joinColumnName)) {
+        errors.push({
+          code: FieldMetadataExceptionCode.INVALID_FIELD_INPUT,
+          message:
+            'Many to one field metadata should carry the join column name in its settings',
+          userFriendlyMessage: msg`A many to one relation field should always declare a join column`,
+        });
+
+        return errors;
+      }
+
+      const flatObjectMetadata = findFlatEntityByIdInFlatEntityMaps({
+        flatEntityId: flatFieldMetadata.objectMetadataId,
+        flatEntityMaps: flatObjectMetadataMaps,
+      });
+
+      if (!isDefined(flatObjectMetadata)) {
+        errors.push({
+          code: FieldMetadataExceptionCode.INVALID_FIELD_INPUT,
+          message: 'Could not find relation field parent flat object',
+          userFriendlyMessage: msg`Could not find relation field parent flat object`,
+        });
+
+        return errors;
+      }
+
+      errors.push(
+        ...validateFlatFieldMetadataNameAvailability({
+          name: flatFieldMetadata.settings.joinColumnName,
+          flatFieldMetadataMaps,
+          flatObjectMetadata,
+        }),
+      );
+      break;
+    }
+    case RelationType.ONE_TO_MANY: {
+      if (isDefined(flatFieldMetadata.settings.joinColumnName)) {
+        errors.push({
+          code: FieldMetadataExceptionCode.INVALID_FIELD_INPUT,
+          message:
+            'One to many field metadata should not carry the join column name in its settings',
+          userFriendlyMessage: msg`A one to many relation field should never declare a join column`,
+        });
+      }
+      break;
+    }
+    default: {
+      assertUnreachable(flatFieldMetadata.settings.relationType);
+    }
+  }
+
+  return errors;
+};
@@ -3,8 +3,12 @@ import { isDefined, isValidUuid } from 'twenty-shared/utils';

 import { FieldMetadataExceptionCode } from 'src/engine/metadata-modules/field-metadata/field-metadata.exception';
 import { type MorphOrRelationFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/types/morph-or-relation-field-metadata-type.type';
+import { findFlatEntityByIdInFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps.util';
 import { type FlatFieldMetadataTypeValidationArgs } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata-type-validator.type';
 import { type FlatFieldMetadataValidationError } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata-validation-error.type';
+import { isMorphOrRelationFlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-morph-or-relation-flat-field-metadata.util';
+import { validateMorphOrRelationFlatFieldJoinColumName } from 'src/engine/metadata-modules/flat-field-metadata/validators/utils/validate-morph-or-relation-flat-field-join-column-name.util';
+import { findFlatEntityPropertyUpdate } from 'src/engine/workspace-manager/workspace-migration-v2/utils/find-flat-entity-property-update.util';

 export const validateMorphOrRelationFlatFieldMetadata = async ({
  flatEntityToValidate: flatFieldMetadataToValidate,
@@ -12,6 +16,7 @@ export const validateMorphOrRelationFlatFieldMetadata = async ({
    flatFieldMetadataMaps,
    flatObjectMetadataMaps,
  },
+  updates,
  remainingFlatEntityMapsToValidate,
 }: FlatFieldMetadataTypeValidationArgs<MorphOrRelationFieldMetadataType>): Promise<
  FlatFieldMetadataValidationError[]
@@ -39,25 +44,29 @@ export const validateMorphOrRelationFlatFieldMetadata = async ({

  const errors: FlatFieldMetadataValidationError[] = [];

-  const targetRelationFlatObjectMetadata =
-    flatObjectMetadataMaps?.byId[relationTargetObjectMetadataId];
+  const targetFlatObjectMetadata = findFlatEntityByIdInFlatEntityMaps({
+    flatEntityId: relationTargetObjectMetadataId,
+    flatEntityMaps: flatObjectMetadataMaps,
+  });

-  if (!isDefined(targetRelationFlatObjectMetadata)) {
+  const targetFlatFieldMetadata =
+    remainingFlatEntityMapsToValidate?.byId[relationTargetFieldMetadataId] ??
+    findFlatEntityByIdInFlatEntityMaps({
+      flatEntityId: relationTargetFieldMetadataId,
+      flatEntityMaps: flatFieldMetadataMaps,
+    });
+
+  if (!isDefined(targetFlatObjectMetadata)) {
    errors.push({
      code: FieldMetadataExceptionCode.OBJECT_METADATA_NOT_FOUND,
      message: 'Relation target object metadata not found',
      userFriendlyMessage: msg`Object targeted by the relation not found`,
    });
+
+    return errors;
  }

-  const targetRelationFlatFieldMetadata =
-    remainingFlatEntityMapsToValidate?.byId[relationTargetFieldMetadataId] ??
-    flatFieldMetadataMaps.byId[relationTargetFieldMetadataId];
-
-  if (
-    isDefined(targetRelationFlatObjectMetadata) &&
-    !isDefined(targetRelationFlatFieldMetadata)
-  ) {
+  if (!isDefined(targetFlatFieldMetadata)) {
    errors.push({
      code: FieldMetadataExceptionCode.FIELD_METADATA_NOT_FOUND,
      message: isDefined(remainingFlatEntityMapsToValidate)
@@ -65,7 +74,33 @@ export const validateMorphOrRelationFlatFieldMetadata = async ({
        : 'Relation field target metadata not found',
      userFriendlyMessage: msg`Relation field target metadata not found`,
    });
+  } else if (!isMorphOrRelationFlatFieldMetadata(targetFlatFieldMetadata)) {
+    errors.push({
+      code: FieldMetadataExceptionCode.INVALID_FIELD_INPUT,
+      message:
+        'A relation field can only target a MORPH_RELATION or another RELATION field',
+      userFriendlyMessage: msg`Invalid relation field target`,
+    });
  }

+  if (
+    !isDefined(updates) ||
+    isDefined(
+      findFlatEntityPropertyUpdate({
+        flatEntityUpdates: updates,
+        property: 'settings',
+      }),
+    )
+  )
+    errors.push(
+      ...validateMorphOrRelationFlatFieldJoinColumName({
+        flatFieldMetadata: flatFieldMetadataToValidate,
+        optimisticFlatEntityMapsAndRelatedFlatEntityMaps: {
+          flatFieldMetadataMaps,
+          flatObjectMetadataMaps,
+        },
+      }),
+    );
+
  return errors;
 };