From cb44b22e1576253c3b855a63692c59511ecc1a5c Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Fri, 27 Mar 2026 18:25:52 +0100 Subject: [PATCH] Fix INDEX view showing labelPlural instead of resolved view name in nav (#19049) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Navigation sidebar was displaying "Notes" instead of "All Notes" for INDEX views - `getNavigationMenuItemLabel` had a special case for INDEX views that returned `objectMetadataItem.labelPlural` instead of the already-resolved `view.name` - Since `viewsSelector` already resolves `{objectLabelPlural}` templates via `resolveViewNamePlaceholders`, the INDEX special case was redundant and incorrect — removed it from both `getNavigationMenuItemLabel` and `getViewNavigationMenuItemLabel` - Added unit tests covering all navigation menu item type branches image --- .../getNavigationMenuItemLabel.test.ts | 205 ++++++++++++++++++ .../getNavigationMenuItemComputedLink.ts | 79 ++----- .../utils/getNavigationMenuItemLabel.ts | 46 ++-- .../utils/getViewNavigationMenuItemLabel.ts | 9 - 4 files changed, 239 insertions(+), 100 deletions(-) create mode 100644 packages/twenty-front/src/modules/navigation-menu-item/display/utils/__tests__/getNavigationMenuItemLabel.test.ts diff --git a/packages/twenty-front/src/modules/navigation-menu-item/display/utils/__tests__/getNavigationMenuItemLabel.test.ts b/packages/twenty-front/src/modules/navigation-menu-item/display/utils/__tests__/getNavigationMenuItemLabel.test.ts new file mode 100644 index 0000000000..6376cfbc03 --- /dev/null +++ b/packages/twenty-front/src/modules/navigation-menu-item/display/utils/__tests__/getNavigationMenuItemLabel.test.ts @@ -0,0 +1,205 @@ +import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem'; +import { type View } from '@/views/types/View'; +import { + NavigationMenuItemType, + ViewKey, + type NavigationMenuItem, +} from '~/generated-metadata/graphql'; + +import { getNavigationMenuItemLabel } from '@/navigation-menu-item/display/utils/getNavigationMenuItemLabel'; + +type ObjectMetadata = Pick< + EnrichedObjectMetadataItem, + 'id' | 'labelPlural' | 'nameSingular' +>; +type ViewMetadata = Pick; + +const objectMetadataItems: ObjectMetadata[] = [ + { id: 'obj-1', labelPlural: 'Notes', nameSingular: 'note' }, + { id: 'obj-2', labelPlural: 'Companies', nameSingular: 'company' }, +]; + +const views: ViewMetadata[] = [ + { + id: 'view-index', + name: 'All Notes', + objectMetadataId: 'obj-1', + key: ViewKey.INDEX, + }, + { + id: 'view-custom', + name: 'My Custom View', + objectMetadataId: 'obj-1', + key: null, + }, +]; + +const baseItem: NavigationMenuItem = { + id: 'nav-1', + type: NavigationMenuItemType.OBJECT, + position: 0, + createdAt: '', + updatedAt: '', +}; + +describe('getNavigationMenuItemLabel', () => { + describe('when type is OBJECT', () => { + it('should return labelPlural for a matching object', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.OBJECT, + targetObjectMetadataId: 'obj-1', + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + 'Notes', + ); + }); + + it('should return empty string when object is not found', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.OBJECT, + targetObjectMetadataId: 'nonexistent', + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + '', + ); + }); + }); + + describe('when type is VIEW', () => { + it('should return the resolved view name for an INDEX view', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.VIEW, + viewId: 'view-index', + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + 'All Notes', + ); + }); + + it('should return the view name for a non-INDEX view', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.VIEW, + viewId: 'view-custom', + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + 'My Custom View', + ); + }); + + it('should return empty string when view is not found', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.VIEW, + viewId: 'nonexistent', + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + '', + ); + }); + }); + + describe('when type is LINK', () => { + it('should return the item name when present', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.LINK, + name: 'Documentation', + link: 'https://docs.example.com', + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + 'Documentation', + ); + }); + + it('should return the link URL when name is null', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.LINK, + name: null, + link: 'https://docs.example.com', + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + 'https://docs.example.com', + ); + }); + + it('should return "Link" when both name and link are empty', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.LINK, + name: null, + link: ' ', + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + 'Link', + ); + }); + }); + + describe('when type is RECORD', () => { + it('should return labelIdentifier from targetRecordIdentifier', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.RECORD, + targetRecordIdentifier: { + id: 'rec-1', + labelIdentifier: 'Acme Corp', + }, + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + 'Acme Corp', + ); + }); + + it('should return empty string when targetRecordIdentifier is missing', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.RECORD, + targetRecordIdentifier: null, + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + '', + ); + }); + }); + + describe('when type is FOLDER', () => { + it('should return the item name when present', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.FOLDER, + name: 'Sales', + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + 'Sales', + ); + }); + + it('should return "Folder" when name is null', () => { + const item = { + ...baseItem, + type: NavigationMenuItemType.FOLDER, + name: null, + }; + + expect(getNavigationMenuItemLabel(item, objectMetadataItems, views)).toBe( + 'Folder', + ); + }); + }); +}); diff --git a/packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemComputedLink.ts b/packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemComputedLink.ts index 7a9e5d9048..71170425c4 100644 --- a/packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemComputedLink.ts +++ b/packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemComputedLink.ts @@ -1,9 +1,10 @@ +import { getLinkNavigationMenuItemComputedLink } from '@/navigation-menu-item/display/link/utils/getLinkNavigationMenuItemComputedLink'; +import { getObjectNavigationMenuItemComputedLink } from '@/navigation-menu-item/display/object/utils/getObjectNavigationMenuItemComputedLink'; +import { getRecordNavigationMenuItemComputedLink } from '@/navigation-menu-item/display/record/utils/getRecordNavigationMenuItemComputedLink'; +import { getViewNavigationMenuItemComputedLink } from '@/navigation-menu-item/display/view/utils/getViewNavigationMenuItemComputedLink'; import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem'; -import { recordIdentifierToObjectRecordIdentifier } from '@/navigation-menu-item/common/utils/recordIdentifierToObjectRecordIdentifier'; import { type View } from '@/views/types/View'; -import { ViewKey } from '@/views/types/ViewKey'; -import { AppPath, NavigationMenuItemType } from 'twenty-shared/types'; -import { getAppPath, isDefined } from 'twenty-shared/utils'; +import { NavigationMenuItemType } from 'twenty-shared/types'; import { type NavigationMenuItem } from '~/generated-metadata/graphql'; export const getNavigationMenuItemComputedLink = ( @@ -12,64 +13,22 @@ export const getNavigationMenuItemComputedLink = ( views: Pick[], ): string => { switch (item.type) { - case NavigationMenuItemType.OBJECT: { - const objectMetadataItem = objectMetadataItems.find( - (meta) => meta.id === item.targetObjectMetadataId, + case NavigationMenuItemType.OBJECT: + return getObjectNavigationMenuItemComputedLink( + item, + objectMetadataItems, + views, ); - if (!isDefined(objectMetadataItem)) { - return ''; - } - const indexView = views.find( - (view) => - view.objectMetadataId === objectMetadataItem.id && - view.key === ViewKey.INDEX, + case NavigationMenuItemType.VIEW: + return getViewNavigationMenuItemComputedLink( + item, + objectMetadataItems, + views, ); - return getAppPath( - AppPath.RecordIndexPage, - { objectNamePlural: objectMetadataItem.namePlural }, - indexView ? { viewId: indexView.id } : {}, - ); - } - case NavigationMenuItemType.VIEW: { - const view = views.find((view) => view.id === item.viewId); - if (!isDefined(view)) { - return ''; - } - const objectMetadataItem = objectMetadataItems.find( - (meta) => meta.id === view.objectMetadataId, - ); - if (!isDefined(objectMetadataItem)) { - return ''; - } - return getAppPath( - AppPath.RecordIndexPage, - { objectNamePlural: objectMetadataItem.namePlural }, - { viewId: item.viewId! }, - ); - } - case NavigationMenuItemType.LINK: { - const linkUrl = (item.link ?? '').trim(); - if (linkUrl.startsWith('http://') || linkUrl.startsWith('https://')) { - return linkUrl; - } - return linkUrl ? `https://${linkUrl}` : ''; - } - case NavigationMenuItemType.RECORD: { - const objectMetadataItem = objectMetadataItems.find( - (meta) => meta.id === item.targetObjectMetadataId, - ); - if ( - !isDefined(objectMetadataItem) || - !isDefined(item.targetRecordIdentifier) - ) { - return ''; - } - const objectRecordIdentifier = recordIdentifierToObjectRecordIdentifier({ - recordIdentifier: item.targetRecordIdentifier, - objectMetadataItem, - }); - return objectRecordIdentifier.linkToShowPage ?? ''; - } + case NavigationMenuItemType.LINK: + return getLinkNavigationMenuItemComputedLink(item); + case NavigationMenuItemType.RECORD: + return getRecordNavigationMenuItemComputedLink(item, objectMetadataItems); default: return ''; } diff --git a/packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemLabel.ts b/packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemLabel.ts index 65bfdfe546..9b76658268 100644 --- a/packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemLabel.ts +++ b/packages/twenty-front/src/modules/navigation-menu-item/display/utils/getNavigationMenuItemLabel.ts @@ -1,8 +1,11 @@ +import { getFolderNavigationMenuItemLabel } from '@/navigation-menu-item/display/folder/utils/getFolderNavigationMenuItemLabel'; +import { getLinkNavigationMenuItemLabel } from '@/navigation-menu-item/display/link/utils/getLinkNavigationMenuItemLabel'; +import { getObjectNavigationMenuItemLabel } from '@/navigation-menu-item/display/object/utils/getObjectNavigationMenuItemLabel'; +import { getRecordNavigationMenuItemLabel } from '@/navigation-menu-item/display/record/utils/getRecordNavigationMenuItemLabel'; +import { getViewNavigationMenuItemLabel } from '@/navigation-menu-item/display/view/utils/getViewNavigationMenuItemLabel'; import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem'; import { type View } from '@/views/types/View'; -import { ViewKey } from '@/views/types/ViewKey'; import { NavigationMenuItemType } from 'twenty-shared/types'; -import { isDefined } from 'twenty-shared/utils'; import { type NavigationMenuItem } from '~/generated-metadata/graphql'; export const getNavigationMenuItemLabel = ( @@ -14,35 +17,16 @@ export const getNavigationMenuItemLabel = ( views: Pick[], ): string => { switch (item.type) { - case NavigationMenuItemType.OBJECT: { - const objectMetadataItem = objectMetadataItems.find( - (meta) => meta.id === item.targetObjectMetadataId, - ); - return objectMetadataItem?.labelPlural ?? ''; - } - case NavigationMenuItemType.VIEW: { - const view = views.find((view) => view.id === item.viewId); - if (!isDefined(view)) { - return ''; - } - if (view.key === ViewKey.INDEX) { - const objectMetadataItem = objectMetadataItems.find( - (meta) => meta.id === view.objectMetadataId, - ); - return objectMetadataItem?.labelPlural ?? view.name; - } - return view.name; - } - case NavigationMenuItemType.LINK: { - const linkUrl = (item.link ?? '').trim(); - return (item.name ?? linkUrl) || 'Link'; - } - case NavigationMenuItemType.RECORD: { - return item.targetRecordIdentifier?.labelIdentifier ?? ''; - } - case NavigationMenuItemType.FOLDER: { - return item.name ?? 'Folder'; - } + case NavigationMenuItemType.OBJECT: + return getObjectNavigationMenuItemLabel(item, objectMetadataItems); + case NavigationMenuItemType.VIEW: + return getViewNavigationMenuItemLabel(item, views); + case NavigationMenuItemType.LINK: + return getLinkNavigationMenuItemLabel(item); + case NavigationMenuItemType.RECORD: + return getRecordNavigationMenuItemLabel(item); + case NavigationMenuItemType.FOLDER: + return getFolderNavigationMenuItemLabel(item); default: return item.name ?? ''; } diff --git a/packages/twenty-front/src/modules/navigation-menu-item/display/view/utils/getViewNavigationMenuItemLabel.ts b/packages/twenty-front/src/modules/navigation-menu-item/display/view/utils/getViewNavigationMenuItemLabel.ts index 709887166d..5a07d7f16b 100644 --- a/packages/twenty-front/src/modules/navigation-menu-item/display/view/utils/getViewNavigationMenuItemLabel.ts +++ b/packages/twenty-front/src/modules/navigation-menu-item/display/view/utils/getViewNavigationMenuItemLabel.ts @@ -1,23 +1,14 @@ -import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem'; import { type View } from '@/views/types/View'; -import { ViewKey } from '@/views/types/ViewKey'; import { isDefined } from 'twenty-shared/utils'; import { type NavigationMenuItem } from '~/generated-metadata/graphql'; export const getViewNavigationMenuItemLabel = ( item: Pick, views: Pick[], - objectMetadataItems: Pick[], ): string => { const view = views.find((view) => view.id === item.viewId); if (!isDefined(view)) { return ''; } - if (view.key === ViewKey.INDEX) { - const objectMetadataItem = objectMetadataItems.find( - (meta) => meta.id === view.objectMetadataId, - ); - return objectMetadataItem?.labelPlural ?? view.name; - } return view.name; };