From 2ae930f3252c15e3b37b60890e07d886c6a85a63 Mon Sep 17 00:00:00 2001 From: m0rph3us1987 Date: Tue, 21 Jul 2026 17:18:54 +0000 Subject: [PATCH] AI Implementation feature(846): First-Start Create Admin Modal 1.05 (#8) --- .kilo/plans/845.md | 147 --------------------- .kilo/plans/846.md | 52 ++++++++ backend/src/modules/setup/setup.service.ts | 96 +++++++++----- docs/api/setup.md | 42 ++++-- docs/architecture/backend-modules.md | 4 +- docs/guides/bootstrap.md | 13 +- docs/index.md | 2 +- tests/backend/setup-create-admin.spec.ts | 55 ++++++-- 8 files changed, 208 insertions(+), 203 deletions(-) delete mode 100644 .kilo/plans/845.md create mode 100644 .kilo/plans/846.md diff --git a/.kilo/plans/845.md b/.kilo/plans/845.md deleted file mode 100644 index da73886..0000000 --- a/.kilo/plans/845.md +++ /dev/null @@ -1,147 +0,0 @@ -# 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`, `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> | 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>`) 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 `` — keep existing `[attr.aria-invalid]="!!usernameError()"` and existing `` block, but rebind to the new method name. -2. Password `