Require all final breadcrumbs to not point to their parents (#3337)

Inspired by a comment on [Charlie's webhooks
PR](https://github.com/oxidecomputer/console/pull/3320/changes#diff-c8f1bef7c0f906ca5002103709d1125057a54691dbbe673d4978670628c61cbeR80).
I was curious on whether there were terminal breadcrumbs that didn't
abide by this reasoning, and inadvertently pointed to parent pages. And
there were!

Breadcrumbs breaking this rule fell into two cases:

1. Pointing to a parent view that defaulted back to "where you just came
from". Not a big deal, but a little silly
2. Pointing to a parent view that then defaulted to _some other place._
That's weird!

Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>

Author: fakemonster

app/pages/project/vpcs/VpcRoutersTab.tsx

@@ -13,6 +13,7 @@ import { api, getListQFn, q, queryClient, useApiMutation, type VpcRouter } from
 
 import { HL } from '~/components/HL'
 import { routeFormMessage } from '~/forms/vpc-router-route-common'
+import { makeCrumb } from '~/hooks/use-crumbs'
 import { getVpcSelector, useVpcSelector } from '~/hooks/use-params'
 import { useQuickActions } from '~/hooks/use-quick-actions'
 import { confirmDelete } from '~/stores/confirm-delete'
@@ -36,7 +37,7 @@ export async function clientLoader({ params }: LoaderFunctionArgs) {
   return null
 }
 
-export const handle = { crumb: 'Routers' }
+export const handle = makeCrumb('Routers', (p) => pb.vpcRouters(getVpcSelector(p)))
 
 export default function VpcRoutersTab() {
   const vpcSelector = useVpcSelector()

app/pages/project/vpcs/VpcSubnetsTab.tsx

@@ -11,6 +11,7 @@ import { Outlet, type LoaderFunctionArgs } from 'react-router'
 
 import { api, getListQFn, queryClient, useApiMutation, type VpcSubnet } from '@oxide/api'
 
+import { makeCrumb } from '~/hooks/use-crumbs'
 import { getVpcSelector, useVpcSelector } from '~/hooks/use-params'
 import { useQuickActions } from '~/hooks/use-quick-actions'
 import { confirmDelete } from '~/stores/confirm-delete'
@@ -36,7 +37,7 @@ export async function clientLoader({ params }: LoaderFunctionArgs) {
   return null
 }
 
-export const handle = { crumb: 'VPC Subnets' }
+export const handle = makeCrumb('VPC Subnets', (p) => pb.vpcSubnets(getVpcSelector(p)))
 
 export default function VpcSubnetsTab() {
   const vpcSelector = useVpcSelector()

app/pages/system/networking/IpPoolsPage.tsx

@@ -18,6 +18,7 @@ import { Badge } from '@oxide/design-system/ui'
 import { DocsPopover } from '~/components/DocsPopover'
 import { HL } from '~/components/HL'
 import { IpVersionBadge } from '~/components/IpVersionBadge'
+import { makeCrumb } from '~/hooks/use-crumbs'
 import { useQuickActions } from '~/hooks/use-quick-actions'
 import { confirmDelete } from '~/stores/confirm-delete'
 import { addToast } from '~/stores/toast'
@@ -87,7 +88,7 @@ export async function clientLoader() {
   return null
 }
 
-export const handle = { crumb: 'IP Pools' }
+export const handle = makeCrumb('IP Pools', pb.ipPools())
 
 export default function IpPoolsPage() {
   const navigate = useNavigate()

app/pages/system/networking/SubnetPoolsPage.tsx

@@ -24,6 +24,7 @@ import { Subnet16Icon, Subnet24Icon } from '@oxide/design-system/icons/react'
 import { DocsPopover } from '~/components/DocsPopover'
 import { HL } from '~/components/HL'
 import { IpVersionBadge } from '~/components/IpVersionBadge'
+import { makeCrumb } from '~/hooks/use-crumbs'
 import { useQuickActions } from '~/hooks/use-quick-actions'
 import { confirmDelete } from '~/stores/confirm-delete'
 import { addToast } from '~/stores/toast'
@@ -86,7 +87,7 @@ export async function clientLoader() {
   return null
 }
 
-export const handle = { crumb: 'Subnet Pools' }
+export const handle = makeCrumb('Subnet Pools', pb.subnetPools())
 
 export default function SubnetPoolsPage() {
   const navigate = useNavigate()

app/routes.tsx

@@ -18,7 +18,7 @@ import {
 import { NotFound } from './components/ErrorPage'
 import { PageSkeleton } from './components/PageSkeleton.tsx'
 import { makeCrumb, type Crumb } from './hooks/use-crumbs'
-import { getInstanceSelector, getVpcSelector } from './hooks/use-params'
+import { getInstanceSelector, getProjectSelector, getVpcSelector } from './hooks/use-params'
 import { pb } from './util/path-builder'
 
 // hack because RR doesn't export the redirect type
@@ -195,7 +195,7 @@ export const routes = createRoutesFromElements(
             lazy={() => import('./pages/system/inventory/DisksTab').then(convert)}
           />
         </Route>
-        <Route path="inventory" handle={{ crumb: 'Inventory' }}>
+        <Route path="inventory" handle={makeCrumb('Inventory', pb.sledInventory())}>
           <Route path="sleds" handle={{ crumb: 'Sleds' }}>
             {/* a crumb for the sled ID looks ridiculous, unfortunately */}
             <Route
@@ -330,7 +330,13 @@ export const routes = createRoutesFromElements(
           lazy={() => import('./layouts/SerialConsoleLayout').then(convert)}
         >
           <Route path="instances" handle={{ crumb: 'Instances' }}>
-            <Route path=":instance" handle={makeCrumb((p) => p.instance!)}>
+            <Route
+              path=":instance"
+              handle={makeCrumb(
+                (p) => p.instance!,
+                (p) => pb.instance(getInstanceSelector(p))
+              )}
+            >
               <Route
                 path="serial-console"
                 lazy={() =>
@@ -506,7 +512,13 @@ export const routes = createRoutesFromElements(
             </Route>
           </Route>
           <Route path="vpcs" handle={{ crumb: 'VPCs' }}>
-            <Route path=":vpc" handle={makeCrumb((p) => p.vpc!)}>
+            <Route
+              path=":vpc"
+              handle={makeCrumb(
+                (p) => p.vpc!,
+                (p) => pb.vpc(getVpcSelector(p))
+              )}
+            >
               <Route path="routers" handle={{ crumb: 'Routers' }}>
                 <Route
                   path=":router"
@@ -600,7 +612,7 @@ export const routes = createRoutesFromElements(
           />
           <Route
             lazy={() => import('./pages/project/affinity/AffinityPage').then(convert)}
-            handle={{ crumb: 'Affinity Groups' }}
+            handle={makeCrumb('Affinity Groups', (p) => pb.affinity(getProjectSelector(p)))}
           >
             <Route
               path="affinity-new"

app/util/__snapshots__/path-builder.spec.ts.snap

@@ -37,7 +37,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "Affinity Groups",
-      "path": "/projects/p/",
+      "path": "/projects/p/affinity",
     },
   ],
   "antiAffinityGroup (/projects/p/affinity/aag)": [
@@ -456,13 +456,13 @@ exports[`breadcrumbs 2`] = `
   "ipPools (/system/networking/ip-pools)": [
     {
       "label": "IP Pools",
-      "path": "/system/networking/",
+      "path": "/system/networking/ip-pools",
     },
   ],
   "ipPoolsNew (/system/networking/ip-pools-new)": [
     {
       "label": "IP Pools",
-      "path": "/system/networking/",
+      "path": "/system/networking/ip-pools",
     },
   ],
   "profile (/settings/profile)": [
@@ -592,7 +592,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "i",
-      "path": "/projects/p/instances/i",
+      "path": "/projects/p/instances/i/storage",
     },
     {
       "label": "Serial Console",
@@ -750,7 +750,7 @@ exports[`breadcrumbs 2`] = `
   "sledInstances (/system/inventory/sleds/5c56b522-c9b8-49e4-9f9a-8d52a89ec3e0/instances)": [
     {
       "label": "Inventory",
-      "path": "/system/inventory",
+      "path": "/system/inventory/sleds",
     },
     {
       "label": "Sleds",
@@ -880,13 +880,13 @@ exports[`breadcrumbs 2`] = `
   "subnetPools (/system/networking/subnet-pools)": [
     {
       "label": "Subnet Pools",
-      "path": "/system/networking/",
+      "path": "/system/networking/subnet-pools",
     },
   ],
   "subnetPoolsNew (/system/networking/subnet-pools-new)": [
     {
       "label": "Subnet Pools",
-      "path": "/system/networking/",
+      "path": "/system/networking/subnet-pools",
     },
   ],
   "systemUpdate (/system/update)": [
@@ -1088,7 +1088,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "v",
-      "path": "/projects/p/vpcs/v",
+      "path": "/projects/p/vpcs/v/firewall-rules",
     },
     {
       "label": "Routers",
@@ -1122,7 +1122,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "Routers",
-      "path": "/projects/p/vpcs/v/",
+      "path": "/projects/p/vpcs/v/routers",
     },
   ],
   "vpcRouterRouteEdit (/projects/p/vpcs/v/routers/r/routes/rr/edit)": [
@@ -1140,7 +1140,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "v",
-      "path": "/projects/p/vpcs/v",
+      "path": "/projects/p/vpcs/v/firewall-rules",
     },
     {
       "label": "Routers",
@@ -1170,7 +1170,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "v",
-      "path": "/projects/p/vpcs/v",
+      "path": "/projects/p/vpcs/v/firewall-rules",
     },
     {
       "label": "Routers",
@@ -1204,7 +1204,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "Routers",
-      "path": "/projects/p/vpcs/v/",
+      "path": "/projects/p/vpcs/v/routers",
     },
   ],
   "vpcRoutersNew (/projects/p/vpcs/v/routers-new)": [
@@ -1226,7 +1226,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "Routers",
-      "path": "/projects/p/vpcs/v/",
+      "path": "/projects/p/vpcs/v/routers",
     },
   ],
   "vpcSubnets (/projects/p/vpcs/v/subnets)": [
@@ -1248,7 +1248,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "VPC Subnets",
-      "path": "/projects/p/vpcs/v/",
+      "path": "/projects/p/vpcs/v/subnets",
     },
   ],
   "vpcSubnetsEdit (/projects/p/vpcs/v/subnets/su/edit)": [
@@ -1270,7 +1270,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "VPC Subnets",
-      "path": "/projects/p/vpcs/v/",
+      "path": "/projects/p/vpcs/v/subnets",
     },
   ],
   "vpcSubnetsNew (/projects/p/vpcs/v/subnets-new)": [
@@ -1292,7 +1292,7 @@ exports[`breadcrumbs 2`] = `
     },
     {
       "label": "VPC Subnets",
-      "path": "/projects/p/vpcs/v/",
+      "path": "/projects/p/vpcs/v/subnets",
     },
   ],
   "vpcs (/projects/p/vpcs)": [

app/util/path-builder.spec.ts

@@ -186,3 +186,29 @@ test('breadcrumbs', async () => {
 
   expect(Object.fromEntries(pairs)).toMatchSnapshot()
 })
+
+// Some pages don't have self-referential breadcrumbs (like /instances-new). But
+// these pages also aren't _targeted_ by breadcrumbs! Any page that's targeted
+// by a breadcrumb should be able to point to itself.
+test('every page reachable by breadcrumb should have a self-referential breadcrumb', async () => {
+  // as far as react router is concerned, /blah and /blah/ are equivalent
+  const dropFinalSlash = (p: string) => p.replace(/\/$/, '')
+
+  const allCrumbs = await Promise.all(
+    Object.values(pb).map(async (fn) => {
+      const pathname = fn(params)
+      const matches = await getMatches(pathname)
+      return matchesToCrumbs(matches).filter(({ titleOnly }) => !titleOnly)
+    })
+  )
+  const allPaths = new Set(allCrumbs.flat().map(({ path }) => path))
+
+  for (const path of allPaths) {
+    const crumbs = matchesToCrumbs(await getMatches(path)).filter(
+      ({ titleOnly }) => !titleOnly
+    )
+    const last = R.last(crumbs)
+    if (last === undefined) expect.fail(`Found no breadcrumbs for ${path}`)
+    expect(dropFinalSlash(path)).toEqual(dropFinalSlash(last.path))
+  }
+})