Fix: Sidebar filter produces duplicate results

src/components/sidebar/helpers/__tests__/get-filtered-nav-items.test.ts

@@ -0,0 +1,187 @@
+/**
+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: MPL-2.0
+ */
+import { describe, it, expect } from 'vitest'
+import { getFilteredNavItems } from '../get-filtered-nav-items'
+import {
+	FilteredNavItem,
+	NavItemWithMetaData,
+	SubmenuNavItemWithMetaData,
+	LinkNavItemWithMetaData,
+} from '../../types'
+
+type NavItemWithRoutes = Extract<NavItemWithMetaData | FilteredNavItem, { routes: unknown }>
+
+const isNavItemWithRoutes = (
+	item: NavItemWithMetaData | FilteredNavItem
+): item is NavItemWithRoutes => 'routes' in item
+
+const expectNavItemWithRoutes = (
+	item: NavItemWithMetaData | FilteredNavItem | undefined
+): NavItemWithRoutes => {
+	if (!item) {
+		throw new Error('Expected nav item with routes but received undefined')
+	}
+
+	if (isNavItemWithRoutes(item)) {
+		return item
+	}
+
+	throw new Error('Expected nav item with routes')
+}
+
+describe('getFilteredNavItems', () => {
+	it('returns all items when filter value is empty', () => {
+		const items: NavItemWithMetaData[] = [
+			{ title: 'Item 1', path: '/item-1' },
+			{ title: 'Item 2', path: '/item-2' },
+		] as LinkNavItemWithMetaData[]
+
+		const result = getFilteredNavItems(items, '')
+
+		expect(result).toEqual(items)
+	})
+
+	it('filters items by title (case insensitive)', () => {
+		const items: NavItemWithMetaData[] = [
+			{ title: 'Getting Started', path: '/getting-started' },
+			{ title: 'Advanced Topics', path: '/advanced' },
+		] as LinkNavItemWithMetaData[]
+
+		const result = getFilteredNavItems(items, 'getting')
+
+		expect(result).toHaveLength(1)
+		expect(result[0]).toMatchObject({ title: 'Getting Started', matchesFilter: true })
+	})
+
+	it('filters items by alias', () => {
+		const items: NavItemWithMetaData[] = [
+			{ title: 'Documentation', path: '/docs', alias: 'guide' },
+			{ title: 'API Reference', path: '/api' },
+		] as LinkNavItemWithMetaData[]
+
+		const result = getFilteredNavItems(items, 'guide')
+
+		expect(result).toHaveLength(1)
+		expect(result[0]).toMatchObject({ title: 'Documentation', matchesFilter: true })
+	})
+
+	it('does not duplicate items when both title and alias match filter', () => {
+		const items: NavItemWithMetaData[] = [
+			{
+				title: 'Enable logs',
+				path: 'deploy/manage/monitor/logs',
+				alias: 'service logs, system logs, audit logging',
+			},
+		] as LinkNavItemWithMetaData[]
+
+		const result = getFilteredNavItems(items, 'logs')
+
+		expect(result).toHaveLength(1)
+		expect(result[0]).toMatchObject({ title: 'Enable logs', matchesFilter: true })
+	})
+
+	it('skips items without title', () => {
+		const items: NavItemWithMetaData[] = [
+			{ heading: 'Section' },
+			{ title: 'Valid Item', path: '/valid' },
+		] as NavItemWithMetaData[]
+
+		const result = getFilteredNavItems(items, 'section')
+
+		expect(result).toHaveLength(0)
+	})
+
+	it('recursively filters submenu items', () => {
+		const items: NavItemWithMetaData[] = [
+			{
+				title: 'Parent',
+				routes: [
+					{ title: 'Child Match', path: '/child-match' },
+					{ title: 'Other Child', path: '/other' },
+				],
+			},
+		] as SubmenuNavItemWithMetaData[]
+
+		const result = getFilteredNavItems(items, 'child match')
+
+		expect(result).toHaveLength(1)
+		const parent = expectNavItemWithRoutes(result[0])
+		expect(parent).toMatchObject({
+			title: 'Parent',
+			hasChildrenMatchingFilter: true,
+		})
+		expect(parent.routes).toHaveLength(1)
+		const child = parent.routes[0]!
+		expect(child).toMatchObject({
+			title: 'Child Match',
+			matchesFilter: true,
+		})
+	})
+
+	it('includes parent and all children when parent matches filter', () => {
+		const items: NavItemWithMetaData[] = [
+			{
+				title: 'Parent Match',
+				routes: [
+					{ title: 'Child 1', path: '/child-1' },
+					{ title: 'Child 2', path: '/child-2' },
+				],
+			},
+		] as SubmenuNavItemWithMetaData[]
+
+		const result = getFilteredNavItems(items, 'parent')
+
+		expect(result).toHaveLength(1)
+		const parent = expectNavItemWithRoutes(result[0])
+		expect(parent).toMatchObject({
+			title: 'Parent Match',
+			matchesFilter: true,
+		})
+		expect(parent.routes).toHaveLength(2)
+	})
+
+	it('excludes parent when no children match filter', () => {
+		const items: NavItemWithMetaData[] = [
+			{
+				title: 'Parent',
+				routes: [
+					{ title: 'Child 1', path: '/child-1' },
+					{ title: 'Child 2', path: '/child-2' },
+				],
+			},
+		] as SubmenuNavItemWithMetaData[]
+
+		const result = getFilteredNavItems(items, 'nomatch')
+
+		expect(result).toHaveLength(0)
+	})
+
+	it('handles deeply nested submenu items', () => {
+		const items: NavItemWithMetaData[] = [
+			{
+				title: 'Level 1',
+				routes: [
+					{
+						title: 'Level 2',
+						routes: [
+							{ title: 'Deep Match', path: '/deep' },
+						],
+					},
+				],
+			},
+		] as SubmenuNavItemWithMetaData[]
+
+		const result = getFilteredNavItems(items, 'deep')
+
+		expect(result).toHaveLength(1)
+		const levelOne = expectNavItemWithRoutes(result[0])
+		const levelTwo = expectNavItemWithRoutes(levelOne.routes[0])
+		const deepMatch = levelTwo.routes[0]!
+		expect(deepMatch).toMatchObject({
+			title: 'Deep Match',
+			matchesFilter: true,
+		})
+	})
+})

