Skip to content

Commit 94d0938

Browse files
authored
Fix regression with edge case where navigator may display wrong data (#916)
This addresses a rare edge case where the Index JSON contains multiple top-level root nodes for distinct trees of navigation data. While that shouldn't normally happen in practice, we would like to prefer the tree with a containing module if available instead of always using the first tree provided in the array. Resolves: rdar://140370212
1 parent 73f7914 commit 94d0938

File tree

2 files changed

+103
-4
lines changed

2 files changed

+103
-4
lines changed

src/utils/navigatorData.js

+20-4
Original file line numberDiff line numberDiff line change
@@ -178,14 +178,29 @@ export function getSiblings(uid, childrenMap, children) {
178178
return getChildren(item.parent, childrenMap, children);
179179
}
180180

181+
function extractRootNode(data) {
182+
// most of the time, it is expected that `data` always has a single item
183+
// that represents the top-level root node of the navigation tree
184+
//
185+
// there may be rare, unexpected scenarios where multiple top-level root
186+
// nodes are provide for some reason—if that happens, we would prefer the
187+
// first one categorized as a "module"
188+
//
189+
// otherwise, the first provided node will be used
190+
return data.length === 1 ? data[0] : (data.find(node => (
191+
node.type === TopicTypes.module
192+
)) ?? data[0]);
193+
}
194+
181195
/**
182196
* Flatten data for each language variant
183197
* @return { languageVariant: NavigatorFlatItem[] }
184198
*/
185199
export function flattenNavigationIndex(indexData) {
186200
return Object.entries(indexData).reduce((acc, [language, data]) => {
201+
const topLevelNode = extractRootNode(data);
187202
acc[language] = flattenNestedData(
188-
data[0].children || [], null, 0, data[0].beta,
203+
topLevelNode.children || [], null, 0, topLevelNode.beta,
189204
);
190205
return acc;
191206
}, {});
@@ -196,10 +211,11 @@ export function flattenNavigationIndex(indexData) {
196211
*/
197212
export function extractTechnologyProps(indexData) {
198213
return Object.entries(indexData).reduce((acc, [language, data]) => {
214+
const topLevelNode = extractRootNode(data);
199215
acc[language] = {
200-
technology: data[0].title,
201-
technologyPath: data[0].path || data[0].url,
202-
isTechnologyBeta: data[0].beta,
216+
technology: topLevelNode.title,
217+
technologyPath: topLevelNode.path || topLevelNode.url,
218+
isTechnologyBeta: topLevelNode.beta,
203219
};
204220
return acc;
205221
}, {});

tests/unit/utils/navigatorData.spec.js

+83
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
import {
1212
convertChildrenArrayToObject,
13+
extractTechnologyProps,
14+
flattenNavigationIndex,
1315
flattenNestedData,
1416
getAllChildren,
1517
getChildren,
@@ -317,3 +319,84 @@ describe('index data', () => {
317319
expect(childNodes).toEqual([root0Child0, root0Child1]);
318320
});
319321
});
322+
323+
describe('when multiple top-level children are provided', () => {
324+
const a = {
325+
type: 'overview',
326+
title: 'a',
327+
path: '/tutorials/a',
328+
children: [
329+
{
330+
type: 'project',
331+
title: 'a1',
332+
path: '/tutorials/a/a1',
333+
},
334+
],
335+
};
336+
const b = {
337+
type: 'module',
338+
title: 'a',
339+
path: '/documentation/b',
340+
children: [
341+
{
342+
type: 'article',
343+
title: 'b1',
344+
path: '/documentation/b/b1',
345+
},
346+
],
347+
};
348+
const c = {
349+
type: 'other',
350+
title: 'c',
351+
path: '/documentation/c',
352+
children: [
353+
{
354+
type: 'article',
355+
title: 'c1',
356+
path: '/documentation/c/c1',
357+
},
358+
],
359+
};
360+
361+
describe('flattenNavigationIndex', () => {
362+
it('prefers modules', () => {
363+
// use first root node if only one is provided
364+
let flattenedIndex = flattenNavigationIndex({ swift: [a] });
365+
expect(flattenedIndex.swift.length).toBe(1);
366+
expect(flattenedIndex.swift[0].title).toBe(a.children[0].title);
367+
flattenedIndex = flattenNavigationIndex({ swift: [b] });
368+
expect(flattenedIndex.swift.length).toBe(1);
369+
expect(flattenedIndex.swift[0].title).toBe(b.children[0].title);
370+
371+
// prefer "module" root when multiple top-level nodes are provided
372+
flattenedIndex = flattenNavigationIndex({ swift: [a, b] });
373+
expect(flattenedIndex.swift.length).toBe(1);
374+
expect(flattenedIndex.swift[0].title).toBe(b.children[0].title);
375+
376+
// fallback to first root node when multiple top-level nodes are provided
377+
// and none of them is a "module"
378+
flattenedIndex = flattenNavigationIndex({ swift: [c, a] });
379+
expect(flattenedIndex.swift.length).toBe(1);
380+
expect(flattenedIndex.swift[0].title).toBe(c.children[0].title);
381+
});
382+
});
383+
384+
describe('extractTechnologyProps', () => {
385+
it('prefers modules', () => {
386+
// use first root node if only one is provided
387+
let props = extractTechnologyProps({ swift: [a] });
388+
expect(props.swift.technology).toBe(a.title);
389+
props = extractTechnologyProps({ swift: [b] });
390+
expect(props.swift.technology).toBe(b.title);
391+
392+
// prefer "module" root when multiple top-level nodes are provided
393+
props = extractTechnologyProps({ swift: [a, b] });
394+
expect(props.swift.technology).toBe(b.title);
395+
396+
// fallback to first root node when multiple top-level nodes are provided
397+
// and none of them is a "module"
398+
props = extractTechnologyProps({ swift: [c, a] });
399+
expect(props.swift.technology).toBe(c.title);
400+
});
401+
});
402+
});

0 commit comments

Comments
 (0)