From e8905be71aefce9bdf81cfd32a52f8c5e489d1d8 Mon Sep 17 00:00:00 2001 From: Etienne <45695613+etiennejouan@users.noreply.github.com> Date: Fri, 4 Jul 2025 23:07:24 +0200 Subject: [PATCH] Import - Improve phone validation (#12901) Context : - Phones import is a bit complex if not all subfields are provided. - Phones subfield validation are absent or different from BE validation. Solution : - Normalize callingCode and countryCode validation (BE/FE) - Ease phone import if only phoneNumber is provided --- ...ildRecordFromImportedStructuredRow.test.ts | 35 +++ .../buildRecordFromImportedStructuredRow.ts | 214 ++++++++---------- ...etSpreadSheetFieldValidationDefinitions.ts | 54 ++++- .../utils/transform-phones-value.util.ts | 23 +- packages/twenty-shared/package.json | 1 + packages/twenty-shared/src/utils/index.ts | 2 + .../getCountryCodesForCallingCode.ts | 15 ++ .../utils/validation/phones-value/index.ts | 1 + .../phones-value/isValidCountryCode.ts | 7 + yarn.lock | 1 + 10 files changed, 208 insertions(+), 145 deletions(-) create mode 100644 packages/twenty-shared/src/utils/validation/phones-value/getCountryCodesForCallingCode.ts create mode 100644 packages/twenty-shared/src/utils/validation/phones-value/index.ts create mode 100644 packages/twenty-shared/src/utils/validation/phones-value/isValidCountryCode.ts diff --git a/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/__tests__/buildRecordFromImportedStructuredRow.test.ts b/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/__tests__/buildRecordFromImportedStructuredRow.test.ts index c702a7ec6..82099dd47 100644 --- a/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/__tests__/buildRecordFromImportedStructuredRow.test.ts +++ b/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/__tests__/buildRecordFromImportedStructuredRow.test.ts @@ -405,4 +405,39 @@ describe('buildRecordFromImportedStructuredRow', () => { ratingField: '4', }); }); + + it('should handle case where user provides only a primaryPhoneNumber without calling code', () => { + const importedStructuredRow: ImportedStructuredRow = { + 'Primary Phone Number (phoneField)': '5550123', + }; + + const fields: FieldMetadataItem[] = [ + { + id: '13', + name: 'phoneField', + label: 'Phone Field', + type: FieldMetadataType.PHONES, + isNullable: true, + isActive: true, + isCustom: false, + isSystem: false, + createdAt: '2023-01-01', + updatedAt: '2023-01-01', + icon: 'IconPhone', + description: null, + }, + ]; + + const result = buildRecordFromImportedStructuredRow({ + importedStructuredRow, + fields, + }); + + expect(result).toEqual({ + phoneField: { + primaryPhoneNumber: '5550123', + primaryPhoneCallingCode: '+1', + }, + }); + }); }); diff --git a/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/buildRecordFromImportedStructuredRow.ts b/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/buildRecordFromImportedStructuredRow.ts index a04f64147..6d5a160fc 100644 --- a/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/buildRecordFromImportedStructuredRow.ts +++ b/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/buildRecordFromImportedStructuredRow.ts @@ -1,14 +1,16 @@ import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; import { FieldActorForInputValue } from '@/object-record/record-field/types/FieldMetadata'; -import { COMPOSITE_FIELD_SUB_FIELD_LABELS } from '@/settings/data-model/constants/CompositeFieldSubFieldLabel'; +import { getSubFieldOptionKey } from '@/object-record/spreadsheet-import/utils/getSubFieldOptionKey'; import { ImportedStructuredRow } from '@/spreadsheet-import/types'; import { isNonEmptyString } from '@sniptt/guards'; +import { CountryCode, parsePhoneNumberWithError } from 'libphonenumber-js'; import { isDefined } from 'twenty-shared/utils'; import { z } from 'zod'; import { FieldMetadataType } from '~/generated-metadata/graphql'; import { castToString } from '~/utils/castToString'; import { convertCurrencyAmountToCurrencyMicros } from '~/utils/convertCurrencyToCurrencyMicros'; import { isEmptyObject } from '~/utils/isEmptyObject'; +import { stripSimpleQuotesFromString } from '~/utils/string/stripSimpleQuotesFromString'; type BuildRecordFromImportedStructuredRowArgs = { importedStructuredRow: ImportedStructuredRow; @@ -16,23 +18,14 @@ type BuildRecordFromImportedStructuredRowArgs = { }; const buildCompositeFieldRecord = ( - fieldName: string, + field: FieldMetadataItem, importedStructuredRow: ImportedStructuredRow, - compositeFieldConfig: Record< - string, - { - labelKey: string; - transform?: (value: any) => any; - } - >, + compositeFieldConfig: Record any) | undefined>, ): Record | undefined => { const compositeFieldRecord = Object.entries(compositeFieldConfig).reduce( - ( - acc, - [compositeFieldKey, { labelKey: compositeFieldLabelKey, transform }], - ) => { + (acc, [compositeFieldKey, transform]) => { const value = - importedStructuredRow[`${compositeFieldLabelKey} (${fieldName})`]; + importedStructuredRow[getSubFieldOptionKey(field, compositeFieldKey)]; return isDefined(value) ? { ...acc, [compositeFieldKey]: transform?.(value) || value } @@ -106,123 +99,49 @@ export const buildRecordFromImportedStructuredRow = ({ const recordToBuild: Record = {}; - const { - ADDRESS: { - addressCity: addressCityLabel, - addressCountry: addressCountryLabel, - addressPostcode: addressPostcodeLabel, - addressState: addressStateLabel, - addressStreet1: addressStreet1Label, - addressStreet2: addressStreet2Label, - }, - CURRENCY: { - amountMicros: amountMicrosLabel, - currencyCode: currencyCodeLabel, - }, - FULL_NAME: { firstName: firstNameLabel, lastName: lastNameLabel }, - LINKS: { - primaryLinkUrl: primaryLinkUrlLabel, - primaryLinkLabel: primaryLinkLabelLabel, - secondaryLinks: secondaryLinksLabel, - }, - EMAILS: { - primaryEmail: primaryEmailLabel, - additionalEmails: additionalEmailsLabel, - }, - PHONES: { - primaryPhoneNumber: primaryPhoneNumberLabel, - primaryPhoneCountryCode: primaryPhoneCountryCodeLabel, - primaryPhoneCallingCode: primaryPhoneCallingCodeLabel, - additionalPhones: additionalPhonesLabel, - }, - RICH_TEXT_V2: { blocknote: blocknoteLabel, markdown: markdownLabel }, - } = COMPOSITE_FIELD_SUB_FIELD_LABELS; - - const COMPOSITE_FIELD_CONFIGS = { + const COMPOSITE_FIELD_TRANSFORM_CONFIGS = { [FieldMetadataType.CURRENCY]: { - amountMicros: { - labelKey: amountMicrosLabel, - transform: (value: any) => - convertCurrencyAmountToCurrencyMicros(Number(value)), - }, - currencyCode: { labelKey: currencyCodeLabel }, + amountMicros: (value: any) => + convertCurrencyAmountToCurrencyMicros(Number(value)), + currencyCode: undefined, }, - [FieldMetadataType.ADDRESS]: { - addressStreet1: { - labelKey: addressStreet1Label, - transform: castToString, - }, - addressStreet2: { - labelKey: addressStreet2Label, - transform: castToString, - }, - addressCity: { labelKey: addressCityLabel, transform: castToString }, - addressPostcode: { - labelKey: addressPostcodeLabel, - transform: castToString, - }, - addressState: { labelKey: addressStateLabel, transform: castToString }, - addressCountry: { - labelKey: addressCountryLabel, - transform: castToString, - }, + addressStreet1: castToString, + addressStreet2: castToString, + addressCity: castToString, + addressPostcode: castToString, + addressState: castToString, + addressCountry: castToString, }, - [FieldMetadataType.LINKS]: { - primaryLinkLabel: { - labelKey: primaryLinkLabelLabel, - transform: castToString, - }, - primaryLinkUrl: { - labelKey: primaryLinkUrlLabel, - transform: castToString, - }, - secondaryLinks: { - labelKey: secondaryLinksLabel, - transform: linkArrayJSONSchema.parse, - }, + primaryLinkLabel: castToString, + primaryLinkUrl: castToString, + secondaryLinks: linkArrayJSONSchema.parse, }, [FieldMetadataType.PHONES]: { - primaryPhoneCountryCode: { - labelKey: primaryPhoneCountryCodeLabel, - transform: castToString, - }, - primaryPhoneNumber: { - labelKey: primaryPhoneNumberLabel, - transform: castToString, - }, - primaryPhoneCallingCode: { - labelKey: primaryPhoneCallingCodeLabel, - transform: castToString, - }, - additionalPhones: { - labelKey: additionalPhonesLabel, - transform: phoneArrayJSONSchema.parse, - }, + primaryPhoneCountryCode: castToString, + primaryPhoneNumber: castToString, + primaryPhoneCallingCode: castToString, + additionalPhones: phoneArrayJSONSchema.parse, }, [FieldMetadataType.RICH_TEXT_V2]: { - blocknote: { labelKey: blocknoteLabel, transform: castToString }, - markdown: { labelKey: markdownLabel, transform: castToString }, + blocknote: castToString, + markdown: castToString, }, [FieldMetadataType.EMAILS]: { - primaryEmail: { labelKey: primaryEmailLabel, transform: castToString }, - additionalEmails: { - labelKey: additionalEmailsLabel, - transform: stringArrayJSONSchema.parse, - }, + primaryEmail: castToString, + additionalEmails: stringArrayJSONSchema.parse, }, - [FieldMetadataType.FULL_NAME]: { - firstName: { labelKey: firstNameLabel }, - lastName: { labelKey: lastNameLabel }, + firstName: undefined, + lastName: undefined, }, [FieldMetadataType.ACTOR]: { - source: { labelKey: 'source', transform: () => 'IMPORT' }, - context: { labelKey: 'context', transform: () => ({}) }, + source: () => 'IMPORT', + context: () => ({}), }, }; @@ -233,20 +152,83 @@ export const buildRecordFromImportedStructuredRow = ({ case FieldMetadataType.CURRENCY: case FieldMetadataType.ADDRESS: case FieldMetadataType.LINKS: - case FieldMetadataType.PHONES: case FieldMetadataType.RICH_TEXT_V2: case FieldMetadataType.EMAILS: case FieldMetadataType.FULL_NAME: { const compositeData = buildCompositeFieldRecord( - field.name, + field, importedStructuredRow, - COMPOSITE_FIELD_CONFIGS[field.type], + COMPOSITE_FIELD_TRANSFORM_CONFIGS[field.type], ); if (isDefined(compositeData)) { recordToBuild[field.name] = compositeData; } break; } + case FieldMetadataType.PHONES: { + const compositeData = buildCompositeFieldRecord( + field, + importedStructuredRow, + COMPOSITE_FIELD_TRANSFORM_CONFIGS[field.type], + ); + if (!isDefined(compositeData)) { + break; + } + recordToBuild[field.name] = compositeData; + + const primaryPhoneNumber = + importedStructuredRow[ + getSubFieldOptionKey(field, 'primaryPhoneNumber') + ]; + + const primaryPhoneCallingCode = + importedStructuredRow[ + getSubFieldOptionKey(field, 'primaryPhoneCallingCode') + ]; + + const hasUserProvidedPrimaryPhoneNumberWithoutCallingCode = + isDefined(primaryPhoneNumber) && + (!isDefined(primaryPhoneCallingCode) || + !isNonEmptyString(primaryPhoneCallingCode)); + + // To meet backend requirements, handle case where user provides only a primaryPhoneNumber without calling code + if (hasUserProvidedPrimaryPhoneNumberWithoutCallingCode) { + const primaryPhoneCountryCode = + importedStructuredRow[ + getSubFieldOptionKey(field, 'primaryPhoneCountryCode') + ]; + + const hasUserProvidedPrimaryPhoneCountryCode = + isDefined(primaryPhoneCountryCode) && + isNonEmptyString(primaryPhoneCountryCode); + + try { + const { + number: parsedNumber, + countryCallingCode: parsedCountryCallingCode, + } = parsePhoneNumberWithError( + primaryPhoneNumber as string, + hasUserProvidedPrimaryPhoneCountryCode + ? (primaryPhoneCountryCode as CountryCode) + : undefined, + ); + + recordToBuild[field.name] = { + primaryPhoneNumber: parsedNumber, + primaryPhoneCallingCode: `+${parsedCountryCallingCode}`, + }; + } catch { + recordToBuild[field.name] = { + primaryPhoneNumber, + primaryPhoneCallingCode: + stripSimpleQuotesFromString( + field?.defaultValue?.primaryPhoneCallingCode, + ) || '+1', + }; + } + } + break; + } case FieldMetadataType.BOOLEAN: recordToBuild[field.name] = importedFieldValue === 'true' || importedFieldValue === true; diff --git a/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/getSpreadSheetFieldValidationDefinitions.ts b/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/getSpreadSheetFieldValidationDefinitions.ts index 2d25daa24..7d6aeec7e 100644 --- a/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/getSpreadSheetFieldValidationDefinitions.ts +++ b/packages/twenty-front/src/modules/object-record/spreadsheet-import/utils/getSpreadSheetFieldValidationDefinitions.ts @@ -4,7 +4,14 @@ import { emailSchema } from '@/object-record/record-field/validation-schemas/ema import { SpreadsheetImportFieldValidationDefinition } from '@/spreadsheet-import/types'; import { t } from '@lingui/core/macro'; import { isDate, isString } from '@sniptt/guards'; -import { absoluteUrlSchema, isDefined, isValidUuid } from 'twenty-shared/utils'; +import { parsePhoneNumberWithError } from 'libphonenumber-js'; +import { + absoluteUrlSchema, + getCountryCodesForCallingCode, + isDefined, + isValidCountryCode, + isValidUuid, +} from 'twenty-shared/utils'; import { FieldMetadataType } from '~/generated-metadata/graphql'; const getNumberValidationDefinition = ( @@ -16,6 +23,20 @@ const getNumberValidationDefinition = ( level: 'error', }); +const isValidPhoneNumber = (value: string) => { + try { + return isDefined( + parsePhoneNumberWithError(value, { defaultCallingCode: '1' }), + ); + } catch { + return false; + } +}; + +const isValidCallingCode = (value: string) => { + return getCountryCodesForCallingCode(value).length > 0; +}; + export const getSpreadSheetFieldValidationDefinitions = ( type: FieldMetadataType, fieldName: string, @@ -143,9 +164,27 @@ export const getSpreadSheetFieldValidationDefinitions = ( case 'primaryPhoneNumber': return [ { - rule: 'regex', - value: '^[0-9]+$', - errorMessage: `${fieldName} ${t`must contain only numbers`}`, + rule: 'function', + isValid: isValidPhoneNumber, + errorMessage: `${fieldName} ${t`is not a valid phone number`}`, + level: 'error', + }, + ]; + case 'primaryPhoneCallingCode': + return [ + { + rule: 'function', + isValid: isValidCallingCode, + errorMessage: `${fieldName} ${t`is not a valid calling code`}`, + level: 'error', + }, + ]; + case 'primaryPhoneCountryCode': + return [ + { + rule: 'function', + isValid: isValidCountryCode, + errorMessage: `${fieldName} ${t`is not a valid country code`}`, level: 'error', }, ]; @@ -165,10 +204,9 @@ export const getSpreadSheetFieldValidationDefinitions = ( callingCode: string; countryCode: string; }) => - isDefined(phone.number) && - /^[0-9]+$/.test(phone.number) && - isDefined(phone.callingCode) && - isDefined(phone.countryCode), + isValidPhoneNumber(phone.number) && + isValidCallingCode(phone.callingCode) && + isValidCountryCode(phone.countryCode), ); } catch { return false; diff --git a/packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-phones-value.util.ts b/packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-phones-value.util.ts index 52f1c6a28..33973cf33 100644 --- a/packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-phones-value.util.ts +++ b/packages/twenty-server/src/engine/core-modules/record-transformer/utils/transform-phones-value.util.ts @@ -2,13 +2,12 @@ import { t } from '@lingui/core/macro'; import { isNonEmptyString } from '@sniptt/guards'; import { CountryCallingCode, - CountryCode, - getCountries, - getCountryCallingCode, parsePhoneNumberWithError, } from 'libphonenumber-js'; import { + getCountryCodesForCallingCode, isDefined, + isValidCountryCode, parseJson, removeUndefinedFields, } from 'twenty-shared/utils'; @@ -22,8 +21,6 @@ import { PhonesMetadata, } from 'src/engine/metadata-modules/field-metadata/composite-types/phones.composite-type'; -const ALL_COUNTRIES_CODE = getCountries(); - export type PhonesFieldGraphQLInput = | Partial< Omit & { @@ -38,22 +35,6 @@ type AdditionalPhoneMetadataWithNumber = Partial & const removePlusFromString = (str: string) => str.replace(/\+/g, ''); -const isValidCountryCode = (input: string): input is CountryCode => { - return ALL_COUNTRIES_CODE.includes(input as unknown as CountryCode); -}; - -const getCountryCodesForCallingCode = (callingCode: string) => { - const cleanCallingCode = callingCode.startsWith('+') - ? callingCode.slice(1) - : callingCode; - - return ALL_COUNTRIES_CODE.filter((country) => { - const countryCallingCode = getCountryCallingCode(country); - - return countryCallingCode === cleanCallingCode; - }); -}; - const validatePrimaryPhoneCountryCodeAndCallingCode = ({ callingCode, countryCode, diff --git a/packages/twenty-shared/package.json b/packages/twenty-shared/package.json index ad9b73f7d..9a2443939 100644 --- a/packages/twenty-shared/package.json +++ b/packages/twenty-shared/package.json @@ -22,6 +22,7 @@ }, "dependencies": { "@sniptt/guards": "^0.2.0", + "libphonenumber-js": "^1.10.26", "zod": "3.23.8" }, "preconstruct": { diff --git a/packages/twenty-shared/src/utils/index.ts b/packages/twenty-shared/src/utils/index.ts index a797b4593..e7d336990 100644 --- a/packages/twenty-shared/src/utils/index.ts +++ b/packages/twenty-shared/src/utils/index.ts @@ -33,3 +33,5 @@ export { isValidLocale } from './validation/isValidLocale'; export { isValidUuid } from './validation/isValidUuid'; export { isValidVariable } from './validation/isValidVariable'; export { normalizeLocale } from './validation/normalizeLocale'; +export { getCountryCodesForCallingCode } from './validation/phones-value/getCountryCodesForCallingCode'; +export { isValidCountryCode } from './validation/phones-value/isValidCountryCode'; diff --git a/packages/twenty-shared/src/utils/validation/phones-value/getCountryCodesForCallingCode.ts b/packages/twenty-shared/src/utils/validation/phones-value/getCountryCodesForCallingCode.ts new file mode 100644 index 000000000..be7a21487 --- /dev/null +++ b/packages/twenty-shared/src/utils/validation/phones-value/getCountryCodesForCallingCode.ts @@ -0,0 +1,15 @@ +import { getCountries, getCountryCallingCode } from 'libphonenumber-js'; + +const ALL_COUNTRIES_CODE = getCountries(); + +export const getCountryCodesForCallingCode = (callingCode: string) => { + const cleanCallingCode = callingCode.startsWith('+') + ? callingCode.slice(1) + : callingCode; + + return ALL_COUNTRIES_CODE.filter((country) => { + const countryCallingCode = getCountryCallingCode(country); + + return countryCallingCode === cleanCallingCode; + }); +}; diff --git a/packages/twenty-shared/src/utils/validation/phones-value/index.ts b/packages/twenty-shared/src/utils/validation/phones-value/index.ts new file mode 100644 index 000000000..d33ba11ca --- /dev/null +++ b/packages/twenty-shared/src/utils/validation/phones-value/index.ts @@ -0,0 +1 @@ +export * from './isValidCountryCode'; diff --git a/packages/twenty-shared/src/utils/validation/phones-value/isValidCountryCode.ts b/packages/twenty-shared/src/utils/validation/phones-value/isValidCountryCode.ts new file mode 100644 index 000000000..0ff5523a3 --- /dev/null +++ b/packages/twenty-shared/src/utils/validation/phones-value/isValidCountryCode.ts @@ -0,0 +1,7 @@ +import { CountryCode, getCountries } from 'libphonenumber-js'; + +const ALL_COUNTRIES_CODE = getCountries(); + +export const isValidCountryCode = (input: string): input is CountryCode => { + return ALL_COUNTRIES_CODE.includes(input as unknown as CountryCode); +}; diff --git a/yarn.lock b/yarn.lock index 0f23bf147..e29e26601 100644 --- a/yarn.lock +++ b/yarn.lock @@ -56856,6 +56856,7 @@ __metadata: "@types/babel__preset-env": "npm:^7" babel-plugin-module-resolver: "npm:^5.0.2" glob: "npm:^11.0.1" + libphonenumber-js: "npm:^1.10.26" tsx: "npm:^4.19.3" zod: "npm:3.23.8" languageName: unknown