Skip to content

Commit 859241d

Browse files
rdelassusremi
andauthored
Fix merge records page accumulating duplicate morph items (#17705)
## Why When opening **Merge records** repeatedly, morph items for the same command-menu page were appended instead of replaced. This could produce duplicated IDs (e.g. `[A,B,B,A]`) in the merge flow and extra duplicate tabs in the UI. ## What - Update `useCommandMenuUpdateNavigationMorphItemsByPage` to replace page morph items instead of appending existing ones. - Add regression tests covering: - replacing existing morph items for the same page - keeping only the latest payload when called twice for the same page ## Notes I could not run the full workspace tests locally in this environment because of existing test/build setup issues unrelated to this change (missing `packages/twenty-front/tsconfig.spec.json` and `temporal-polyfill` resolution in dependent tasks). Co-authored-by: remi <remi@labox-apps.com>
1 parent 41e413d commit 859241d

File tree

2 files changed

+117
-12
lines changed

2 files changed

+117
-12
lines changed
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import { useCommandMenuUpdateNavigationMorphItemsByPage } from '@/command-menu/hooks/useCommandMenuUpdateNavigationMorphItemsByPage';
2+
import { commandMenuNavigationMorphItemsByPageState } from '@/command-menu/states/commandMenuNavigationMorphItemsByPageState';
3+
import { renderHook } from '@testing-library/react';
4+
import { act } from 'react';
5+
import { RecoilRoot, useRecoilValue } from 'recoil';
6+
7+
const pageId = 'merge-page-id';
8+
const objectMetadataId = 'company-metadata-id';
9+
10+
const Wrapper = ({
11+
children,
12+
initialRecordIds,
13+
}: {
14+
children: React.ReactNode;
15+
initialRecordIds: string[];
16+
}) => (
17+
<RecoilRoot
18+
initializeState={({ set }) => {
19+
set(
20+
commandMenuNavigationMorphItemsByPageState,
21+
new Map([
22+
[
23+
pageId,
24+
initialRecordIds.map((recordId) => ({
25+
objectMetadataId,
26+
recordId,
27+
})),
28+
],
29+
]),
30+
);
31+
}}
32+
>
33+
{children}
34+
</RecoilRoot>
35+
);
36+
37+
const renderHooks = (initialRecordIds: string[]) =>
38+
renderHook(
39+
() => {
40+
const { updateCommandMenuNavigationMorphItemsByPage } =
41+
useCommandMenuUpdateNavigationMorphItemsByPage();
42+
const commandMenuNavigationMorphItemsByPage = useRecoilValue(
43+
commandMenuNavigationMorphItemsByPageState,
44+
);
45+
46+
return {
47+
updateCommandMenuNavigationMorphItemsByPage,
48+
commandMenuNavigationMorphItemsByPage,
49+
};
50+
},
51+
{
52+
wrapper: ({ children }) => (
53+
<Wrapper initialRecordIds={initialRecordIds}>{children}</Wrapper>
54+
),
55+
},
56+
);
57+
58+
describe('useCommandMenuUpdateNavigationMorphItemsByPage', () => {
59+
it('should replace existing items for a page instead of appending', async () => {
60+
const { result } = renderHooks(['record-1', 'record-2']);
61+
62+
await act(async () => {
63+
await result.current.updateCommandMenuNavigationMorphItemsByPage({
64+
pageId,
65+
objectMetadataId,
66+
objectRecordIds: ['record-2', 'record-1'],
67+
});
68+
});
69+
70+
expect(
71+
result.current.commandMenuNavigationMorphItemsByPage.get(pageId),
72+
).toEqual([
73+
{
74+
objectMetadataId,
75+
recordId: 'record-2',
76+
},
77+
{
78+
objectMetadataId,
79+
recordId: 'record-1',
80+
},
81+
]);
82+
});
83+
84+
it('should keep only the latest payload when called twice for the same page', async () => {
85+
const { result } = renderHooks([]);
86+
87+
await act(async () => {
88+
await result.current.updateCommandMenuNavigationMorphItemsByPage({
89+
pageId,
90+
objectMetadataId,
91+
objectRecordIds: ['record-1', 'record-2'],
92+
});
93+
await result.current.updateCommandMenuNavigationMorphItemsByPage({
94+
pageId,
95+
objectMetadataId,
96+
objectRecordIds: ['record-2', 'record-1'],
97+
});
98+
});
99+
100+
expect(
101+
result.current.commandMenuNavigationMorphItemsByPage.get(pageId),
102+
).toEqual([
103+
{
104+
objectMetadataId,
105+
recordId: 'record-2',
106+
},
107+
{
108+
objectMetadataId,
109+
recordId: 'record-1',
110+
},
111+
]);
112+
});
113+
});

packages/twenty-front/src/modules/command-menu/hooks/useCommandMenuUpdateNavigationMorphItemsByPage.tsx

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { commandMenuNavigationMorphItemsByPageState } from '@/command-menu/states/commandMenuNavigationMorphItemsByPageState';
2-
import { isNonEmptyArray } from '@sniptt/guards';
32
import { useRecoilCallback } from 'recoil';
43

54
type UpdateNavigationMorphItemsByPageParams = {
@@ -20,17 +19,10 @@ export const useCommandMenuUpdateNavigationMorphItemsByPage = () => {
2019
.getLoadable(commandMenuNavigationMorphItemsByPageState)
2120
.getValue();
2221

23-
const currentMorphItemsForPage = currentMorphItems.get(pageId);
24-
25-
const newMorphItems = [
26-
...(isNonEmptyArray(currentMorphItemsForPage)
27-
? currentMorphItemsForPage
28-
: []),
29-
...objectRecordIds.map((recordId) => ({
30-
objectMetadataId,
31-
recordId,
32-
})),
33-
];
22+
const newMorphItems = objectRecordIds.map((recordId) => ({
23+
objectMetadataId,
24+
recordId,
25+
}));
3426

3527
const newMorphItemsMap = new Map(currentMorphItems);
3628
newMorphItemsMap.set(pageId, newMorphItems);

0 commit comments

Comments
 (0)