From c7aa59491bec9c935e99edad5f869325842ddcbe Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Thu, 20 Nov 2025 13:58:40 +0100 Subject: [PATCH] Fix merge button (#15899) Fixes https://github.com/twentyhq/private-issues/issues/362 **How to reproduce** Link a person that has a duplicate to an opportunity. (you can create a duplicate by giving two people the same linkedin link). Open the opportunity record from opportunity table. Click on the related person (in "Point of contact"). In front of "Duplicates", click on the merge button (two arrows becoming one). You should here see an empty "Merge preview" and an error when clicking "First" tab. **Issue** The issue is that CommandMenuMergeRecordPage is getting the referenced objectMetadataItem from useContextStoreObjectMetadataItemOrThrow without an instance id. contextStoreObjectMetadataItem is still "opportunity" as it should be, being on an opportunity view. So further down it attempts to display the record page according the label identifier field from opportunity, which is a text field, "name", and it breaks because the record is actually a person for who the "name" field is not a text but a full_name type. **Fix** I suggested a fix that offers the possibility to find the referenced objectMetadataItem from the MergeRecords instanceId. But I still gave flexibility to avoid having to set that state everytime we open the merge tab, by falling back to the default contextStoreObjectMetadataItem. (this is used when we merge records from ticking two records from a view and open the command menu). Im not sure this is the best option. Open to suggestions ! --------- Co-authored-by: Charles Bochet --- .../useOpenMergeRecordsPageInCommandMenu.tsx | 55 ++++++++++++++----- .../components/CommandMenuMergeRecordPage.tsx | 29 ++++++---- .../components/MergePreviewTab.tsx | 4 +- .../components/MergeRecordsContainer.tsx | 10 +--- .../components/MergeSettingsTab.tsx | 12 +--- .../hooks/useMergeRecordsActions.ts | 6 +- .../hooks/useMergeRecordsSelectedRecords.ts | 30 ++++++++++ ...gePreview.ts => usePerformMergePreview.ts} | 30 ++++------ .../common-merge-many-query-runner.service.ts | 4 +- 9 files changed, 112 insertions(+), 68 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergeRecordsSelectedRecords.ts rename packages/twenty-front/src/modules/object-record/record-merge/hooks/{useMergePreview.ts => usePerformMergePreview.ts} (73%) diff --git a/packages/twenty-front/src/modules/command-menu/hooks/useOpenMergeRecordsPageInCommandMenu.tsx b/packages/twenty-front/src/modules/command-menu/hooks/useOpenMergeRecordsPageInCommandMenu.tsx index 04e5198ef8..199a42556b 100644 --- a/packages/twenty-front/src/modules/command-menu/hooks/useOpenMergeRecordsPageInCommandMenu.tsx +++ b/packages/twenty-front/src/modules/command-menu/hooks/useOpenMergeRecordsPageInCommandMenu.tsx @@ -1,11 +1,15 @@ import { useCommandMenuUpdateNavigationMorphItemsByPage } from '@/command-menu/hooks/useCommandMenuUpdateNavigationMorphItemsByPage'; import { useNavigateCommandMenu } from '@/command-menu/hooks/useNavigateCommandMenu'; import { CommandMenuPages } from '@/command-menu/types/CommandMenuPages'; +import { contextStoreCurrentObjectMetadataItemIdComponentState } from '@/context-store/states/contextStoreCurrentObjectMetadataItemIdComponentState'; import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; import { useLazyFindManyRecords } from '@/object-record/hooks/useLazyFindManyRecords'; import { useUpsertRecordsInStore } from '@/object-record/record-store/hooks/useUpsertRecordsInStore'; + import { msg, t } from '@lingui/core/macro'; +import { useRecoilCallback } from 'recoil'; import { IconArrowMerge } from 'twenty-ui/display'; +import { v4 } from 'uuid'; type UseOpenMergeRecordsPageInCommandMenuProps = { objectNameSingular: string; @@ -35,22 +39,43 @@ export const useOpenMergeRecordsPageInCommandMenu = ({ }, }); - const openMergeRecordsPageInCommandMenu = async () => { - await updateCommandMenuNavigationMorphItemsByPage({ - pageId: CommandMenuPages.MergeRecords, - objectMetadataId: objectMetadataItem.id, - objectRecordIds, - }); - const { records } = await findManyRecordsLazy(); - upsertRecordsInStore(records ?? []); + const openMergeRecordsPageInCommandMenu = useRecoilCallback( + ({ set }) => { + return async () => { + const pageId = v4(); - navigateCommandMenu({ - page: CommandMenuPages.MergeRecords, - pageTitle: t(msg`Merge records`), - pageIcon: IconArrowMerge, - pageId: CommandMenuPages.MergeRecords, - }); - }; + set( + contextStoreCurrentObjectMetadataItemIdComponentState.atomFamily({ + instanceId: pageId, + }), + objectMetadataItem.id, + ); + + await updateCommandMenuNavigationMorphItemsByPage({ + pageId, + objectMetadataId: objectMetadataItem.id, + objectRecordIds, + }); + const { records } = await findManyRecordsLazy(); + upsertRecordsInStore(records ?? []); + + navigateCommandMenu({ + page: CommandMenuPages.MergeRecords, + pageTitle: t(msg`Merge records`), + pageIcon: IconArrowMerge, + pageId, + }); + }; + }, + [ + objectMetadataItem.id, + objectRecordIds, + findManyRecordsLazy, + upsertRecordsInStore, + navigateCommandMenu, + updateCommandMenuNavigationMorphItemsByPage, + ], + ); return { openMergeRecordsPageInCommandMenu, diff --git a/packages/twenty-front/src/modules/command-menu/pages/record-page/components/CommandMenuMergeRecordPage.tsx b/packages/twenty-front/src/modules/command-menu/pages/record-page/components/CommandMenuMergeRecordPage.tsx index ea97c77ab6..7a084f8386 100644 --- a/packages/twenty-front/src/modules/command-menu/pages/record-page/components/CommandMenuMergeRecordPage.tsx +++ b/packages/twenty-front/src/modules/command-menu/pages/record-page/components/CommandMenuMergeRecordPage.tsx @@ -1,6 +1,7 @@ import { ActionMenuComponentInstanceContext } from '@/action-menu/states/contexts/ActionMenuComponentInstanceContext'; import { CommandMenuPageComponentInstanceContext } from '@/command-menu/states/contexts/CommandMenuPageComponentInstanceContext'; import { useContextStoreObjectMetadataItemOrThrow } from '@/context-store/hooks/useContextStoreObjectMetadataItemOrThrow'; +import { ContextStoreComponentInstanceContext } from '@/context-store/states/contexts/ContextStoreComponentInstanceContext'; import { RecordComponentInstanceContextsWrapper } from '@/object-record/components/RecordComponentInstanceContextsWrapper'; import { MergeRecordsContainer } from '@/object-record/record-merge/components/MergeRecordsContainer'; import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile'; @@ -20,12 +21,14 @@ const StyledRightDrawerRecord = styled.div<{ export const CommandMenuMergeRecordPage = () => { const isMobile = useIsMobile(); - const { objectMetadataItem } = useContextStoreObjectMetadataItemOrThrow(); - const commandMenuPageInstanceId = useComponentInstanceStateContext( CommandMenuPageComponentInstanceContext, )?.instanceId; + const { objectMetadataItem } = useContextStoreObjectMetadataItemOrThrow( + commandMenuPageInstanceId, + ); + if (!commandMenuPageInstanceId) { throw new Error('Command menu page instance id is not defined'); } @@ -34,15 +37,21 @@ export const CommandMenuMergeRecordPage = () => { - - - - - + + + + + + ); }; diff --git a/packages/twenty-front/src/modules/object-record/record-merge/components/MergePreviewTab.tsx b/packages/twenty-front/src/modules/object-record/record-merge/components/MergePreviewTab.tsx index af5c92a154..2025be24ef 100644 --- a/packages/twenty-front/src/modules/object-record/record-merge/components/MergePreviewTab.tsx +++ b/packages/twenty-front/src/modules/object-record/record-merge/components/MergePreviewTab.tsx @@ -1,4 +1,4 @@ -import { useMergePreview } from '@/object-record/record-merge/hooks/useMergePreview'; +import { usePerformMergePreview } from '@/object-record/record-merge/hooks/usePerformMergePreview'; import { SummaryCard } from '@/object-record/record-show/components/SummaryCard'; import { CardType } from '@/object-record/record-show/types/CardType'; import { getCardComponent } from '@/object-record/record-show/utils/getCardComponent'; @@ -14,7 +14,7 @@ type MergePreviewTabProps = { export const MergePreviewTab = ({ objectNameSingular, }: MergePreviewTabProps) => { - const { mergePreviewRecord, isGeneratingPreview } = useMergePreview({ + const { mergePreviewRecord, isGeneratingPreview } = usePerformMergePreview({ objectNameSingular, }); diff --git a/packages/twenty-front/src/modules/object-record/record-merge/components/MergeRecordsContainer.tsx b/packages/twenty-front/src/modules/object-record/record-merge/components/MergeRecordsContainer.tsx index 2c10579cd3..28a1edc635 100644 --- a/packages/twenty-front/src/modules/object-record/record-merge/components/MergeRecordsContainer.tsx +++ b/packages/twenty-front/src/modules/object-record/record-merge/components/MergeRecordsContainer.tsx @@ -8,7 +8,7 @@ import { TabListComponentInstanceContext } from '@/ui/layout/tab-list/states/con import { useRecoilComponentValue } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValue'; import { CommandMenuPageComponentInstanceContext } from '@/command-menu/states/contexts/CommandMenuPageComponentInstanceContext'; -import { useMergePreview } from '@/object-record/record-merge/hooks/useMergePreview'; +import { useMergeRecordsSelectedRecords } from '@/object-record/record-merge/hooks/useMergeRecordsSelectedRecords'; import { MergeRecordsTabId } from '@/object-record/record-merge/types/MergeRecordsTabId'; import { useAvailableComponentInstanceIdOrThrow } from '@/ui/utilities/state/component-state/hooks/useAvailableComponentInstanceIdOrThrow'; import { useMergeRecordsContainerTabs } from '../hooks/useMergeRecordsContainerTabs'; @@ -45,9 +45,7 @@ type MergeRecordsContainerProps = { export const MergeRecordsContainer = ({ objectNameSingular, }: MergeRecordsContainerProps) => { - const { selectedRecords } = useMergePreview({ - objectNameSingular, - }); + const { selectedRecords } = useMergeRecordsSelectedRecords(); const { tabs } = useMergeRecordsContainerTabs(selectedRecords); @@ -76,9 +74,7 @@ export const MergeRecordsContainer = ({ {activeTabId === MergeRecordsTabId.MERGE_PREVIEW && ( )} - {activeTabId === MergeRecordsTabId.SETTINGS && ( - - )} + {activeTabId === MergeRecordsTabId.SETTINGS && } {selectedRecords.some((record) => record.id === activeTabId) && ( { +export const MergeSettingsTab = () => { const { mergeSettings, updatePriorityRecordIndex } = useMergeRecordsSettings(); - const { selectedRecords } = useMergePreview({ - objectNameSingular, - }); + const { selectedRecords } = useMergeRecordsSelectedRecords(); const priorityOptions = selectedRecords.map((_, index) => ({ value: index, diff --git a/packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergeRecordsActions.ts b/packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergeRecordsActions.ts index 7c6b10f59b..28f88be4e3 100644 --- a/packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergeRecordsActions.ts +++ b/packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergeRecordsActions.ts @@ -3,7 +3,7 @@ import { useRecoilValue, useSetRecoilState } from 'recoil'; import { useCommandMenu } from '@/command-menu/hooks/useCommandMenu'; import { useMergeManyRecords } from '@/object-record/hooks/useMergeManyRecords'; -import { useMergePreview } from '@/object-record/record-merge/hooks/useMergePreview'; +import { useMergeRecordsSelectedRecords } from '@/object-record/record-merge/hooks/useMergeRecordsSelectedRecords'; import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar'; import { AppPath } from 'twenty-shared/types'; import { useNavigateApp } from '~/hooks/useNavigateApp'; @@ -19,9 +19,7 @@ export const useMergeRecordsActions = ({ }: UseMergeRecordsActionsProps) => { const mergeSettings = useRecoilValue(mergeSettingsState); - const { selectedRecords } = useMergePreview({ - objectNameSingular, - }); + const { selectedRecords } = useMergeRecordsSelectedRecords(); const { mergeManyRecords, loading: isMerging } = useMergeManyRecords({ objectNameSingular, diff --git a/packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergeRecordsSelectedRecords.ts b/packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergeRecordsSelectedRecords.ts new file mode 100644 index 0000000000..fdc7a69afe --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergeRecordsSelectedRecords.ts @@ -0,0 +1,30 @@ +import { commandMenuNavigationMorphItemsByPageState } from '@/command-menu/states/commandMenuNavigationMorphItemsByPageState'; +import { CommandMenuPageComponentInstanceContext } from '@/command-menu/states/contexts/CommandMenuPageComponentInstanceContext'; +import { recordStoreRecordsSelector } from '@/object-record/record-store/states/selectors/recordStoreRecordsSelector'; +import { useComponentInstanceStateContext } from '@/ui/utilities/state/component-state/hooks/useComponentInstanceStateContext'; +import { useRecoilValue } from 'recoil'; + +export const useMergeRecordsSelectedRecords = () => { + const mergeRecordsPageInstanceId = useComponentInstanceStateContext( + CommandMenuPageComponentInstanceContext, + )?.instanceId; + + const commandMenuNavigationMorphItemsByPage = useRecoilValue( + commandMenuNavigationMorphItemsByPageState, + ); + + const selectedRecordIds = + commandMenuNavigationMorphItemsByPage + .get(mergeRecordsPageInstanceId ?? '') + ?.map((morphItem) => morphItem.recordId) ?? []; + + const selectedRecords = useRecoilValue( + recordStoreRecordsSelector({ + recordIds: selectedRecordIds, + }), + ); + + return { + selectedRecords, + }; +}; diff --git a/packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergePreview.ts b/packages/twenty-front/src/modules/object-record/record-merge/hooks/usePerformMergePreview.ts similarity index 73% rename from packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergePreview.ts rename to packages/twenty-front/src/modules/object-record/record-merge/hooks/usePerformMergePreview.ts index a9b634952e..6f41bd6f5e 100644 --- a/packages/twenty-front/src/modules/object-record/record-merge/hooks/useMergePreview.ts +++ b/packages/twenty-front/src/modules/object-record/record-merge/hooks/usePerformMergePreview.ts @@ -1,9 +1,7 @@ -import { commandMenuNavigationMorphItemsByPageState } from '@/command-menu/states/commandMenuNavigationMorphItemsByPageState'; -import { CommandMenuPages } from '@/command-menu/types/CommandMenuPages'; import { getRecordFromRecordNode } from '@/object-record/cache/utils/getRecordFromRecordNode'; import { useMergeManyRecords } from '@/object-record/hooks/useMergeManyRecords'; +import { useMergeRecordsSelectedRecords } from '@/object-record/record-merge/hooks/useMergeRecordsSelectedRecords'; import { useUpsertRecordsInStore } from '@/object-record/record-store/hooks/useUpsertRecordsInStore'; -import { recordStoreRecordsSelector } from '@/object-record/record-store/states/selectors/recordStoreRecordsSelector'; import { type ObjectRecord } from '@/object-record/types/ObjectRecord'; import { useEffect, useState } from 'react'; import { useRecoilValue } from 'recoil'; @@ -14,7 +12,7 @@ type UseMergePreviewProps = { objectNameSingular: string; }; -export const useMergePreview = ({ +export const usePerformMergePreview = ({ objectNameSingular, }: UseMergePreviewProps) => { const [mergePreviewRecord, setMergePreviewRecord] = @@ -29,26 +27,20 @@ export const useMergePreview = ({ objectNameSingular, }); - const commandMenuNavigationMorphItemsByPage = useRecoilValue( - commandMenuNavigationMorphItemsByPageState, - ); - - const selectedRecordIds = - commandMenuNavigationMorphItemsByPage - .get(CommandMenuPages.MergeRecords) - ?.map((morphItem) => morphItem.recordId) ?? []; - const selectedRecords = useRecoilValue( - recordStoreRecordsSelector({ - recordIds: selectedRecordIds, - }), - ); + const { selectedRecords } = useMergeRecordsSelectedRecords(); const { upsertRecordsInStore } = useUpsertRecordsInStore(); useEffect(() => { const fetchPreview = async () => { - if (selectedRecords.length < 2 || isMergeInProgress || isInitialized) + if ( + selectedRecords.length < 2 || + isMergeInProgress || + isInitialized || + isGeneratingPreview + ) { return; + } setIsGeneratingPreview(true); try { @@ -83,13 +75,13 @@ export const useMergePreview = ({ selectedRecords, mergeSettings, isMergeInProgress, + isGeneratingPreview, mergeManyRecords, upsertRecordsInStore, isInitialized, ]); return { - selectedRecords, mergePreviewRecord, isGeneratingPreview: isGeneratingPreview, }; diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-merge-many-query-runner.service.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-merge-many-query-runner.service.ts index 9ddbd566f3..72b839353a 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-merge-many-query-runner.service.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-merge-many-query-runner.service.ts @@ -5,10 +5,10 @@ import { QUERY_MAX_RECORDS, } from 'twenty-shared/constants'; import { + FieldMetadataRelationSettings, FieldMetadataType, ObjectRecord, RelationType, - FieldMetadataRelationSettings, } from 'twenty-shared/types'; import { isDefined } from 'twenty-shared/utils'; import { FindOptionsRelations, In, ObjectLiteral } from 'typeorm'; @@ -95,7 +95,7 @@ export class CommonMergeManyQueryRunnerService extends CommonBaseQueryRunnerServ }); await queryBuilder - .softDelete() + .delete() .whereInIds(idsToDelete) .returning(columnsToReturn) .execute();