Fix enum defaultValue issues (twentyhq#5307)

This PR fixes several issues: - enum naming should be: {tableName}_{fieldName}_enum and respecting the case - defaultValue format handled in the FE should respect the one in the BE In my opinion we should refactor the defaultValue: - we should respect backend format: "'myDefault'" for constant default and "0" for float, "now" for expressions, "true" for booleans. we can rename it to defaultValueExpression if it is more clear but we should not maintain a parallel system - we should deprecate option: isDefaultValue which is confusing - we should re-work backend to have a more unified approach between fields and avoid having if everywhere about select, multiselect, and currency cases. one unified "computeDefaultValue" function should do the job What is still broken: - currency default Value on creation. I think we should do the refactor first - select default value edition. These cases do not break the schema but are ignored currently
synapsenet-arena · May 6, 2024 · 2c9f50e · 2c9f50e
1 parent ff77a4e
commit 2c9f50e
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 26 deletions.
diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/useFieldMetadataItem.ts b/packages/twenty-front/src/modules/object-metadata/hooks/useFieldMetadataItem.ts
@@ -26,6 +26,7 @@ export const useFieldMetadataItem = () => {
   ) => {
     const formattedInput = formatFieldMetadataItemInput(input);
 
+    debugger;
     const defaultValue = getDefaultValueForBackend(
       input.defaultValue ?? formattedInput.defaultValue,
       input.type,
@@ -51,24 +52,24 @@ export const useFieldMetadataItem = () => {
       | 'options'
     >,
   ) => {
-    const formattedInput = formatFieldMetadataItemInput(input);
-    const defaultValue = input.defaultValue
-      ? typeof input.defaultValue == 'string'
-        ? `'${input.defaultValue}'`
-        : input.defaultValue
-      : formattedInput.defaultValue ?? undefined;
+    // In Edit mode, all options need an id,
+    // so we generate an id for newly created options.
+    const inputOptions = input.options?.map((option: FieldMetadataOption) =>
+      option.id ? option : { ...option, id: v4() },
+    );
+    const formattedInput = formatFieldMetadataItemInput({
+      ...input,
+      options: inputOptions,
+    });
+
+    const defaultValue = input.defaultValue ?? formattedInput.defaultValue;
 
     return updateOneFieldMetadataItem({
       fieldMetadataIdToUpdate: input.id,
-      updatePayload: formatFieldMetadataItemInput({
-        ...input,
+      updatePayload: {
+        ...formattedInput,
         defaultValue,
-        // In Edit mode, all options need an id,
-        // so we generate an id for newly created options.
-        options: input.options?.map((option: FieldMetadataOption) =>
-          option.id ? option : { ...option, id: v4() },
-        ),
-      }),
+      },
     });
   };
 

diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts b/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts
@@ -28,7 +28,13 @@ export const useUpdateOneFieldMetadataItem = () => {
     fieldMetadataIdToUpdate: UpdateOneFieldMetadataItemMutationVariables['idToUpdate'];
     updatePayload: Pick<
       UpdateOneFieldMetadataItemMutationVariables['updatePayload'],
-      'description' | 'icon' | 'isActive' | 'label' | 'name'
+      | 'description'
+      | 'icon'
+      | 'isActive'
+      | 'label'
+      | 'name'
+      | 'defaultValue'
+      | 'options'
     >;
   }) => {
     return await mutate({

diff --git a/...ty-front/src/modules/object-metadata/utils/__tests__/formatFieldMetadataItemInput.test.ts b/...ty-front/src/modules/object-metadata/utils/__tests__/formatFieldMetadataItemInput.test.ts
@@ -77,7 +77,7 @@ describe('formatFieldMetadataItemInput', () => {
           value: 'OPTION_2',
         },
       ],
-      defaultValue: "'OPTION_1'",
+      defaultValue: 'OPTION_1',
     };
 
     const result = formatFieldMetadataItemInput(input);
@@ -140,7 +140,7 @@ describe('formatFieldMetadataItemInput', () => {
           value: 'OPTION_2',
         },
       ],
-      defaultValue: ["'OPTION_1'", "'OPTION_2'"],
+      defaultValue: ['OPTION_1', 'OPTION_2'],
     };
 
     const result = formatFieldMetadataItemInput(input);

diff --git a/packages/twenty-front/src/modules/object-metadata/utils/formatFieldMetadataItemInput.ts b/packages/twenty-front/src/modules/object-metadata/utils/formatFieldMetadataItemInput.ts
@@ -32,14 +32,14 @@ export const formatFieldMetadataItemInput = (
     const defaultOptions = options?.filter((option) => option.isDefault);
     if (isDefined(defaultOptions)) {
       defaultValue = defaultOptions.map(
-        (defaultOption) => `'${getOptionValueFromLabel(defaultOption.label)}'`,
+        (defaultOption) => `${getOptionValueFromLabel(defaultOption.label)}`,
       );
     }
   }
   if (input.type === FieldMetadataType.Select) {
     const defaultOption = options?.find((option) => option.isDefault);
     defaultValue = isDefined(defaultOption)
-      ? `'${getOptionValueFromLabel(defaultOption.label)}'`
+      ? `${getOptionValueFromLabel(defaultOption.label)}`
       : undefined;
   }
 

diff --git a/packages/twenty-front/src/modules/object-metadata/utils/getDefaultValueForBackend.ts b/packages/twenty-front/src/modules/object-metadata/utils/getDefaultValueForBackend.ts
@@ -1,3 +1,4 @@
+import { CurrencyCode } from '@/object-record/record-field/types/CurrencyCode';
 import { FieldCurrencyValue } from '@/object-record/record-field/types/FieldMetadata';
 import { FieldMetadataType } from '~/generated-metadata/graphql';
 
@@ -9,10 +10,12 @@ export const getDefaultValueForBackend = (
     const currencyDefaultValue = defaultValue as FieldCurrencyValue;
     return {
       amountMicros: currencyDefaultValue.amountMicros,
-      currencyCode: `'${currencyDefaultValue.currencyCode}'` as any,
+      currencyCode: `'${currencyDefaultValue.currencyCode}'` as CurrencyCode,
     } satisfies FieldCurrencyValue;
-  } else if (typeof defaultValue === 'string') {
+  } else if (fieldMetadataType === FieldMetadataType.Select) {
     return `'${defaultValue}'`;
+  } else if (fieldMetadataType === FieldMetadataType.MultiSelect) {
+    return defaultValue.map((value: string) => `'${value}'`);
   }
 
   return defaultValue;

diff --git a/...erver/src/engine/metadata-modules/field-metadata/validators/is-quoted-string.validator.ts b/...erver/src/engine/metadata-modules/field-metadata/validators/is-quoted-string.validator.ts
@@ -13,7 +13,7 @@ export function IsQuotedString(validationOptions?: ValidationOptions) {
       options: validationOptions,
       validator: {
         validate(value: any) {
-          return typeof value === 'string' && /^'.*'$/.test(value);
+          return typeof value === 'string' && /^'{1}.*'{1}$/.test(value);
         },
         defaultMessage(args: ValidationArguments) {
           return `${args.property} must be a quoted string`;

diff --git a/...rver/src/engine/workspace-manager/workspace-health/services/database-structure.service.ts b/...rver/src/engine/workspace-manager/workspace-health/services/database-structure.service.ts
@@ -166,9 +166,8 @@ export class DatabaseStructureService {
       // Compute enum name to compare data type properly
       if (typeORMType === 'enum') {
         const objectName = fieldMetadata.object?.nameSingular;
-        const prefix = fieldMetadata.isCustom ? '_' : '';
 
-        return `${objectName}_${prefix}${columnName}_enum`;
+        return `${objectName}_${columnName}_enum`;
       }
 
       return mainDataSource.driver.normalizeType({

diff --git a/...workspace-manager/workspace-migration-runner/services/workspace-migration-enum.service.ts b/...workspace-manager/workspace-migration-runner/services/workspace-migration-enum.service.ts
@@ -32,9 +32,9 @@ export class WorkspaceMigrationEnumService {
     }
 
     const columnDefinition = migrationColumn.alteredColumnDefinition;
-    const oldEnumTypeName =
-      `${tableName}_${migrationColumn.currentColumnDefinition.columnName}_enum`.toLowerCase();
+    const oldEnumTypeName = `${tableName}_${migrationColumn.currentColumnDefinition.columnName}_enum`;
     const tempEnumTypeName = `${oldEnumTypeName}_temp`;
+    const newEnumTypeName = `${tableName}_${columnDefinition.columnName}_enum`;
     const enumValues =
       columnDefinition.enum?.map((enumValue) => {
         if (typeof enumValue === 'string') {
@@ -76,6 +76,7 @@ export class WorkspaceMigrationEnumService {
         type: columnDefinition.columnType,
         default: columnDefinition.defaultValue,
         enum: enumValues,
+        enumName: newEnumTypeName,
         isArray: columnDefinition.isArray,
         isNullable: columnDefinition.isNullable,
       }),

diff --git a/...engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.ts b/...engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.ts
@@ -307,6 +307,8 @@ export class WorkspaceMigrationRunnerService {
       return;
     }
 
+    const enumName = `${tableName}_${migrationColumn.columnName}_enum`;
+
     await queryRunner.addColumn(
       `${schemaName}.${tableName}`,
       new TableColumn({
@@ -316,6 +318,7 @@ export class WorkspaceMigrationRunnerService {
         enum: migrationColumn.enum?.filter(
           (value): value is string => typeof value === 'string',
         ),
+        enumName: enumName,
         isArray: migrationColumn.isArray,
         isNullable: migrationColumn.isNullable,
       }),