fix: include calling code in phones unique constraint
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.
This commit is contained in:
+1
-1
@@ -209,7 +209,7 @@ export const SETTINGS_COMPOSITE_FIELD_TYPE_CONFIGS = {
|
||||
.primaryPhoneCallingCode,
|
||||
isImportable: true,
|
||||
isFilterable: true,
|
||||
isIncludedInUniqueConstraint: false,
|
||||
isIncludedInUniqueConstraint: true,
|
||||
},
|
||||
{
|
||||
subFieldName:
|
||||
|
||||
+34
@@ -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);
|
||||
|
||||
+6
-10
@@ -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)}`,
|
||||
}));
|
||||
});
|
||||
};
|
||||
|
||||
+5
-7
@@ -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) {
|
||||
|
||||
@@ -23,6 +23,7 @@ export const phonesCompositeType: CompositeType = {
|
||||
type: FieldMetadataType.TEXT,
|
||||
hidden: false,
|
||||
isRequired: false,
|
||||
isIncludedInUniqueConstraint: true,
|
||||
},
|
||||
{
|
||||
name: 'additionalPhones',
|
||||
|
||||
Reference in New Issue
Block a user