[UI-01] React app scaffold and routing - #175
Conversation
Coverage Report for CI Build 32278629164Coverage remained the same at 87.883%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
c1dc175 to
ef12bdd
Compare
🚀 PR Environment
Updated on every push. Torn down automatically when this PR closes. |
There was a problem hiding this comment.
Pull request overview
Adds a new standalone React/Vite-based admin UI application under apps/ui within the monorepo, including routing, auth scaffolding (mock + OIDC placeholder), an axios API client layer, and a dedicated UI CI job—while explicitly keeping the UI out of the root NestJS/Jest/ESLint toolchain.
Changes:
- Introduces
apps/uias an isolated npm package (Vite + React Router + Tailwind + shadcn/ui) with its own lint/test/build pipeline. - Implements initial route tree (public
/login, authenticated shell, tenants routes) plus placeholder pages and layouts. - Adds a normalized axios API client (Bearer + 401 refresh single-flight) with MSW/Vitest tests, and updates CI to run UI checks separately.
Reviewed changes
Copilot reviewed 59 out of 62 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Excludes UI from root Jest |
| eslint.config.mjs | Excludes UI from root ESLint |
| docs/ui-bc-design-system-planning.md | Adds BCDS alignment notes |
| docs/DEVELOPER.md | Documents UI dev workflow |
| apps/ui/vite.config.ts | Vite config + proxy + vitest |
| apps/ui/tsconfig.node.json | TS config for Vite config |
| apps/ui/tsconfig.json | TS project references |
| apps/ui/tsconfig.app.json | TS config for app sources |
| apps/ui/src/test/setup.ts | Vitest + MSW test setup |
| apps/ui/src/test/msw/server.ts | MSW server bootstrap |
| apps/ui/src/test/msw/handlers.ts | MSW tenants API handlers |
| apps/ui/src/routes/routes.tsx | Route tree + lazy splitting |
| apps/ui/src/routes/routes.test.tsx | Routing/auth guard tests |
| apps/ui/src/routes/require-auth.tsx | Auth-guard layout route |
| apps/ui/src/pages/TenantsPage.tsx | Tenants list page UI |
| apps/ui/src/pages/TenantsPage.test.tsx | Tenants page MSW test |
| apps/ui/src/pages/TenantOverviewPage.tsx | Tenant overview placeholder |
| apps/ui/src/pages/SettingsPage.tsx | Settings placeholder page |
| apps/ui/src/pages/RouteErrorPage.tsx | Router error boundary page |
| apps/ui/src/pages/PlaceholderPage.tsx | Shared “coming soon” page |
| apps/ui/src/pages/NotFoundPage.tsx | 404 page |
| apps/ui/src/pages/LoginPage.tsx | Login page (mock/oidc) |
| apps/ui/src/pages/DashboardPage.tsx | Dashboard placeholder cards |
| apps/ui/src/main.tsx | App entry + router mount |
| apps/ui/src/lib/utils.ts | Tailwind cn() helper |
| apps/ui/src/lib/env.ts | Env parsing (auth mode) |
| apps/ui/src/lib/auth/types.ts | AuthClient/AuthUser contracts |
| apps/ui/src/lib/auth/oidc-auth.ts | OIDC auth client scaffold |
| apps/ui/src/lib/auth/mock-auth.ts | Mock auth implementation |
| apps/ui/src/lib/auth/mock-auth.test.ts | Mock auth tests |
| apps/ui/src/lib/auth/context.ts | React auth context hook |
| apps/ui/src/lib/auth/auth-context.tsx | AuthProvider wiring + seam |
| apps/ui/src/lib/api/types.gen.ts | Generated OpenAPI types |
| apps/ui/src/lib/api/resources/tenants.ts | Tenants resource functions |
| apps/ui/src/lib/api/queries/tenants.ts | TanStack Query hooks |
| apps/ui/src/lib/api/pagination.ts | List envelope normalization |
| apps/ui/src/lib/api/errors.ts | Error shape normalization |
| apps/ui/src/lib/api/constants.ts | API base path constant |
| apps/ui/src/lib/api/client.ts | Axios client + refresh logic |
| apps/ui/src/lib/api/client.test.ts | Client interceptor tests |
| apps/ui/src/layouts/TenantLayout.tsx | Tenant tabbed layout |
| apps/ui/src/layouts/RootLayout.tsx | App providers layout |
| apps/ui/src/layouts/AppShell.tsx | Sidebar/header shell layout |
| apps/ui/src/index.css | Tailwind + shadcn theme tokens |
| apps/ui/src/components/ui/table.tsx | shadcn table primitive |
| apps/ui/src/components/ui/skeleton.tsx | shadcn skeleton primitive |
| apps/ui/src/components/ui/separator.tsx | shadcn separator primitive |
| apps/ui/src/components/ui/label.tsx | shadcn label primitive |
| apps/ui/src/components/ui/input.tsx | shadcn input primitive |
| apps/ui/src/components/ui/dropdown-menu.tsx | shadcn dropdown primitive |
| apps/ui/src/components/ui/card.tsx | shadcn card primitive |
| apps/ui/src/components/ui/button.tsx | shadcn button primitive |
| apps/ui/src/components/ui/badge.tsx | shadcn badge primitive |
| apps/ui/src/components/ui/avatar.tsx | shadcn avatar primitive |
| apps/ui/README.md | UI package docs/conventions |
| apps/ui/package.json | UI deps/scripts/toolchain |
| apps/ui/index.html | Vite HTML entry |
| apps/ui/eslint.config.mjs | UI ESLint flat config |
| apps/ui/components.json | shadcn CLI config |
| apps/ui/.env.example | UI env example (proxy/auth) |
| .github/workflows/ci-checks.yml | Adds dedicated ui CI job |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
54e9a2f to
ae63216
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 62 out of 65 changed files in this pull request and generated no new comments.
Suppressed comments (4)
apps/ui/src/routes/require-auth.tsx:15
- The guard claims to preserve the intended URL, but this drops fragment identifiers, so a protected deep link such as
/tenants/123#detailsreturns to/tenants/123after sign-in. Preservelocation.hashas well.
docs/ui-bc-design-system-planning.md:6 - This is already scheduled as issue #180, which is linked from this PR and blocked by #82, so describing it as a candidate issue is outdated. Point readers to the existing tracker.
apps/ui/README.md:69 - The root Jest config now explicitly ignores
apps/ui(package.json:93-97), so it will not pick up*.spec.tsfiles here. Keep the naming convention, but avoid documenting a reason contradicted by this PR's configuration.
- Test files are `*.test.ts(x)` (never `*.spec.ts` — the root Jest config would
pick those up).
apps/ui/src/lib/api/client.ts:54
- When a refresh succeeds but the retried request is still unauthorized, every concurrent retried request reaches this branch and calls
onAuthFailure. In OIDC mode that can launch multiple competing sign-out redirects, despite the single-flight guarantee documented below. Route both refresh failure and post-retry 401s through one shared auth-failure operation (reset when auth state changes).
if (!config || config._retried) {
authHandlers.onAuthFailure();
jamshale
left a comment
There was a problem hiding this comment.
Looks like you knew what you were doing 👍 I didn't go over every component in detail... I know we are waiting to have the logout merged, but it would be nice to have the app land on the login instead of hello world even if login is disabled.
That, and some screenshots of what some of the pages look like without needing to pull the branch and set everything up would be nice.
Looks well done though so I'll approve.
|
Follow-ups from review, filed so they aren't lost on merge:
|
Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
ae63216 to
3b51ee8
Compare
For #82
Adds the frontend application as
apps/ui, a standalone npm package inside the monorepo with its ownpackage.json, lockfile, and toolchain. It is not part of the root NestJS build, root ESLint config, or root Jest run.App Details
Stack
typescript-eslintstill stops at <6.1Routing
layout routes per the issue: public
/login; auth-guarded shell (sidebar/header) wrapping/dashboard,/tenants,/tenants/:tenantId/*(tabbed layout: overview, users, connections, credentials, audit-logs, logs, settings), and/settings; plus 404 and a route error boundary. Pages are code-split via route-levellazy(). Placeholder pages reference their tracking issues in the repo (see #84–#89 on parent epic).API Client - Axios
lib/api/resources/, when the flat routes move to the spec's nested/tenants/{id}/…form, only those files change.npm run types:spec), not hand-written interfaces. generated, don't touchThe tenants pages use it against the real API (
GET /api/v1/tenants,GET /api/v1/tenants/:id).Auth
An
AuthClientwith two implementations currently flagged byVITE_AUTH_MODEThis is while Auth is work-in progress.
mock(default): fake session insessionStorage; login/logout/refresh and the route guard genuinely work. Used until interactive OIDC exists.oidc: realoidc-client-tsUserManager(Authorization Code + PKCE against this origin's/oidc). Dormant until AU-02 lands and an SPA client can be registered, completed by [UI-02] Login and auth integration #83 in this epic.Same-origin by design — the Vite dev server proxies
/api,/oidc,/healthto the API (VITE_PROXY_TARGET, defaultlocalhost:3000), mirroring the production Caddy reverse proxy planned in #160. Every URL in the SPA is relative: noVITE_API_URL, no CORS on the API ever, one build artifact for all environments.CI, tests, etc
Tests — 13 Vitest tests: unauthenticated redirect + mock login returning to the originally requested page, 401 single-flight refresh/retry, both error-shape normalizations, mock-auth session lifecycle, and the tenants table rendering from a bare-array response via MSW.
CI — new
uijob inci-checks.yml(Node 24, npm cache scoped toapps/ui/package-lock.json):npm ci, lint, format check, tests, build. Backend jobs untouched.Docs —
apps/ui/README.md(structure + conventions), a Frontend section indocs/DEVELOPER.md, anddocs/ui-bc-design-system-planning.mdcapturing how the interim theme maps onto BC Design System adoption (feeds #180).Local Run
See docs above
npm run devwith the Vite proxy. Local Docker/Caddy same-origin serving is [UI-10] Local Docker/Caddy serving for the UI (same-origin dev stack) #181.