HIPCTF2/.kilo/plans/845.md

# Implementation Plan: Job 845 — First-Start Create Admin Modal 1.04 (surface field validation)

## 1. Architectural Reconnaissance

- **Codebase style & conventions:**
  - NestJS API + Angular 17 SPA monorepo (workspaces in `package.json`).
  - Frontend: **Standalone components**, `ChangeDetectionStrategy.OnPush`, Angular **signals** (`signal`/`computed`), no NgModules. Style guide mandates `OnPush` and signals for state.
  - Reactive Forms via `ReactiveFormsModule` with `fb.nonNullable.group(...)` (non-nullable typed controls).
  - All services are `providedIn: 'root'`. Two HTTP interceptors wired in `frontend/src/main.ts` (`csrfInterceptor`, then `authInterceptor`).
  - Frontend tests are **pure-logic / no TestBed**: `tests/frontend/**/*.spec.ts` import pure functions (validators, session-storage helpers, shell decision helpers) and rely on `jest` (`testEnvironment: 'jsdom'`) runnable with the root command `npm test` (which uses `jest --config tests/jest.config.js --selectProjects frontend`).
  - Strictly-typed reactive forms per `angular-forms` skill (`FormGroup<T>`, `fb.nonNullable`).
  - Data attributes / `data-testid` selectors are the stable DOM hooks (see `docs/guides/bootstrap.md`).
- **Data Layer:** N/A for this Job — purely a frontend UI/validation correctness fix. The backend already returns a structured `400 VALIDATION_FAILED` envelope; the bug is solely that the SPA short-circuits on `form.invalid` without ever surfacing per-field error messages.
- **Test Framework & Structure:**
  - Jest with `ts-jest`, jsdom environment for frontend.
  - Tests live **only** in `tests/frontend/` (never co-located with source).
  - Root command: `npm test` (runs all projects via `jest --config tests/jest.config.js`).
  - Existing precedent at `tests/frontend/setup-create-admin.spec.ts` is a pure-logic suite (mirrors component logic in standalone helper functions). **We will follow that pattern** — no TestBed, no DOM rendering required, no visual confirmation.
- **Required Tools & Dependencies:** No new dependencies, system tools, or `setup.sh` changes needed. The fix is a localized source-code change in the standalone component + its template + a pure-helper module that mirrors the per-field error messages (for parity testing). The npm workspace already has `@angular/forms ^17.3` and `jest`.

## 2. Impacted Files

### To Modify
- **`frontend/src/app/features/setup/setup-create-admin.component.ts`** — fix the reactivity of the per-field error calculations and expose `passwordErrors()`. Convert the `computed(...)`s whose bodies read `FormControl` properties (which are NOT signals in Angular 17) into plain **methods** so the template re-evaluates them on every change-detection cycle (cheap, OnPush-friendly, no extra subscriptions). Alternatively keep them as computed but reflect `touched` into a local `signal()` driven by `statusChanges`/`valueChanges` + an effect; **plain methods are the minimal, idiomatic fix** and are used elsewhere in the same component style. Keep all existing public surface (`form`, `submitting`, `serverError`, `serverErrorCode`, `canRetry`, `policyDescription`, `submit`, `swallowBackdrop`, `swallowEscape`) intact.
- **`frontend/src/app/features/setup/setup-create-admin.component.html`** — render the new Password field-error block, mirror `aria-invalid` on the Password and Confirm Password inputs (Username already has it), and keep the `data-testid` hooks the bootstrap guide documents (`setup-username-error`, add `setup-password-error`, keep `setup-confirm-error`).
- **`frontend/src/app/features/setup/setup-create-admin.component.css`** — no functional change required; `.field-error` styling already exists.
- **`tests/frontend/setup-create-admin.spec.ts`** — extend the existing pure-logic suite to cover the new password-error helper and to assert that, given an all-empty (or partially-filled) post-submit state, the per-field error strings the template renders are correct.

