From 696a202bb9d648ef832d99d1764a5a5594e13366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Bosi?= <71827178+bosiraphael@users.noreply.github.com> Date: Mon, 30 Mar 2026 15:50:52 +0200 Subject: [PATCH] Fix navigation menu edition (#19115) # Description Fix "Already in navbar" false positive in the sidebar object picker: `getObjectMetadataIdsInDraft` was collecting `objectMetadataId` from all navigation menu item types (OBJECT, VIEW, RECORD), so having a pinned view or record for an object type would incorrectly block re-adding that object. Now only OBJECT-type items contribute to the duplicate check. # Video QA ## Before https://github.com/user-attachments/assets/7b853a55-6d7a-444c-92ee-bf6a75d9927c ## After https://github.com/user-attachments/assets/65e651e2-9df8-45fd-872d-00782d6b6490 Co-authored-by: Charles Bochet --- .../getObjectMetadataIdsInDraft.test.ts | 111 ++++++++++++++++++ .../utils/getObjectMetadataIdsInDraft.ts | 24 ++-- .../dnd/hooks/useHandleAddToNavigationDrop.ts | 9 +- .../hooks/useWorkspaceNavigationMenuItems.ts | 19 +-- ...seNavigationMenuObjectMetadataFromDraft.ts | 16 ++- 5 files changed, 133 insertions(+), 46 deletions(-) create mode 100644 packages/twenty-front/src/modules/navigation-menu-item/common/utils/__tests__/getObjectMetadataIdsInDraft.test.ts diff --git a/packages/twenty-front/src/modules/navigation-menu-item/common/utils/__tests__/getObjectMetadataIdsInDraft.test.ts b/packages/twenty-front/src/modules/navigation-menu-item/common/utils/__tests__/getObjectMetadataIdsInDraft.test.ts new file mode 100644 index 0000000000..ccda6fdb55 --- /dev/null +++ b/packages/twenty-front/src/modules/navigation-menu-item/common/utils/__tests__/getObjectMetadataIdsInDraft.test.ts @@ -0,0 +1,111 @@ +import { getObjectMetadataIdsInDraft } from '@/navigation-menu-item/common/utils/getObjectMetadataIdsInDraft'; +import { NavigationMenuItemType } from 'twenty-shared/types'; + +describe('getObjectMetadataIdsInDraft', () => { + it('should collect objectMetadataId from OBJECT-type items', () => { + const draft = [ + { + type: NavigationMenuItemType.OBJECT, + targetObjectMetadataId: 'object-people', + }, + ]; + + const result = getObjectMetadataIdsInDraft(draft); + + expect(result).toEqual(new Set(['object-people'])); + }); + + it('should NOT collect objectMetadataId from VIEW-type items', () => { + const draft = [ + { + type: NavigationMenuItemType.VIEW, + targetObjectMetadataId: 'object-people', + }, + ]; + + const result = getObjectMetadataIdsInDraft(draft); + + expect(result.size).toBe(0); + }); + + it('should NOT collect objectMetadataId from RECORD-type items', () => { + const draft = [ + { + type: NavigationMenuItemType.RECORD, + targetObjectMetadataId: 'object-people', + }, + ]; + + const result = getObjectMetadataIdsInDraft(draft); + + expect(result.size).toBe(0); + }); + + it('should skip RECORD and VIEW items but collect from OBJECT items', () => { + const draft = [ + { + type: NavigationMenuItemType.RECORD, + targetObjectMetadataId: 'object-people', + }, + { + type: NavigationMenuItemType.VIEW, + targetObjectMetadataId: 'object-people', + }, + { + type: NavigationMenuItemType.OBJECT, + targetObjectMetadataId: 'object-companies', + }, + ]; + + const result = getObjectMetadataIdsInDraft(draft); + + expect(result.has('object-people')).toBe(false); + expect(result.has('object-companies')).toBe(true); + }); + + it('should collect from multiple OBJECT items', () => { + const draft = [ + { + type: NavigationMenuItemType.OBJECT, + targetObjectMetadataId: 'object-people', + }, + { + type: NavigationMenuItemType.OBJECT, + targetObjectMetadataId: 'object-companies', + }, + ]; + + const result = getObjectMetadataIdsInDraft(draft); + + expect(result).toEqual(new Set(['object-people', 'object-companies'])); + }); + + it('should skip FOLDER and LINK items', () => { + const draft = [ + { type: NavigationMenuItemType.FOLDER }, + { type: NavigationMenuItemType.LINK }, + ]; + + const result = getObjectMetadataIdsInDraft(draft); + + expect(result.size).toBe(0); + }); + + it('should handle an empty draft', () => { + const result = getObjectMetadataIdsInDraft([]); + + expect(result.size).toBe(0); + }); + + it('should skip OBJECT items without targetObjectMetadataId', () => { + const draft = [ + { + type: NavigationMenuItemType.OBJECT, + }, + ]; + + const result = getObjectMetadataIdsInDraft(draft); + + expect(result.size).toBe(0); + }); +}); diff --git a/packages/twenty-front/src/modules/navigation-menu-item/common/utils/getObjectMetadataIdsInDraft.ts b/packages/twenty-front/src/modules/navigation-menu-item/common/utils/getObjectMetadataIdsInDraft.ts index 18e62f4e21..fc1c878a54 100644 --- a/packages/twenty-front/src/modules/navigation-menu-item/common/utils/getObjectMetadataIdsInDraft.ts +++ b/packages/twenty-front/src/modules/navigation-menu-item/common/utils/getObjectMetadataIdsInDraft.ts @@ -1,28 +1,18 @@ +import { NavigationMenuItemType } from 'twenty-shared/types'; import { isDefined } from 'twenty-shared/utils'; - -type NavigationMenuItemDraftForObjectIds = { - viewId?: string | null; - targetObjectMetadataId?: string | null; -}; - -type ViewForObjectIds = { - id: string; - objectMetadataId: string; -}; +import { type NavigationMenuItem } from '~/generated-metadata/graphql'; export const getObjectMetadataIdsInDraft = ( - draft: NavigationMenuItemDraftForObjectIds[], - views: ViewForObjectIds[], + draft: Pick[], ): Set => draft.reduce>((ids, item) => { - const view = isDefined(item.viewId) - ? views.find((view) => view.id === item.viewId) - : undefined; - if (isDefined(view)) { - ids.add(view.objectMetadataId); + if (item.type !== NavigationMenuItemType.OBJECT) { + return ids; } + if (isDefined(item.targetObjectMetadataId)) { ids.add(item.targetObjectMetadataId); } + return ids; }, new Set()); diff --git a/packages/twenty-front/src/modules/navigation-menu-item/display/dnd/hooks/useHandleAddToNavigationDrop.ts b/packages/twenty-front/src/modules/navigation-menu-item/display/dnd/hooks/useHandleAddToNavigationDrop.ts index 38dd127cf9..97f0b7dfa9 100644 --- a/packages/twenty-front/src/modules/navigation-menu-item/display/dnd/hooks/useHandleAddToNavigationDrop.ts +++ b/packages/twenty-front/src/modules/navigation-menu-item/display/dnd/hooks/useHandleAddToNavigationDrop.ts @@ -9,7 +9,6 @@ import { addToNavPayloadRegistryState } from '@/navigation-menu-item/common/stat import { navigationMenuItemsDraftState } from '@/navigation-menu-item/common/states/navigationMenuItemsDraftState'; import { openNavigationMenuItemFolderIdsState } from '@/navigation-menu-item/common/states/openNavigationMenuItemFolderIdsState'; import { canNavigationMenuItemBeDroppedIn } from '@/navigation-menu-item/common/utils/canNavigationMenuItemBeDroppedIn'; -import { getObjectColorWithFallback } from '@/object-metadata/utils/getObjectColorWithFallback'; import { getObjectMetadataIdsInDraft } from '@/navigation-menu-item/common/utils/getObjectMetadataIdsInDraft'; import { validateAndExtractWorkspaceFolderId } from '@/navigation-menu-item/common/utils/validateAndExtractWorkspaceFolderId'; import { useAddFolderToNavigationMenuDraft } from '@/navigation-menu-item/edit/folder/hooks/useAddFolderToNavigationMenuDraft'; @@ -20,6 +19,7 @@ import { useAddObjectToNavigationMenuDraft } from '@/navigation-menu-item/edit/o import { useAddRecordToNavigationMenuDraft } from '@/navigation-menu-item/edit/record/hooks/useAddRecordToNavigationMenuDraft'; import { useAddViewToNavigationMenuDraft } from '@/navigation-menu-item/edit/view/hooks/useAddViewToNavigationMenuDraft'; import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; +import { getObjectColorWithFallback } from '@/object-metadata/utils/getObjectColorWithFallback'; import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue'; import { useSetAtomState } from '@/ui/utilities/state/jotai/hooks/useSetAtomState'; import { viewsSelector } from '@/views/states/selectors/viewsSelector'; @@ -130,10 +130,9 @@ export const useHandleAddToNavigationDrop = () => { return; } case NavigationMenuItemType.OBJECT: { - const objectMetadataIdsInWorkspace = getObjectMetadataIdsInDraft( - currentDraft, - views, - ); + const objectMetadataIdsInWorkspace = + getObjectMetadataIdsInDraft(currentDraft); + if (objectMetadataIdsInWorkspace.has(payload.objectMetadataId)) { return; } diff --git a/packages/twenty-front/src/modules/navigation-menu-item/display/hooks/useWorkspaceNavigationMenuItems.ts b/packages/twenty-front/src/modules/navigation-menu-item/display/hooks/useWorkspaceNavigationMenuItems.ts index 2e3daff1ca..38258e7f36 100644 --- a/packages/twenty-front/src/modules/navigation-menu-item/display/hooks/useWorkspaceNavigationMenuItems.ts +++ b/packages/twenty-front/src/modules/navigation-menu-item/display/hooks/useWorkspaceNavigationMenuItems.ts @@ -1,6 +1,4 @@ -import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue'; -import { viewsSelector } from '@/views/states/selectors/viewsSelector'; - +import { NavigationMenuItemType } from 'twenty-shared/types'; import { isDefined } from 'twenty-shared/utils'; import { useNavigationMenuItemsData } from './useNavigationMenuItemsData'; @@ -9,22 +7,13 @@ export const useWorkspaceNavigationMenuItems = (): { } => { const { workspaceNavigationMenuItems: rawWorkspaceNavigationMenuItems } = useNavigationMenuItemsData(); - const views = useAtomStateValue(viewsSelector); - const workspaceNavViewIds = new Set( + const objectMetadataIdsInWorkspaceNav = new Set( rawWorkspaceNavigationMenuItems - .map((item) => item.viewId) - .filter((viewId) => isDefined(viewId)), - ); - - const objectMetadataIdsInWorkspaceNav = new Set([ - ...views - .filter((view) => workspaceNavViewIds.has(view.id)) - .map((view) => view.objectMetadataId), - ...rawWorkspaceNavigationMenuItems + .filter((item) => item.type === NavigationMenuItemType.OBJECT) .map((item) => item.targetObjectMetadataId) .filter((objectMetadataId) => isDefined(objectMetadataId)), - ]); + ); return { objectMetadataIdsInWorkspaceNav, diff --git a/packages/twenty-front/src/modules/navigation-menu-item/edit/hooks/useNavigationMenuObjectMetadataFromDraft.ts b/packages/twenty-front/src/modules/navigation-menu-item/edit/hooks/useNavigationMenuObjectMetadataFromDraft.ts index dd1147d50e..8b81ce91eb 100644 --- a/packages/twenty-front/src/modules/navigation-menu-item/edit/hooks/useNavigationMenuObjectMetadataFromDraft.ts +++ b/packages/twenty-front/src/modules/navigation-menu-item/edit/hooks/useNavigationMenuObjectMetadataFromDraft.ts @@ -4,22 +4,20 @@ import { getObjectMetadataIdsInDraft } from '@/navigation-menu-item/common/utils import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue'; import { viewsSelector } from '@/views/states/selectors/viewsSelector'; import { ViewKey } from '@/views/types/ViewKey'; +import { type NavigationMenuItem } from '~/generated-metadata/graphql'; -type NavigationMenuItemDraft = { - id?: string; - viewId?: string | null; - targetObjectMetadataId?: string | null; -}; +type NavigationMenuItemDraft = Pick< + NavigationMenuItem, + 'id' | 'type' | 'viewId' | 'targetObjectMetadataId' +>; export const useNavigationMenuObjectMetadataFromDraft = ( currentDraft: NavigationMenuItemDraft[], ) => { const views = useAtomStateValue(viewsSelector); - const objectMetadataIdsInWorkspace = getObjectMetadataIdsInDraft( - currentDraft, - views, - ); + const objectMetadataIdsInWorkspace = + getObjectMetadataIdsInDraft(currentDraft); const objectMetadataIdsWithIndexView = new Set( views