From 0a99f784eb2fa7d454d6461fef4186fbdf998093 Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Mon, 15 Jun 2026 13:56:35 +0200 Subject: [PATCH] fix(front): dedupe morph relation fields in view field pickers (#21580) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Issue Reported in quality-feedbacks: **"Issues with morph relation view field"** — a morph relation column added to a view **disappears after refresh** (and can be added several times). ## Root cause — the SSE metadata sync A morph relation is stored as **one `fieldMetadata` row per target object**, all sharing a `morphId`. Collapsing those rows into the single field that represents the relation is a **read-time projection** in the server's `objects.fieldsList` resolver — it is *not* a storage invariant, and the rows are never merged. The frontend metadata store is kept in sync with the raw rows **one row at a time over SSE** (`MetadataStoreSSEEffect`): every metadata change broadcasts a single created/updated record that's pushed straight into the store. Creating a morph relation creates N rows (one per target), so **N `create` events arrive and N raw sub-fields land in the store — bypassing the `fieldsList` projection entirely.** The view-field pickers read straight from that store, so they saw the morph relation **once per target**. Each could be added as a column referencing a different sub-field id; after a refresh the view reloads from the projected (deduped) data, the non-survivor columns no longer resolve, and they disappear. ## Fix & architecture note Because the store deliberately mirrors raw rows (that's what the SSE sync maintains), the fix applies the **same read-time projection on the client** — deduping morph rows by `morphId` in `useActiveFieldMetadataItems` — rather than filtering rows at each insert path (SSE, optimistic create, …). This matches how the backend already models morph fields and is robust regardless of which path delivered the rows. The survivor-selection rule (which sub-field id represents the relation) now lives in `twenty-shared` (`pickMorphGroupSurvivor`) so client and server can't drift. --- .../hooks/useActiveFieldMetadataItems.ts | 19 ++- ...upeMorphRelationFieldMetadataItems.test.ts | 146 ++++++++++++++++++ .../dedupeMorphRelationFieldMetadataItems.ts | 38 +++++ .../pick-morph-group-survivor.util.spec.ts | 85 ---------- ...er-morph-relation-duplicate-fields.util.ts | 4 +- .../utils/pick-morph-group-survivor.util.ts | 19 --- ...-relation-from-flat-field-metadata.util.ts | 4 +- ...pick-morph-group-survivor-or-throw.spec.ts | 61 ++++++++ .../pick-morph-group-survivor-or-throw.ts | 32 ++++ packages/twenty-shared/src/utils/index.ts | 1 + 10 files changed, 293 insertions(+), 116 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-metadata/utils/__tests__/dedupeMorphRelationFieldMetadataItems.test.ts create mode 100644 packages/twenty-front/src/modules/object-metadata/utils/dedupeMorphRelationFieldMetadataItems.ts delete mode 100644 packages/twenty-server/src/engine/dataloaders/utils/__tests__/pick-morph-group-survivor.util.spec.ts delete mode 100644 packages/twenty-server/src/engine/dataloaders/utils/pick-morph-group-survivor.util.ts create mode 100644 packages/twenty-shared/src/utils/fieldMetadata/__tests__/pick-morph-group-survivor-or-throw.spec.ts create mode 100644 packages/twenty-shared/src/utils/fieldMetadata/pick-morph-group-survivor-or-throw.ts diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/useActiveFieldMetadataItems.ts b/packages/twenty-front/src/modules/object-metadata/hooks/useActiveFieldMetadataItems.ts index d1a5ec7dae..1a261693f0 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/useActiveFieldMetadataItems.ts +++ b/packages/twenty-front/src/modules/object-metadata/hooks/useActiveFieldMetadataItems.ts @@ -1,5 +1,6 @@ import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem'; import { isDefined } from 'twenty-shared/utils'; +import { dedupeMorphRelationFieldMetadataItems } from '@/object-metadata/utils/dedupeMorphRelationFieldMetadataItems'; import { isActiveFieldMetadataItem } from '@/object-metadata/utils/isActiveFieldMetadataItem'; import { useMemo } from 'react'; @@ -11,14 +12,16 @@ export const useActiveFieldMetadataItems = ({ const activeFieldMetadataItems = useMemo( () => isDefined(objectMetadataItem) - ? objectMetadataItem.readableFields.filter( - ({ id, isActive, isSystem, name }) => - isActiveFieldMetadataItem({ - objectNameSingular: objectMetadataItem.nameSingular, - fieldMetadata: { isActive, isSystem, name }, - }) || - // Allow label identifier field even if it's a system field - id === objectMetadataItem.labelIdentifierFieldMetadataId, + ? dedupeMorphRelationFieldMetadataItems( + objectMetadataItem.readableFields.filter( + ({ id, isActive, isSystem, name }) => + isActiveFieldMetadataItem({ + objectNameSingular: objectMetadataItem.nameSingular, + fieldMetadata: { isActive, isSystem, name }, + }) || + // Allow label identifier field even if it's a system field + id === objectMetadataItem.labelIdentifierFieldMetadataId, + ), ) : [], [objectMetadataItem], diff --git a/packages/twenty-front/src/modules/object-metadata/utils/__tests__/dedupeMorphRelationFieldMetadataItems.test.ts b/packages/twenty-front/src/modules/object-metadata/utils/__tests__/dedupeMorphRelationFieldMetadataItems.test.ts new file mode 100644 index 0000000000..898ced85fa --- /dev/null +++ b/packages/twenty-front/src/modules/object-metadata/utils/__tests__/dedupeMorphRelationFieldMetadataItems.test.ts @@ -0,0 +1,146 @@ +import { FieldMetadataType } from 'twenty-shared/types'; + +import { type FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; +import { dedupeMorphRelationFieldMetadataItems } from '@/object-metadata/utils/dedupeMorphRelationFieldMetadataItems'; + +const buildField = ( + field: Partial & Pick, +): FieldMetadataItem => + ({ + isActive: true, + isSystem: false, + morphId: null, + ...field, + }) as FieldMetadataItem; + +describe('dedupeMorphRelationFieldMetadataItems', () => { + it('should keep non-morph fields untouched', () => { + const fields = [ + buildField({ id: '1', type: FieldMetadataType.TEXT }), + buildField({ id: '2', type: FieldMetadataType.NUMBER }), + ]; + + expect(dedupeMorphRelationFieldMetadataItems(fields)).toEqual(fields); + }); + + it('should keep a single field per morphId', () => { + const fields = [ + buildField({ + id: 'b', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-1', + }), + buildField({ + id: 'a', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-1', + }), + ]; + + const result = dedupeMorphRelationFieldMetadataItems(fields); + + expect(result).toHaveLength(1); + expect(result[0].id).toBe('a'); + }); + + it('should preserve the position of the surviving morph field', () => { + const fields = [ + buildField({ id: 'name', type: FieldMetadataType.TEXT }), + buildField({ + id: 'a', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-1', + }), + buildField({ + id: 'z', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-1', + }), + buildField({ id: 'tag', type: FieldMetadataType.TEXT }), + ]; + + const result = dedupeMorphRelationFieldMetadataItems(fields); + + expect(result.map((field) => field.id)).toEqual(['name', 'a', 'tag']); + }); + + it('should prefer active non-system fields over system ones', () => { + const fields = [ + buildField({ + id: 'a', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-1', + isSystem: true, + }), + buildField({ + id: 'z', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-1', + isSystem: false, + }), + ]; + + const result = dedupeMorphRelationFieldMetadataItems(fields); + + expect(result).toHaveLength(1); + expect(result[0].id).toBe('z'); + }); + + it('should keep the active morph field over an inactive one', () => { + const fields = [ + buildField({ + id: 'a', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-1', + isActive: false, + }), + buildField({ + id: 'z', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-1', + isActive: true, + }), + ]; + + const result = dedupeMorphRelationFieldMetadataItems(fields); + + expect(result).toHaveLength(1); + expect(result[0].id).toBe('z'); + }); + + it('should dedupe each morphId independently', () => { + const fields = [ + buildField({ + id: 'a1', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-1', + }), + buildField({ + id: 'a2', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-1', + }), + buildField({ + id: 'b1', + type: FieldMetadataType.MORPH_RELATION, + morphId: 'morph-2', + }), + ]; + + const result = dedupeMorphRelationFieldMetadataItems(fields); + + expect(result.map((field) => field.id).sort()).toEqual(['a1', 'b1']); + }); + + it('should keep morph fields without a morphId', () => { + const fields = [ + buildField({ + id: 'a', + type: FieldMetadataType.MORPH_RELATION, + morphId: null, + }), + ]; + + expect(dedupeMorphRelationFieldMetadataItems(fields)).toEqual(fields); + }); +}); diff --git a/packages/twenty-front/src/modules/object-metadata/utils/dedupeMorphRelationFieldMetadataItems.ts b/packages/twenty-front/src/modules/object-metadata/utils/dedupeMorphRelationFieldMetadataItems.ts new file mode 100644 index 0000000000..3c81d75140 --- /dev/null +++ b/packages/twenty-front/src/modules/object-metadata/utils/dedupeMorphRelationFieldMetadataItems.ts @@ -0,0 +1,38 @@ +import { FieldMetadataType } from 'twenty-shared/types'; +import { isDefined, pickMorphGroupSurvivorOrThrow } from 'twenty-shared/utils'; + +import { type FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; + +export const dedupeMorphRelationFieldMetadataItems = ( + fieldMetadataItems: FieldMetadataItem[], +): FieldMetadataItem[] => { + const morphGroupsByMorphId = new Map(); + + for (const fieldMetadataItem of fieldMetadataItems) { + if ( + fieldMetadataItem.type !== FieldMetadataType.MORPH_RELATION || + !isDefined(fieldMetadataItem.morphId) + ) { + continue; + } + + const group = morphGroupsByMorphId.get(fieldMetadataItem.morphId) ?? []; + + group.push(fieldMetadataItem); + morphGroupsByMorphId.set(fieldMetadataItem.morphId, group); + } + + const survivorIdByMorphId = new Map(); + + for (const [morphId, group] of morphGroupsByMorphId) { + survivorIdByMorphId.set(morphId, pickMorphGroupSurvivorOrThrow(group).id); + } + + return fieldMetadataItems.filter( + (fieldMetadataItem) => + fieldMetadataItem.type !== FieldMetadataType.MORPH_RELATION || + !isDefined(fieldMetadataItem.morphId) || + survivorIdByMorphId.get(fieldMetadataItem.morphId) === + fieldMetadataItem.id, + ); +}; diff --git a/packages/twenty-server/src/engine/dataloaders/utils/__tests__/pick-morph-group-survivor.util.spec.ts b/packages/twenty-server/src/engine/dataloaders/utils/__tests__/pick-morph-group-survivor.util.spec.ts deleted file mode 100644 index 702f562755..0000000000 --- a/packages/twenty-server/src/engine/dataloaders/utils/__tests__/pick-morph-group-survivor.util.spec.ts +++ /dev/null @@ -1,85 +0,0 @@ -import { FieldMetadataType } from 'twenty-shared/types'; - -import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; -import { pickMorphGroupSurvivor } from 'src/engine/dataloaders/utils/pick-morph-group-survivor.util'; - -const makeMorphField = ( - overrides: Partial> & { - id: string; - }, -): FlatFieldMetadata => - ({ - type: FieldMetadataType.MORPH_RELATION, - isActive: true, - isSystem: false, - morphId: 'morph-1', - ...overrides, - }) as FlatFieldMetadata; - -describe('pickMorphGroupSurvivor', () => { - it('should return the only field when group has one element', () => { - const field = makeMorphField({ id: 'a' }); - - expect(pickMorphGroupSurvivor([field])).toBe(field); - }); - - it('should prefer active non-system over active system', () => { - const standard = makeMorphField({ - id: 'b', - isActive: true, - isSystem: false, - }); - const system = makeMorphField({ - id: 'a', - isActive: true, - isSystem: true, - }); - - expect(pickMorphGroupSurvivor([system, standard])).toBe(standard); - }); - - it('should prefer active over inactive', () => { - const active = makeMorphField({ - id: 'b', - isActive: true, - isSystem: true, - }); - const inactive = makeMorphField({ - id: 'a', - isActive: false, - isSystem: false, - }); - - expect(pickMorphGroupSurvivor([inactive, active])).toBe(active); - }); - - it('should break ties by smallest id', () => { - const fieldA = makeMorphField({ - id: 'aaa', - isActive: true, - isSystem: false, - }); - const fieldB = makeMorphField({ - id: 'bbb', - isActive: true, - isSystem: false, - }); - - expect(pickMorphGroupSurvivor([fieldB, fieldA])).toBe(fieldA); - }); - - it('should prefer active+non-system (score 3) over inactive+non-system (score 1)', () => { - const best = makeMorphField({ - id: 'z', - isActive: true, - isSystem: false, - }); - const worse = makeMorphField({ - id: 'a', - isActive: false, - isSystem: false, - }); - - expect(pickMorphGroupSurvivor([worse, best])).toBe(best); - }); -}); diff --git a/packages/twenty-server/src/engine/dataloaders/utils/filter-morph-relation-duplicate-fields.util.ts b/packages/twenty-server/src/engine/dataloaders/utils/filter-morph-relation-duplicate-fields.util.ts index 465c0eddb0..e5e7c4651e 100644 --- a/packages/twenty-server/src/engine/dataloaders/utils/filter-morph-relation-duplicate-fields.util.ts +++ b/packages/twenty-server/src/engine/dataloaders/utils/filter-morph-relation-duplicate-fields.util.ts @@ -1,8 +1,8 @@ import { FieldMetadataType } from 'twenty-shared/types'; +import { pickMorphGroupSurvivorOrThrow } from 'twenty-shared/utils'; import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; import { isFlatFieldMetadataOfType } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-flat-field-metadata-of-type.util'; -import { pickMorphGroupSurvivor } from 'src/engine/dataloaders/utils/pick-morph-group-survivor.util'; export const filterMorphRelationDuplicateFields = ( flatFieldMetadatas: FlatFieldMetadata[], @@ -36,7 +36,7 @@ export const filterMorphRelationDuplicateFields = ( []; for (const group of morphGroupsByMorphId.values()) { - filteredMorphFlatFieldMetadatas.push(pickMorphGroupSurvivor(group)); + filteredMorphFlatFieldMetadatas.push(pickMorphGroupSurvivorOrThrow(group)); } return [...otherFlatFieldMetadatas, ...filteredMorphFlatFieldMetadatas]; diff --git a/packages/twenty-server/src/engine/dataloaders/utils/pick-morph-group-survivor.util.ts b/packages/twenty-server/src/engine/dataloaders/utils/pick-morph-group-survivor.util.ts deleted file mode 100644 index 782dd6168a..0000000000 --- a/packages/twenty-server/src/engine/dataloaders/utils/pick-morph-group-survivor.util.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { type FieldMetadataType } from 'twenty-shared/types'; - -import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; - -// Prefers active non-system fields (standard targets) over system ones -// (auto-created for custom objects). Smallest id breaks ties. -const scoreMorphField = ( - field: FlatFieldMetadata, -): number => (field.isActive ? 2 : 0) + (field.isSystem ? 0 : 1); - -export const pickMorphGroupSurvivor = ( - group: FlatFieldMetadata[], -): FlatFieldMetadata => { - return group.reduce((best, current) => { - const diff = scoreMorphField(current) - scoreMorphField(best); - - return diff > 0 || (diff === 0 && current.id < best.id) ? current : best; - }); -}; diff --git a/packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/resolve-relation-from-flat-field-metadata.util.ts b/packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/resolve-relation-from-flat-field-metadata.util.ts index d3304c5e86..3ea981a77c 100644 --- a/packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/resolve-relation-from-flat-field-metadata.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/flat-field-metadata/utils/resolve-relation-from-flat-field-metadata.util.ts @@ -1,6 +1,6 @@ import { FieldMetadataType } from 'twenty-shared/types'; +import { pickMorphGroupSurvivorOrThrow } from 'twenty-shared/utils'; -import { pickMorphGroupSurvivor } from 'src/engine/dataloaders/utils/pick-morph-group-survivor.util'; import { RelationDTO } from 'src/engine/metadata-modules/field-metadata/dtos/relation.dto'; import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; import { findFlatEntityByIdInFlatEntityMapsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps-or-throw.util'; @@ -64,7 +64,7 @@ export const resolveRelationFromFlatFieldMetadata = ({ }), ]; - const survivorMorphField = pickMorphGroupSurvivor( + const survivorMorphField = pickMorphGroupSurvivorOrThrow( allMorphFlatFieldMetadatas, ); diff --git a/packages/twenty-shared/src/utils/fieldMetadata/__tests__/pick-morph-group-survivor-or-throw.spec.ts b/packages/twenty-shared/src/utils/fieldMetadata/__tests__/pick-morph-group-survivor-or-throw.spec.ts new file mode 100644 index 0000000000..b716fd5c05 --- /dev/null +++ b/packages/twenty-shared/src/utils/fieldMetadata/__tests__/pick-morph-group-survivor-or-throw.spec.ts @@ -0,0 +1,61 @@ +import { pickMorphGroupSurvivorOrThrow } from '@/utils/fieldMetadata/pick-morph-group-survivor-or-throw'; + +const makeMorphField = (overrides: { + id: string; + isActive?: boolean; + isSystem?: boolean; +}) => ({ + isActive: true, + isSystem: false, + ...overrides, +}); + +describe('pickMorphGroupSurvivorOrThrow', () => { + it('should return the only field when group has one element', () => { + const field = makeMorphField({ id: 'a' }); + + expect(pickMorphGroupSurvivorOrThrow([field])).toBe(field); + }); + + it('should prefer active non-system over active system', () => { + const standard = makeMorphField({ + id: 'b', + isActive: true, + isSystem: false, + }); + const system = makeMorphField({ id: 'a', isActive: true, isSystem: true }); + + expect(pickMorphGroupSurvivorOrThrow([system, standard])).toBe(standard); + }); + + it('should prefer active over inactive', () => { + const active = makeMorphField({ id: 'b', isActive: true, isSystem: true }); + const inactive = makeMorphField({ + id: 'a', + isActive: false, + isSystem: false, + }); + + expect(pickMorphGroupSurvivorOrThrow([inactive, active])).toBe(active); + }); + + it('should break ties by smallest id', () => { + const fieldA = makeMorphField({ id: 'aaa' }); + const fieldB = makeMorphField({ id: 'bbb' }); + + expect(pickMorphGroupSurvivorOrThrow([fieldB, fieldA])).toBe(fieldA); + }); + + it('should treat nullish isActive/isSystem as falsy', () => { + const nullishField = { id: 'a', isActive: null, isSystem: null }; + const activeField = makeMorphField({ id: 'b', isActive: true }); + + expect(pickMorphGroupSurvivorOrThrow([nullishField, activeField])).toBe( + activeField, + ); + }); + + it('should throw on an empty group', () => { + expect(() => pickMorphGroupSurvivorOrThrow([])).toThrow(); + }); +}); diff --git a/packages/twenty-shared/src/utils/fieldMetadata/pick-morph-group-survivor-or-throw.ts b/packages/twenty-shared/src/utils/fieldMetadata/pick-morph-group-survivor-or-throw.ts new file mode 100644 index 0000000000..abde5680c7 --- /dev/null +++ b/packages/twenty-shared/src/utils/fieldMetadata/pick-morph-group-survivor-or-throw.ts @@ -0,0 +1,32 @@ +import { CustomError } from '@/utils/errors'; + +type MorphGroupSurvivorCandidate = { + id: string; + isActive?: boolean | null; + isSystem?: boolean | null; +}; + +const scoreMorphField = (field: MorphGroupSurvivorCandidate): number => + (field.isActive ? 2 : 0) + (field.isSystem ? 0 : 1); + +export const pickMorphGroupSurvivorOrThrow = < + T extends MorphGroupSurvivorCandidate, +>( + group: T[], +): T => { + if (group.length === 0) { + throw new CustomError( + 'pickMorphGroupSurvivorOrThrow requires a non-empty morph group', + 'EMPTY_MORPH_GROUP', + ); + } + + return group.reduce((best, current) => { + const scoreDifference = scoreMorphField(current) - scoreMorphField(best); + + return scoreDifference > 0 || + (scoreDifference === 0 && current.id < best.id) + ? current + : best; + }); +}; diff --git a/packages/twenty-shared/src/utils/index.ts b/packages/twenty-shared/src/utils/index.ts index ce7dbf402f..8fc434dd36 100644 --- a/packages/twenty-shared/src/utils/index.ts +++ b/packages/twenty-shared/src/utils/index.ts @@ -58,6 +58,7 @@ export { isFieldMetadataNumericKind } from './fieldMetadata/isFieldMetadataNumer export { isFieldMetadataSelectKind } from './fieldMetadata/isFieldMetadataSelectKind'; export { isFieldMetadataSupportedInGroupBy } from './fieldMetadata/isFieldMetadataSupportedInGroupBy'; export { isFieldMetadataTextKind } from './fieldMetadata/isFieldMetadataTextKind'; +export { pickMorphGroupSurvivorOrThrow } from './fieldMetadata/pick-morph-group-survivor-or-throw'; export { shouldExcludeFieldFromAgentToolSchema } from './fieldMetadata/shouldExcludeFieldFromAgentToolSchema'; export { extractFolderPathFilenameAndTypeOrThrow } from './files/extractFolderPathFilenameAndTypeOrThrow.util'; export { checkIfShouldComputeEmptinessFilter } from './filter/checkIfShouldComputeEmptinessFilter';