fix(server): normalize empty composite phone sub-fields to NULL (#19775)

Fixed using Opus 4.7, I wanted to test this model out and in this repo I know you guys care about quality, pls let me know if this is good code. It looks good to me Fixes #19740. ## Summary PostgreSQL UNIQUE indexes treat two `''` values as duplicates but two `NULL`s as distinct. `validateAndInferPhoneInput` was persisting blank `primaryPhoneNumber` as `''` instead of `NULL`, so a second record with an empty unique phone failed with a constraint violation. The sibling composite transforms (`transformEmailsValue`, `removeEmptyLinks`, `transformTextField`) already canonicalize null-equivalent values; phones was the outlier. - Empty-string phone sub-fields now normalize to `null`. `undefined` is preserved so partial updates leave columns the user did not touch alone. - `PhonesFieldGraphQLInput` drops the aspirational `CountryCode` brand on input. GraphQL delivers raw strings at the boundary; branding happens during validation. --------- Co-authored-by: Charles Bochet <charles@twenty.com>
2026-04-18 05:36:43 +10:00
parent ab85946102
commit 59e4ed715a
3 changed files with 271 additions and 21 deletions
@@ -0,0 +1,83 @@
+import { transformPhonesValue } from 'src/engine/core-modules/record-transformer/utils/transform-phones-value.util';
+
+describe('transformPhonesValue', () => {
+  it('should return null when input is null', () => {
+    const result = transformPhonesValue({ input: null });
+
+    expect(result).toBeNull();
+  });
+
+  it('should normalize all empty primary sub-fields to null', () => {
+    const result = transformPhonesValue({
+      input: {
+        primaryPhoneNumber: '',
+        primaryPhoneCallingCode: '',
+        primaryPhoneCountryCode: '',
+      },
+    });
+
+    expect(result).toEqual({
+      additionalPhones: null,
+      primaryPhoneNumber: null,
+      primaryPhoneCallingCode: null,
+      primaryPhoneCountryCode: null,
+    });
+  });
+
+  it('should normalize empty number inside an additionalPhones entry to null', () => {
+    const result = transformPhonesValue({
+      input: {
+        primaryPhoneNumber: '',
+        additionalPhones: JSON.stringify([{ number: '' }]),
+      },
+    });
+
+    expect(result?.additionalPhones).toBe(JSON.stringify([{ number: null }]));
+  });
+
+  it('should parse a valid international phone number into its canonical parts', () => {
+    const result = transformPhonesValue({
+      input: { primaryPhoneNumber: '+14155552671' },
+    });
+
+    expect(result).toEqual({
+      additionalPhones: null,
+      primaryPhoneNumber: '4155552671',
+      primaryPhoneCallingCode: '+1',
+      primaryPhoneCountryCode: 'US',
+    });
+  });
+
+  it('should infer callingCode from the number when callingCode is an empty string', () => {
+    const result = transformPhonesValue({
+      input: {
+        primaryPhoneNumber: '+14155552671',
+        primaryPhoneCallingCode: '',
+      },
+    });
+
+    expect(result).toEqual({
+      additionalPhones: null,
+      primaryPhoneNumber: '4155552671',
+      primaryPhoneCallingCode: '+1',
+      primaryPhoneCountryCode: 'US',
+    });
+  });
+
+  it('should accept additionalPhones as an array of phone objects', () => {
+    const result = transformPhonesValue({
+      input: {
+        primaryPhoneNumber: '+14155552671',
+        additionalPhones: [
+          { number: '+442071838750', callingCode: '+44', countryCode: 'GB' },
+        ],
+      },
+    });
+
+    expect(result?.additionalPhones).toBe(
+      JSON.stringify([
+        { countryCode: 'GB', callingCode: '+44', number: '2071838750' },
+      ]),
+    );
+  });
+});
@@ -5,10 +5,7 @@ import {
  parsePhoneNumberWithError,
 } from 'libphonenumber-js';
 import isEmpty from 'lodash.isempty';
-import {
-  type AdditionalPhoneMetadata,
-  type PhonesMetadata,
-} from 'twenty-shared/types';
+import { type AdditionalPhoneMetadata } from 'twenty-shared/types';
 import {
  getCountryCodesForCallingCode,
  isDefined,
@@ -23,11 +20,12 @@ import {
 } from 'src/engine/core-modules/record-transformer/record-transformer.exception';

 export type PhonesFieldGraphQLInput =
-  | Partial<
-      Omit<PhonesMetadata, 'additionalPhones'> & {
-        additionalPhones: string | null;
-      }
-    >
+  | {
+      primaryPhoneNumber?: string | null;
+      primaryPhoneCountryCode?: string | null;
+      primaryPhoneCallingCode?: string | null;
+      additionalPhones?: string | Partial<AdditionalPhoneMetadata>[] | null;
+    }
  | null
  | undefined;

@@ -36,10 +34,16 @@ type AdditionalPhoneMetadataWithNumber = Partial<AdditionalPhoneMetadata> &

 const removePlusFromString = (str: string) => str.replace(/\+/g, '');

+const nullIfEmptyString = (value: string | null | undefined) =>
+  !isDefined(value) ? value : isNonEmptyString(value) ? value : null;
+
 const validatePrimaryPhoneCountryCodeAndCallingCode = ({
  callingCode,
  countryCode,
-}: Partial<Omit<AdditionalPhoneMetadata, 'number'>>) => {
+}: {
+  callingCode?: string | null;
+  countryCode?: string | null;
+}) => {
  if (isNonEmptyString(countryCode) && !isValidCountryCode(countryCode)) {
    throw new RecordTransformerException(
      `Invalid country code ${countryCode}`,
@@ -154,24 +158,28 @@ const validateAndInferPhoneInput = ({
  callingCode,
  countryCode,
  number,
-}: Partial<AdditionalPhoneMetadata>) => {
-  validatePrimaryPhoneCountryCodeAndCallingCode({
-    callingCode,
-    countryCode,
-  });
+}: {
+  callingCode?: string | null;
+  countryCode?: string | null;
+  number?: string | null;
+}) => {
+  validatePrimaryPhoneCountryCodeAndCallingCode({ callingCode, countryCode });

-  if (isDefined(number) && isNonEmptyString(number)) {
+  if (isNonEmptyString(number)) {
    return validateAndInferMetadataFromPrimaryPhoneNumber({
      number,
-      callingCode,
-      countryCode,
+      callingCode: isNonEmptyString(callingCode) ? callingCode : undefined,
+      countryCode:
+        isNonEmptyString(countryCode) && isValidCountryCode(countryCode)
+          ? countryCode
+          : undefined,
    });
  }

  return {
-    callingCode,
-    countryCode,
-    number,
+    callingCode: nullIfEmptyString(callingCode),
+    countryCode: nullIfEmptyString(countryCode),
+    number: nullIfEmptyString(number),
  };
 };

@@ -0,0 +1,159 @@
+import { faker } from '@faker-js/faker';
+import { createOneOperation } from 'test/integration/graphql/utils/create-one-operation.util';
+import { createOneFieldMetadata } from 'test/integration/metadata/suites/field-metadata/utils/create-one-field-metadata.util';
+import { createOneObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/create-one-object-metadata.util';
+import { deleteOneObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/delete-one-object-metadata.util';
+import { updateOneObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/update-one-object-metadata.util';
+import { deleteRecordsByIds } from 'test/integration/utils/delete-records-by-ids';
+import { FieldMetadataType } from 'twenty-shared/types';
+
+const OBJECT_SINGULAR = 'uniquePhonesTestObject';
+const OBJECT_PLURAL = 'uniquePhonesTestObjects';
+const FIELD_NAME = 'phone';
+
+describe('unique PHONES field with empty values', () => {
+  let createdObjectMetadataId: string;
+  let createdRecordIdsForCleaning: string[] = [];
+
+  beforeAll(async () => {
+    const { data } = await createOneObjectMetadata({
+      input: {
+        nameSingular: OBJECT_SINGULAR,
+        namePlural: OBJECT_PLURAL,
+        labelSingular: 'Unique Phones Test Object',
+        labelPlural: 'Unique Phones Test Objects',
+        icon: 'IconPhone',
+        isLabelSyncedWithName: false,
+      },
+    });
+
+    createdObjectMetadataId = data.createOneObject.id;
+
+    await createOneFieldMetadata({
+      input: {
+        name: FIELD_NAME,
+        label: 'Phone',
+        type: FieldMetadataType.PHONES,
+        objectMetadataId: createdObjectMetadataId,
+        isUnique: true,
+        isLabelSyncedWithName: false,
+      },
+      gqlFields: `
+        id
+        name
+        isUnique
+      `,
+    });
+  });
+
+  afterEach(async () => {
+    if (createdRecordIdsForCleaning.length > 0) {
+      await deleteRecordsByIds(OBJECT_SINGULAR, createdRecordIdsForCleaning);
+      createdRecordIdsForCleaning = [];
+    }
+  });
+
+  afterAll(async () => {
+    await updateOneObjectMetadata({
+      expectToFail: false,
+      input: {
+        idToUpdate: createdObjectMetadataId,
+        updatePayload: { isActive: false },
+      },
+    });
+    await deleteOneObjectMetadata({
+      input: { idToDelete: createdObjectMetadataId },
+    });
+  });
+
+  it('should allow creating two records with empty primaryPhoneNumber on a unique PHONES field', async () => {
+    const firstId = faker.string.uuid();
+    const secondId = faker.string.uuid();
+
+    const firstResponse = await createOneOperation({
+      objectMetadataSingularName: OBJECT_SINGULAR,
+      input: {
+        id: firstId,
+        [FIELD_NAME]: { primaryPhoneNumber: '' },
+      },
+      gqlFields: `id`,
+    });
+
+    expect(firstResponse.errors).toBeUndefined();
+    expect(firstResponse.data.createOneResponse.id).toBe(firstId);
+    createdRecordIdsForCleaning.push(firstId);
+
+    const secondResponse = await createOneOperation({
+      objectMetadataSingularName: OBJECT_SINGULAR,
+      input: {
+        id: secondId,
+        [FIELD_NAME]: { primaryPhoneNumber: '' },
+      },
+      gqlFields: `id`,
+    });
+
+    expect(secondResponse.errors).toBeUndefined();
+    expect(secondResponse.data.createOneResponse.id).toBe(secondId);
+    createdRecordIdsForCleaning.push(secondId);
+  });
+
+  it('should allow creating two records with no PHONES payload at all', async () => {
+    const firstId = faker.string.uuid();
+    const secondId = faker.string.uuid();
+
+    const firstResponse = await createOneOperation({
+      objectMetadataSingularName: OBJECT_SINGULAR,
+      input: { id: firstId },
+      gqlFields: `id`,
+    });
+
+    expect(firstResponse.errors).toBeUndefined();
+    createdRecordIdsForCleaning.push(firstId);
+
+    const secondResponse = await createOneOperation({
+      objectMetadataSingularName: OBJECT_SINGULAR,
+      input: { id: secondId },
+      gqlFields: `id`,
+    });
+
+    expect(secondResponse.errors).toBeUndefined();
+    createdRecordIdsForCleaning.push(secondId);
+  });
+
+  it('should still enforce uniqueness when two records share the same non-empty primaryPhoneNumber', async () => {
+    const firstId = faker.string.uuid();
+    const secondId = faker.string.uuid();
+
+    const firstResponse = await createOneOperation({
+      objectMetadataSingularName: OBJECT_SINGULAR,
+      input: {
+        id: firstId,
+        [FIELD_NAME]: {
+          primaryPhoneNumber: '4155552671',
+          primaryPhoneCallingCode: '+1',
+          primaryPhoneCountryCode: 'US',
+        },
+      },
+      gqlFields: `id`,
+    });
+
+    expect(firstResponse.errors).toBeUndefined();
+    createdRecordIdsForCleaning.push(firstId);
+
+    const secondResponse = await createOneOperation({
+      objectMetadataSingularName: OBJECT_SINGULAR,
+      input: {
+        id: secondId,
+        [FIELD_NAME]: {
+          primaryPhoneNumber: '4155552671',
+          primaryPhoneCallingCode: '+1',
+          primaryPhoneCountryCode: 'US',
+        },
+      },
+      gqlFields: `id`,
+      expectToFail: true,
+    });
+
+    expect(secondResponse.errors).toBeDefined();
+  });
+});