Skip to content

Commit 85b45a8

Browse files
nikolasrieblenikolasrieble
authored
Nikolasrieble.sidebar.reproduce bug in test (#2914)
* test: Add unit tests for getFilteredNavItems * refactor: do not yield an item twice if title and alias fit
1 parent 5d5be2c commit 85b45a8

File tree

2 files changed

+216
-47
lines changed

2 files changed

+216
-47
lines changed

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

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
/**
2+
* Copyright (c) HashiCorp, Inc.
3+
* SPDX-License-Identifier: MPL-2.0
4+
*/
5+
import { describe, it, expect } from 'vitest'
6+
import { getFilteredNavItems } from '../get-filtered-nav-items'
7+
import {
8+
FilteredNavItem,
9+
NavItemWithMetaData,
10+
SubmenuNavItemWithMetaData,
11+
LinkNavItemWithMetaData,
12+
} from '../../types'
13+
14+
type NavItemWithRoutes = Extract<NavItemWithMetaData | FilteredNavItem, { routes: unknown }>
15+
16+
const isNavItemWithRoutes = (
17+
item: NavItemWithMetaData | FilteredNavItem
18+
): item is NavItemWithRoutes => 'routes' in item
19+
20+
const expectNavItemWithRoutes = (
21+
item: NavItemWithMetaData | FilteredNavItem | undefined
22+
): NavItemWithRoutes => {
23+
if (!item) {
24+
throw new Error('Expected nav item with routes but received undefined')
25+
}
26+
27+
if (isNavItemWithRoutes(item)) {
28+
return item
29+
}
30+
31+
throw new Error('Expected nav item with routes')
32+
}
33+
34+
describe('getFilteredNavItems', () => {
35+
it('returns all items when filter value is empty', () => {
36+
const items: NavItemWithMetaData[] = [
37+
{ title: 'Item 1', path: '/item-1' },
38+
{ title: 'Item 2', path: '/item-2' },
39+
] as LinkNavItemWithMetaData[]
40+
41+
const result = getFilteredNavItems(items, '')
42+
43+
expect(result).toEqual(items)
44+
})
45+
46+
it('filters items by title (case insensitive)', () => {
47+
const items: NavItemWithMetaData[] = [
48+
{ title: 'Getting Started', path: '/getting-started' },
49+
{ title: 'Advanced Topics', path: '/advanced' },
50+
] as LinkNavItemWithMetaData[]
51+
52+
const result = getFilteredNavItems(items, 'getting')
53+
54+
expect(result).toHaveLength(1)
55+
expect(result[0]).toMatchObject({ title: 'Getting Started', matchesFilter: true })
56+
})
57+
58+
it('filters items by alias', () => {
59+
const items: NavItemWithMetaData[] = [
60+
{ title: 'Documentation', path: '/docs', alias: 'guide' },
61+
{ title: 'API Reference', path: '/api' },
62+
] as LinkNavItemWithMetaData[]
63+
64+
const result = getFilteredNavItems(items, 'guide')
65+
66+
expect(result).toHaveLength(1)
67+
expect(result[0]).toMatchObject({ title: 'Documentation', matchesFilter: true })
68+
})
69+
70+
it('does not duplicate items when both title and alias match filter', () => {
71+
const items: NavItemWithMetaData[] = [
72+
{
73+
title: 'Enable logs',
74+
path: 'deploy/manage/monitor/logs',
75+
alias: 'service logs, system logs, audit logging',
76+
},
77+
] as LinkNavItemWithMetaData[]
78+
79+
const result = getFilteredNavItems(items, 'logs')
80+
81+
expect(result).toHaveLength(1)
82+
expect(result[0]).toMatchObject({ title: 'Enable logs', matchesFilter: true })
83+
})
84+
85+
it('skips items without title', () => {
86+
const items: NavItemWithMetaData[] = [
87+
{ heading: 'Section' },
88+
{ title: 'Valid Item', path: '/valid' },
89+
] as NavItemWithMetaData[]
90+
91+
const result = getFilteredNavItems(items, 'section')
92+
93+
expect(result).toHaveLength(0)
94+
})
95+
96+
it('recursively filters submenu items', () => {
97+
const items: NavItemWithMetaData[] = [
98+
{
99+
title: 'Parent',
100+
routes: [
101+
{ title: 'Child Match', path: '/child-match' },
102+
{ title: 'Other Child', path: '/other' },
103+
],
104+
},
105+
] as SubmenuNavItemWithMetaData[]
106+
107+
const result = getFilteredNavItems(items, 'child match')
108+
109+
expect(result).toHaveLength(1)
110+
const parent = expectNavItemWithRoutes(result[0])
111+
expect(parent).toMatchObject({
112+
title: 'Parent',
113+
hasChildrenMatchingFilter: true,
114+
})
115+
expect(parent.routes).toHaveLength(1)
116+
const child = parent.routes[0]!
117+
expect(child).toMatchObject({
118+
title: 'Child Match',
119+
matchesFilter: true,
120+
})
121+
})
122+
123+
it('includes parent and all children when parent matches filter', () => {
124+
const items: NavItemWithMetaData[] = [
125+
{
126+
title: 'Parent Match',
127+
routes: [
128+
{ title: 'Child 1', path: '/child-1' },
129+
{ title: 'Child 2', path: '/child-2' },
130+
],
131+
},
132+
] as SubmenuNavItemWithMetaData[]
133+
134+
const result = getFilteredNavItems(items, 'parent')
135+
136+
expect(result).toHaveLength(1)
137+
const parent = expectNavItemWithRoutes(result[0])
138+
expect(parent).toMatchObject({
139+
title: 'Parent Match',
140+
matchesFilter: true,
141+
})
142+
expect(parent.routes).toHaveLength(2)
143+
})
144+
145+
it('excludes parent when no children match filter', () => {
146+
const items: NavItemWithMetaData[] = [
147+
{
148+
title: 'Parent',
149+
routes: [
150+
{ title: 'Child 1', path: '/child-1' },
151+
{ title: 'Child 2', path: '/child-2' },
152+
],
153+
},
154+
] as SubmenuNavItemWithMetaData[]
155+
156+
const result = getFilteredNavItems(items, 'nomatch')
157+
158+
expect(result).toHaveLength(0)
159+
})
160+
161+
it('handles deeply nested submenu items', () => {
162+
const items: NavItemWithMetaData[] = [
163+
{
164+
title: 'Level 1',
165+
routes: [
166+
{
167+
title: 'Level 2',
168+
routes: [
169+
{ title: 'Deep Match', path: '/deep' },
170+
],
171+
},
172+
],
173+
},
174+
] as SubmenuNavItemWithMetaData[]
175+
176+
const result = getFilteredNavItems(items, 'deep')
177+
178+
expect(result).toHaveLength(1)
179+
const levelOne = expectNavItemWithRoutes(result[0])
180+
const levelTwo = expectNavItemWithRoutes(levelOne.routes[0])
181+
const deepMatch = levelTwo.routes[0]!
182+
expect(deepMatch).toMatchObject({
183+
title: 'Deep Match',
184+
matchesFilter: true,
185+
})
186+
})
187+
})

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

Lines changed: 29 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,7 @@
33
* SPDX-License-Identifier: MPL-2.0
44
*/
55

6-
import {
7-
EnrichedNavItem,
8-
FilteredNavItem,
9-
LinkNavItemWithMetaData,
10-
NavItemWithMetaData,
11-
SubmenuNavItemWithMetaData,
12-
} from '../types'
6+
import { FilteredNavItem, NavItemWithMetaData, SubmenuNavItemWithMetaData } from '../types'
137

148
/**
159
* This does not use Array.filter because we need to add metadata to each item
@@ -23,68 +17,56 @@ export const getFilteredNavItems = (
2317
return items as NavItemWithMetaData[]
2418
}
2519

20+
const normalizedFilterValue = filterValue.toLowerCase()
2621
const filteredItems = []
22+
const includesFilter = (value?: string) =>
23+
typeof value === 'string' &&
24+
value.toLowerCase().includes(normalizedFilterValue)
2725

28-
items.forEach((item: EnrichedNavItem) => {
29-
const isSubmenuNavItem = item.hasOwnProperty('routes')
30-
const isLinkNavItem =
31-
item.hasOwnProperty('path') || item.hasOwnProperty('href')
32-
const doesNotHaveTitle = !(isSubmenuNavItem || isLinkNavItem)
33-
if (doesNotHaveTitle) {
34-
return
26+
for (const item of items) {
27+
const isSubmenuNavItem = 'routes' in item
28+
const isLinkNavItem = 'path' in item || 'href' in item
29+
const doesNotHaveTitle = !('title' in item)
30+
if (doesNotHaveTitle || !(isSubmenuNavItem || isLinkNavItem)) {
31+
continue
3532
}
3633

37-
const doesTitleMatchFilter = (
38-
item as SubmenuNavItemWithMetaData | LinkNavItemWithMetaData
39-
).title
40-
?.toLowerCase()
41-
.includes(filterValue.toLowerCase())
34+
const doesTitleMatchFilter = includesFilter(item.title)
35+
const doesAliasMatchFilter =
36+
'alias' in item && includesFilter(item.alias)
4237

43-
// Check and filter alias
44-
const hasAlias = item.hasOwnProperty('alias')
45-
if (hasAlias) {
46-
const doesAliasMatchFilter = (
47-
item as SubmenuNavItemWithMetaData | LinkNavItemWithMetaData
48-
).alias
49-
?.toLowerCase()
50-
.includes(filterValue.toLowerCase())
51-
52-
// Add to filtered items if filter value is in alias
53-
if (doesAliasMatchFilter) {
54-
filteredItems.push({ ...item, matchesFilter: true })
55-
}
38+
// Flag items where either title or alias matches the active filter value.
39+
if (doesTitleMatchFilter || doesAliasMatchFilter) {
40+
filteredItems.push({ ...item, matchesFilter: true })
41+
continue
5642
}
5743

5844
/**
59-
* If an item's title matches the filter, we want to include it and its
60-
* children in the filter results. `matchesFilter` is added to all items
61-
* with a title that matches, and is used in `SidebarNavSubmenu` to
62-
* determine if a submenu should be open when searching.
45+
* If an item's title or alias matches the filter, we include it in the
46+
* results and mark it with `matchesFilter`. This metadata is consumed by
47+
* `SidebarNavSubmenu` to control which submenus start open while searching.
6348
*
64-
* If an item's title doesn't match the filter, then we need to recursively
65-
* look at the children of a submenu to see if any of those have titles or
66-
* subemnus that match the filter.
67-
*
68-
* TODO: write test cases to document this functionality more clearly
49+
* When neither title nor alias matches we recurse into submenu children to
50+
* surface any descendants that do match and annotate the parent so it stays
51+
* open.
6952
*/
70-
if (doesTitleMatchFilter) {
71-
filteredItems.push({ ...item, matchesFilter: true })
72-
} else if (isSubmenuNavItem) {
53+
if (isSubmenuNavItem) {
54+
const submenuItem = item as SubmenuNavItemWithMetaData
7355
const matchingChildren = getFilteredNavItems(
74-
(item as SubmenuNavItemWithMetaData).routes,
56+
submenuItem.routes,
7557
filterValue
7658
)
7759
const hasChildrenMatchingFilter = matchingChildren.length > 0
7860

7961
if (hasChildrenMatchingFilter) {
8062
filteredItems.push({
81-
...item,
63+
...submenuItem,
8264
hasChildrenMatchingFilter,
8365
routes: matchingChildren,
8466
})
8567
}
8668
}
87-
})
69+
}
8870

8971
return filteredItems as FilteredNavItem[]
9072
}

0 commit comments

Comments
 (0)