Respect data app nav items (#25416)

* Allow overwriting `SidebarLink` name styles

* Add `DataAppPageSidebarLink` component

* Render data app sidebar page tree

* Hide pages marked as hidden

* Add `getParentDataAppPageId` helper

* Highlight parent page when hidden page is selected

* Extract `getSelectedItems` and add tests

* Fix messed up ordering bug

frontend/src/metabase/entities/data-apps/utils.ts

 import _ from "underscore";
-import type { Collection, DataApp, Dashboard } from "metabase-types/api";
+import type {
+  Collection,
+  DataApp,
+  DataAppNavItem,
+  DataAppPageId,
+  Dashboard,
+} from "metabase-types/api";
 
 export function getDataAppIcon(app?: DataApp) {
   return { name: "star" };
@@ -16,3 +22,45 @@ export function getDataAppHomePageId(dataApp: DataApp, pages: Dashboard[]) {
   const [firstPage] = _.sortBy(pages, "name");
   return firstPage?.id;
 }
+
+function isParentPage(
+  targetPageIndent: number,
+  maybeParentNavItem: DataAppNavItem,
+) {
+  if (maybeParentNavItem.hidden) {
+    return false;
+  }
+
+  // For `indent: 1` the expected parent indent is 0.
+  // But it can be coming as `undefined` from the API,
+  // so we need to ensure we accept both `undefined` and 0.
+  if (targetPageIndent === 1) {
+    return !maybeParentNavItem.indent || maybeParentNavItem.indent === 0;
+  }
+
+  return maybeParentNavItem.indent === targetPageIndent - 1;
+}
+
+export function getParentDataAppPageId(
+  pageId: DataAppPageId,
+  navItems: DataAppNavItem[],
+): DataAppPageId | null {
+  const pageIndex = navItems.findIndex(navItem => navItem.page_id === pageId);
+  if (pageIndex === -1) {
+    return null;
+  }
+
+  const { indent } = navItems[pageIndex];
+
+  // Top-level page
+  if (typeof indent !== "number" || indent === 0) {
+    return null;
+  }
+
+  const pagesBeforeTarget = navItems.slice(0, pageIndex);
+  const parentPageIndex = _.findLastIndex(pagesBeforeTarget, navItem =>
+    isParentPage(indent, navItem),
+  );
+
+  return pagesBeforeTarget[parentPageIndex]?.page_id || null;
+}

frontend/src/metabase/entities/data-apps/utils.unit.spec.ts

+import type { DataAppNavItem } from "metabase-types/api";
 import {
   createMockDataApp,
   createMockDataAppPage,
 } from "metabase-types/api/mocks";
-import { getDataAppHomePageId } from "./utils";
+import { getDataAppHomePageId, getParentDataAppPageId } from "./utils";
 
 describe("data app utils", () => {
   const dataAppWithoutHomepage = createMockDataApp({ dashboard_id: null });
@@ -42,4 +43,52 @@ describe("data app utils", () => {
       });
     });
   });
+
+  describe("getParentDataAppPageId", () => {
+    const navItems: DataAppNavItem[] = [
+      { page_id: 1 },
+      { page_id: 2, indent: 1 },
+      { page_id: 3, indent: 0 },
+      { page_id: 4, indent: 1 },
+      { page_id: 5, indent: 1, hidden: true },
+      { page_id: 6, indent: 2 },
+    ];
+
+    it("returns null if can't find page", () => {
+      expect(
+        getParentDataAppPageId(2, [
+          { page_id: 1, indent: 1 },
+          { page_id: 2, indent: 1 },
+        ]),
+      ).toBeNull();
+    });
+
+    it("returns null if page list is empty", () => {
+      expect(getParentDataAppPageId(2, [])).toBeNull();
+    });
+
+    it("returns null for top-level page with undefined indent", () => {
+      expect(getParentDataAppPageId(1, navItems)).toBeNull();
+    });
+
+    it("returns null for top-level page with 0 indent", () => {
+      expect(getParentDataAppPageId(3, navItems)).toBeNull();
+    });
+
+    it("returns ID of the closest top-level page when indent is 1", () => {
+      expect(getParentDataAppPageId(2, navItems)).toBe(1);
+    });
+
+    it("returns ID of the closest parent page by indent for visible pages", () => {
+      expect(getParentDataAppPageId(4, navItems)).toBe(3);
+    });
+
+    it("returns ID of the closest parent page by indent for hidden pages", () => {
+      expect(getParentDataAppPageId(5, navItems)).toBe(3);
+    });
+
+    it("skips hidden pages when looking for a parent", () => {
+      expect(getParentDataAppPageId(6, navItems)).toBe(4);
+    });
+  });
 });

frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbar/DataAppNavbarContainer.tsx

@@ -6,7 +6,7 @@ import Modal from "metabase/components/Modal";
 
 import * as Urls from "metabase/lib/urls";
 
-import DataApps, { getDataAppHomePageId } from "metabase/entities/data-apps";
+import DataApps from "metabase/entities/data-apps";
 import Dashboards from "metabase/entities/dashboards";
 import Search from "metabase/entities/search";
 
@@ -18,16 +18,12 @@ import type { State } from "metabase-types/store";
 import { MainNavbarProps, MainNavbarOwnProps, SelectedItem } from "../types";
 import NavbarLoadingView from "../NavbarLoadingView";
 
+import getSelectedItems from "./getSelectedItems";
 import DataAppNavbarView from "./DataAppNavbarView";
 
 const FETCHING_SEARCH_MODELS = ["page"];
 const LIMIT = 100;
 
-function isAtDataAppHomePage(selectedItems: SelectedItem[]) {
-  const [selectedItem] = selectedItems;
-  return selectedItems.length === 1 && selectedItem.type === "data-app";
-}
-
 type NavbarModal =
   | "MODAL_ADD_DATA"
   | "MODAL_APP_SETTINGS"
@@ -61,33 +57,20 @@ function DataAppNavbarContainer({
 }: DataAppNavbarContainerProps & { onReloadNavbar: () => Promise<void> }) {
   const [modal, setModal] = useState<NavbarModal>(null);
 
-  const finalSelectedItems: SelectedItem[] = useMemo(() => {
-    const isHomepage = isAtDataAppHomePage(selectedItems);
-
-    // Once a data app is launched, the first view is going to be the app homepage
-    // Homepage is an app page specified by a user or picked automatically (just the first one)
-    // The homepage doesn't have a regular page path like /a/1/page/1, but an app one like /a/1
-    // So we need to overwrite the selectedItems list here and specify the homepage
-    if (isHomepage) {
-      return [
-        {
-          type: "data-app-page",
-          id: getDataAppHomePageId(dataApp, pages),
-        },
-      ];
-    }
-
-    return selectedItems;
-  }, [dataApp, pages, selectedItems]);
+  const finalSelectedItems: SelectedItem[] = useMemo(
+    () => getSelectedItems({ dataApp, pages, selectedItems }),
+    [dataApp, pages, selectedItems],
+  );
 
   const handleNewDataAdded = useCallback(
     async (nextDataAppState: DataApp) => {
       // refresh navbar content to show scaffolded pages
       await onReloadNavbar();
 
-      // New pages are added to the end of data app's nav_items list,
-      // so 1st non-hidden page from the end is a good candidate to navigate to
-      const reversedNavItems = nextDataAppState.nav_items.reverse();
+      // 1. New pages are added to the end of data app's nav_items list,
+      // so 1st non-hidden page from the end is a good candidate to navigate to.
+      // 2. Array.prototype.reverse is mutating and it's important not to mess up the real ordering
+      const reversedNavItems = [...nextDataAppState.nav_items].reverse();
       const newPageNavItem = reversedNavItems.find(
         navItem => typeof navItem.page_id === "number" && !navItem.hidden,
       );

frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbar/DataAppNavbarView.tsx

-import React from "react";
+import React, { useCallback, useMemo } from "react";
 import _ from "underscore";
 
-import * as Urls from "metabase/lib/urls";
-
-import type { DataApp } from "metabase-types/api";
+import type { DataApp, DataAppNavItem } from "metabase-types/api";
 
 import { MainNavbarProps, SelectedItem } from "../types";
 import {
-  PaddedSidebarLink,
   SidebarContentRoot,
   SidebarHeading,
   SidebarHeadingWrapper,
@@ -15,6 +12,8 @@ import {
 } from "../MainNavbar.styled";
 import DataAppActionPanel from "./DataAppActionPanel";
 
+import DataAppPageSidebarLink from "./DataAppPageSidebarLink";
+
 interface Props extends MainNavbarProps {
   dataApp: DataApp;
   pages: any[];
@@ -37,23 +36,36 @@ function DataAppNavbarView({
     item => item.type,
   );
 
+  const pageMap = useMemo(() => _.indexBy(pages, "id"), [pages]);
+
+  const renderNavItem = useCallback(
+    (navItem: DataAppNavItem) => {
+      const page = pageMap[navItem.page_id];
+
+      if (!page || navItem.hidden) {
+        return null;
+      }
+
+      return (
+        <DataAppPageSidebarLink
+          key={page.id}
+          dataApp={dataApp}
+          page={page}
+          isSelected={dataAppPage?.id === page.id}
+          indent={navItem.indent}
+        />
+      );
+    },
+    [dataApp, pageMap, dataAppPage],
+  );
+
   return (
     <SidebarContentRoot>
       <SidebarSection>
         <SidebarHeadingWrapper>
           <SidebarHeading>{dataApp.collection.name}</SidebarHeading>
         </SidebarHeadingWrapper>
-        <ul>
-          {pages.map(page => (
-            <PaddedSidebarLink
-              key={page.id}
-              url={Urls.dataAppPage(dataApp, page)}
-              isSelected={dataAppPage?.id === page.id}
-            >
-              {page.name}
-            </PaddedSidebarLink>
-          ))}
-        </ul>
+        <ul>{dataApp.nav_items.map(renderNavItem)}</ul>
       </SidebarSection>
       <DataAppActionPanel
         dataApp={dataApp}

frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbar/DataAppPageSidebarLink/DataAppPageSidebarLink.styled.tsx

+import styled from "@emotion/styled";
+import { SidebarLink } from "../../SidebarItems";
+import { PaddedSidebarLink } from "../../MainNavbar.styled";
+
+export const DataAppPageLink = styled(PaddedSidebarLink)<{ indent: number }>`
+  ${SidebarLink.NameContainers.join(",")} {
+    margin-left: calc(${props => props.indent} * 1rem);
+  }
+`;

frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbar/DataAppPageSidebarLink/DataAppPageSidebarLink.tsx

+import React from "react";
+
+import * as Urls from "metabase/lib/urls";
+
+import type { DataApp, Dashboard } from "metabase-types/api";
+
+import { DataAppPageLink } from "./DataAppPageSidebarLink.styled";
+
+interface Props {
+  dataApp: DataApp;
+  page: Dashboard;
+  isSelected: boolean;
+  indent?: number;
+}
+
+function DataAppPageSidebarLink({
+  dataApp,
+  page,
+  isSelected,
+  indent = 0,
+}: Props) {
+  return (
+    <DataAppPageLink
+      key={page.id}
+      url={Urls.dataAppPage(dataApp, page)}
+      isSelected={isSelected}
+      indent={indent}
+    >
+      {page.name}
+    </DataAppPageLink>
+  );
+}
+
+export default DataAppPageSidebarLink;

frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbar/DataAppPageSidebarLink/index.ts

+export { default } from "./DataAppPageSidebarLink";

frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbar/getSelectedItems.ts

+import {
+  getDataAppHomePageId,
+  getParentDataAppPageId,
+} from "metabase/entities/data-apps";
+
+import type { DataApp, Dashboard } from "metabase-types/api";
+
+import { SelectedItem } from "../types";
+
+function isAtDataAppHomePage(selectedItems: SelectedItem[]) {
+  const [selectedItem] = selectedItems;
+  return selectedItems.length === 1 && selectedItem.type === "data-app";
+}
+
+function isDataAppPageSelected(selectedItems: SelectedItem[]) {
+  const [selectedItem] = selectedItems;
+  return selectedItems.length === 1 && selectedItem.type === "data-app-page";
+}
+
+type Opts = {
+  dataApp: DataApp;
+  pages: Dashboard[];
+  selectedItems: SelectedItem[];
+};
+
+function getSelectedItems({
+  dataApp,
+  pages,
+  selectedItems,
+}: Opts): SelectedItem[] {
+  const isHomepage = isAtDataAppHomePage(selectedItems);
+
+  // Once a data app is launched, the first view is going to be the app homepage
+  // Homepage is an app page specified by a user or picked automatically (just the first one)
+  // The homepage doesn't have a regular page path like /a/1/page/1, but an app one like /a/1
+  // So we need to overwrite the selectedItems list here and specify the homepage
+  if (isHomepage) {
+    return [
+      {
+        type: "data-app-page",
+        id: getDataAppHomePageId(dataApp, pages),
+      },
+    ];
+  }
+
+  if (isDataAppPageSelected(selectedItems)) {
+    const [selectedPage] = selectedItems;
+    const navItem = dataApp.nav_items.find(
+      item => item.page_id === selectedPage.id,
+    );
+
+    // If the selected page is hidden, there's nothing to highlight,
+    // so we want to highlight the parent
+    if (navItem?.hidden) {
+      const parentPageId = getParentDataAppPageId(
+        selectedPage.id as number,
+        dataApp.nav_items,
+      );
+      return [
+        {
+          type: "data-app-page",
+          id: parentPageId || getDataAppHomePageId(dataApp, pages),
+        },
+      ];
+    }
+  }
+
+  return selectedItems;
+}
+
+export default getSelectedItems;

frontend/src/metabase/nav/containers/MainNavbar/DataAppNavbar/getSelectedItems.unit.spec.ts

+import {
+  createMockDataApp,
+  createMockDataAppPage,
+} from "metabase-types/api/mocks";
+import getSelectedItems from "./getSelectedItems";
+
+describe("getSelectedItems", () => {
+  const dataApp = createMockDataApp({
+    dashboard_id: 2,
+    nav_items: [
+      { page_id: 1 },
+      { page_id: 2 },
+      { page_id: 3, indent: 1, hidden: true },
+      { page_id: 4, indent: 1 },
+    ],
+  });
+
+  const pages = [
+    createMockDataAppPage({ id: 1 }),
+    createMockDataAppPage({ id: 2 }),
+    createMockDataAppPage({ id: 3 }),
+    createMockDataAppPage({ id: 4 }),
+  ];
+
+  it("should select the homepage when no page is selected explicitly", () => {
+    expect(
+      getSelectedItems({
+        dataApp,
+        pages,
+        selectedItems: [{ type: "data-app", id: dataApp.id }],
+      }),
+    ).toEqual([{ type: "data-app-page", id: dataApp.dashboard_id }]);
+  });
+
+  it("should select the homepage when it's explicitly selected", () => {
+    expect(
+      getSelectedItems({
+        dataApp,
+        pages,
+        selectedItems: [
+          { type: "data-app-page", id: dataApp.dashboard_id as number },
+        ],
+      }),
+    ).toEqual([{ type: "data-app-page", id: dataApp.dashboard_id }]);
+  });
+
+  it("should select the first page when no page is selected explicitly and homepage is not set", () => {
+    expect(
+      getSelectedItems({
+        dataApp: { ...dataApp, dashboard_id: null },
+        pages,
+        selectedItems: [{ type: "data-app", id: dataApp.id }],
+      }),
+    ).toEqual([{ type: "data-app-page", id: pages[0].id }]);
+  });
+
+  it("should select a regular top-level page", () => {
+    expect(
+      getSelectedItems({
+        dataApp,
+        pages,
+        selectedItems: [{ type: "data-app-page", id: pages[0].id }],
+      }),
+    ).toEqual([{ type: "data-app-page", id: pages[0].id }]);
+  });
+
+  it("should select a regular nested page", () => {
+    expect(
+      getSelectedItems({
+        dataApp,
+        pages,
+        selectedItems: [{ type: "data-app-page", id: pages[3].id }],
+      }),
+    ).toEqual([{ type: "data-app-page", id: pages[3].id }]);
+  });
+
+  it("should select a parent page when hidden page is open", () => {
+    expect(
+      getSelectedItems({
+        dataApp,
+        pages,
+        selectedItems: [{ type: "data-app-page", id: pages[2].id }],
+      }),
+    ).toEqual([{ type: "data-app-page", id: pages[1].id }]);
+  });
+});

frontend/src/metabase/nav/containers/MainNavbar/SidebarItems/SidebarItems.styled.tsx

@@ -109,7 +109,7 @@ export const FullWidthLink = styled(Link)`
 const ITEM_NAME_LENGTH_TOOLTIP_THRESHOLD = 35;
 const ITEM_NAME_LABEL_WIDTH = Math.round(parseInt(NAV_SIDEBAR_WIDTH, 10) * 0.7);
 
-const ItemName = styled(TreeNode.NameContainer)`
+export const ItemName = styled(TreeNode.NameContainer)`
   width: ${ITEM_NAME_LABEL_WIDTH}px;
   padding: 6px 3px;
   white-space: nowrap;

frontend/src/metabase/nav/containers/MainNavbar/SidebarItems/SidebarLink.tsx

@@ -6,6 +6,7 @@ import { IconProps } from "metabase/components/Icon";
 
 import {
   FullWidthLink,
+  ItemName,
   NameContainer,
   NodeRoot,
   SidebarIcon,
@@ -92,4 +93,6 @@ function SidebarLink({
   );
 }
 
-export default SidebarLink;
+export default Object.assign(SidebarLink, {
+  NameContainers: [ItemName, TreeNode.NameContainer],
+});