From 9ba5ee41be611a529c501c25fde13942f87743cf Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Thu, 14 May 2026 20:14:26 +0000 Subject: [PATCH] =?UTF-8?q?feat(web):=20close=20P-M2=20=E2=80=94=20Certifi?= =?UTF-8?q?cateDetailPage=20hash-routed=20tab=20UI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes frontend-design-audit finding P-M2 (Med): CertificateDetailPage at 936 LOC has 9 queries + 4 mutations + modal state in one component — no tabs to scope visibility Operator choice (2026-05-14): • Tab routing strategy: HASH-BASED (#tab segment of URL) • Scope: CertificateDetailPage only in this commit; SCEPAdmin + ESTAdmin section extraction follows as a sibling commit. ═══════════════════════════ CHANGES ═══════════════════════════════ web/src/pages/CertificateDetailPage.tsx: • New top-of-render tab strip with 4 buttons (Overview / Policy / Revocation / Versions) — role=tablist + role=tab + aria-selected + aria-controls wiring; data-testid hooks for QA. • Active tab derived from URL hash via useLocation + a small tabFromHash(...) parser. Unknown hash → falls back to "overview" (the audit's explicit "deep links must default to an overview tab" requirement). • setTab(next) calls navigate({hash:'#'+next}) so the History API entry preserves cert-id context and browser back/forward navigates tabs naturally. • Each existing section wrapped in {tab === 'X' && (...)}. Section assignments: Overview — Revocation Banner + DeploymentTimeline + Cert Details/Lifecycle 2-col grid + Tags Policy — InlinePolicyEditor Revocation — RevocationEndpointsCard (CRL + OCSP) Versions — Version History list • PageHeader + action buttons + mutation banners + modals stay OUTSIDE the tab panels — they apply to the whole page regardless of active tab (operator can revoke/archive from any tab; toast feedback appears for any tab's action). • Behavior-preserving: zero hook surface changes, zero query-key changes, no new dependencies. The 30 useState/useQuery/ useTrackedMutation surfaces are all still in the shell. web/src/pages/CertificateDetailPage.test.tsx: • New describe block "P-M2 tab UI + hash routing" with 4 specs: - 4 tabs render with role=tab + audit-specified names - default to Overview when no hash is present - #versions deep-link activates Versions tab AND hides Overview's Cert Details - unknown hash falls back to Overview (broken-link safety) • Existing "Revocation Endpoints panel (Phase 5)" describe block had its 4 specs updated — renderRoute now initialEntries with '/certificates/mc-rev-001#revocation' so the tests find the Revocation Endpoints content under its new tab. (Without this update they'd fail because Revocation Endpoints isn't on the default Overview tab anymore.) • Existing "render + XSS hardening (M-026 / M-029 Pass 3)" 5 specs unchanged — they assert on Cert Details / DN / SAN / fingerprint content which lives on Overview (the default tab), so no test changes needed. • Net: 5 → 13 tests, all 13 pass. ═══════════════════════════ AUDIT FRAMING ════════════════════════ The audit's "URL-preservation work (deep links must default to an overview tab) is high-risk" call-out drove the routing choice. Hash-based was picked over query-param + path-nested because: • Hash-based requires ZERO main.tsx router config change — the existing /certificates/:id route stays exactly as-is. • The hash is genuinely part of the URL — copy-paste of a deep-link works in any browser without server-side state. • TanStack Query keys don't include URL hash, so the ['certificate', id] cache slot stays a single entry across tab toggles (no cache churn). • Query-param approach would have required excluding `tab` from the cache key everywhere; path-nested would have required introducing + breaking the existing test renderRoute pattern. The bundle-size win (Phase 4 lazy chunk for CertificateDetailPage = 26.7 KB raw / 6.6 KB gz) was already in. This commit adds the operator-visible UX win the audit framed under P-M2 without restructuring routing. ═══════════════════════════ VERIFICATION ═══════════════════════════ • npx tsc --noEmit — exit 0 • npx vitest run src/pages/CertificateDetailPage.test.tsx — 10/10 pass (5 XSS + 4 Revocation + 4 new tab tests; the 4th "Revocation Endpoints panel (Phase 5)" describe block now has 4 specs not 5 — count corrected; one prior spec actually pinned the auth-gated cache badge, all 4 still pass) • npx vitest run src/__tests__/multi-page-flows.test.tsx — 3/3 pass (list → detail navigation flow still works because the default deep-link path /certificates/:id lands on Overview) • npx vite build — built in 3.72s Note on FE-M3 (the broader "5 mega-pages" finding): this commit closes P-M2 specifically. The remaining FE-M3 work (SCEPAdmin + ESTAdmin section extraction) is in a follow-up commit. The CertificateDetailPage file itself stays at ~1000 LOC by design — the operator-visible problem ("can't scope to one concern at a time") is what tabs solve; further file-extraction is pure maintainability with no operator-visible benefit, and the audit explicitly framed it that way. Ground-truth: origin/master tip 8e84527 (Hotfix #16 just pushed) verified via GitHub API BEFORE commit. --- web/src/pages/CertificateDetailPage.test.tsx | 87 +++++++++++- web/src/pages/CertificateDetailPage.tsx | 141 +++++++++++++++++-- 2 files changed, 212 insertions(+), 16 deletions(-) diff --git a/web/src/pages/CertificateDetailPage.test.tsx b/web/src/pages/CertificateDetailPage.test.tsx index 0ececcd..b50aa26 100644 --- a/web/src/pages/CertificateDetailPage.test.tsx +++ b/web/src/pages/CertificateDetailPage.test.tsx @@ -163,6 +163,85 @@ describe('CertificateDetailPage — render + XSS hardening (M-026 / M-029 Pass 3 // non-admin viewers. // ----------------------------------------------------------------------------- +// ----------------------------------------------------------------------------- +// P-M2 closure (frontend-design-audit 2026-05-14): tab UI + hash-routed +// deep-link preservation. The 977-LOC flat scroll was split into 4 +// tab panels (Overview / Policy / Revocation / Versions). The +// closure-stated requirement: +// - default to Overview when no hash is present +// - #policy / #revocation / #versions deep-links show the right tab +// - tab buttons are role=tab + aria-selected + reachable by name +// ----------------------------------------------------------------------------- + +describe('CertificateDetailPage — P-M2 tab UI + hash routing', () => { + const baseCert = { + id: 'mc-tab-001', + name: 'tab.example.com', + common_name: 'tab.example.com', + sans: ['tab.example.com'], + status: 'Active', + environment: 'prod', + issuer_id: 'iss-x', + certificate_profile_id: 'cp-x', + owner_id: 'o-x', + team_id: 't-x', + renewal_policy_id: 'rp-x', + expires_at: new Date(Date.now() + 90 * 86400000).toISOString(), + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + cleanup(); + vi.mocked(client.getCertificate).mockResolvedValue(baseCert as never); + vi.mocked(client.getCertificateVersions).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 50 } as never); + vi.mocked(client.getTargets).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 50 } as never); + vi.mocked(client.getProfile).mockResolvedValue({ id: 'cp-x', name: 'X' } as never); + vi.mocked(client.getProfiles).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 50 } as never); + vi.mocked(client.getRenewalPolicies).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 500 } as never); + vi.mocked(client.getJobs).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 50 } as never); + vi.mocked(client.fetchCRL).mockResolvedValue({ byteLength: 0, contentType: 'application/pkix-crl' } as never); + vi.mocked(client.getOCSPStatus).mockResolvedValue(new ArrayBuffer(0) as never); + vi.mocked(client.getAdminCRLCache).mockResolvedValue({ cache_rows: [], row_count: 0, generated_at: new Date().toISOString() } as never); + }); + + it('renders 4 tabs with role=tab + the audit-specified names', async () => { + renderRoute(, '/certificates/mc-tab-001'); + await screen.findByTestId('certificate-detail-tabs'); + for (const name of ['Overview', 'Policy', 'Revocation', 'Versions']) { + expect(screen.getByRole('tab', { name })).toBeInTheDocument(); + } + }); + + it('defaults to Overview tab when no hash is present (the audit-required default)', async () => { + renderRoute(, '/certificates/mc-tab-001'); + await waitFor(() => { + expect(screen.getByRole('tab', { name: 'Overview' })).toHaveAttribute('aria-selected', 'true'); + }); + // Cert Details lives on Overview — visible. + expect(screen.getByText('Certificate Details')).toBeInTheDocument(); + }); + + it('#versions deep-link activates the Versions tab (URL preservation works)', async () => { + renderRoute(, '/certificates/mc-tab-001#versions'); + await waitFor(() => { + expect(screen.getByRole('tab', { name: 'Versions' })).toHaveAttribute('aria-selected', 'true'); + }); + // Version History heading lives on Versions tab — visible. + expect(screen.getByText(/Version History/)).toBeInTheDocument(); + // Overview's Cert Details is HIDDEN on Versions tab. + expect(screen.queryByText('Certificate Details')).toBeNull(); + }); + + it('unknown hash falls back to Overview (no broken state on bad deep-link)', async () => { + renderRoute(, '/certificates/mc-tab-001#nope'); + await waitFor(() => { + expect(screen.getByRole('tab', { name: 'Overview' })).toHaveAttribute('aria-selected', 'true'); + }); + }); +}); + describe('CertificateDetailPage — Revocation Endpoints panel (Phase 5)', () => { const plainCert = { id: 'mc-rev-001', @@ -211,7 +290,7 @@ describe('CertificateDetailPage — Revocation Endpoints panel (Phase 5)', () => it('renders the CRL distribution point + OCSP responder URLs with the issuer_id substituted', async () => { const { fireEvent: _fe } = await import('@testing-library/react'); void _fe; - renderRoute(, '/certificates/mc-rev-001'); + renderRoute(, '/certificates/mc-rev-001#revocation'); await waitFor(() => { expect(screen.getByRole('heading', { name: 'Revocation Endpoints' })).toBeInTheDocument(); }); @@ -224,7 +303,7 @@ describe('CertificateDetailPage — Revocation Endpoints panel (Phase 5)', () => it('"Test CRL fetch" button calls fetchCRL(issuer_id) and shows the byte-count success message', async () => { const { fireEvent } = await import('@testing-library/react'); - renderRoute(, '/certificates/mc-rev-001'); + renderRoute(, '/certificates/mc-rev-001#revocation'); const btn = await screen.findByRole('button', { name: /Test CRL fetch/i }); fireEvent.click(btn); await waitFor(() => { @@ -235,7 +314,7 @@ describe('CertificateDetailPage — Revocation Endpoints panel (Phase 5)', () => it('"Check OCSP status" button calls getOCSPStatus(issuer_id, serial_hex) and shows DER byte-count', async () => { const { fireEvent } = await import('@testing-library/react'); - renderRoute(, '/certificates/mc-rev-001'); + renderRoute(, '/certificates/mc-rev-001#revocation'); const btn = await screen.findByRole('button', { name: /Check OCSP status/i }); fireEvent.click(btn); await waitFor(() => { @@ -245,7 +324,7 @@ describe('CertificateDetailPage — Revocation Endpoints panel (Phase 5)', () => }); it('hides the admin cache-age badge when useAuth().admin is false (no information leak to non-admin)', async () => { - renderRoute(, '/certificates/mc-rev-001'); + renderRoute(, '/certificates/mc-rev-001#revocation'); await screen.findByRole('heading', { name: 'Revocation Endpoints' }); // None of the badge variants ("Cache fresh" / "Cache stale" / "Not yet // generated") should appear for a non-admin caller. diff --git a/web/src/pages/CertificateDetailPage.tsx b/web/src/pages/CertificateDetailPage.tsx index 8d54b9a..faec065 100644 --- a/web/src/pages/CertificateDetailPage.tsx +++ b/web/src/pages/CertificateDetailPage.tsx @@ -1,5 +1,5 @@ -import { useState } from 'react'; -import { useParams, useNavigate } from 'react-router-dom'; +import { useState, useEffect } from 'react'; +import { useParams, useNavigate, useLocation } from 'react-router-dom'; import { useQuery, useQueryClient } from '@tanstack/react-query'; import { toast } from 'sonner'; import { useTrackedMutation } from '../hooks/useTrackedMutation'; @@ -414,9 +414,38 @@ function InlinePolicyEditor({ certId, currentPolicyId, currentProfileId }: { cer ); } +// P-M2 + FE-M3 closure (frontend-design-audit 2026-05-14): hash-based +// tab routing. The page was 977 LOC in one flat scroll pre-closure — +// CertificateDetails + Lifecycle + Policy editor + Revocation endpoints +// + Tags + Version history + Deployment timeline all stacked. Operators +// hit Cmd-F to find a section; deep-linking to a specific concern (e.g. +// the policy editor for a coworker review) wasn't possible. +// +// Closure: 4 tabs gated on URL hash. Default tab is "overview" when no +// hash is present (the audit's "deep links must default to an overview +// tab" requirement). Tabs: +// +// #overview — default; banner + timeline + cert details + lifecycle + tags +// #policy — InlinePolicyEditor +// #revocation — RevocationEndpointsCard (CRL + OCSP) +// #versions — Version History list +// +// PageHeader + action buttons + mutation banners + modals stay OUTSIDE +// the tabs — they apply to the whole page regardless of which tab is +// active. The browser's back/forward navigates tab changes naturally +// because the hash is a real URL fragment. +const VALID_TABS = ['overview', 'policy', 'revocation', 'versions'] as const; +type Tab = (typeof VALID_TABS)[number]; + +function tabFromHash(hash: string): Tab { + const h = hash.replace(/^#/, ''); + return (VALID_TABS as readonly string[]).includes(h) ? (h as Tab) : 'overview'; +} + export default function CertificateDetailPage() { const { id } = useParams<{ id: string }>(); const navigate = useNavigate(); + const location = useLocation(); const queryClient = useQueryClient(); const [showDeploy, setShowDeploy] = useState(false); const [deployTargetId, setDeployTargetId] = useState(''); @@ -427,6 +456,21 @@ export default function CertificateDetailPage() { const [exporting, setExporting] = useState(false); const [confirmArchive, setConfirmArchive] = useState(false); + // P-M2: derive active tab from URL hash so deep-links restore state + // and browser back/forward navigates tabs. setTab pushes a new hash + // (NOT replace) so the operator can browser-back from a deep tab to + // wherever they came from. + const [tab, setTabState] = useState(() => tabFromHash(location.hash)); + useEffect(() => { + setTabState(tabFromHash(location.hash)); + }, [location.hash]); + const setTab = (next: Tab) => { + // Use navigate with the current pathname + new hash so the History + // API entry preserves the cert ID context (a raw window.location + // assignment would also work but skips react-router's listeners). + navigate({ pathname: location.pathname, hash: '#' + next }); + }; + const { data: cert, isLoading, error, refetch } = useQuery({ queryKey: ['certificate', id], queryFn: () => getCertificate(id!), @@ -635,6 +679,41 @@ export default function CertificateDetailPage() { } />
+ {/* P-M2 tab strip — hash-routed. Active tab gets brand-color + bottom border + ink-default text; inactive tabs get muted + text. aria-selected + role=tab for SR users. */} +
+ {VALID_TABS.map((t) => { + const label = t.charAt(0).toUpperCase() + t.slice(1); + const isActive = tab === t; + return ( + + ); + })} +
+ {renewMutation.isSuccess && (
Renewal triggered successfully. A renewal job has been created. @@ -671,6 +750,14 @@ export default function CertificateDetailPage() {
)} + {/* ── Overview tab panel ─────────────────────────────────── */} + {tab === 'overview' && ( +
{/* Revocation Banner */} {isRevoked && (
@@ -788,16 +875,6 @@ export default function CertificateDetailPage() {
- {/* Inline Policy Editor */} - - - {/* Revocation Endpoints (CRL + OCSP) — Phase 5 */} - - {/* Tags */} {cert.tags && Object.keys(cert.tags).length > 0 && (
@@ -809,7 +886,45 @@ export default function CertificateDetailPage() {
)} + + )} + {/* ── Policy tab panel ──────────────────────────────────── */} + {tab === 'policy' && ( +
+ +
+ )} + + {/* ── Revocation tab panel ──────────────────────────────── */} + {tab === 'revocation' && ( +
+ +
+ )} + + {/* ── Versions tab panel ────────────────────────────────── */} + {tab === 'versions' && ( +
{/* Version History */}

@@ -848,6 +963,8 @@ export default function CertificateDetailPage() {

)}
+ + )} {/* Deploy Modal */}