#6094 Prevent creating a custom field with an existing name (#6100)

Fixes #6094 Description: Added logic inside SettingsObjectNewFieldStep2.tsx to prevent form submission Current Behaviours: <img width="947" alt="Screenshot 2024-07-03 at 1 45 31 PM" src="https://github.com/twentyhq/twenty/assets/95612797/bef54bc4-fc83-48f3-894a-34445ec64723"> --------- Co-authored-by: Marie Stoppa <marie.stoppa@essec.edu>
2024-07-17 15:13:57 +05:30
parent 37ae99390e
commit 94c2358c89
11 changed files with 212 additions and 151 deletions
--- a/packages/twenty-front/src/modules/object-metadata/validation-schemas/tests/metadataLabelSchema.test.ts
+++ b/packages/twenty-front/src/modules/object-metadata/validation-schemas/tests/metadataLabelSchema.test.ts
@ -6,7 +6,7 @@ describe('metadataLabelSchema', () => {
    const validMetadataLabel = 'Option 1';

    // When
-    const result = metadataLabelSchema.parse(validMetadataLabel);
+    const result = metadataLabelSchema().parse(validMetadataLabel);

    // Then
    expect(result).toEqual(validMetadataLabel);
@ -16,7 +16,7 @@ describe('metadataLabelSchema', () => {
    const validMetadataLabel = 'עִבְרִי';

    // When
-    const result = metadataLabelSchema.parse(validMetadataLabel);
+    const result = metadataLabelSchema().parse(validMetadataLabel);

    // Then
    expect(result).toEqual(validMetadataLabel);
--- a/packages/twenty-front/src/modules/object-metadata/validation-schemas/fieldMetadataItemSchema.ts
+++ b/packages/twenty-front/src/modules/object-metadata/validation-schemas/fieldMetadataItemSchema.ts
@ -10,97 +10,99 @@ import {
 } from '~/generated-metadata/graphql';
 import { camelCaseStringSchema } from '~/utils/validation-schemas/camelCaseStringSchema';

-export const fieldMetadataItemSchema = z.object({
-  __typename: z.literal('field').optional(),
-  createdAt: z.string().datetime(),
-  defaultValue: z.any().optional(),
-  description: z.string().trim().nullable().optional(),
-  fromRelationMetadata: z
-    .object({
-      __typename: z.literal('relation').optional(),
-      id: z.string().uuid(),
-      relationType: z.nativeEnum(RelationMetadataType),
-      toFieldMetadataId: z.string().uuid(),
-      toObjectMetadata: z.object({
-        __typename: z.literal('object').optional(),
-        dataSourceId: z.string().uuid(),
+export const fieldMetadataItemSchema = (existingLabels?: string[]) => {
+  return z.object({
+    __typename: z.literal('field').optional(),
+    createdAt: z.string().datetime(),
+    defaultValue: z.any().optional(),
+    description: z.string().trim().nullable().optional(),
+    fromRelationMetadata: z
+      .object({
+        __typename: z.literal('relation').optional(),
        id: z.string().uuid(),
-        isRemote: z.boolean(),
-        isSystem: z.boolean(),
-        namePlural: z.string().trim().min(1),
-        nameSingular: z.string().trim().min(1),
-      }),
-    })
-    .nullable()
-    .optional(),
-  icon: z.string().startsWith('Icon').trim().nullable(),
-  id: z.string().uuid(),
-  isActive: z.boolean(),
-  isCustom: z.boolean(),
-  isNullable: z.boolean(),
-  isSystem: z.boolean(),
-  label: metadataLabelSchema,
-  name: camelCaseStringSchema,
-  options: z
-    .array(
-      z.object({
-        color: themeColorSchema,
+        relationType: z.nativeEnum(RelationMetadataType),
+        toFieldMetadataId: z.string().uuid(),
+        toObjectMetadata: z.object({
+          __typename: z.literal('object').optional(),
+          dataSourceId: z.string().uuid(),
+          id: z.string().uuid(),
+          isRemote: z.boolean(),
+          isSystem: z.boolean(),
+          namePlural: z.string().trim().min(1),
+          nameSingular: z.string().trim().min(1),
+        }),
+      })
+      .nullable()
+      .optional(),
+    icon: z.string().startsWith('Icon').trim().nullable(),
+    id: z.string().uuid(),
+    isActive: z.boolean(),
+    isCustom: z.boolean(),
+    isNullable: z.boolean(),
+    isSystem: z.boolean(),
+    label: metadataLabelSchema(existingLabels),
+    name: camelCaseStringSchema,
+    options: z
+      .array(
+        z.object({
+          color: themeColorSchema,
+          id: z.string().uuid(),
+          label: z.string().trim().min(1),
+          position: z.number(),
+          value: z.string().trim().min(1),
+        }),
+      )
+      .nullable()
+      .optional(),
+    relationDefinition: z
+      .object({
+        __typename: z.literal('RelationDefinition').optional(),
+        relationId: z.string().uuid(),
+        direction: z.nativeEnum(RelationDefinitionType),
+        sourceFieldMetadata: z.object({
+          __typename: z.literal('field').optional(),
+          id: z.string().uuid(),
+          name: z.string().trim().min(1),
+        }),
+        sourceObjectMetadata: z.object({
+          __typename: z.literal('object').optional(),
+          id: z.string().uuid(),
+          namePlural: z.string().trim().min(1),
+          nameSingular: z.string().trim().min(1),
+        }),
+        targetFieldMetadata: z.object({
+          __typename: z.literal('field').optional(),
+          id: z.string().uuid(),
+          name: z.string().trim().min(1),
+        }),
+        targetObjectMetadata: z.object({
+          __typename: z.literal('object').optional(),
+          id: z.string().uuid(),
+          namePlural: z.string().trim().min(1),
+          nameSingular: z.string().trim().min(1),
+        }),
+      })
+      .nullable()
+      .optional(),
+    toRelationMetadata: z
+      .object({
+        __typename: z.literal('relation').optional(),
        id: z.string().uuid(),
-        label: z.string().trim().min(1),
-        position: z.number(),
-        value: z.string().trim().min(1),
-      }),
-    )
-    .nullable()
-    .optional(),
-  relationDefinition: z
-    .object({
-      __typename: z.literal('RelationDefinition').optional(),
-      relationId: z.string().uuid(),
-      direction: z.nativeEnum(RelationDefinitionType),
-      sourceFieldMetadata: z.object({
-        __typename: z.literal('field').optional(),
-        id: z.string().uuid(),
-        name: z.string().trim().min(1),
-      }),
-      sourceObjectMetadata: z.object({
-        __typename: z.literal('object').optional(),
-        id: z.string().uuid(),
-        namePlural: z.string().trim().min(1),
-        nameSingular: z.string().trim().min(1),
-      }),
-      targetFieldMetadata: z.object({
-        __typename: z.literal('field').optional(),
-        id: z.string().uuid(),
-        name: z.string().trim().min(1),
-      }),
-      targetObjectMetadata: z.object({
-        __typename: z.literal('object').optional(),
-        id: z.string().uuid(),
-        namePlural: z.string().trim().min(1),
-        nameSingular: z.string().trim().min(1),
-      }),
-    })
-    .nullable()
-    .optional(),
-  toRelationMetadata: z
-    .object({
-      __typename: z.literal('relation').optional(),
-      id: z.string().uuid(),
-      relationType: z.nativeEnum(RelationMetadataType),
-      fromFieldMetadataId: z.string().uuid(),
-      fromObjectMetadata: z.object({
-        __typename: z.literal('object').optional(),
-        id: z.string().uuid(),
-        dataSourceId: z.string().uuid(),
-        isRemote: z.boolean(),
-        isSystem: z.boolean(),
-        namePlural: z.string().trim().min(1),
-        nameSingular: z.string().trim().min(1),
-      }),
-    })
-    .nullable()
-    .optional(),
-  type: z.nativeEnum(FieldMetadataType),
-  updatedAt: z.string().datetime(),
-}) satisfies z.ZodType<FieldMetadataItem>;
+        relationType: z.nativeEnum(RelationMetadataType),
+        fromFieldMetadataId: z.string().uuid(),
+        fromObjectMetadata: z.object({
+          __typename: z.literal('object').optional(),
+          id: z.string().uuid(),
+          dataSourceId: z.string().uuid(),
+          isRemote: z.boolean(),
+          isSystem: z.boolean(),
+          namePlural: z.string().trim().min(1),
+          nameSingular: z.string().trim().min(1),
+        }),
+      })
+      .nullable()
+      .optional(),
+    type: z.nativeEnum(FieldMetadataType),
+    updatedAt: z.string().datetime(),
+  }) satisfies z.ZodType<FieldMetadataItem>;
+};
--- a/packages/twenty-front/src/modules/object-metadata/validation-schemas/metadataLabelSchema.ts
+++ b/packages/twenty-front/src/modules/object-metadata/validation-schemas/metadataLabelSchema.ts
@ -1,23 +1,42 @@
+import { errors } from '@/settings/data-model/fields/forms/utils/errorMessages';
 import { z } from 'zod';

 import { METADATA_LABEL_VALID_PATTERN } from '~/pages/settings/data-model/constants/MetadataLabelValidPattern';
 import { computeMetadataNameFromLabelOrThrow } from '~/pages/settings/data-model/utils/compute-metadata-name-from-label.utils';
-
-export const metadataLabelSchema = z
-  .string()
-  .trim()
-  .min(1)
-  .regex(METADATA_LABEL_VALID_PATTERN)
-  .refine(
-    (label) => {
-      try {
-        computeMetadataNameFromLabelOrThrow(label);
-        return true;
-      } catch (error) {
-        return false;
-      }
-    },
-    {
-      message: 'Label is not formattable',
-    },
-  ); // allows non-latin char
+export const metadataLabelSchema = (existingLabels?: string[]) => {
+  return z
+    .string()
+    .trim()
+    .min(1, errors.LabelEmpty)
+    .regex(METADATA_LABEL_VALID_PATTERN, errors.LabelNotFormattable)
+    .refine(
+      (label) => {
+        try {
+          computeMetadataNameFromLabelOrThrow(label);
+          return true;
+        } catch (error) {
+          return false;
+        }
+      },
+      {
+        message: errors.LabelNotFormattable,
+      },
+    )
+    .refine(
+      (label) => {
+        try {
+          if (!existingLabels || !label?.length) {
+            return true;
+          }
+          return !existingLabels.includes(
+            computeMetadataNameFromLabelOrThrow(label),
+          );
+        } catch (error) {
+          return false;
+        }
+      },
+      {
+        message: errors.LabelNotUnique,
+      },
+    );
+};
--- a/packages/twenty-front/src/modules/object-metadata/validation-schemas/objectMetadataItemSchema.ts
+++ b/packages/twenty-front/src/modules/object-metadata/validation-schemas/objectMetadataItemSchema.ts
@ -10,7 +10,7 @@ export const objectMetadataItemSchema = z.object({
  createdAt: z.string().datetime(),
  dataSourceId: z.string().uuid(),
  description: z.string().trim().nullable().optional(),
-  fields: z.array(fieldMetadataItemSchema),
+  fields: z.array(fieldMetadataItemSchema()),
  icon: z.string().startsWith('Icon').trim(),
  id: z.string().uuid(),
  imageIdentifierFieldMetadataId: z.string().uuid().nullable(),
@ -19,8 +19,8 @@ export const objectMetadataItemSchema = z.object({
  isRemote: z.boolean(),
  isSystem: z.boolean(),
  labelIdentifierFieldMetadataId: z.string().uuid().nullable(),
-  labelPlural: metadataLabelSchema,
-  labelSingular: metadataLabelSchema,
+  labelPlural: metadataLabelSchema(),
+  labelSingular: metadataLabelSchema(),
  namePlural: camelCaseStringSchema,
  nameSingular: camelCaseStringSchema,
  updatedAt: z.string().datetime(),
--- a/packages/twenty-front/src/modules/settings/data-model/fields/forms/components/SettingsDataModelFieldAboutForm.tsx
+++ b/packages/twenty-front/src/modules/settings/data-model/fields/forms/components/SettingsDataModelFieldAboutForm.tsx
@ -1,22 +1,27 @@
-import { Controller, useFormContext } from 'react-hook-form';
 import styled from '@emotion/styled';
+import { Controller, useFormContext } from 'react-hook-form';
 import { z } from 'zod';

 import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
 import { fieldMetadataItemSchema } from '@/object-metadata/validation-schemas/fieldMetadataItemSchema';
+import { getErrorMessageFromError } from '@/settings/data-model/fields/forms/utils/errorMessages';
 import { IconPicker } from '@/ui/input/components/IconPicker';
 import { TextArea } from '@/ui/input/components/TextArea';
 import { TextInput } from '@/ui/input/components/TextInput';

-export const settingsDataModelFieldAboutFormSchema =
-  fieldMetadataItemSchema.pick({
+export const settingsDataModelFieldAboutFormSchema = (
+  existingLabels?: string[],
+) => {
+  return fieldMetadataItemSchema(existingLabels || []).pick({
    description: true,
    icon: true,
    label: true,
  });
+};

+// Correctly infer the type from the returned schema
 type SettingsDataModelFieldAboutFormValues = z.infer<
-  typeof settingsDataModelFieldAboutFormSchema
+  ReturnType<typeof settingsDataModelFieldAboutFormSchema>
 >;

 type SettingsDataModelFieldAboutFormProps = {
@ -32,13 +37,18 @@ const StyledInputsContainer = styled.div`
  width: 100%;
 `;

+const LABEL = 'label';
+
 export const SettingsDataModelFieldAboutForm = ({
  disabled,
  fieldMetadataItem,
  maxLength,
 }: SettingsDataModelFieldAboutFormProps) => {
-  const { control } = useFormContext<SettingsDataModelFieldAboutFormValues>();
-
+  const {
+    control,
+    trigger,
+    formState: { errors },
+  } = useFormContext<SettingsDataModelFieldAboutFormValues>();
  return (
    <>
      <StyledInputsContainer>
@ -56,14 +66,18 @@ export const SettingsDataModelFieldAboutForm = ({
          )}
        />
        <Controller
-          name="label"
+          name={LABEL}
          control={control}
          defaultValue={fieldMetadataItem?.label}
          render={({ field: { onChange, value } }) => (
            <TextInput
              placeholder="Employees"
              value={value}
-              onChange={onChange}
+              onChange={(e) => {
+                onChange(e);
+                trigger(LABEL);
+              }}
+              error={getErrorMessageFromError(errors.label?.message)}
              disabled={disabled}
              maxLength={maxLength}
              fullWidth
--- a/packages/twenty-front/src/modules/settings/data-model/fields/forms/relation/components/SettingsDataModelFieldRelationForm.tsx
+++ b/packages/twenty-front/src/modules/settings/data-model/fields/forms/relation/components/SettingsDataModelFieldRelationForm.tsx
@ -17,7 +17,7 @@ import { TextInput } from '@/ui/input/components/TextInput';

 export const settingsDataModelFieldRelationFormSchema = z.object({
  relation: z.object({
-    field: fieldMetadataItemSchema.pick({
+    field: fieldMetadataItemSchema().pick({
      icon: true,
      label: true,
    }),
--- a/packages/twenty-front/src/modules/settings/data-model/fields/forms/utils/errorMessages.ts
+++ b/packages/twenty-front/src/modules/settings/data-model/fields/forms/utils/errorMessages.ts
@ -0,0 +1,18 @@
+export enum errors {
+  LabelNotUnique = 'LABEL_NOT_UNIQUE',
+  LabelNotFormattable = 'LABEL_NOT_FORMATTABLE',
+  LabelEmpty = 'LABEL_EMPTY',
+}
+
+export const getErrorMessageFromError = (error?: string) => {
+  switch (error) {
+    case errors.LabelEmpty:
+      return 'Name cannot be empty.';
+    case errors.LabelNotFormattable:
+      return 'Name should start with a letter.';
+    case errors.LabelNotUnique:
+      return 'This name is already used.';
+    default:
+      return '';
+  }
+};
--- a/packages/twenty-front/src/modules/settings/data-model/fields/forms/validation-schemas/settingsFieldFormSchema.ts
+++ b/packages/twenty-front/src/modules/settings/data-model/fields/forms/validation-schemas/settingsFieldFormSchema.ts
@ -4,8 +4,10 @@ import { settingsDataModelFieldAboutFormSchema } from '@/settings/data-model/fie
 import { settingsDataModelFieldSettingsFormSchema } from '@/settings/data-model/fields/forms/components/SettingsDataModelFieldSettingsFormCard';
 import { settingsDataModelFieldTypeFormSchema } from '@/settings/data-model/fields/forms/components/SettingsDataModelFieldTypeSelect';

-export const settingsFieldFormSchema = z
-  .object({})
-  .merge(settingsDataModelFieldAboutFormSchema)
-  .merge(settingsDataModelFieldTypeFormSchema)
-  .and(settingsDataModelFieldSettingsFormSchema);
+export const settingsFieldFormSchema = (existingLabels?: string[]) => {
+  return z
+    .object({})
+    .merge(settingsDataModelFieldAboutFormSchema(existingLabels))
+    .merge(settingsDataModelFieldTypeFormSchema)
+    .and(settingsDataModelFieldSettingsFormSchema);
+};
--- a/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx
+++ b/packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx
@ -1,16 +1,15 @@
+import { useTheme } from '@emotion/react';
+import styled from '@emotion/styled';
 import {
  ChangeEvent,
  FocusEventHandler,
  ForwardedRef,
-  forwardRef,
  InputHTMLAttributes,
+  forwardRef,
  useRef,
  useState,
 } from 'react';
-import { useTheme } from '@emotion/react';
-import styled from '@emotion/styled';
-import { IconAlertCircle, IconComponent, IconEye, IconEyeOff } from 'twenty-ui';
-
+import { IconComponent, IconEye, IconEyeOff } from 'twenty-ui';
 import { useCombinedRefs } from '~/hooks/useCombinedRefs';

 const StyledContainer = styled.div<
@ -35,10 +34,12 @@ const StyledInputContainer = styled.div`
 `;

 const StyledInput = styled.input<
-  Pick<TextInputV2ComponentProps, 'fullWidth' | 'LeftIcon'>
+  Pick<TextInputV2ComponentProps, 'fullWidth' | 'LeftIcon' | 'error'>
 >`
  background-color: ${({ theme }) => theme.background.transparent.lighter};
-  border: 1px solid ${({ theme }) => theme.border.color.medium};
+  border: 1px solid
+    ${({ theme, error }) =>
+      error ? theme.border.color.danger : theme.border.color.medium};
  border-bottom-left-radius: ${({ theme, LeftIcon }) =>
    !LeftIcon && theme.border.radius.sm};
  border-right: none;
@ -86,10 +87,14 @@ const StyledLeftIconContainer = styled.div`
  padding-left: ${({ theme }) => theme.spacing(2)};
 `;

-const StyledTrailingIconContainer = styled.div`
+const StyledTrailingIconContainer = styled.div<
+  Pick<TextInputV2ComponentProps, 'error'>
+>`
  align-items: center;
  background-color: ${({ theme }) => theme.background.transparent.lighter};
-  border: 1px solid ${({ theme }) => theme.border.color.medium};
+  border: 1px solid
+    ${({ theme, error }) =>
+      error ? theme.border.color.danger : theme.border.color.medium};
  border-bottom-right-radius: ${({ theme }) => theme.border.radius.sm};
  border-left: none;
  border-top-right-radius: ${({ theme }) => theme.border.radius.sm};
@ -191,14 +196,10 @@ const TextInputV2Component = (
            value,
            LeftIcon,
            maxLength,
+            error,
          }}
        />
-        <StyledTrailingIconContainer>
-          {error && (
-            <StyledTrailingIcon>
-              <IconAlertCircle size={16} color={theme.color.red} />
-            </StyledTrailingIcon>
-          )}
+        <StyledTrailingIconContainer {...{ error }}>
          {!error && type === INPUT_TYPE_PASSWORD && (
            <StyledTrailingIcon
              onClick={handleTogglePasswordVisibility}
--- a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx
+++ b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx
@ -38,7 +38,7 @@ import { FieldMetadataType } from '~/generated-metadata/graphql';
 import { isDefined } from '~/utils/isDefined';

 type SettingsDataModelFieldEditFormValues = z.infer<
-  typeof settingsFieldFormSchema
+  ReturnType<typeof settingsFieldFormSchema>
 >;

 const StyledSettingsObjectFieldTypeSelect = styled(
@ -93,7 +93,7 @@ export const SettingsObjectFieldEdit = () => {

  const formConfig = useForm<SettingsDataModelFieldEditFormValues>({
    mode: 'onTouched',
-    resolver: zodResolver(settingsFieldFormSchema),
+    resolver: zodResolver(settingsFieldFormSchema()),
  });

  useEffect(() => {
--- a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx
+++ b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx
@ -36,7 +36,7 @@ import { isDefined } from '~/utils/isDefined';
 import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull';

 type SettingsDataModelNewFieldFormValues = z.infer<
-  typeof settingsFieldFormSchema
+  ReturnType<typeof settingsFieldFormSchema>
 >;

 const StyledSettingsObjectFieldTypeSelect = styled(
@ -59,7 +59,11 @@ export const SettingsObjectNewFieldStep2 = () => {

  const formConfig = useForm<SettingsDataModelNewFieldFormValues>({
    mode: 'onTouched',
-    resolver: zodResolver(settingsFieldFormSchema),
+    resolver: zodResolver(
+      settingsFieldFormSchema(
+        activeObjectMetadataItem?.fields.map((value) => value.name),
+      ),
+    ),
  });

  useEffect(() => {
@ -169,8 +173,9 @@ export const SettingsObjectNewFieldStep2 = () => {

  return (
    <RecordFieldValueSelectorContextProvider>
-      {/* eslint-disable-next-line react/jsx-props-no-spreading */}
-      <FormProvider {...formConfig}>
+      <FormProvider // eslint-disable-next-line react/jsx-props-no-spreading
+        {...formConfig}
+      >
        <SubMenuTopBarContainer Icon={IconSettings} title="Settings">
          <SettingsPageContainer>
            <SettingsHeaderContainer>