From 353d1e89d5d6971069868366ba77477da4cb37ef Mon Sep 17 00:00:00 2001 From: Thomas Trompette Date: Mon, 13 Apr 2026 17:50:58 +0200 Subject: [PATCH] Fix merge with null value + reset data virtualization before init load (#19633) **Merge records fix:** selectPriorityFieldValue throws when merging records if the priority record has no value for a field (e.g., null/empty) but 2+ other records do. The recordsWithValues array is pre-filtered to only records with non-empty values, so the priority record isn't in the list. The fix: instead of throwing, fall back to null since this is the priority record actual value **Duplicated IDs fix** https://github.com/user-attachments/assets/bd6d7d08-d079-49a5-aad4-740b59a3c246 When applying a filter that reduces the record count, the virtualized table's record ID array keeps stale entries from the previous larger result set. loadRecordsToVirtualRows clones the old array (e.g., 60 entries) and only overwrites the first N positions (e.g., 9) with the new filtered results, leaving positions 9-59 with old IDs. If any old ID matches a new one, it appears twice in the selection, causing "-> 2 selected" for a single click and a duplicate ID in the merge mutation payload. The fix: clear the record IDs array in useTriggerInitialRecordTableDataLoad before repopulating it with fresh data. --------- Co-authored-by: Charles Bochet --- .../useTriggerInitialRecordTableDataLoad.ts | 30 +++- .../default-merge-field-value.util.spec.ts | 128 ++++++++++++++++++ .../__tests__/merge-field-values.util.spec.ts | 19 ++- .../select-priority-field-value.util.spec.ts | 111 --------------- ...l.ts => default-merge-field-value.util.ts} | 16 ++- .../utils/merge-field-values.util.ts | 8 +- ...on-field-values-for-dry-run-record.util.ts | 4 +- 7 files changed, 176 insertions(+), 140 deletions(-) create mode 100644 packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/default-merge-field-value.util.spec.ts delete mode 100644 packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/select-priority-field-value.util.spec.ts rename packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/{select-priority-field-value.util.ts => default-merge-field-value.util.ts} (60%) diff --git a/packages/twenty-front/src/modules/object-record/record-table/virtualization/hooks/useTriggerInitialRecordTableDataLoad.ts b/packages/twenty-front/src/modules/object-record/record-table/virtualization/hooks/useTriggerInitialRecordTableDataLoad.ts index 86b2715647c..fbd5f8deb70 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/virtualization/hooks/useTriggerInitialRecordTableDataLoad.ts +++ b/packages/twenty-front/src/modules/object-record/record-table/virtualization/hooks/useTriggerInitialRecordTableDataLoad.ts @@ -1,8 +1,12 @@ -import { useCallback } from 'react'; import { useStore } from 'jotai'; +import { useCallback } from 'react'; import { useRecordIndexTableLazyQuery } from '@/object-record/record-index/hooks/useRecordIndexTableLazyQuery'; -import { recordIndexAllRecordIdsComponentSelector } from '@/object-record/record-index/states/selectors/recordIndexAllRecordIdsComponentSelector'; +import { recordIndexRecordIdsByGroupComponentFamilyState } from '@/object-record/record-index/states/recordIndexRecordIdsByGroupComponentFamilyState'; +import { + NO_RECORD_GROUP_FAMILY_KEY, + recordIndexAllRecordIdsComponentSelector, +} from '@/object-record/record-index/states/selectors/recordIndexAllRecordIdsComponentSelector'; import { useUpsertRecordsInStore } from '@/object-record/record-store/hooks/useUpsertRecordsInStore'; import { RECORD_TABLE_HORIZONTAL_SCROLL_SHADOW_VISIBILITY_CSS_VARIABLE_NAME } from '@/object-record/record-table/constants/RecordTableHorizontalScrollShadowVisibilityCssVariableName'; import { RECORD_TABLE_VERTICAL_SCROLL_SHADOW_VISIBILITY_CSS_VARIABLE_NAME } from '@/object-record/record-table/constants/RecordTableVerticalScrollShadowVisibilityCssVariableName'; @@ -17,20 +21,21 @@ import { useReapplyRowSelection } from '@/object-record/record-table/virtualizat import { useResetTableFocuses } from '@/object-record/record-table/virtualization/hooks/useResetTableFocuses'; import { useResetVirtualizedRowTreadmill } from '@/object-record/record-table/virtualization/hooks/useResetVirtualizedRowTreadmill'; +import { dataLoadingStatusByRealIndexComponentState } from '@/object-record/record-table/virtualization/states/dataLoadingStatusByRealIndexComponentState'; import { dataPagesLoadedComponentState } from '@/object-record/record-table/virtualization/states/dataPagesLoadedComponentState'; import { isInitializingVirtualTableDataLoadingComponentState } from '@/object-record/record-table/virtualization/states/isInitializingVirtualTableDataLoadingComponentState'; import { lastRealIndexSetComponentState } from '@/object-record/record-table/virtualization/states/lastRealIndexSetComponentState'; import { lastScrollPositionComponentState } from '@/object-record/record-table/virtualization/states/lastScrollPositionComponentState'; +import { recordIdByRealIndexComponentState } from '@/object-record/record-table/virtualization/states/recordIdByRealIndexComponentState'; import { scrollAtRealIndexComponentState } from '@/object-record/record-table/virtualization/states/scrollAtRealIndexComponentState'; import { totalNumberOfRecordsToVirtualizeComponentState } from '@/object-record/record-table/virtualization/states/totalNumberOfRecordsToVirtualizeComponentState'; import { type ObjectRecord } from '@/object-record/types/ObjectRecord'; import { SIGN_IN_BACKGROUND_MOCK_COMPANIES } from '@/sign-in-background-mock/constants/SignInBackgroundMockCompanies'; import { useShowAuthModal } from '@/ui/layout/hooks/useShowAuthModal'; -import { useAtomComponentStateCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateCallbackState'; -import { dataLoadingStatusByRealIndexComponentState } from '@/object-record/record-table/virtualization/states/dataLoadingStatusByRealIndexComponentState'; -import { recordIdByRealIndexComponentState } from '@/object-record/record-table/virtualization/states/recordIdByRealIndexComponentState'; -import { useSetAtomComponentState } from '@/ui/utilities/state/jotai/hooks/useSetAtomComponentState'; +import { useAtomComponentFamilyStateCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentFamilyStateCallbackState'; import { useAtomComponentSelectorCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentSelectorCallbackState'; +import { useAtomComponentStateCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateCallbackState'; +import { useSetAtomComponentState } from '@/ui/utilities/state/jotai/hooks/useSetAtomComponentState'; import { isDefined } from 'twenty-shared/utils'; export const useTriggerInitialRecordTableDataLoad = () => { @@ -62,6 +67,11 @@ export const useTriggerInitialRecordTableDataLoad = () => { const store = useStore(); + const recordIndexRecordIdsByGroupFamilyState = + useAtomComponentFamilyStateCallbackState( + recordIndexRecordIdsByGroupComponentFamilyState, + ); + const recordIdByRealIndexCallbackState = useAtomComponentStateCallbackState( recordIdByRealIndexComponentState, ); @@ -122,6 +132,8 @@ export const useTriggerInitialRecordTableDataLoad = () => { store.set(isInitializingVirtualTableDataLoadingCallbackState, true); try { + store.set(isRecordTableInitialLoading, true); + resetTableFocuses(); resetVirtualizedRowTreadmill(); @@ -165,6 +177,11 @@ export const useTriggerInitialRecordTableDataLoad = () => { newDataLoadingStatusByRealIndex, ); + store.set( + recordIndexRecordIdsByGroupFamilyState(NO_RECORD_GROUP_FAMILY_KEY), + [], + ); + const { records: findManyRecords, totalCount: findManyTotalCount } = await findManyRecordsLazy(); @@ -211,6 +228,7 @@ export const useTriggerInitialRecordTableDataLoad = () => { resetTableFocuses, resetVirtualizedRowTreadmill, recordIndexAllRecordIds, + recordIndexRecordIdsByGroupFamilyState, store, showAuthModal, dataPagesLoadedCallbackState, diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/default-merge-field-value.util.spec.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/default-merge-field-value.util.spec.ts new file mode 100644 index 00000000000..209d65f6ca7 --- /dev/null +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/default-merge-field-value.util.spec.ts @@ -0,0 +1,128 @@ +import { defaultMergeFieldValue } from 'src/engine/api/graphql/graphql-query-runner/utils/default-merge-field-value.util'; + +describe('defaultMergeFieldValue', () => { + it('should return priority record value when available', () => { + const recordsWithValues = [ + { + recordId: '1', + value: 'priority value', + }, + { + recordId: '2', + value: 'other value', + }, + ]; + + const result = defaultMergeFieldValue(recordsWithValues, '1'); + + expect(result).toBe('priority value'); + }); + + it('should fall back to first record with value when priority record not found', () => { + const recordsWithValues = [ + { + recordId: '2', + value: 'first value', + }, + { + recordId: '3', + value: 'second value', + }, + ]; + + const result = defaultMergeFieldValue(recordsWithValues, '1'); + + expect(result).toBe('first value'); + }); + + it('should fall back to other record when priority record has null value', () => { + const recordsWithValues = [ + { + recordId: '1', + value: null, + }, + { + recordId: '2', + value: 'fallback value', + }, + ]; + + const result = defaultMergeFieldValue(recordsWithValues, '1'); + + expect(result).toBe('fallback value'); + }); + + it('should fall back to other record when priority record has empty string', () => { + const recordsWithValues = [ + { + recordId: '1', + value: '', + }, + { + recordId: '2', + value: 'fallback value', + }, + ]; + + const result = defaultMergeFieldValue(recordsWithValues, '1'); + + expect(result).toBe('fallback value'); + }); + + it('should fall back to other record when priority record has undefined value', () => { + const recordsWithValues = [ + { + recordId: '1', + value: undefined, + }, + { + recordId: '2', + value: 'fallback value', + }, + ]; + + const result = defaultMergeFieldValue(recordsWithValues, '1'); + + expect(result).toBe('fallback value'); + }); + + it('should return null when no records exist', () => { + const result = defaultMergeFieldValue([], '1'); + + expect(result).toBeNull(); + }); + + it('should return null when no record has a value', () => { + const recordsWithValues = [ + { + recordId: '1', + value: null, + }, + { + recordId: '2', + value: '', + }, + ]; + + const result = defaultMergeFieldValue(recordsWithValues, '1'); + + expect(result).toBeNull(); + }); + + it('should handle complex object values', () => { + const recordsWithValues = [ + { + recordId: '1', + value: { name: 'priority object', id: 1 }, + }, + { + recordId: '2', + value: { name: 'fallback object', id: 2 }, + }, + ]; + + const result = defaultMergeFieldValue(recordsWithValues, '1'); + + expect(result).toEqual({ name: 'priority object', id: 1 }); + }); +}); diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/merge-field-values.util.spec.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/merge-field-values.util.spec.ts index 69ef4ef938e..c4db97e7641 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/merge-field-values.util.spec.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/merge-field-values.util.spec.ts @@ -21,20 +21,19 @@ describe('mergeFieldValues', () => { expect(result).toBe('value2'); }); - it('should throw error when priority record is not found', () => { - const recordsWithoutPriorityValue = [ + it('should fall back to first record with value when priority record is not found', () => { + const recordsWithValues = [ { value: 'value1', recordId: 'record1' }, - { value: null, recordId: PRIORITY_RECORD_ID }, { value: 'value3', recordId: 'record3' }, ]; - expect(() => - mergeFieldValues( - FieldMetadataType.TEXT, - recordsWithoutPriorityValue, - 'non-existent-id', - ), - ).toThrow('Priority record with ID non-existent-id not found'); + const result = mergeFieldValues( + FieldMetadataType.TEXT, + recordsWithValues, + 'non-existent-id', + ); + + expect(result).toBe('value1'); }); }); diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/select-priority-field-value.util.spec.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/select-priority-field-value.util.spec.ts deleted file mode 100644 index 329f66e739f..00000000000 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/select-priority-field-value.util.spec.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { selectPriorityFieldValue } from 'src/engine/api/graphql/graphql-query-runner/utils/select-priority-field-value.util'; - -describe('selectPriorityFieldValue', () => { - it('should return priority record value when available', () => { - const recordsWithValues = [ - { - recordId: '1', - value: 'priority value', - }, - { - recordId: '2', - value: 'other value', - }, - ]; - - const result = selectPriorityFieldValue(recordsWithValues, '1'); - - expect(result).toBe('priority value'); - }); - - it('should throw error when priority record not found', () => { - const recordsWithValues = [ - { - recordId: '2', - value: 'first value', - }, - { - recordId: '3', - value: 'second value', - }, - ]; - - expect(() => selectPriorityFieldValue(recordsWithValues, '1')).toThrow( - 'Priority record with ID 1 not found in merge candidates', - ); - }); - - it('should return null when priority record has no value', () => { - const recordsWithValues = [ - { - recordId: '1', - value: null, - }, - { - recordId: '2', - value: 'fallback value', - }, - ]; - - const result = selectPriorityFieldValue(recordsWithValues, '1'); - - expect(result).toBeNull(); - }); - - it('should return null when priority record has empty string', () => { - const recordsWithValues = [ - { - recordId: '1', - value: '', - }, - { - recordId: '2', - value: 'fallback value', - }, - ]; - - const result = selectPriorityFieldValue(recordsWithValues, '1'); - - expect(result).toBeNull(); - }); - - it('should return null when priority record has undefined value', () => { - const recordsWithValues = [ - { - recordId: '1', - value: undefined, - }, - { - recordId: '2', - value: 'fallback value', - }, - ]; - - const result = selectPriorityFieldValue(recordsWithValues, '1'); - - expect(result).toBeNull(); - }); - - it('should throw error when no records exist', () => { - expect(() => selectPriorityFieldValue([], '1')).toThrow( - 'Priority record with ID 1 not found in merge candidates', - ); - }); - - it('should handle complex object values', () => { - const recordsWithValues = [ - { - recordId: '1', - value: { name: 'priority object', id: 1 }, - }, - { - recordId: '2', - value: { name: 'fallback object', id: 2 }, - }, - ]; - - const result = selectPriorityFieldValue(recordsWithValues, '1'); - - expect(result).toEqual({ name: 'priority object', id: 1 }); - }); -}); diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/select-priority-field-value.util.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/default-merge-field-value.util.ts similarity index 60% rename from packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/select-priority-field-value.util.ts rename to packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/default-merge-field-value.util.ts index 6d1afdbb2bc..a87b4da8b0d 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/select-priority-field-value.util.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/default-merge-field-value.util.ts @@ -2,7 +2,7 @@ import { isDefined } from 'twenty-shared/utils'; import { hasRecordFieldValue } from 'src/engine/api/graphql/graphql-query-runner/utils/has-record-field-value.util'; -export const selectPriorityFieldValue = ( +export const defaultMergeFieldValue = ( recordsWithValues: { value: T; recordId: string }[], priorityRecordId: string, ): T | null => { @@ -10,14 +10,16 @@ export const selectPriorityFieldValue = ( (record) => record.recordId === priorityRecordId, ); - if (!isDefined(priorityRecord)) { - throw new Error( - `Priority record with ID ${priorityRecordId} not found in merge candidates`, - ); + if (isDefined(priorityRecord) && hasRecordFieldValue(priorityRecord.value)) { + return priorityRecord.value; } - if (hasRecordFieldValue(priorityRecord.value)) { - return priorityRecord.value; + const fallbackRecord = recordsWithValues.find((record) => + hasRecordFieldValue(record.value), + ); + + if (fallbackRecord) { + return fallbackRecord.value; } return null; diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/merge-field-values.util.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/merge-field-values.util.ts index 1ca4a6f75b1..2a61d45d77e 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/merge-field-values.util.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/merge-field-values.util.ts @@ -1,9 +1,9 @@ import { FieldMetadataType, - type RelationType, type EmailsMetadata, type LinksMetadata, type PhonesMetadata, + type RelationType, } from 'twenty-shared/types'; import { mergeArrayFieldValues } from './merge-array-field-values.util'; @@ -11,7 +11,7 @@ import { mergeEmailsFieldValues } from './merge-emails-field-values.util'; import { mergeLinksFieldValues } from './merge-links-field-values.util'; import { mergePhonesFieldValues } from './merge-phones-field-values.util'; import { mergeRelationFieldValuesForDryRunRecord } from './merge-relation-field-values-for-dry-run-record.util'; -import { selectPriorityFieldValue } from './select-priority-field-value.util'; +import { defaultMergeFieldValue } from './default-merge-field-value.util'; export const mergeFieldValues = ( fieldType: FieldMetadataType, @@ -34,7 +34,7 @@ export const mergeFieldValues = ( ); } - return selectPriorityFieldValue(recordsWithValues, priorityRecordId); + return defaultMergeFieldValue(recordsWithValues, priorityRecordId); case FieldMetadataType.EMAILS: return mergeEmailsFieldValues( @@ -55,6 +55,6 @@ export const mergeFieldValues = ( ); default: - return selectPriorityFieldValue(recordsWithValues, priorityRecordId); + return defaultMergeFieldValue(recordsWithValues, priorityRecordId); } }; diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/merge-relation-field-values-for-dry-run-record.util.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/merge-relation-field-values-for-dry-run-record.util.ts index 586bb6f28c7..93c4c6ee6a6 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/merge-relation-field-values-for-dry-run-record.util.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/merge-relation-field-values-for-dry-run-record.util.ts @@ -1,7 +1,7 @@ import { type ObjectRecord, RelationType } from 'twenty-shared/types'; import { isDefined } from 'twenty-shared/utils'; -import { selectPriorityFieldValue } from 'src/engine/api/graphql/graphql-query-runner/utils/select-priority-field-value.util'; +import { defaultMergeFieldValue } from 'src/engine/api/graphql/graphql-query-runner/utils/default-merge-field-value.util'; export const mergeRelationFieldValuesForDryRunRecord = ( recordsWithValues: { value: unknown; recordId: string }[], @@ -12,7 +12,7 @@ export const mergeRelationFieldValuesForDryRunRecord = ( return mergeOneToManyRelationArrays(recordsWithValues); } - return selectPriorityFieldValue( + return defaultMergeFieldValue( recordsWithValues as { value: ObjectRecord | null; recordId: string }[], priorityRecordId, );