### To Create
- **`frontend/src/app/features/setup/setup-create-admin.field-errors.ts`** — pure helper functions that return the per-field error string (or `null`) for a given `(value, touched)` snapshot. Keeping them as pure functions (no Angular DI, no signals) is deliberate: it lets the component call them from methods **and** lets the pure-logic test suite assert them without spinning up Angular. Mirrors the pattern already used by `setup-create-admin.validators.ts` and tested in `setup-create-admin.spec.ts`.

## 3. Proposed Changes

### Root cause
Angular 17 `FormControl.touched`, `.valid`, and `.errors` are plain class properties, **not** signals. The existing `computed(() => … usernameErrors …)` (component file lines 70–78 and 80–87) reads those plain properties and so registers **zero signal dependencies**. Even after `submit()` calls `form.markAllAsTouched()`, the computed is not re-evaluated — its cached `null` is returned and the `@if (usernameErrors()) { … }` block stays hidden. The same applies to `confirmErrors()` for the `passwordConfirm` control. There is also no `passwordErrors()` at all and no `aria-invalid` binding on the Password/Confirm inputs, which matches the user's observation.

### 3.1 New pure helper module

`frontend/src/app/features/setup/setup-create-admin.field-errors.ts`:

```ts
export type FieldErrorKey =
  | 'required'
  | 'minlength'
  | 'maxlength'
  | 'pattern'
  | 'passwordMismatch';

export interface FieldErrorContext {
  value: string;
  touched: boolean;
  errors: Partial<Record<FieldErrorKey, true>> | null;
  groupMismatch?: boolean;
}

export function usernameError(ctx: FieldErrorContext): string | null { … }
export function passwordError(ctx: FieldErrorContext): string | null { … }   // currently required-only; password POLICY is server-enforced
export function confirmError(ctx: FieldErrorContext): string | null { … }
```

Rules encoded (single source of truth, identical to current component intent):

- **Username:**
  - not touched or no errors → `null`
  - `required` → `'Username is required'`
  - `minlength` → `'Username must be at least 3 characters'`
  - `maxlength` → `'Username must be at most 32 characters'`
  - `pattern` → `'Username may only contain letters, digits, dot, dash and underscore'`
  - else → `'Invalid username'`
- **Password:**
  - not touched or no errors → `null`
  - `required` → `'Password is required'`
  - else → `null` (policy is intentionally server-side only; this component never surfaces a "weak password" field error pre-submit)
- **Confirm:**
  - not touched and no `passwordMismatch` group error → `null`
  - `required` → `'Please confirm your password'`
  - `passwordMismatch` (group-level) → `'Passwords do not match'`
  - else → `'Invalid value'`

These signatures accept the small slice of state they need so the component can pass `c.value`, `c.touched`, and `c.errors` (cast through `Partial<Record<…, true>>`) and so the test suite can call them directly without touching Angular.

### 3.2 Component changes

`setup-create-admin.component.ts`:

1. **Replace the two `computed(...)`s with plain methods** so the template re-renders them on every CD pass:
   - `usernameError(): string | null { return usernameError({ value: this.form.controls.username.value, touched: this.form.controls.username.touched, errors: this.form.controls.username.errors }); }`
   - `passwordError(): string | null { return passwordError({ value: this.form.controls.password.value, touched: this.form.controls.password.touched, errors: this.form.controls.password.errors }); }`  ← **new**
   - `confirmError(): string | null { return confirmError({ value: this.form.controls.passwordConfirm.value, touched: this.form.controls.passwordConfirm.touched, errors: this.form.controls.passwordConfirm.errors, groupMismatch: !!this.form.errors?.['passwordMismatch'] }); }`
2. **Delete the orphaned `usernameErrors` and `confirmErrors` signal properties** (template rewires to methods).
3. Leave `submit()` exactly as-is: `markAllAsTouched()` then early-return on invalid. The early return is now correct and observable (because `…Error()` is re-evaluated by the next CD cycle that runs after `markAllAsTouched` synchronously mutates `touched` on every control). No new `effect()` needed; this is the minimal fix.
4. Keep OnPush (`ChangeDetectionStrategy.OnPush`), keep all existing imports/decorators/HostListener for `escape` and `swallowBackdrop`. The host binding keyboard handler stays so the modal remains non-dismissible.

