From 63387424c37ce6f11b53ba33b3bcc448ded76d18 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Wed, 15 May 2024 21:43:58 +0200 Subject: [PATCH] Fix transliteration for metadata + transliterate select options (#5430) ## Context Fixes #5403 Transliteration is now integrated to form validation through the schema. While it does not impede inputting an invalid value, it impedes submitting a form that will fail as the transliteration is not possible. Until then we were only performing the transliteration at save time in the front-end, but it's best to provide the information as soon as possible. Later we will add helpers to guide the user (eg "This name is not valid": https://github.com/twentyhq/twenty/issues/5428). --------- Co-authored-by: Charles Bochet --- .../components/RightDrawerCalendarEvent.tsx | 4 +- .../utils/formatFieldMetadataItemInput.ts | 4 +- .../__tests__/metadataLabelSchema.test.ts | 24 ++++++++ .../validation-schemas/metadataLabelSchema.ts | 18 +++++- .../validation-schemas/selectOptionsSchema.ts | 16 +++++- .../forms/utils/getOptionValueFromLabel.ts | 21 ++++--- .../settingsCreateObjectInputSchema.ts | 8 +-- .../settingsUpdateObjectInputSchema.ts | 6 +- .../constants/MetadataLabelValidPattern.ts | 1 + .../constants/MetadataNameValidPattern.ts | 1 + .../constants/OptionValueValidPattern.ts | 1 + .../compute-metadata-name-from-label.test.ts | 27 +++++++++ .../compute-option-value-from-label.test.ts | 27 +++++++++ .../format-metadata-label-to-name.spec.ts | 57 ------------------- .../compute-metadata-name-from-label.utils.ts | 6 ++ .../compute-option-value-from-label.utils.ts | 6 ++ ...l.ts => transliterate-and-format.utils.ts} | 9 ++- .../is-valid-graphql-enum-name.validator.ts | 2 +- 18 files changed, 151 insertions(+), 87 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-metadata/validation-schemas/__tests__/metadataLabelSchema.test.ts create mode 100644 packages/twenty-front/src/pages/settings/data-model/constants/MetadataLabelValidPattern.ts create mode 100644 packages/twenty-front/src/pages/settings/data-model/constants/MetadataNameValidPattern.ts create mode 100644 packages/twenty-front/src/pages/settings/data-model/constants/OptionValueValidPattern.ts create mode 100644 packages/twenty-front/src/pages/settings/data-model/utils/__tests__/compute-metadata-name-from-label.test.ts create mode 100644 packages/twenty-front/src/pages/settings/data-model/utils/__tests__/compute-option-value-from-label.test.ts delete mode 100644 packages/twenty-front/src/pages/settings/data-model/utils/__tests__/format-metadata-label-to-name.spec.ts create mode 100644 packages/twenty-front/src/pages/settings/data-model/utils/compute-metadata-name-from-label.utils.ts create mode 100644 packages/twenty-front/src/pages/settings/data-model/utils/compute-option-value-from-label.utils.ts rename packages/twenty-front/src/pages/settings/data-model/utils/{format-metadata-label-to-name.util.ts => transliterate-and-format.utils.ts} (65%) diff --git a/packages/twenty-front/src/modules/activities/calendar/right-drawer/components/RightDrawerCalendarEvent.tsx b/packages/twenty-front/src/modules/activities/calendar/right-drawer/components/RightDrawerCalendarEvent.tsx index 0134713a1..4f760bf95 100644 --- a/packages/twenty-front/src/modules/activities/calendar/right-drawer/components/RightDrawerCalendarEvent.tsx +++ b/packages/twenty-front/src/modules/activities/calendar/right-drawer/components/RightDrawerCalendarEvent.tsx @@ -18,7 +18,9 @@ export const RightDrawerCalendarEvent = () => { onCompleted: (record) => setRecords([record]), }); - if (!calendarEvent) return null; + if (!calendarEvent) { + return null; + } return ; }; diff --git a/packages/twenty-front/src/modules/object-metadata/utils/formatFieldMetadataItemInput.ts b/packages/twenty-front/src/modules/object-metadata/utils/formatFieldMetadataItemInput.ts index cbaae2cd5..15bc319a9 100644 --- a/packages/twenty-front/src/modules/object-metadata/utils/formatFieldMetadataItemInput.ts +++ b/packages/twenty-front/src/modules/object-metadata/utils/formatFieldMetadataItemInput.ts @@ -1,5 +1,5 @@ import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; -import { formatMetadataLabelToMetadataNameOrThrows } from '~/pages/settings/data-model/utils/format-metadata-label-to-name.util'; +import { computeMetadataNameFromLabelOrThrow } from '~/pages/settings/data-model/utils/compute-metadata-name-from-label.utils'; export const formatFieldMetadataItemInput = ( input: Partial< @@ -16,7 +16,7 @@ export const formatFieldMetadataItemInput = ( description: input.description?.trim() ?? null, icon: input.icon, label, - name: label ? formatMetadataLabelToMetadataNameOrThrows(label) : undefined, + name: label ? computeMetadataNameFromLabelOrThrow(label) : undefined, options: input.options, }; }; diff --git 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 new file mode 100644 index 000000000..822e84f08 --- /dev/null +++ b/packages/twenty-front/src/modules/object-metadata/validation-schemas/__tests__/metadataLabelSchema.test.ts @@ -0,0 +1,24 @@ +import { metadataLabelSchema } from '@/object-metadata/validation-schemas/metadataLabelSchema'; + +describe('metadataLabelSchema', () => { + it('validates a valid label', () => { + // Given + const validMetadataLabel = 'Option 1'; + + // When + const result = metadataLabelSchema.parse(validMetadataLabel); + + // Then + expect(result).toEqual(validMetadataLabel); + }); + it('validates a label with non-latin characters', () => { + // Given + const validMetadataLabel = 'עִבְרִי'; + + // When + const result = metadataLabelSchema.parse(validMetadataLabel); + + // Then + expect(result).toEqual(validMetadataLabel); + }); +}); diff --git a/packages/twenty-front/src/modules/object-metadata/validation-schemas/metadataLabelSchema.ts b/packages/twenty-front/src/modules/object-metadata/validation-schemas/metadataLabelSchema.ts index 8f23b365f..8b696f3e9 100644 --- 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,7 +1,23 @@ 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(/^[a-zA-Z][a-zA-Z0-9 ()]*$/); + .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 diff --git a/packages/twenty-front/src/modules/object-metadata/validation-schemas/selectOptionsSchema.ts b/packages/twenty-front/src/modules/object-metadata/validation-schemas/selectOptionsSchema.ts index 8ba481836..4b634a3ac 100644 --- a/packages/twenty-front/src/modules/object-metadata/validation-schemas/selectOptionsSchema.ts +++ b/packages/twenty-front/src/modules/object-metadata/validation-schemas/selectOptionsSchema.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { FieldMetadataItemOption } from '@/object-metadata/types/FieldMetadataItem'; import { getOptionValueFromLabel } from '@/settings/data-model/fields/forms/utils/getOptionValueFromLabel'; import { themeColorSchema } from '@/ui/theme/utils/themeColorSchema'; +import { computeOptionValueFromLabelOrThrow } from '~/pages/settings/data-model/utils/compute-option-value-from-label.utils'; const selectOptionSchema = z .object({ @@ -14,7 +15,20 @@ const selectOptionSchema = z }) .refine((option) => option.value === getOptionValueFromLabel(option.label), { message: 'Value does not match label', - }) satisfies z.ZodType; + }) + .refine( + (option) => { + try { + computeOptionValueFromLabelOrThrow(option.label); + return true; + } catch (error) { + return false; + } + }, + { + message: 'Label is not transliterable', + }, + ) satisfies z.ZodType; export const selectOptionsSchema = z .array(selectOptionSchema) diff --git a/packages/twenty-front/src/modules/settings/data-model/fields/forms/utils/getOptionValueFromLabel.ts b/packages/twenty-front/src/modules/settings/data-model/fields/forms/utils/getOptionValueFromLabel.ts index 6c256fabc..9a263fa6c 100644 --- a/packages/twenty-front/src/modules/settings/data-model/fields/forms/utils/getOptionValueFromLabel.ts +++ b/packages/twenty-front/src/modules/settings/data-model/fields/forms/utils/getOptionValueFromLabel.ts @@ -1,15 +1,14 @@ import snakeCase from 'lodash.snakecase'; -export const getOptionValueFromLabel = (label: string) => { - // Remove accents - const unaccentedLabel = label - .normalize('NFD') - .replace(/[\u0300-\u036f]/g, ''); - // Remove special characters - const noSpecialCharactersLabel = unaccentedLabel.replace( - /[^a-zA-Z0-9 ]/g, - '', - ); +import { computeOptionValueFromLabelOrThrow } from '~/pages/settings/data-model/utils/compute-option-value-from-label.utils'; - return snakeCase(noSpecialCharactersLabel).toUpperCase(); +export const getOptionValueFromLabel = (label: string) => { + let transliteratedLabel = label; + try { + transliteratedLabel = computeOptionValueFromLabelOrThrow(label); + } catch (error) { + return label; + } + + return snakeCase(transliteratedLabel).toUpperCase(); }; diff --git a/packages/twenty-front/src/modules/settings/data-model/validation-schemas/settingsCreateObjectInputSchema.ts b/packages/twenty-front/src/modules/settings/data-model/validation-schemas/settingsCreateObjectInputSchema.ts index f35093ab5..458f6605e 100644 --- a/packages/twenty-front/src/modules/settings/data-model/validation-schemas/settingsCreateObjectInputSchema.ts +++ b/packages/twenty-front/src/modules/settings/data-model/validation-schemas/settingsCreateObjectInputSchema.ts @@ -1,6 +1,6 @@ import { objectMetadataItemSchema } from '@/object-metadata/validation-schemas/objectMetadataItemSchema'; import { CreateObjectInput } from '~/generated-metadata/graphql'; -import { formatMetadataLabelToMetadataNameOrThrows } from '~/pages/settings/data-model/utils/format-metadata-label-to-name.util'; +import { computeMetadataNameFromLabelOrThrow } from '~/pages/settings/data-model/utils/compute-metadata-name-from-label.utils'; export const settingsCreateObjectInputSchema = objectMetadataItemSchema .pick({ @@ -11,8 +11,6 @@ export const settingsCreateObjectInputSchema = objectMetadataItemSchema }) .transform((value) => ({ ...value, - nameSingular: formatMetadataLabelToMetadataNameOrThrows( - value.labelSingular, - ), - namePlural: formatMetadataLabelToMetadataNameOrThrows(value.labelPlural), + nameSingular: computeMetadataNameFromLabelOrThrow(value.labelSingular), + namePlural: computeMetadataNameFromLabelOrThrow(value.labelPlural), })); diff --git a/packages/twenty-front/src/modules/settings/data-model/validation-schemas/settingsUpdateObjectInputSchema.ts b/packages/twenty-front/src/modules/settings/data-model/validation-schemas/settingsUpdateObjectInputSchema.ts index 5a9a2cf1a..e594a7bc4 100644 --- a/packages/twenty-front/src/modules/settings/data-model/validation-schemas/settingsUpdateObjectInputSchema.ts +++ b/packages/twenty-front/src/modules/settings/data-model/validation-schemas/settingsUpdateObjectInputSchema.ts @@ -1,6 +1,6 @@ import { objectMetadataItemSchema } from '@/object-metadata/validation-schemas/objectMetadataItemSchema'; import { UpdateObjectPayload } from '~/generated-metadata/graphql'; -import { formatMetadataLabelToMetadataNameOrThrows } from '~/pages/settings/data-model/utils/format-metadata-label-to-name.util'; +import { computeMetadataNameFromLabelOrThrow } from '~/pages/settings/data-model/utils/compute-metadata-name-from-label.utils'; export const settingsUpdateObjectInputSchema = objectMetadataItemSchema .pick({ @@ -16,9 +16,9 @@ export const settingsUpdateObjectInputSchema = objectMetadataItemSchema .transform((value) => ({ ...value, nameSingular: value.labelSingular - ? formatMetadataLabelToMetadataNameOrThrows(value.labelSingular) + ? computeMetadataNameFromLabelOrThrow(value.labelSingular) : undefined, namePlural: value.labelPlural - ? formatMetadataLabelToMetadataNameOrThrows(value.labelPlural) + ? computeMetadataNameFromLabelOrThrow(value.labelPlural) : undefined, })); diff --git a/packages/twenty-front/src/pages/settings/data-model/constants/MetadataLabelValidPattern.ts b/packages/twenty-front/src/pages/settings/data-model/constants/MetadataLabelValidPattern.ts new file mode 100644 index 000000000..8cc887e6a --- /dev/null +++ b/packages/twenty-front/src/pages/settings/data-model/constants/MetadataLabelValidPattern.ts @@ -0,0 +1 @@ +export const METADATA_LABEL_VALID_PATTERN = /^[^0-9].*$/; diff --git a/packages/twenty-front/src/pages/settings/data-model/constants/MetadataNameValidPattern.ts b/packages/twenty-front/src/pages/settings/data-model/constants/MetadataNameValidPattern.ts new file mode 100644 index 000000000..ac00bbc80 --- /dev/null +++ b/packages/twenty-front/src/pages/settings/data-model/constants/MetadataNameValidPattern.ts @@ -0,0 +1 @@ +export const METADATA_NAME_VALID_PATTERN = /^[a-zA-Z][a-zA-Z0-9]*$/; diff --git a/packages/twenty-front/src/pages/settings/data-model/constants/OptionValueValidPattern.ts b/packages/twenty-front/src/pages/settings/data-model/constants/OptionValueValidPattern.ts new file mode 100644 index 000000000..2d8ef0210 --- /dev/null +++ b/packages/twenty-front/src/pages/settings/data-model/constants/OptionValueValidPattern.ts @@ -0,0 +1 @@ +export const OPTION_VALUE_VALID_PATTERN = /^[a-zA-Z0-9]+$/; diff --git a/packages/twenty-front/src/pages/settings/data-model/utils/__tests__/compute-metadata-name-from-label.test.ts b/packages/twenty-front/src/pages/settings/data-model/utils/__tests__/compute-metadata-name-from-label.test.ts new file mode 100644 index 000000000..972bcaafd --- /dev/null +++ b/packages/twenty-front/src/pages/settings/data-model/utils/__tests__/compute-metadata-name-from-label.test.ts @@ -0,0 +1,27 @@ +import { computeMetadataNameFromLabelOrThrow } from '~/pages/settings/data-model/utils/compute-metadata-name-from-label.utils'; + +describe('computeMetadataNameFromLabel', () => { + it('throws if empty label', () => { + const label = ' '; + + expect(() => computeMetadataNameFromLabelOrThrow(label)).toThrow(); + }); + + it('computes name for 1 char long label', () => { + const label = 'a'; + + expect(computeMetadataNameFromLabelOrThrow(label)).toEqual('a'); + }); + + it('throws if label starts with digits', () => { + const label = '1string'; + + expect(() => computeMetadataNameFromLabelOrThrow(label)).toThrow(); + }); + + it('computes name for label with non-latin char', () => { + const label = 'λλλ!'; + + expect(computeMetadataNameFromLabelOrThrow(label)).toEqual('lll'); + }); +}); diff --git a/packages/twenty-front/src/pages/settings/data-model/utils/__tests__/compute-option-value-from-label.test.ts b/packages/twenty-front/src/pages/settings/data-model/utils/__tests__/compute-option-value-from-label.test.ts new file mode 100644 index 000000000..72b524d8d --- /dev/null +++ b/packages/twenty-front/src/pages/settings/data-model/utils/__tests__/compute-option-value-from-label.test.ts @@ -0,0 +1,27 @@ +import { computeOptionValueFromLabelOrThrow } from '~/pages/settings/data-model/utils/compute-option-value-from-label.utils'; + +describe('computeOptionValueFromLabel', () => { + it('throws if empty label', () => { + const label = ' '; + + expect(() => computeOptionValueFromLabelOrThrow(label)).toThrow(); + }); + + it('computes name for 1 char long label', () => { + const label = 'a'; + + expect(computeOptionValueFromLabelOrThrow(label)).toEqual('a'); + }); + + it('compute name if starts with digits', () => { + const label = '1'; + + expect(computeOptionValueFromLabelOrThrow(label)).toEqual('1'); + }); + + it('computes name for label with non-latin char', () => { + const label = 'λλλ'; + + expect(computeOptionValueFromLabelOrThrow(label)).toEqual('lll'); + }); +}); diff --git a/packages/twenty-front/src/pages/settings/data-model/utils/__tests__/format-metadata-label-to-name.spec.ts b/packages/twenty-front/src/pages/settings/data-model/utils/__tests__/format-metadata-label-to-name.spec.ts deleted file mode 100644 index c2bdda8a5..000000000 --- a/packages/twenty-front/src/pages/settings/data-model/utils/__tests__/format-metadata-label-to-name.spec.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { formatMetadataLabelToMetadataNameOrThrows } from '~/pages/settings/data-model/utils/format-metadata-label-to-name.util'; - -const VALID_STRING_PATTERN = /^[a-zA-Z][a-zA-Z0-9 ]*$/; - -describe('formatMetadataLabelToMetadataNameOrThrows', () => { - it('leaves strings unchanged if only latin characters', () => { - const input = 'testName'; - - expect( - formatMetadataLabelToMetadataNameOrThrows(input).match( - VALID_STRING_PATTERN, - )?.length, - ).toBe(1); - expect(formatMetadataLabelToMetadataNameOrThrows(input)).toEqual(input); - }); - - it('leaves strings unchanged if only latin characters and digits', () => { - const input = 'testName123'; - - expect( - formatMetadataLabelToMetadataNameOrThrows(input).match( - VALID_STRING_PATTERN, - )?.length, - ).toBe(1); - expect(formatMetadataLabelToMetadataNameOrThrows(input)).toEqual(input); - }); - - it('format strings with non latin characters', () => { - const input = 'בְרִבְרִ'; - const expected = 'bRibRi'; - - expect( - formatMetadataLabelToMetadataNameOrThrows(input).match( - VALID_STRING_PATTERN, - )?.length, - ).toBe(1); - expect(formatMetadataLabelToMetadataNameOrThrows(input)).toEqual(expected); - }); - - it('format strings with mixed characters', () => { - const input = 'aa2בְרִבְרִ'; - const expected = 'aa2BRibRi'; - - expect( - formatMetadataLabelToMetadataNameOrThrows(input).match( - VALID_STRING_PATTERN, - )?.length, - ).toBe(1); - expect(formatMetadataLabelToMetadataNameOrThrows(input)).toEqual(expected); - }); - - it('throws error if could not format', () => { - const input = '$$$***'; - - expect(() => formatMetadataLabelToMetadataNameOrThrows(input)).toThrow(); - }); -}); diff --git a/packages/twenty-front/src/pages/settings/data-model/utils/compute-metadata-name-from-label.utils.ts b/packages/twenty-front/src/pages/settings/data-model/utils/compute-metadata-name-from-label.utils.ts new file mode 100644 index 000000000..25545bb4d --- /dev/null +++ b/packages/twenty-front/src/pages/settings/data-model/utils/compute-metadata-name-from-label.utils.ts @@ -0,0 +1,6 @@ +import { METADATA_NAME_VALID_PATTERN } from '~/pages/settings/data-model/constants/MetadataNameValidPattern'; +import { transliterateAndFormatOrThrow } from '~/pages/settings/data-model/utils/transliterate-and-format.utils'; + +export const computeMetadataNameFromLabelOrThrow = (label: string): string => { + return transliterateAndFormatOrThrow(label, METADATA_NAME_VALID_PATTERN); +}; diff --git a/packages/twenty-front/src/pages/settings/data-model/utils/compute-option-value-from-label.utils.ts b/packages/twenty-front/src/pages/settings/data-model/utils/compute-option-value-from-label.utils.ts new file mode 100644 index 000000000..ebf66346a --- /dev/null +++ b/packages/twenty-front/src/pages/settings/data-model/utils/compute-option-value-from-label.utils.ts @@ -0,0 +1,6 @@ +import { OPTION_VALUE_VALID_PATTERN } from '~/pages/settings/data-model/constants/OptionValueValidPattern'; +import { transliterateAndFormatOrThrow } from '~/pages/settings/data-model/utils/transliterate-and-format.utils'; + +export const computeOptionValueFromLabelOrThrow = (label: string): string => { + return transliterateAndFormatOrThrow(label, OPTION_VALUE_VALID_PATTERN); +}; diff --git a/packages/twenty-front/src/pages/settings/data-model/utils/format-metadata-label-to-name.util.ts b/packages/twenty-front/src/pages/settings/data-model/utils/transliterate-and-format.utils.ts similarity index 65% rename from packages/twenty-front/src/pages/settings/data-model/utils/format-metadata-label-to-name.util.ts rename to packages/twenty-front/src/pages/settings/data-model/utils/transliterate-and-format.utils.ts index 84e5bbc04..b3b441902 100644 --- a/packages/twenty-front/src/pages/settings/data-model/utils/format-metadata-label-to-name.util.ts +++ b/packages/twenty-front/src/pages/settings/data-model/utils/transliterate-and-format.utils.ts @@ -3,14 +3,13 @@ import { slugify, transliterate } from 'transliteration'; import { isDefined } from '~/utils/isDefined'; -const VALID_STRING_PATTERN = /^[a-zA-Z][a-zA-Z0-9 ]*$/; - -export const formatMetadataLabelToMetadataNameOrThrows = ( +export const transliterateAndFormatOrThrow = ( string: string, + validStringPattern: RegExp, ): string => { let formattedString = string; - if (isDefined(formattedString.match(VALID_STRING_PATTERN))) { + if (isDefined(formattedString.match(validStringPattern))) { return toCamelCase(formattedString); } @@ -18,7 +17,7 @@ export const formatMetadataLabelToMetadataNameOrThrows = ( slugify(transliterate(formattedString, { trim: true })), ); - if (!formattedString.match(VALID_STRING_PATTERN)) { + if (!formattedString.match(validStringPattern)) { throw new Error(`"${string}" is not a valid name`); } diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/validators/is-valid-graphql-enum-name.validator.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/validators/is-valid-graphql-enum-name.validator.ts index 44b60cf16..1b45983b3 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/validators/is-valid-graphql-enum-name.validator.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/validators/is-valid-graphql-enum-name.validator.ts @@ -4,7 +4,7 @@ import { ValidationArguments, } from 'class-validator'; -const graphQLEnumNameRegex = /^[_A-Za-z][_0-9A-Za-z]+$/; +const graphQLEnumNameRegex = /^[_A-Za-z][_0-9A-Za-z]*$/; export function IsValidGraphQLEnumName(validationOptions?: ValidationOptions) { return function (object: object, propertyName: string) {