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';