feat(ux): URL-addressable sub-tabs (default-first) for Inventory + Admin (closes #902) (#905)

Resolve QA A.4: Inventory & Coverage remembered its last sub-tab via
hidden in-memory session state while Admin always read its sub-tab from
the /admin/<subtab> URL. The inconsistency was the defect.

Make Inventory sub-tabs URL-addressable as /inventory/<subtab>, matching
the existing Admin convention:

- loadInventory() now derives the sub-section from the URL path
  (getInventorySubTabFromPath), not the module-level currentSubSection, so
  a fresh /inventory lands on the default (active-commitments) and a
  /inventory/<subtab> deep link lands on that sub-tab.
- A sub-nav click routes through navigation.switchInventorySubTab, which
  pushes /inventory/<subtab> via history.pushState so the view is
  shareable/bookmarkable and browser back/forward works.
- The history push lives in navigation.ts alongside switchSettingsSubTab so
  the single historyId counter stays authoritative for the back/forward
  dirty-guard; inventory.ts keeps the pure DOM view switch.

Supersedes the partial PR #757 session-memory behavior with explicit URL
state. Both pages now behave identically; existing /admin/* deep links are
unaffected.

Tests assert, for each page: no param -> default sub-tab, explicit param ->
that sub-tab, switching updates the URL (query/hash preserved, no duplicate
entry), and unknown param -> default.

Author: cristim

frontend/src/__tests__/inventory.test.ts

@@ -34,7 +34,21 @@ jest.mock('../state', () => ({
   getCurrentAccountIDs: jest.fn(() => []),
 }));
 
+// inventory.ts routes sub-nav clicks through navigation.switchInventorySubTab
+// so the click both switches the view AND pushes /inventory/<subtab>
+// (QA A.4). Mock it to delegate to the real (pure) view switcher: that
+// keeps the click->DOM behaviour these tests assert, while letting us spy
+// on the router call. The URL-push half of switchInventorySubTab is tested
+// in navigation.test.ts where the router owns history.
+jest.mock('../navigation', () => {
+  const actual = jest.requireActual('../inventory');
+  return {
+    switchInventorySubTab: jest.fn((name: string) => actual.switchInventorySubSection(name)),
+  };
+});
+
 import { loadInventory, switchInventorySubSection, loadActiveCommitments, loadCoverageBreakdown } from '../inventory';
+import { switchInventorySubTab } from '../navigation';
 import { loadRIExchange } from '../riexchange';
 import * as api from '../api';
 import * as state from '../state';
@@ -202,13 +216,67 @@ describe('Inventory & Coverage sub-section switching', () => {
     expect(document.getElementById('inventory-active-commitments')?.classList.contains('hidden')).toBe(false);
     expect(document.getElementById('inventory-ri-exchange')?.classList.contains('hidden')).toBe(true);
 
-    // Clicking a sub-tab button switches the section.
+    // Clicking a sub-tab button routes through the router (QA A.4) so the
+    // click both switches the view AND pushes /inventory/<subtab>. The
+    // mocked router delegates to the real view switcher, so the DOM flips.
     const coverageBtn = document.querySelector<HTMLButtonElement>('[data-inv-subtab="coverage"]')!;
     coverageBtn.click();
+    expect(switchInventorySubTab).toHaveBeenCalledWith('coverage');
+    expect(document.getElementById('inventory-coverage')?.classList.contains('hidden')).toBe(false);
+    expect(document.getElementById('inventory-active-commitments')?.classList.contains('hidden')).toBe(true);
+  });
+
+});
+
+// QA A.4: Inventory sub-tabs are URL-addressable (/inventory/<subtab>),
+// default-first, and shareable. inventory.ts owns the view switch + the
+// click->router wiring; the URL push itself lives in navigation.ts and is
+// covered in navigation.test.ts. Here we assert:
+//   - the pure switcher returns the resolved (validated) sub-section,
+//   - loadInventory honours an explicit sub-section, defaults when absent,
+//     and falls back when unknown (default-first),
+//   - a sub-nav click routes through navigation.switchInventorySubTab so
+//     the URL gets updated (no hidden session state).
+describe('Inventory & Coverage sub-tab addressing (QA A.4)', () => {
+  beforeEach(() => {
+    buildInventoryDOM();
+    (loadRIExchange as jest.Mock).mockClear();
+    (switchInventorySubTab as jest.Mock).mockClear();
+    (api.listActiveCommitments as jest.Mock).mockReset().mockResolvedValue([]);
+    (api.getCoverageBreakdown as jest.Mock).mockReset().mockResolvedValue({ providers: [] });
+    (state.subscribeProvider as jest.Mock).mockReset().mockReturnValue(jest.fn());
+    (state.subscribeAccount as jest.Mock).mockReset().mockReturnValue(jest.fn());
+    (state.getCurrentProvider as jest.Mock).mockReturnValue('');
+    (state.getCurrentAccountIDs as jest.Mock).mockReturnValue([]);
+  });
+
+  afterEach(() => {
+    clearDOM();
+  });
+
+  test('switchInventorySubSection returns the resolved sub-section', () => {
+    expect(switchInventorySubSection('coverage')).toBe('coverage');
+    // Unknown input resolves to the default (default-first).
+    expect(switchInventorySubSection('bogus')).toBe('active-commitments');
+  });
+
+  test('(a) loadInventory(undefined) -> default sub-section (active-commitments)', () => {
+    loadInventory(undefined);
+    expect(document.getElementById('inventory-active-commitments')?.classList.contains('hidden')).toBe(false);
+    expect(api.listActiveCommitments).toHaveBeenCalled();
+  });
+
+  test('(b) loadInventory(<subtab>) -> that sub-section', () => {
+    loadInventory('coverage');
     expect(document.getElementById('inventory-coverage')?.classList.contains('hidden')).toBe(false);
     expect(document.getElementById('inventory-active-commitments')?.classList.contains('hidden')).toBe(true);
+    expect(api.getCoverageBreakdown).toHaveBeenCalled();
   });
 
+  test('(d) loadInventory(<unknown>) -> falls back to default', () => {
+    loadInventory('bogus-subtab');
+    expect(document.getElementById('inventory-active-commitments')?.classList.contains('hidden')).toBe(false);
+  });
 });
 
 describe('loadActiveCommitments — fetch + render flow', () => {

frontend/src/__tests__/navigation.test.ts

@@ -1,7 +1,7 @@
 /**
  * Navigation module tests
  */
-import { switchTab, switchSettingsSubTab, getSettingsSubTabFromPath } from '../navigation';
+import { switchTab, switchSettingsSubTab, switchInventorySubTab, getSettingsSubTabFromPath, getInventorySubTabFromPath } from '../navigation';
 
 // Mock the dependent modules
 jest.mock('../dashboard', () => ({
@@ -33,6 +33,21 @@ jest.mock('../riexchange', () => ({
 jest.mock('../auth', () => ({
   isAdmin: jest.fn().mockReturnValue(true),
 }));
+// Mock inventory so navigation tests stay focused on routing/history and
+// don't pull in the real fetch/render machinery. switchInventorySubSection
+// must still resolve+return the sub-section (default-first) because
+// navigation.switchInventorySubTab uses the return value to build the URL.
+jest.mock('../inventory', () => {
+  const VALID = ['active-commitments', 'coverage', 'ri-exchange'];
+  const DEFAULT = 'active-commitments';
+  const isValid = (n: string): boolean => VALID.includes(n);
+  return {
+    DEFAULT_INVENTORY_SUB_SECTION: DEFAULT,
+    isValidInventorySubSection: isValid,
+    switchInventorySubSection: jest.fn((n: string) => (isValid(n) ? n : DEFAULT)),
+    loadInventory: jest.fn(),
+  };
+});
 
 import { loadDashboard } from '../dashboard';
 import { loadRecommendations } from '../recommendations';
@@ -164,6 +179,28 @@ describe('Navigation Module', () => {
       expect(homeBtn?.classList.contains('active')).toBe(false);
     });
 
+    // QA A.4: a bare inventory switch lands on the default sub-tab and the
+    // canonical URL carries the sub-tab segment (/inventory/active-commitments),
+    // mirroring how the admin switch pushes /admin/<subtab>.
+    test('switching to inventory pushes /inventory/<default-subtab>', () => {
+      // currentTab is module state that may already be 'inventory' from a
+      // prior test; switch away first so the inventory switch is genuine
+      // (a self-switch would correctly skip the push).
+      switchTab('home');
+      window.history.replaceState(null, '', '/');
+      switchTab('inventory');
+      expect(window.location.pathname).toBe('/inventory/active-commitments');
+    });
+
+    // A deep link to a specific inventory sub-tab is honoured: switchTab
+    // reads the path and the canonical URL keeps that sub-tab.
+    test('switching to inventory honours a /inventory/<subtab> deep link', () => {
+      switchTab('home');
+      window.history.replaceState(null, '', '/inventory/coverage');
+      switchTab('inventory');
+      expect(window.location.pathname).toBe('/inventory/coverage');
+    });
+
     test('deactivates previously active tab', () => {
       // Dashboard is initially active
       const dashboardBtn = document.querySelector('[data-tab="home"]');
@@ -300,6 +337,50 @@ describe('Navigation Module', () => {
     });
   });
 
+  // QA A.4: switchInventorySubTab owns the /inventory/<subtab> history push,
+  // mirroring switchSettingsSubTab. The DOM switch is delegated to the
+  // (mocked) inventory module. Placed BEFORE the *FromPath describes, which
+  // destructively replace window.location with a plain object and would
+  // otherwise break the real history.pushState these tests rely on.
+  describe('switchInventorySubTab', () => {
+    beforeEach(() => {
+      window.history.replaceState(null, '', '/inventory/active-commitments');
+    });
+
+    test('(c) pushes /inventory/<subtab> on a real switch', () => {
+      switchInventorySubTab('coverage');
+      expect(window.location.pathname).toBe('/inventory/coverage');
+    });
+
+    test('(c) preserves existing query params and hash', () => {
+      window.history.replaceState(null, '', '/inventory/active-commitments?provider=aws#frag');
+      switchInventorySubTab('ri-exchange');
+      expect(window.location.pathname).toBe('/inventory/ri-exchange');
+      expect(window.location.search).toBe('?provider=aws');
+      expect(window.location.hash).toBe('#frag');
+    });
+
+    test('(d) an unknown sub-tab resolves to the default in the URL', () => {
+      window.history.replaceState(null, '', '/inventory/coverage');
+      switchInventorySubTab('bogus');
+      expect(window.location.pathname).toBe('/inventory/active-commitments');
+    });
+
+    test('does NOT push a duplicate entry when already on the target sub-tab', () => {
+      window.history.replaceState(null, '', '/inventory/coverage');
+      const before = window.history.length;
+      switchInventorySubTab('coverage');
+      expect(window.location.pathname).toBe('/inventory/coverage');
+      expect(window.history.length).toBe(before);
+    });
+
+    test('push: false switches the view without touching history', () => {
+      switchInventorySubTab('coverage', { push: false });
+      // URL unchanged: the caller (initial load / popstate) owns the URL.
+      expect(window.location.pathname).toBe('/inventory/active-commitments');
+    });
+  });
+
   describe('getSettingsSubTabFromPath', () => {
     // Canonical /admin/* paths (issue #340 IA rename)
     test('returns general for root admin path', () => {
@@ -351,4 +432,32 @@ describe('Navigation Module', () => {
       expect(getSettingsSubTabFromPath()).toBe('general');
     });
   });
+
+  // QA A.4: Inventory sub-tabs become URL-addressable (/inventory/<subtab>),
+  // matching the Admin /admin/<subtab> convention.
+  describe('getInventorySubTabFromPath', () => {
+    test('returns the default (active-commitments) for a bare /inventory path', () => {
+      delete (window as unknown as Record<string, unknown>).location;
+      (window as unknown as Record<string, unknown>).location = { pathname: '/inventory' } as Location;
+      expect(getInventorySubTabFromPath()).toBe('active-commitments');
+    });
+
+    test('returns coverage for /inventory/coverage', () => {
+      delete (window as unknown as Record<string, unknown>).location;
+      (window as unknown as Record<string, unknown>).location = { pathname: '/inventory/coverage' } as Location;
+      expect(getInventorySubTabFromPath()).toBe('coverage');
+    });
+
+    test('returns ri-exchange for /inventory/ri-exchange', () => {
+      delete (window as unknown as Record<string, unknown>).location;
+      (window as unknown as Record<string, unknown>).location = { pathname: '/inventory/ri-exchange' } as Location;
+      expect(getInventorySubTabFromPath()).toBe('ri-exchange');
+    });
+
+    test('falls back to the default for an unknown sub-tab', () => {
+      delete (window as unknown as Record<string, unknown>).location;
+      (window as unknown as Record<string, unknown>).location = { pathname: '/inventory/bogus' } as Location;
+      expect(getInventorySubTabFromPath()).toBe('active-commitments');
+    });
+  });
 });

frontend/src/inventory.ts

@@ -16,6 +16,7 @@ import { loadRIExchange } from './riexchange';
 import { showSkeletonRows, teardownSkeleton } from './lib/skeleton';
 import { formatCurrency, formatDate } from './utils';
 import * as state from './state';
+import { switchInventorySubTab } from './navigation';
 
 type InventorySubSection = 'active-commitments' | 'coverage' | 'ri-exchange';
 
@@ -25,22 +26,38 @@ const SUB_SECTION_IDS: Record<InventorySubSection, string> = {
   'ri-exchange': 'inventory-ri-exchange',
 };
 
-const DEFAULT_SUB_SECTION: InventorySubSection = 'active-commitments';
+export const DEFAULT_INVENTORY_SUB_SECTION: InventorySubSection = 'active-commitments';
 
 let currentSubSection: InventorySubSection | undefined;
 let listenersWired = false;
 
-function isValidSubSection(name: string): name is InventorySubSection {
+/**
+ * Type guard for the Inventory sub-section identifiers. Exported so the
+ * router (navigation.ts) can validate the `/inventory/<subtab>` path
+ * segment without duplicating the closed set.
+ */
+export function isValidInventorySubSection(name: string): name is InventorySubSection {
   return name === 'active-commitments' || name === 'coverage' || name === 'ri-exchange';
 }
 
 /**
  * Show one sub-section, hide the others. Activates the matching sub-nav
  * button and (for ri-exchange) triggers the RI exchange data load so the
  * existing flow stays identical to its pre-#340 behaviour.
+ *
+ * This is the pure view switcher: it does NOT touch the URL. URL history
+ * (the `/inventory/<subtab>` addressing from QA A.4) is owned by
+ * navigation.ts' switchInventorySubTab, mirroring how switchSettingsSubTab
+ * owns the `/admin/<subtab>` history so a single counter (historyId) stays
+ * authoritative for the back/forward dirty-guard.
+ *
+ * Returns the resolved (validated, default-substituted) sub-section so the
+ * caller can reflect the same value in the URL.
  */
-export function switchInventorySubSection(name: string): void {
-  const target: InventorySubSection = isValidSubSection(name) ? name : DEFAULT_SUB_SECTION;
+export function switchInventorySubSection(name: string): InventorySubSection {
+  const target: InventorySubSection = isValidInventorySubSection(name)
+    ? name
+    : DEFAULT_INVENTORY_SUB_SECTION;
 
   document.querySelectorAll<HTMLButtonElement>('#inventory-tab .sub-tab-btn').forEach((btn) => {
     const isActive = btn.dataset['invSubtab'] === target;
@@ -62,6 +79,7 @@ export function switchInventorySubSection(name: string): void {
   }
 
   currentSubSection = target;
+  return target;
 }
 
 // ──────────────────────────────────────────────
@@ -417,8 +435,11 @@ function wireSubNavListeners(): void {
   if (buttons.length === 0) return;
   buttons.forEach((btn) => {
     btn.addEventListener('click', () => {
-      const name = btn.dataset['invSubtab'] ?? DEFAULT_SUB_SECTION;
-      switchInventorySubSection(name);
+      const name = btn.dataset['invSubtab'] ?? DEFAULT_INVENTORY_SUB_SECTION;
+      // Route through the router so the click both switches the view AND
+      // pushes /inventory/<subtab> (QA A.4), keeping history consistent
+      // with the Admin sub-tab flow.
+      switchInventorySubTab(name);
     });
   });
   listenersWired = true;
@@ -485,11 +506,22 @@ function wireChipSubscriptions(): void {
 
 /**
  * Initialize the Inventory & Coverage section. Called by navigation.ts'
- * switchTab when 'inventory' is selected. Defaults to active-commitments
- * if the user hasn't selected a sub-section this session.
+ * switchTab when 'inventory' is selected, passing the sub-section parsed
+ * from the `/inventory/<subtab>` URL path (QA A.4).
+ *
+ * The sub-section comes from the URL, not hidden session state: a fresh
+ * `/inventory` with no sub-segment lands on the default (active-commitments)
+ * and a `/inventory/<subtab>` deep link lands on that sub-section. The
+ * switch is URL-driven (push: false) so re-entering the tab doesn't stack
+ * a redundant history entry on top of the one switchTab already pushed.
  */
-export function loadInventory(): void {
+export function loadInventory(subSection?: string): void {
   wireSubNavListeners();
   wireChipSubscriptions();
-  switchInventorySubSection(currentSubSection ?? DEFAULT_SUB_SECTION);
+  const target = subSection !== undefined && isValidInventorySubSection(subSection)
+    ? subSection
+    : DEFAULT_INVENTORY_SUB_SECTION;
+  // Pure view switch (no history push): switchTab already pushed the
+  // canonical /inventory/<subtab> URL when this tab was entered.
+  switchInventorySubSection(target);
 }