feat: First-Start Create Admin Modal 1.00
This commit is contained in:
OpenVelo Agent
@@ -1,86 +0,0 @@
|
||||
# Implementation Plan: Fix Angular build error — Property 'code' does not exist on type 'CreateAdminResult'
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
|
||||
- **Codebase style & conventions:** Angular 17 standalone components, signals, typed reactive forms. `frontend/tsconfig.json` uses `strict: false` and `strictTemplates: false`. The project still uses `@angular/compiler` (ngc) which performs its own type checking on top of TS narrowing rules.
|
||||
- **Data Layer:** N/A — this is a frontend-only type-narrowing fix.
|
||||
- **Test Framework & Structure:** `tests/jest.config.js`. Tests live under `tests/frontend/`. All compile via `npm test`.
|
||||
- **Required Tools & Dependencies:** No new dependencies. The fix is purely TypeScript / component-internal.
|
||||
|
||||
## 2. Impacted Files
|
||||
|
||||
- **To Modify:** `/repo/frontend/src/app/features/setup/setup-create-admin.component.ts` — fix the type-narrowing that Angular's compiler refuses to accept.
|
||||
- **No new files, no test changes, no docs changes.**
|
||||
|
||||
## 3. Proposed Changes
|
||||
|
||||
### Root cause
|
||||
|
||||
In `setup-create-admin.component.ts:88-117` the `submit()` method awaits the service, then reads `result.ok`. After the `if (result.ok) { ... return; }` branch, TS should narrow `result` to `CreateAdminFailure` (which has `code`). However Angular's compiler (used by `ng build`) reports:
|
||||
|
||||
```
|
||||
TS2339: Property 'code' does not exist on type 'CreateAdminResult'.
|
||||
Property 'code' does not exist on type 'CreateAdminSuccess'.
|
||||
```
|
||||
|
||||
at the two reads `result.code` on lines 109 and 116. With `strict: false` + `strictTemplates: false`, narrowing across an awaited union *should* succeed, but Angular's compiler's narrowing model is stricter in some code paths (notably after `await` of an external service) and conservatively widens back to the full union. The error message proves it: type is `CreateAdminResult`, not `CreateAdminFailure`.
|
||||
|
||||
### Minimal fix
|
||||
|
||||
Replace the discriminated-union narrowing with an explicit type guard in the service and an `else` branch in the component. This avoids relying on the compiler's narrowing at all.
|
||||
|
||||
### Step-by-step
|
||||
|
||||
1. **Add a type predicate to the service** (`setup-create-admin.service.ts`):
|
||||
- Add `export function isCreateAdminFailure(r: CreateAdminResult): r is CreateAdminFailure { return !r.ok; }`.
|
||||
- No behaviour change.
|
||||
|
||||
2. **Refactor `submit()`** in `setup-create-admin.component.ts`:
|
||||
- Replace the `if (result.ok) { ... return; } ... if (result.code === ...) ...` sequence with an `if (result.ok) { ... return; } const failure = result;` pattern. Because `result.ok` is `true | false`, the negation narrows the local copy identically. The safest cross-compiler approach is:
|
||||
```ts
|
||||
if (result.ok) {
|
||||
this.auth.setSession(result.accessToken, result.user);
|
||||
this.bootstrap.markInitialized();
|
||||
await this.router.navigateByUrl('/challenges');
|
||||
return;
|
||||
}
|
||||
const failure: CreateAdminFailure = result; // explicit local of narrowed type
|
||||
if (failure.code === 'USERNAME_TAKEN') {
|
||||
this.serverError.set('Username already exists');
|
||||
this.serverErrorCode.set('USERNAME_TAKEN');
|
||||
return;
|
||||
}
|
||||
this.serverError.set('Setup failed, please retry');
|
||||
this.serverErrorCode.set(failure.code);
|
||||
```
|
||||
- Import `CreateAdminFailure` from `./setup-create-admin.service`.
|
||||
- The cast `const failure: CreateAdminFailure = result` after the `return` is safe (TS will assert assignability; if the union ever changes it becomes a compile error here rather than two ambiguous ones).
|
||||
|
||||
Alternatively, an explicit cast: `const failure = result as CreateAdminFailure;`. Either is acceptable; the `as`-cast is one token shorter and more idiomatic. Choose:
|
||||
|
||||
```ts
|
||||
const failure = result as CreateAdminFailure;
|
||||
```
|
||||
|
||||
3. **No HTML/CSS/service file changes.**
|
||||
|
||||
## 4. Test Strategy
|
||||
|
||||
- **Target Unit Test File:** no changes. The existing `tests/frontend/setup-create-admin.spec.ts` already covers the validator and rules; the build error is a compile-time-only issue that the existing tests do not exercise.
|
||||
- **Mocking Strategy:** N/A.
|
||||
- **Verification:**
|
||||
1. `npm --workspace frontend run build` must complete without `TS2339` errors.
|
||||
2. `npm test` must still pass (20 suites, 97 tests).
|
||||
- **Do not** touch any test file. The fix is internal to the component.
|
||||
|
||||
## 5. Risk / Non-Goals
|
||||
|
||||
- **No behaviour change.** The runtime control flow is identical; the change only adds an explicit local that the compiler can definitively narrow.
|
||||
- **No public API change.** `CreateAdminResult`, `CreateAdminSuccess`, `CreateAdminFailure` shapes remain as defined.
|
||||
- **No docs change required** — implementation detail only.
|
||||
|
||||
## 6. Execution Checklist
|
||||
|
||||
1. Edit `setup-create-admin.component.ts`: import `CreateAdminFailure`, refactor lines 102-116 to use `const failure = result as CreateAdminFailure;` after the success branch returns.
|
||||
2. Run `npm --workspace frontend run build` and confirm 0 errors.
|
||||
3. Run `npm test` and confirm `Test Suites: 20 passed, 20 total`.
|
||||
@@ -0,0 +1,178 @@
|
||||
# Implementation Plan: Job 841 — First-Start Create Admin Modal: Authenticated Shell & Admin Navigation
|
||||
|
||||
## 0. Already-Implemented Check
|
||||
|
||||
The job is **NOT** fully implemented. Specifically:
|
||||
|
||||
- `frontend/src/app/features/setup/setup-create-admin.component.ts:105` already calls `this.router.navigateByUrl('/challenges')` on success, but there is **no `/challenges` route registered** anywhere in `frontend/src/app/app.routes.ts` (only `bootstrap`, `login`, and `''` → `HomeComponent`).
|
||||
- `frontend/src/app/features/home/home.component.ts` renders a plain `<div class="container">` with no nav bar, no sidebar, no menu, and no `routerLink` to any admin area.
|
||||
- `frontend/src/app/app.component.ts` only renders `<router-outlet></router-outlet>`; there is no shared shell/sidebar component.
|
||||
- `frontend/src/app/app.routes.ts` registers **no** `/admin`, `/challenges`, `/scoreboard`, or any other authenticated feature route, and **no admin guard** exists in `frontend/src/app/core/guards/` (only `auth.guard.ts`).
|
||||
- A grep across `frontend/src/app/features/` for `Admin` / `routerLink` / `nav` / `sidebar` / `menu` returns **no** matches outside the create-admin / login flows.
|
||||
- The backend admin surface exists (`backend/src/modules/admin/admin.controller.ts` exposing `/api/v1/admin/users`), but the frontend never exposes a link to it.
|
||||
|
||||
Result: after a successful `POST /api/v1/setup/create-admin`, the operator is dropped on `HomeComponent` which shows only "HIPCTF" + the username, with **no way to reach any Admin functionality**.
|
||||
|
||||
The remediation below is the minimal, focused change required to satisfy the job's Expected Behavior: authenticated shell must display the created username **and** provide admin-only navigation entries (including a link to the Admin area).
|
||||
|
||||
## 1. Architectural Reconnaissance
|
||||
|
||||
- **Codebase style & conventions:**
|
||||
- Frontend: Angular 20 standalone components, signals (`signal`/`computed`), `inject()`, functional `CanActivateFn` guards. No NgModules. Inline templates are inline; the more complex `SetupCreateAdminComponent` uses `templateUrl` + `styleUrls`.
|
||||
- Backend: NestJS, TypeORM, Zod-validated pipes, `AdminGuard` + `@Roles('admin')` on `AdminController`. REST surface at `/api/v1/...`.
|
||||
- Test style: Jest via `tests/jest.config.js` with two projects (`backend` Node, `frontend` jsdom). Frontend tests live under `tests/frontend/`, are runnable with `npm test` from root, never import from inside `src/`.
|
||||
- **Data Layer:** TypeORM with SQLite (file DB at `DATABASE_PATH`, `:memory:` in tests). No schema change is required for this job — `user.role` already supports `'admin'` and is returned on `/api/v1/setup/create-admin`.
|
||||
- **Test Framework & Structure:** Jest. Tests **MUST** live under `tests/` (never inside `frontend/src/`). Run with `npm test` from repo root.
|
||||
- **Required Tools & Dependencies:** None new. The existing `jest`, `ts-jest`, `@types/jest`, `jest-environment-jsdom`, `@angular/router`, `@angular/core`, `@angular/common` are already present. No `setup.sh` change is required.
|
||||
|
||||
## 2. Impacted Files
|
||||
|
||||
### To Modify
|
||||
|
||||
- `frontend/src/app/app.routes.ts` — register the new admin-protected child route(s) and the admin guard.
|
||||
- `frontend/src/app/features/home/home.component.ts` — replace plain container with a small shell that renders username and admin-only nav links (or host the new shell component).
|
||||
- `frontend/src/app/features/setup/setup-create-admin.component.ts` — fix the post-success navigation target so the operator lands somewhere valid (`/admin` is the natural target since this user is the freshly-created admin).
|
||||
|
||||
### To Create
|
||||
|
||||
- `frontend/src/app/core/guards/admin.guard.ts` — functional `CanActivateFn` that allows only users with `role === 'admin'` (mirrors `authGuard`).
|
||||
- `frontend/src/app/core/services/admin.service.ts` — thin wrapper over the existing `/api/v1/admin/users` REST endpoint (so the Admin area page can list users immediately, exercising the admin nav link end-to-end).
|
||||
- `frontend/src/app/features/admin/admin-users.component.ts` — minimal standalone "Admin area" landing page reachable via the new nav link. Renders a list of users by calling `AdminService.list()`. Lazy-loaded.
|
||||
- `tests/frontend/admin-navigation.spec.ts` — focused unit tests for the new guard and the nav-link rendering logic.
|
||||
- `tests/frontend/admin-shell.spec.ts` — focused unit test verifying the authenticated shell shows username + admin nav and that a `player` role does not see admin nav.
|
||||
|
||||
## 3. Proposed Changes
|
||||
|
||||
### 1. Schema / Data
|
||||
No DB migration required. `user.role` already supports `'admin'`, and `CurrentUser.role` is already returned by `/api/v1/setup/create-admin` (see `frontend/src/app/features/setup/setup-create-admin.service.ts:15`).
|
||||
|
||||
### 2. Backend Logic & APIs
|
||||
No backend changes required. The existing `AdminController` (`backend/src/modules/admin/admin.controller.ts`) at `/api/v1/admin/users` is sufficient for the Admin area page to display something real.
|
||||
|
||||
### 3. Frontend UI Integration
|
||||
|
||||
**3.1 Add `adminGuard` (functional guard)**
|
||||
|
||||
File: `frontend/src/app/core/guards/admin.guard.ts`
|
||||
|
||||
```ts
|
||||
import { inject } from '@angular/core';
|
||||
import { CanActivateFn, Router } from '@angular/router';
|
||||
import { AuthService } from '../services/auth.service';
|
||||
import { BootstrapService } from '../services/bootstrap.service';
|
||||
|
||||
export const adminGuard: CanActivateFn = () => {
|
||||
const auth = inject(AuthService);
|
||||
const bootstrap = inject(BootstrapService);
|
||||
const router = inject(Router);
|
||||
|
||||
if (!bootstrap.initialized()) return router.createUrlTree(['/bootstrap']);
|
||||
if (!auth.isAuthenticated()) return router.createUrlTree(['/login']);
|
||||
if (auth.currentUser()?.role !== 'admin') return router.createUrlTree(['/']);
|
||||
return true;
|
||||
};
|
||||
```
|
||||
|
||||
Rationale: mirrors `authGuard` (`frontend/src/app/core/guards/auth.guard.ts`) and reuses the same signal-driven `AuthService` + `BootstrapService`.
|
||||
|
||||
**3.2 Add `AdminService` (thin HTTP wrapper)**
|
||||
|
||||
File: `frontend/src/app/core/services/admin.service.ts`
|
||||
|
||||
- `listUsers(): Promise<Array<{ id: string; username: string; role: 'admin' | 'player' }>>` calls `GET /api/v1/admin/users` via `HttpClient` with `withCredentials: true`.
|
||||
- Single-purpose; no caching layer needed for this job.
|
||||
|
||||
**3.3 Add `AdminUsersComponent` (lazy-loaded Admin landing page)**
|
||||
|
||||
File: `frontend/src/app/features/admin/admin-users.component.ts`
|
||||
|
||||
- Standalone, `ChangeDetectionStrategy.OnPush`, signals (`users = signal<...>([])`, `loading = signal(false)`, `error = signal<string|null>(null)`).
|
||||
- On `ngOnInit`, calls `AdminService.listUsers()` and stores the result.
|
||||
- Renders a heading ("Admin area") plus a list of users with `username` and `role`.
|
||||
- Uses inline template (no extra `.html` file required, matching the existing `HomeComponent` and `LoginComponent` style).
|
||||
|
||||
**3.4 Wire routes**
|
||||
|
||||
File: `frontend/src/app/app.routes.ts`
|
||||
|
||||
Add an authenticated `admin` child under the `''` route (still `authGuard`-protected) so the admin nav link resolves:
|
||||
|
||||
```ts
|
||||
{
|
||||
path: '',
|
||||
loadComponent: () => import('./features/home/home.component').then(m => m.HomeComponent),
|
||||
canActivate: [authGuard],
|
||||
children: [
|
||||
{
|
||||
path: 'admin',
|
||||
canActivate: [adminGuard],
|
||||
loadComponent: () => import('./features/admin/admin-users.component').then(m => m.AdminUsersComponent),
|
||||
},
|
||||
],
|
||||
},
|
||||
```
|
||||
|
||||
And add `<router-outlet></router-outlet>` inside `HomeComponent`'s template so the child route renders below the nav.
|
||||
|
||||
**3.5 Authenticated shell with admin nav**
|
||||
|
||||
File: `frontend/src/app/features/home/home.component.ts`
|
||||
|
||||
- Replace the current plain template with a shell that always renders:
|
||||
- A header bar with the page title (`bootstrap.payload()?.pageTitle ?? 'HIPCTF'`) on the left.
|
||||
- On the right: the signed-in username + role (`Signed in as <b>{{ username }}</b> ({{ role }})`).
|
||||
- A `<nav>` element below the header containing admin-only links, conditionally rendered with `@if (auth.currentUser()?.role === 'admin')`. Minimum required entry: `<a routerLink="/admin" data-testid="nav-admin">Admin</a>`.
|
||||
- The existing welcome / Sign-in block for unauthenticated viewers stays.
|
||||
- `<router-outlet />` below the nav so the new `/admin` child route renders.
|
||||
- Keep the component class minimal; just `inject(AuthService)` + `inject(BootstrapService)`.
|
||||
|
||||
**3.6 Fix post-admin-creation navigation target**
|
||||
|
||||
File: `frontend/src/app/features/setup/setup-create-admin.component.ts:105`
|
||||
|
||||
- Change `await this.router.navigateByUrl('/challenges');` → `await this.router.navigateByUrl('/admin');`. The current `/challenges` target is dead (no route registered). `/admin` is the appropriate landing page for a freshly-created admin user and is consistent with the job's Expected Behavior ("a link to the Admin area").
|
||||
- The bootstrap flow doc (`docs/guides/bootstrap.md:49`) says "navigates to `/challenges`" — update the doc string in lockstep (one-line edit, no behavior change beyond the URL).
|
||||
|
||||
### 4. `/data` Usage
|
||||
This job writes no persistent assets — purely UI routing changes and a thin service. No `/data` directory writes required.
|
||||
|
||||
## 4. Test Strategy
|
||||
|
||||
Tests live in the existing `tests/frontend/` folder and run via `npm test` (which invokes `jest --config tests/jest.config.js`). No visual / Playwright tests are introduced; the rule explicitly forbids them.
|
||||
|
||||
### 4.1 `tests/frontend/admin-shell.spec.ts`
|
||||
|
||||
Focus: the authenticated shell renders username + admin nav, and hides admin nav for non-admins.
|
||||
|
||||
- Use Angular's `TestBed` with `provideRouter([])` and stub `AuthService` / `BootstrapService` with minimal signal-shaped fakes:
|
||||
- `auth.currentUser = signal({ id: 'u1', username: 'firstadmin', role: 'admin' })`
|
||||
- `auth.isAuthenticated = computed(() => true)`
|
||||
- `bootstrap.initialized = signal(true)`, `bootstrap.payload = signal({ pageTitle: 'HIPCTF' })`
|
||||
- Render `HomeComponent`, assert fixture contains the username `firstadmin` and the `[data-testid="nav-admin"]` link.
|
||||
- Re-render with `role: 'player'` and assert the admin nav link is **not** present.
|
||||
|
||||
Mocking strategy: do **not** mock `HttpClient`. `HomeComponent` template is static (no HTTP); no network is touched. `RouterLink`/`RouterOutlet` are provided via `provideRouter([{ path: 'admin', children: [] }])` so the link renders without real navigation.
|
||||
|
||||
### 4.2 `tests/frontend/admin-navigation.spec.ts`
|
||||
|
||||
Focus: `adminGuard` redirect logic.
|
||||
|
||||
- Provide a stub `AuthService` + `BootstrapService` and an injected `Router` (from `TestBed.inject(Router)`).
|
||||
- Use `TestBed.runInInjectionContext(() => adminGuard({}, {} as any))` to invoke the guard.
|
||||
- Cases:
|
||||
- Uninitialized → returns `UrlTree('/bootstrap')`.
|
||||
- Unauthenticated → returns `UrlTree('/login')`.
|
||||
- Authenticated non-admin → returns `UrlTree('/')`.
|
||||
- Authenticated admin → returns `true`.
|
||||
|
||||
Mocking strategy: pure DI stubs; no HTTP, no DOM navigation.
|
||||
|
||||
### 4.3 No new backend tests
|
||||
|
||||
`/api/v1/admin/users` is already covered by `tests/backend/admin-guard.spec.ts` (200 for admin, 403 for player, 401 unauth). This job introduces no backend surface area.
|
||||
|
||||
## 5. Out of Scope (Explicit Non-Goals)
|
||||
|
||||
- No `/challenges`, `/scoreboard`, or other future feature routes — only `/admin` is wired because that is what the job requires.
|
||||
- No new backend endpoints, no DB migrations, no theme/UI redesign beyond the minimal nav addition.
|
||||
- No Playwright/visual tests (explicitly forbidden by the test rule).
|
||||
Reference in New Issue
Block a user