From 4b607213e62222647459825637a355f636f8210e Mon Sep 17 00:00:00 2001 From: Sonarly Claude Code Date: Sat, 2 May 2026 08:25:18 +0000 Subject: [PATCH] fix: include calling code in phones unique constraint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://sonarly.com/issue/33478?type=bug When a unique constraint is enabled on a PHONES field, the unique index only covers the `primaryPhoneNumber` sub-column (national number), ignoring `primaryPhoneCallingCode`. This causes false duplicate errors when two records have the same national number but different country calling codes (e.g., +1 123456789 vs +32 123456789). Fix: **Problem:** The unique constraint on PHONES composite fields only covered the `primaryPhoneNumber` sub-column, ignoring `primaryPhoneCallingCode`. Phone numbers like `+1 123456789` and `+32 123456789` both store `123456789` as their `primaryPhoneNumber` (the national number), with the calling code stored separately. This caused false unique constraint violations for different international phone numbers sharing the same national number. **Changes:** 1. **`packages/twenty-shared/src/types/composite-types/phones.composite-type.ts`** — Added `isIncludedInUniqueConstraint: true` to the `primaryPhoneCallingCode` property. This is the canonical source of truth consumed by all code paths that build unique indexes and conflict detection logic. 2. **`packages/twenty-front/src/modules/settings/data-model/constants/SettingsCompositeFieldTypeConfigs.ts`** — Changed `isIncludedInUniqueConstraint` from `false` to `true` for `primaryPhoneCallingCode` in the frontend PHONES config. This ensures the spreadsheet import validation and other frontend consumers correctly consider both columns. 3. **`packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-conflicting-fields.util.ts`** — Changed `.find()` to `.filter()` when looking up unique constraint properties, and returns entries for ALL matching properties (not just the first). This was previously a latent bug — it only returned one sub-field per composite type. The DB-level index creation code (`computeFlatIndexFieldColumnNames`) and GraphQL input type generator already used `.filter()` correctly. 4. **`packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/find-conflicting-record.util.ts`** — Changed `.find()` to `.filter()` + `.some()` for matching the column name from a Postgres constraint error to the correct unique field. This ensures the error enrichment (finding the conflicting record ID) works when any of multiple unique constraint sub-columns triggers the violation. 5. **`packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/__tests__/get-conflicting-fields.util.spec.ts`** — Added a new test case that verifies a unique PHONES field returns both `primaryPhoneNumber` and `primaryPhoneCallingCode` as conflicting fields. **Effect:** After this fix, the unique index on PHONES fields will be a compound index on both `(primaryPhoneNumber, primaryPhoneCallingCode)`. This means `(123456789, +1)` and `(123456789, +32)` are correctly treated as different phone numbers. **Note for existing workspaces:** Users who already have a unique constraint on a PHONES field will need to drop and re-create the index for the new compound constraint to take effect at the database level. New unique constraints created after this fix will automatically include both columns. --- .../SettingsCompositeFieldTypeConfigs.ts | 2 +- .../get-conflicting-fields.util.spec.ts | 34 +++++++++++++++++++ .../utils/get-conflicting-fields.util.ts | 16 ++++----- .../utils/find-conflicting-record.util.ts | 12 +++---- .../composite-types/phones.composite-type.ts | 1 + 5 files changed, 47 insertions(+), 18 deletions(-) diff --git a/packages/twenty-front/src/modules/settings/data-model/constants/SettingsCompositeFieldTypeConfigs.ts b/packages/twenty-front/src/modules/settings/data-model/constants/SettingsCompositeFieldTypeConfigs.ts index 7cd02af8cca..9a20d83577c 100644 --- a/packages/twenty-front/src/modules/settings/data-model/constants/SettingsCompositeFieldTypeConfigs.ts +++ b/packages/twenty-front/src/modules/settings/data-model/constants/SettingsCompositeFieldTypeConfigs.ts @@ -209,7 +209,7 @@ export const SETTINGS_COMPOSITE_FIELD_TYPE_CONFIGS = { .primaryPhoneCallingCode, isImportable: true, isFilterable: true, - isIncludedInUniqueConstraint: false, + isIncludedInUniqueConstraint: true, }, { subFieldName: diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/__tests__/get-conflicting-fields.util.spec.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/__tests__/get-conflicting-fields.util.spec.ts index 9d72bc0e650..306f3b1f43a 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/__tests__/get-conflicting-fields.util.spec.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/__tests__/get-conflicting-fields.util.spec.ts @@ -61,6 +61,13 @@ describe('getConflictingFields', () => { isUnique: false, }); + const phonesUniqueField = createMockField({ + id: 'phones-unique-id', + name: 'phonesField', + type: FieldMetadataType.PHONES, + isUnique: true, + }); + const addressUniqueFieldNoIncludedProp = createMockField({ id: 'address-unique-id', name: 'addressField', @@ -170,6 +177,33 @@ describe('getConflictingFields', () => { expect(result).toEqual([{ baseField: 'id', fullPath: 'id', column: 'id' }]); }); + it('returns all unique constraint subfields for composite fields with multiple included properties', () => { + const fields = [idField, phonesUniqueField]; + const flatObjectMetadata = buildFlatObjectMetadata(fields); + const flatFieldMetadataMaps = buildFlatFieldMetadataMaps(fields); + + const result = getConflictingFields( + flatObjectMetadata, + flatFieldMetadataMaps, + ); + + expect(result).toEqual( + expect.arrayContaining([ + { baseField: 'id', fullPath: 'id', column: 'id' }, + { + baseField: 'phonesField', + fullPath: 'phonesField.primaryPhoneNumber', + column: 'phonesFieldPrimaryPhoneNumber', + }, + { + baseField: 'phonesField', + fullPath: 'phonesField.primaryPhoneCallingCode', + column: 'phonesFieldPrimaryPhoneCallingCode', + }, + ]), + ); + }); + it('ignores non-unique fields', () => { const fields = [idField, phonesNotUniqueField]; const flatObjectMetadata = buildFlatObjectMetadata(fields); diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-conflicting-fields.util.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-conflicting-fields.util.ts index 4207ef12da4..7a85a6c43ae 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-conflicting-fields.util.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-create-many-query-runner/utils/get-conflicting-fields.util.ts @@ -32,18 +32,14 @@ export const getConflictingFields = ( ]; } - const property = compositeType.properties.find( + const uniqueProperties = compositeType.properties.filter( (prop) => prop.isIncludedInUniqueConstraint, ); - return property - ? [ - { - baseField: field.name, - fullPath: `${field.name}.${property.name}`, - column: `${field.name}${capitalize(property.name)}`, - }, - ] - : []; + return uniqueProperties.map((property) => ({ + baseField: field.name, + fullPath: `${field.name}.${property.name}`, + column: `${field.name}${capitalize(property.name)}`, + })); }); }; diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/find-conflicting-record.util.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/find-conflicting-record.util.ts index d3ea578dcff..0f75d90fbdb 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/find-conflicting-record.util.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/find-conflicting-record.util.ts @@ -28,17 +28,15 @@ export const findConflictingRecord = async ( return field.name === columnName; } - const property = compositeType.properties.find( + const uniqueProperties = compositeType.properties.filter( (prop) => prop.isIncludedInUniqueConstraint, ); - if (!property) { - return false; - } + return uniqueProperties.some((property) => { + const expectedColumnName = `${field.name}${capitalize(property.name)}`; - const expectedColumnName = `${field.name}${capitalize(property.name)}`; - - return expectedColumnName === columnName; + return expectedColumnName === columnName; + }); }); if (!matchingField) { diff --git a/packages/twenty-shared/src/types/composite-types/phones.composite-type.ts b/packages/twenty-shared/src/types/composite-types/phones.composite-type.ts index dbba0a336c8..022877d49bc 100644 --- a/packages/twenty-shared/src/types/composite-types/phones.composite-type.ts +++ b/packages/twenty-shared/src/types/composite-types/phones.composite-type.ts @@ -23,6 +23,7 @@ export const phonesCompositeType: CompositeType = { type: FieldMetadataType.TEXT, hidden: false, isRequired: false, + isIncludedInUniqueConstraint: true, }, { name: 'additionalPhones',