> Optional, NOT recommended for this Job (kept out of scope to honor "minimal changes"): a reactive alternative would be to subscribe to `statusChanges` / `valueChanges` via `toSignal()` in the constructor and gate rendering on those signals. The plain-method approach above is the minimum-surface fix that fully resolves the bug and matches the component's current style.

### 3.3 Template changes

`setup-create-admin.component.html`:

1. Username `<input>` — keep existing `[attr.aria-invalid]="!!usernameError()"` and existing `<small class="field-error">` block, but rebind to the new method name.
2. Password `<label class="field">` — add `aria-invalid` and add the missing error block below the policy hint:
   ```html
   <input … formControlName="password"
          [attr.aria-invalid]="!!passwordError()"
          data-testid="setup-password" />
   <small class="hint">{{ policyDescription() }}</small>
   @if (passwordError()) {
     <small class="field-error" data-testid="setup-password-error">{{ passwordError() }}</small>
   }
   ```
3. Confirm `<input>` — add `aria-invalid`:
   ```html
   <input … formControlName="passwordConfirm"
          [attr.aria-invalid]="!!confirmError()"
          data-testid="setup-password-confirm" />
   @if (confirmError()) {
     <small class="field-error" data-testid="setup-confirm-error">{{ confirmError() }}</small>
   }
   ```
4. Submit button — **unchanged**. The Job accepts either "visible per-field error messages" **or** "disabled submit button until valid" — we keep the click-to-trigger-feedback behavior (already used by `markAllAsTouched`) and now surface real errors, which is the original design intent. We deliberately do **not** disable the button on invalid (preserves parity with the existing guide which says the button is "disabled only while submitting").

### 3.4 CSS

No changes. `.field-error` color/size is already defined.

### 3.5 No backend changes

The backend's `SetupController.createAdmin` already responds with `400 VALIDATION_FAILED` and a `details` array; that contract is unchanged. The client never reached it before, and after the fix the client still won't for an empty form (which is correct — the per-field client-side errors are now visible; the server is reachable when the client actually submits valid input).

## 4. Test Strategy

- **Target Unit Test File:** `tests/frontend/setup-create-admin.spec.ts` (extend the existing pure-logic file; do **not** create a co-located spec under `frontend/`).
  - The existing file already exports nothing new — it lives entirely in `tests/frontend/` per project convention and runs with the root command `npm test` (Jest, jsdom).
- **Mocking Strategy:**
  - **No TestBed, no HTTP, no DOM rendering.** The new helpers (`usernameError`, `passwordError`, `confirmError`) are pure functions in `setup-create-admin.field-errors.ts` — they are imported and called directly with hand-built `FieldErrorContext` literals, mirroring the existing `usernameErrors`/`isFormReadyForSubmit` helpers in the same file.
  - The existing `passwordMatchValidator` tests and username rules act as the de-facto model; we extend the "Composed submit gate" suite to also assert the rendered error strings.
- **New test cases (≤ 8, focused on the bug paths):**
  1. `usernameError({ value: '', touched: true, errors: { required: true } })` returns `'Username is required'`.
  2. `usernameError({ value: 'ab', touched: true, errors: { minlength: true } })` returns the minlength message.
  3. `passwordError({ value: '', touched: true, errors: { required: true } })` returns `'Password is required'`.
  4. `passwordError({ value: 'x', touched: true, errors: null })` returns `null` (policy is server-side only; no pre-submit hint).
  5. `confirmError({ value: '', touched: true, errors: { required: true } })` returns `'Please confirm your password'`.
  6. `confirmError({ value: 'a', touched: true, errors: null, groupMismatch: true })` returns `'Passwords do not match'`.
  7. `confirmError({ value: 'a', touched: false, errors: null })` returns `null` (no spurious visibility pre-touch).
  8. `usernameError({ value: '', touched: false, errors: { required: true } })` returns `null` (no spurious visibility pre-touch).
- **How to run:** `npm test` (root). Selectors: `npm run test:frontend` for fast feedback. No browser, no screenshot, no Visual regression.
- **Out of scope (explicitly):** TestBed rendering, HTTP mock for `SetupCreateAdminService.create`, fixture-based aria assertion, and end-to-end visual confirmation — all forbidden by the test rules.