From 6397cc613a056b908ea24ee26efb2e5668a2ebf7 Mon Sep 17 00:00:00 2001 From: Andrew Jiang Date: Fri, 4 Oct 2024 15:06:34 -0400 Subject: [PATCH] add more unit tests --- .../__test__/pruneNavigationTree.test.ts | 211 ++++++++++++++++++ .../src/navigation/utils/deleteChild.ts | 75 +++++-- .../versions/latest/NavigationNodePage.ts | 13 +- .../traversers/__test__/prunetree.test.ts | 67 +++--- .../fdr-sdk/src/utils/traversers/prunetree.ts | 33 ++- .../fdr-sdk/src/utils/traversers/types.ts | 6 +- 6 files changed, 322 insertions(+), 83 deletions(-) diff --git a/packages/fdr-sdk/src/navigation/utils/__test__/pruneNavigationTree.test.ts b/packages/fdr-sdk/src/navigation/utils/__test__/pruneNavigationTree.test.ts index 0dfd3494ac..092d2f69e5 100644 --- a/packages/fdr-sdk/src/navigation/utils/__test__/pruneNavigationTree.test.ts +++ b/packages/fdr-sdk/src/navigation/utils/__test__/pruneNavigationTree.test.ts @@ -95,4 +95,215 @@ describe("pruneNavigationTree", () => { expect(result).toBeUndefined(); }); + + it("should not prune section children even if section itself is pruned", () => { + const root: FernNavigation.NavigationNode = { + type: "section", + id: FernNavigation.NodeId("root"), + slug: FernNavigation.Slug("root"), + title: "Root", + overviewPageId: FernNavigation.PageId("overview.mdx"), // this is a visitable page + children: [ + { + type: "page", + id: FernNavigation.NodeId("page"), + slug: FernNavigation.Slug("root/page"), + title: "Page", + pageId: FernNavigation.PageId("page.mdx"), + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + }, + ], + collapsed: undefined, + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + pointsTo: undefined, + }; + + const result = pruneNavigationTree(root, (node) => node.id !== "root"); + + // structuredClone should duplicate the object + expect(result === root).toBe(false); + + expect(result).toStrictEqual({ + type: "section", + id: FernNavigation.NodeId("root"), + slug: FernNavigation.Slug("root"), + overviewPageId: undefined, // this should be deleted + title: "Root", + children: [ + { + type: "page", + id: FernNavigation.NodeId("page"), + slug: FernNavigation.Slug("root/page"), + title: "Page", + pageId: FernNavigation.PageId("page.mdx"), + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + }, + ], + collapsed: undefined, + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + pointsTo: undefined, + }); + }); + + it("should not prune non-leaf nodes", () => { + const root: FernNavigation.NavigationNode = { + type: "section", + id: FernNavigation.NodeId("root"), + slug: FernNavigation.Slug("root"), + title: "Root", + overviewPageId: undefined, + children: [ + { + type: "page", + id: FernNavigation.NodeId("page"), + slug: FernNavigation.Slug("root/page"), + title: "Page", + pageId: FernNavigation.PageId("page.mdx"), + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + }, + ], + collapsed: undefined, + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + pointsTo: undefined, + }; + + const result = pruneNavigationTree(root, (node) => node.id !== "root"); + + // structuredClone should duplicate the object + expect(result === root).toBe(false); + + expect(result).toStrictEqual({ + type: "section", + id: FernNavigation.NodeId("root"), + slug: FernNavigation.Slug("root"), + overviewPageId: undefined, // this should be deleted + title: "Root", + children: [ + { + type: "page", + id: FernNavigation.NodeId("page"), + slug: FernNavigation.Slug("root/page"), + title: "Page", + pageId: FernNavigation.PageId("page.mdx"), + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + }, + ], + collapsed: undefined, + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + pointsTo: undefined, + }); + }); + + it("should delete leaf node and its parent if no siblings left", () => { + const root: FernNavigation.NavigationNode = { + type: "section", + id: FernNavigation.NodeId("root"), + slug: FernNavigation.Slug("root"), + title: "Root", + overviewPageId: undefined, + children: [ + { + type: "section", + id: FernNavigation.NodeId("section2"), + slug: FernNavigation.Slug("root/section2"), + title: "Section 2", + overviewPageId: undefined, + children: [ + { + type: "page", + id: FernNavigation.NodeId("page1"), + slug: FernNavigation.Slug("root/section2/page"), + title: "Page", + pageId: FernNavigation.PageId("page.mdx"), + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + }, + ], + collapsed: undefined, + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + pointsTo: undefined, + }, + { + type: "page", + id: FernNavigation.NodeId("page2"), + slug: FernNavigation.Slug("root/page"), + title: "Page", + pageId: FernNavigation.PageId("page.mdx"), + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + }, + ], + collapsed: undefined, + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + pointsTo: undefined, + }; + + const result = pruneNavigationTree(root, (node) => node.id !== "page1"); + + // structuredClone should duplicate the object + expect(result === root).toBe(false); + + expect(result).toStrictEqual({ + type: "section", + id: FernNavigation.NodeId("root"), + slug: FernNavigation.Slug("root"), + overviewPageId: undefined, // this should be deleted + title: "Root", + children: [ + { + type: "page", + id: FernNavigation.NodeId("page2"), + slug: FernNavigation.Slug("root/page"), + title: "Page", + pageId: FernNavigation.PageId("page.mdx"), + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + }, + ], + collapsed: undefined, + canonicalSlug: undefined, + icon: undefined, + hidden: undefined, + noindex: undefined, + + // NOTE: points to is updated! + pointsTo: "root/page", + }); + }); }); diff --git a/packages/fdr-sdk/src/navigation/utils/deleteChild.ts b/packages/fdr-sdk/src/navigation/utils/deleteChild.ts index bd04391243..6aebaa9f42 100644 --- a/packages/fdr-sdk/src/navigation/utils/deleteChild.ts +++ b/packages/fdr-sdk/src/navigation/utils/deleteChild.ts @@ -1,5 +1,6 @@ -import { UnreachableCaseError } from "ts-essentials"; +import { MarkOptional, UnreachableCaseError } from "ts-essentials"; import { FernNavigation } from "../.."; +import { DeleterAction } from "../../utils/traversers/types"; /** * @param parent delete node from this parent (mutable) @@ -7,65 +8,95 @@ import { FernNavigation } from "../.."; * @returns the id of the deleted node or null if the node was not deletable from the parent */ export function mutableDeleteChild( - parent: FernNavigation.NavigationNodeParent, + parent: FernNavigation.NavigationNodeParent | undefined, node: FernNavigation.NavigationNode, -): FernNavigation.NodeId | null { +): DeleterAction { + /** + * The idea here is we should only delete leaf nodes (we're treating changelogs here like a leaf node) + * + * In the case that we have sections that have content, deleting it from its parent would delete all its children as well. + * Instead, we'll just remove the overviewPageId, which will make the section a non-visitable node, yet still retain its children. + */ + if ( + !FernNavigation.isLeaf(node) && + FernNavigation.isPage(node) && + FernNavigation.getChildren(node).length > 0 && + node.type !== "changelog" + ) { + // if the node to be deleted is a section, remove the overviewPageId + if (FernNavigation.isSectionOverview(node)) { + (node as MarkOptional).overviewPageId = undefined; + return "noop"; + } else { + throw new UnreachableCaseError(node); + } + } + + // if the node is not a leaf node, don't delete it from the parent unless it has no children + if (!FernNavigation.isLeaf(node) && FernNavigation.getChildren(node).length > 0) { + return "noop"; + } + + if (parent == null) { + return "deleted"; + } + switch (parent.type) { case "apiPackage": parent.children = parent.children.filter((child) => child.id !== node.id); - return node.id; + return "deleted"; case "apiReference": parent.children = parent.children.filter((child) => child.id !== node.id); parent.changelog = parent.changelog?.id === node.id ? undefined : parent.changelog; - return node.id; + return "deleted"; case "changelog": parent.children = parent.children.filter((child) => child.id !== node.id); - return node.id; + return "deleted"; case "changelogYear": parent.children = parent.children.filter((child) => child.id !== node.id); - return node.id; + return "deleted"; case "changelogMonth": parent.children = parent.children.filter((child) => child.id !== node.id); - return node.id; + return "deleted"; case "endpointPair": - return null; + return "should-delete-parent"; case "productgroup": parent.children = parent.children.filter((child) => child.id !== node.id); parent.landingPage = parent.landingPage?.id === node.id ? undefined : parent.landingPage; - return node.id; + return "deleted"; case "product": - return null; + return "should-delete-parent"; case "root": - return null; + return "should-delete-parent"; case "unversioned": if (node.id === parent.landingPage?.id) { parent.landingPage = undefined; - return node.id; + return "deleted"; } - return null; + return "should-delete-parent"; case "section": parent.children = parent.children.filter((child) => child.id !== node.id); - return node.id; + return "deleted"; case "sidebarGroup": parent.children = parent.children.filter((child) => child.id !== node.id); - return node.id; + return "deleted"; case "tab": - return null; + return "should-delete-parent"; case "sidebarRoot": parent.children = parent.children.filter((child) => child.id !== node.id); - return node.id; + return "deleted"; case "tabbed": parent.children = parent.children.filter((child) => child.id !== node.id); - return node.id; + return "deleted"; case "version": if (node.id === parent.landingPage?.id) { parent.landingPage = undefined; - return node.id; + return "deleted"; } - return null; + return "should-delete-parent"; case "versioned": parent.children = parent.children.filter((child) => child.id !== node.id); - return node.id; + return "deleted"; default: throw new UnreachableCaseError(parent); } diff --git a/packages/fdr-sdk/src/navigation/versions/latest/NavigationNodePage.ts b/packages/fdr-sdk/src/navigation/versions/latest/NavigationNodePage.ts index 159b90ee8a..c16871db34 100644 --- a/packages/fdr-sdk/src/navigation/versions/latest/NavigationNodePage.ts +++ b/packages/fdr-sdk/src/navigation/versions/latest/NavigationNodePage.ts @@ -1,4 +1,4 @@ -import type { ChangelogMonthNode, ChangelogNode, ChangelogYearNode } from "."; +import type { ChangelogNode } from "."; import type { NavigationNode } from "./NavigationNode"; import { isApiLeaf, type NavigationNodeApiLeaf } from "./NavigationNodeApiLeaf"; import { hasMarkdown, type NavigationNodeWithMarkdown } from "./NavigationNodeMarkdown"; @@ -6,14 +6,11 @@ import { hasMarkdown, type NavigationNodeWithMarkdown } from "./NavigationNodeMa /** * A navigation node that represents a visitable page in the documentation */ -export type NavigationNodePage = - | NavigationNodeWithMarkdown - | NavigationNodeApiLeaf - | ChangelogNode - | ChangelogYearNode - | ChangelogMonthNode; +export type NavigationNodePage = NavigationNodeWithMarkdown | NavigationNodeApiLeaf | ChangelogNode; +// | ChangelogYearNode +// | ChangelogMonthNode; -export function isPage(node: NavigationNode): node is NavigationNodePage { +export function isPage(node: N): node is N & NavigationNodePage { return ( isApiLeaf(node) || node.type === "changelog" || diff --git a/packages/fdr-sdk/src/utils/traversers/__test__/prunetree.test.ts b/packages/fdr-sdk/src/utils/traversers/__test__/prunetree.test.ts index 03604c4454..fbf5f020ca 100644 --- a/packages/fdr-sdk/src/utils/traversers/__test__/prunetree.test.ts +++ b/packages/fdr-sdk/src/utils/traversers/__test__/prunetree.test.ts @@ -1,48 +1,43 @@ import { prunetree } from "../prunetree"; +import { DeleterAction } from "../types"; import { FIXTURE, Record } from "./fixture"; +const DELETER = (parent: Record | undefined, child: Record): DeleterAction => { + if (parent == null) { + return "deleted"; + } + parent.children = parent.children.filter((c) => c.id !== child.id); + return "deleted"; +}; + +const testPruner = (predicate: (node: Record) => boolean): Record | undefined => { + const [pruned] = prunetree(structuredClone(FIXTURE), { + predicate, + getChildren: (node) => node.children, + deleter: DELETER, + getPointer: (node) => node.id, + }); + return pruned; +}; + describe("prunetree", () => { it("should return the same tree if the predicate returns true for all nodes", () => { - const [pruned] = prunetree(structuredClone(FIXTURE), { - predicate: () => { - return true; - }, - getChildren: (node) => node.children, - deleter: (parent, child) => { - parent.children = parent.children.filter((c) => c.id !== child.id); - return child.id; - }, - getPointer: (node) => node.id, + const pruned = testPruner(() => { + return true; }); expect(pruned).toStrictEqual(FIXTURE); }); it("should return undefined if the predicate returns false for all nodes", () => { - const [pruned] = prunetree(structuredClone(FIXTURE), { - predicate: () => { - return false; - }, - getChildren: (node) => node.children, - deleter: (parent, child) => { - parent.children = parent.children.filter((c) => c.id !== child.id); - return child.id; - }, - getPointer: (node) => node.id, + const pruned = testPruner(() => { + return false; }); expect(pruned).toBeUndefined(); }); it("should prune the tree if the predicate returns false for some nodes", () => { - const [pruned] = prunetree(structuredClone(FIXTURE), { - predicate: (node) => { - return node.id !== 1; - }, - getChildren: (node) => node.children, - deleter: (parent, child) => { - parent.children = parent.children.filter((c) => c.id !== child.id); - return child.id; - }, - getPointer: (node) => node.id, + const pruned = testPruner((node) => { + return node.id !== 1; }); expect(pruned).toStrictEqual({ id: 0, @@ -61,16 +56,8 @@ describe("prunetree", () => { }); it("should prune parents that don't have children, but not leaf nodes", () => { - const [pruned] = prunetree(structuredClone(FIXTURE), { - predicate: (node) => { - return node.id !== 7 && node.id !== 3; - }, - getChildren: (node) => node.children, - deleter: (parent, child) => { - parent.children = parent.children.filter((c) => c.id !== child.id); - return child.id; - }, - getPointer: (node) => node.id, + const pruned = testPruner((node) => { + return node.id !== 7 && node.id !== 3; }); expect(pruned).toStrictEqual({ id: 0, diff --git a/packages/fdr-sdk/src/utils/traversers/prunetree.ts b/packages/fdr-sdk/src/utils/traversers/prunetree.ts index 73b2937649..815e021ff2 100644 --- a/packages/fdr-sdk/src/utils/traversers/prunetree.ts +++ b/packages/fdr-sdk/src/utils/traversers/prunetree.ts @@ -1,4 +1,6 @@ +import { UnreachableCaseError } from "ts-essentials"; import { bfs } from "./bfs"; +import { DeleterAction } from "./types"; interface PruneTreeOptions { /** @@ -13,7 +15,7 @@ interface PruneTreeOptions { * @param child the child that should be deleted * @returns the pointer to the child node, or **null** if the child cannot be deleted */ - deleter: (parent: PARENT, child: NODE) => POINTER | null; + deleter: (parent: PARENT | undefined, child: NODE) => DeleterAction; /** * After the child is deleted, we can check if the parent should be deleted too, @@ -84,7 +86,7 @@ export function prunetree { - deleter: (parent: PARENT, child: NODE) => POINTER | null; + deleter: (parent: PARENT | undefined, child: NODE) => DeleterAction; shouldDeleteParent: (parent: PARENT) => boolean; getPointer: (node: NODE) => POINTER; } @@ -99,23 +101,32 @@ function deleteChildAndMaybeParent = (node: N, parents: readonly P[]) => Next; +export type TraverserVisit = (node: N, parents: readonly P[]) => Action; export type TraverserGetChildren = (parent: P) => readonly N[]; + +export type DeleterAction = "deleted" | "should-delete-parent" | "noop";