src/components/sidebar/helpers/get-filtered-nav-items.ts

@@ -3,13 +3,7 @@
  * SPDX-License-Identifier: MPL-2.0
  */
 
-import {
-	EnrichedNavItem,
-	FilteredNavItem,
-	LinkNavItemWithMetaData,
-	NavItemWithMetaData,
-	SubmenuNavItemWithMetaData,
-} from '../types'
+import { FilteredNavItem, NavItemWithMetaData, SubmenuNavItemWithMetaData } from '../types'
 
 /**
  * This does not use Array.filter because we need to add metadata to each item
@@ -23,68 +17,56 @@ export const getFilteredNavItems = (
 		return items as NavItemWithMetaData[]
 	}
 
+	const normalizedFilterValue = filterValue.toLowerCase()
 	const filteredItems = []
+	const includesFilter = (value?: string) =>
+		typeof value === 'string' &&
+		value.toLowerCase().includes(normalizedFilterValue)
 
-	items.forEach((item: EnrichedNavItem) => {
-		const isSubmenuNavItem = item.hasOwnProperty('routes')
-		const isLinkNavItem =
-			item.hasOwnProperty('path') || item.hasOwnProperty('href')
-		const doesNotHaveTitle = !(isSubmenuNavItem || isLinkNavItem)
-		if (doesNotHaveTitle) {
-			return
+	for (const item of items) {
+		const isSubmenuNavItem = 'routes' in item
+		const isLinkNavItem = 'path' in item || 'href' in item
+		const doesNotHaveTitle = !('title' in item)
+		if (doesNotHaveTitle || !(isSubmenuNavItem || isLinkNavItem)) {
+			continue
 		}
 
-		const doesTitleMatchFilter = (
-			item as SubmenuNavItemWithMetaData | LinkNavItemWithMetaData
-		).title
-			?.toLowerCase()
-			.includes(filterValue.toLowerCase())
+		const doesTitleMatchFilter = includesFilter(item.title)
+		const doesAliasMatchFilter =
+			'alias' in item && includesFilter(item.alias)
 
-		// Check and filter alias
-		const hasAlias = item.hasOwnProperty('alias')
-		if (hasAlias) {
-			const doesAliasMatchFilter = (
-				item as SubmenuNavItemWithMetaData | LinkNavItemWithMetaData
-			).alias
-				?.toLowerCase()
-				.includes(filterValue.toLowerCase())
-
-			// Add to filtered items if filter value is in alias
-			if (doesAliasMatchFilter) {
-				filteredItems.push({ ...item, matchesFilter: true })
-			}
+		// Flag items where either title or alias matches the active filter value.
+		if (doesTitleMatchFilter || doesAliasMatchFilter) {
+			filteredItems.push({ ...item, matchesFilter: true })
+			continue
 		}
 
 		/**
-		 * If an item's title matches the filter, we want to include it and its
-		 * children in the filter results. `matchesFilter` is added to all items
-		 * with a title that matches, and is used in `SidebarNavSubmenu` to
-		 * determine if a submenu should be open when searching.
+		 * If an item's title or alias matches the filter, we include it in the
+		 * results and mark it with `matchesFilter`. This metadata is consumed by
+		 * `SidebarNavSubmenu` to control which submenus start open while searching.
 		 *
-		 * If an item's title doesn't match the filter, then we need to recursively
-		 * look at the children of a submenu to see if any of those have titles or
-		 * subemnus that match the filter.
-		 *
-		 * TODO: write test cases to document this functionality more clearly
+		 * When neither title nor alias matches we recurse into submenu children to
+		 * surface any descendants that do match and annotate the parent so it stays
+		 * open.
 		 */
-		if (doesTitleMatchFilter) {
-			filteredItems.push({ ...item, matchesFilter: true })
-		} else if (isSubmenuNavItem) {
+		if (isSubmenuNavItem) {
+			const submenuItem = item as SubmenuNavItemWithMetaData
 			const matchingChildren = getFilteredNavItems(
-				(item as SubmenuNavItemWithMetaData).routes,
+				submenuItem.routes,
 				filterValue
 			)
 			const hasChildrenMatchingFilter = matchingChildren.length > 0
 
 			if (hasChildrenMatchingFilter) {
 				filteredItems.push({
-					...item,
+					...submenuItem,
 					hasChildrenMatchingFilter,
 					routes: matchingChildren,
 				})
 			}
 		}
-	})
+	}
 
 	return filteredItems as FilteredNavItem[]
 }