refactor(frontend): gate analytics + admin nav on permissions

[zaira-bibi]

📚 Stacked on #539 (analytics dashboard) → which is on #533 (GET /permissions/can). Review/merge #533 → #539 → this; the diff is against #539's branch. Follow-up to the analytics-dashboard nav gating; part of the analytics epic #434.

What

Render the Analytics and Admin sidebar entries through a small config-driven, permission-gated mechanism that mirrors how plugin entries render — replacing the ad-hoc canViewAnalytics boolean (Analytics) and the coarse is_admin flag (Admin).

Changes

• refactor(frontend): GATED_NAV config + resolveNavPermissions — resolves each gated entry once per token via GET /permissions/can (one check per item, failing closed) into a Record<key, boolean>
• refactor(frontend): shared NavItem component (extracted from the repeated nav-link blocks)
• refactor(frontend): the dashboard layout resolves navPermissions once and threads it to both sidebars, which render GATED_NAV.filter(i => navPermissions?.[i.key]).map(NavItem); canViewAnalytics removed
• refactor(frontend): Admin nav gates on email.whitelist.read (whitelist scope) instead of is_admin
• fix(frontend): the whitelist page's client guard is aligned to the same email.whitelist.read check (was !is_admin), so nav visibility and page access agree
• test(frontend): resolveNavPermissions, NavItem, rewritten AppSidebar gating (per key), WhitelistPage guard

How to Test

1. make test.frontend.vitest → green (190 tests).
2. cd frontend && bun run typecheck, make lint.frontend, make test.frontend.api → all clean.
3. Manual (backend + frontend running): a user with analytics.read sees the Analytics entry; a user with email.whitelist.read sees the Admin entry and can open /dashboard/admin/whitelist; a user with neither sees neither entry, and direct navigation to the whitelist page redirects to /dashboard.

Notes

• Stacked PR — merge after feat(frontend): add analytics dashboard at /dashboard/analytics #539 (gh stack sync --prune once feat(frontend): add analytics dashboard at /dashboard/analytics #539 lands).
• No backend / generated.ts change; no new dependencies. Single permission check per gated entry (no batch endpoint).
• UI gating is a convenience, not a security boundary — every endpoint still enforces its own permission and returns 403; the client checks fail closed.

This PR description was written with the assistance of an LLM (Claude).

[zaira-bibi]

@claude-review

[claude]

Claude finished @zaira-bibi's task in 2m 22s —— View job

---

Claude Code Review

Reviewed the diff against origin/zaira/feat/analytics-dashboard-ui (10 files, +322/-72). This is a clean, well-scoped refactor that replaces the ad-hoc canViewAnalytics boolean and the coarse is_admin flag with a config-driven, permission-gated nav mechanism. Nicely done overall — good abstraction, strong test coverage, and the fail-closed behaviour is consistent throughout.

✅ Strengths

• GATED_NAV + resolveNavPermissions is a clean, extensible pattern — adding a gated entry is now a single config object. Checks run in parallel (Promise.all), fail closed per-item, and log with context (no bare except-style swallowing).
• NavItem correctly de-duplicates the repeated link markup; keyed by item.key (not array index) ✔.
• Effect hygiene — both permission effects use an active cleanup flag to avoid post-unmount state updates, and depend only on [token]. Good.
• Guard/nav alignment — moving the whitelist page guard to the same email.whitelist.read check so nav visibility and page access agree is the right call.
• Tests cover the config shape, resolveNavPermissions (map/scope pass-through/fail-closed), NavItem rendering, per-key AppSidebar gating, and the WhitelistPage allow/deny guard. Matches the TDD expectation for new functionality.

🟡 Suggestions (non-blocking)

Medium — WhitelistPage fetches before the permission check resolves (inline)
The fetchEntries effect runs on mount while access === "checking", so getWhitelist(token) fires for every user — including those about to be denied (an unauthorized GET /whitelist → 403 → state.error, just before the redirect). Gate it on access === "allowed". Not a security issue (endpoint enforces its own permission), but avoids a wasted request and an error flash. Fix this →

Low — infinite spinner when token is absent
WhitelistPage.tsx:87-100 returns early on !token, leaving access === "checking" forever (permanent "Loading whitelist..." spinner). This relies on the dashboard layout guaranteeing an authenticated token before this page mounts — which appears to hold — but it's a fragile implicit assumption. A brief note or an explicit unauthenticated branch would make it robust.

Low — duplicated spinner JSX
WhitelistPage.tsx:165-174 (the checking spinner) is identical to the state.loading spinner at 176-185. Could be collapsed into one if (access === "checking" || state.loading) branch.

Security

No concerns. UI gating is correctly framed as convenience, not enforcement; every gated entry still fails closed, and the backend enforces each endpoint independently.

Verdict

Approve-worthy pending the maintainer's call on the Medium suggestion. No blocking issues; the fetch-timing item is a small correctness/efficiency polish rather than a defect.
· branch zaira/feat/permission-gated-nav

[regisb]

Again, I'm definitely not the right person to review this but as far as I'm concerned, this is good to go.

[hamza-56]

Nice follow-up to #539