diff --git a/.kilo/plans/895.md b/.kilo/plans/895.md deleted file mode 100644 index 3caae65..0000000 --- a/.kilo/plans/895.md +++ /dev/null @@ -1,276 +0,0 @@ -#[ALREADY_IMPLEMENTED] - -# Implementation Plan: Admin Area General Settings and Categories 1.13 (Job 895) - -## Status - -**The job's required behavior is already fully implemented in this repository.** -No additional source changes, schema migrations, or new tests are required to -satisfy the acceptance criteria. The components, validators, endpoints, and -documented behavior already exist; the failure modes described in the bug -report (HTTP 401, malformed `datetime-local`, End ≤ Start, invalid challenge -IP, missing event-state derivation) are all addressed by code that already -lives in the repo. - -This plan therefore documents **where** each requirement is already fulfilled -and explains the evidence under `/repo`. The implementer should NOT add new -features — only verify the existing tests pass and the existing code path -matches the acceptance criteria. - ---- - -## 1. Architectural Reconnaissance - -- **Codebase style & conventions:** TypeScript end-to-end. NestJS 10 + Express - on the backend (modular structure under `backend/src/modules/**`), Angular - 17 standalone components with signals + `ChangeDetectionStrategy.OnPush` on - the frontend under `frontend/src/app/features/**`. Pure helpers live - alongside their consumers (`*.pure.ts`). -- **Data Layer:** SQLite via `better-sqlite3`, accessed through `DatabaseService` - and a `SettingsService` that persists key/value rows in a `setting` table. - Migrations live under `backend/src/database/migrations/`. Persistent user - data and uploads live under `/data/hipctf` (created by `setup.sh`). -- **Test Framework & Structure:** Jest 29 + `ts-jest`. A single root Jest - config at `tests/jest.config.js` defines two projects (`backend` = - `ts-jest` + `node`, `frontend` = `ts-jest` + `jsdom`). All tests live - under the dedicated `tests/` folder (never co-located with source) and - are runnable from the root with `npm test` (and `npm run test:backend`, - `npm run test:frontend`). -- **Required Tools & Dependencies:** No new dependencies are needed. The - existing `package.json` already declares `jest`, `ts-jest`, `jsdom`, - `jest-environment-jsdom`, `@types/jest`, `@types/jsdom`, `typescript`, - `sharp`, plus the workspace dependencies for NestJS and Angular. The - `setup.sh` script already installs root + workspace deps, rebuilds - `better-sqlite3`, and builds the frontend and backend. - -### Frontend / backend topology - -- The Angular SPA is built into `frontend/dist/` by `npm --workspace frontend run build`. -- `main.ts` (backend) statically serves `frontend/dist` (or `…/browser`) when - present, and `SpaFallbackMiddleware` (`backend/src/frontend/spa.controller.ts`) - serves `index.html` for any non-`/api` non-asset `GET`. **No Angular dev - proxy is required** — the SPA and the API share the same origin (default - `http://localhost:3000`), which is exactly what the bug report calls - out as the working configuration. -- All in-app HTTP calls use **relative** `/api/v1/...` paths - (`frontend/src/app/core/services/admin.service.ts:76-141` and throughout - `auth.service.ts`, `bootstrap.service.ts`, …). The `withCredentials: true` - flag is set on every call so the refresh-token cookie is sent. - ---- - -## 2. Impacted Files - -### Existing files that already implement the job (no edits required) - -No files will be modified. The job is fully covered by the following -already-present files: - -- **Frontend pure helpers** — `frontend/src/app/features/admin/general.pure.ts` -- **Frontend component** — `frontend/src/app/features/admin/general.component.ts` -- **Frontend service** — `frontend/src/app/core/services/admin.service.ts` -- **Frontend SPA wiring** — `backend/src/frontend/spa.controller.ts` + `backend/src/main.ts` -- **Frontend tests** — `tests/frontend/admin-general-pure.spec.ts` -- **Backend DTO** — `backend/src/modules/admin/dto/general.dto.ts` -- **Backend controller** — `backend/src/modules/admin/admin-general.controller.ts` -- **Backend service** — `backend/src/modules/admin/general.service.ts` -- **Backend tests** — `tests/backend/admin-general-event-window.spec.ts`, `tests/backend/admin-general-event.spec.ts`, `tests/backend/admin-general-list-themes.spec.ts`, `tests/backend/admin-general-service.spec.ts`, `tests/backend/admin-validation.spec.ts` -- **Categories (already embedded in the same page)** — `frontend/src/app/features/admin/categories/**`, `backend/src/modules/admin/admin-categories.controller.ts`, `backend/src/modules/admin/categories.service.ts`, `backend/src/modules/admin/dto/categories.dto.ts`, `tests/backend/admin-categories-service.spec.ts` - -### To Create - -None. - ---- - -## 3. Proposed Changes - -**None required.** Each acceptance criterion from the job is already -satisfied by the existing implementation. Evidence per requirement: - -### 1. "Accept valid UTC schedules" - -- `frontend/src/app/features/admin/general.pure.ts:126-140` defines - `toIsoUtc(local)`, which parses `YYYY-MM-DDTHH:mm[:ss[.fff]]` via a - regex and constructs `new Date(Date.UTC(...))` so the wall-clock - components are interpreted as **UTC** regardless of the browser's - timezone. The same file defines `toDatetimeLocal(iso)` which uses - `getUTC*` getters to render the picker. -- `backend/src/modules/admin/dto/general.dto.ts:57-58` requires both - `eventStartUtc` and `eventEndUtc` to be `z.string().datetime(...)` - (the zod ISO-8601 helper), and `superRefine` enforces - `eventEndUtc > eventStartUtc` (lines 62-72). -- Lifecycle: `AdminGeneralComponent.applySettings` (`general.component.ts:407-433`) - converts the stored UTC ISO strings to `datetime-local` strings on - load; `onSubmit` (lines 435-463) converts them back via `toIsoUtc` - before issuing `PUT /api/v1/admin/general/settings`. -- Locked in by `tests/frontend/admin-general-pure.spec.ts:79-112` - (datetime helpers) and `tests/backend/admin-general-event-window.spec.ts:19-22`. - -### 2. "Reject malformed values while preserving the prior schedule/value" - -- `isoDatetimeValidator` (`general.pure.ts:142-146`) returns - `{ invalidDatetime: true }` for empty/unparseable strings, including - `''` (this is the strict policy). -- `endAfterStartValidator` (`general.pure.ts:106-116`) returns - `{ endBeforeStart: true }` when both fields are valid and end is not - strictly after start. -- The reactive form uses `Validators.required` + the custom validators - (lines 231-243) and the Save button is bound to - `[disabled]="submitting() || form.invalid"` (line 184). When the - server returns `400 VALIDATION_FAILED`, the assignable values are - unchanged and the response is rendered into `general-save-error` - (lines 185-187). The component also calls `applySettings(updated)` - on success so the form is reset to its validated state. -- The backend `GeneralSettingsSchema` (`general.dto.ts:51-72`) applies - the same trim + ISO-8601 + ordering rules on the server. The - `SettingsService.set` calls in `backend/src/modules/admin/general.service.ts:62-71` - are only invoked after `ZodValidationPipe` has validated the body. -- Locked in by `tests/frontend/admin-general-pure.spec.ts:187-244` - (validator + message helpers) and `tests/backend/admin-general-event-window.spec.ts:26-67`. - -### 3. "Expose timestamp-derived event states and working controls" - -- `deriveEventState(start, end)` (`general.pure.ts:95-104`) returns - `'unconfigured' | 'countdown' | 'running' | 'stopped'` based on the - current `Date.now()`. -- `eventStateLabel` is a `computed` signal bound to the disabled - `general-event-toggle` button (`general.component.ts:247-252` and - template lines 174-176), so the label is always derived from the - current timestamps in the form. -- All other form controls are enabled; only the event-toggle button is - intentionally disabled (it's a status display, not an action). -- Locked in by `tests/frontend/admin-general-pure.spec.ts:18-49` (the - `deriveEventState` suite). - -### 4. "HTTP 401 UNAUTHORIZED on GET /api/v1/admin/general/settings" - -- `AdminGeneralController` (`backend/src/modules/admin/admin-general.controller.ts:11-13`) - is mounted under `UseGuards(AdminGuard)` and `@Roles('admin')`. -- The global `JwtAuthGuard` rejects unauthenticated requests with 401 - *before* the route handler runs. Verified by `tests/backend/admin-guard.spec.ts` - and `tests/backend/csrf-protected-routes.spec.ts`. The component - treats any load failure as `loadError` and renders - `general-error` (`general.component.ts:52-53, 379-382`). - -### 5. "Frontend requests to localhost:4200 returning 404 because no proxy was configured; API on localhost:3000" - -- This configuration is already prevented by the deployment topology: - the Angular SPA is built and served by the **same** NestJS process on - port 3000 (`backend/src/main.ts:96-105` + `backend/src/frontend/spa.controller.ts`). - All HTTP calls use **relative** `/api/v1/...` paths (see `admin.service.ts:76-141`), - so they resolve to `http://localhost:3000/api/v1/...` regardless of - where the SPA was opened from. No `proxy.conf.json` is required and - none is present in `frontend/angular.json`. -- `CORS_ORIGINS` defaults to `http://localhost:4200,http://localhost:3000` - (`main.ts:26`), so the SPA can also be opened from a dev server on - 4200 in development; in production both the SPA and the API live on - 3000. - -### 6. "datetime-local rejected malformed text at the browser control layer" - -- The native browser `datetime-local` rejects malformed text by - reporting the value as an empty string to JavaScript. The component - then sets the form value to `''`, which triggers - `isoDatetimeValidator` → `{ invalidDatetime: true }` → form invalid - → Save disabled. An inline error region - (`general-eventStart-error` / `general-eventEnd-error`) renders the - per-field message via `datetimeMessage` / `eventEndFieldMessage` - helpers (`general.pure.ts:148-176`, template lines 109-128). - -### 7. "End <= Start left Save disabled" - -- `endAfterStartValidator` (`general.pure.ts:106-116`) emits - `{ endBeforeStart: true }` form-level error. -- `eventEndFieldMessage` (`general.pure.ts:165-176`) renders - `"Event end must be after event start."` into - `general-eventEnd-error` when the per-field ISO check passes but the - cross-field `endBeforeStart` error is set. -- `template` lines 121-128 bind the inline error and apply - `aria-invalid="true"` / `aria-describedby` to the end input. -- Locked in by `tests/frontend/admin-general-pure.spec.ts:51-77, 227-244`. - -### 8. "Default challenge IP — valid IPv4 / hostname" - -- `isValidDefaultChallengeAddress` (`general.pure.ts:28-31`) accepts - full IPv4 (no leading zeros on multi-digit octets, four 0–255 parts) - or a valid RFC-1123 hostname with at least two labels and no - all-numeric labels. -- `defaultChallengeAddressError` / `defaultChallengeAddressMessage` - (`general.pure.ts:33-64`) produce the required and format messages. -- `defaultChallengeAddressValidator` (component line 394-405) attaches - `defaultChallengeAddressFormat` to the control when malformed. -- The backend `defaultChallengeIpSchema` (`general.dto.ts:22-49`) trims - and re-validates with `isIP` + the same hostname regex, emitting - `defaultChallengeIp must be a valid IPv4 address or hostname` on - failure. -- Locked in by `tests/frontend/admin-general-pure.spec.ts:246-328` and - `tests/backend/admin-general-event-window.spec.ts` (the schema - requires a valid `defaultChallengeIp` for any successful parse). - -### 9. "Admin Categories page alongside General settings" - -- Embedded via the `` element in the General - Settings template (`general.component.ts:195`). -- `backend/src/modules/admin/admin-categories.controller.ts` exposes - `GET/POST /api/v1/admin/categories` and `PUT/DELETE /:id`. -- `backend/src/modules/admin/categories.service.ts` enforces the - system-row protection, challenge-attached protection, and abbreviation - uppercase + uniqueness rules. Verified by - `tests/backend/admin-categories-service.spec.ts`. - -### 10. "Emit `general` SSE so other tabs see the new theme" - -- `AdminGeneralService.updateSettings` calls `SseHubService.emitEvent` - with `{ topic: 'general', themeKey, registrationsEnabled }` - (`backend/src/modules/admin/general.service.ts:72-76`). Documented in - `docs/guides/admin-general-settings.md` and `docs/api/admin.md`. - ---- - -## 4. Test Strategy - -**No new tests are required.** The repository already contains a single -focused Jest project per technology that locks in the relevant behavior -and is runnable from the root via `npm test`: - -- **Frontend logic (`tests/frontend/admin-general-pure.spec.ts`)** — - covers `deriveEventState`, `endAfterStartValidator`, `toIsoUtc`, - `toDatetimeLocal`, `isoDatetimeValidator`, `datetimeMessage`, - `eventEndFieldMessage`, `pageTitleError` / `pageTitleMessage`, - `isValidDefaultChallengeAddress` / `defaultChallengeAddressError` / - `defaultChallengeAddressMessage`, and `normalizeDefaultChallengeAddress`. - These exercise the pure helpers that the component uses to decide - when Save is enabled and which inline error message to render. -- **Frontend shell + state** — `tests/frontend/admin-shell.spec.ts`, - `tests/frontend/admin-navigation.spec.ts`, - `tests/frontend/shell-led.spec.ts`, `tests/frontend/shell-active-section.spec.ts`. -- **Backend validation** — - `tests/backend/admin-general-event-window.spec.ts` (likely-end sort - and end-before-start), `tests/backend/admin-general-event.spec.ts`, - `tests/backend/admin-general-service.spec.ts`, - `tests/backend/admin-general-list-themes.spec.ts`, - `tests/backend/admin-validation.spec.ts` (the zod schema). -- **Backend auth/guard** — `tests/backend/admin-guard.spec.ts`, - `tests/backend/csrf-protected-routes.spec.ts`. -- **Backend categories** — `tests/backend/admin-categories-service.spec.ts`. - -### Mocking strategy (already in place) - -- Frontend Jest project uses `jsdom` + `tests/frontend/jest.setup.ts` - and consumes only the pure helper module (`*.pure.ts`), so no - `TestBed`, HTTP mocks, or component fixtures are needed for the - validation regressions. -- Backend tests use `tests/backend/db-helper.ts` to provision an - isolated SQLite database under a temp directory and exercise the - `ZodValidationPipe` directly, so no HTTP-level mocking is needed for - the validation cases. - -### Verification command - -```bash -npm test -``` - -This single command runs all Jest projects (`backend` and `frontend`) -from the repo root and confirms every behavior the job requires. diff --git a/.kilo/plans/896.md b/.kilo/plans/896.md new file mode 100644 index 0000000..41da5e4 --- /dev/null +++ b/.kilo/plans/896.md @@ -0,0 +1,274 @@ +# Implementation Plan: Admin Area General Settings and Categories 1.14 (Job 896) + +## 0. Status — NOT yet implemented + +The required behaviour for this Job is **NOT** fully implemented in this +repository. The category edit modal opens but the `Name`, `Abbreviation`, +and `Description` reactive-form controls are not populated with the +selected category's existing values when the user clicks the edit button +(e.g. `cat-edit-CRY`), and the abbreviation input is therefore not marked +`readonly` for system rows. The bug is reproduced every time an existing +category is opened for edit, regardless of whether the user previously +opened it in create mode or not. + +Evidence (read-only inspection): +* `frontend/src/app/features/admin/categories/category-form-modal.component.ts:91-110` + declares a constructor `effect()` that *intends* to call + `this.form.patchValue({ name, abbreviation, description })` and + `this.abbreviationReadonly.set(c.isSystem)` when `mode() === 'edit' && + category()` is truthy. The rest of the source tree (parent component, + service, backend controller/service/DTO, schemas, migrations, and the + existing backend Jest suite) already implements everything the Guide + describes; only the frontend modal's edit-prefill step is broken. + +Because the fix lives entirely in one frontend component (and a new +Jest test), no DB migration, backend code change, dependency addition, +or `setup.sh` change is required. + +--- + +## 1. Architectural Reconnaissance + +- **Codebase style & conventions:** + - Frontend: Angular 17.3 (`/repo/frontend/package.json`), standalone + components, `ChangeDetectionStrategy.OnPush`, signal inputs / + outputs, reactive forms via `fb.nonNullable.group`. + - Backend: NestJS 10 + Express, modular layout under + `backend/src/modules/admin/**`, TypeORM + `better-sqlite3`. + - Tests: Jest 29 with two projects (`backend` = `ts-jest`+`node`, + `frontend` = `ts-jest`+`jsdom`) configured in + `/repo/tests/jest.config.js`. A single `npm test` from the repo + root runs every spec. Frontend specs live under `tests/frontend/` + alongside existing admin specs. +- **Data Layer:** + - SQLite via TypeORM; `category` table has + `id`, `system_key`, `name`, `abbreviation`, `description`, + `icon_path`, `created_at`, `updated_at` columns + (`backend/src/database/entities/category.entity.ts`). + - Categories are CRUD-ed via `AdminCategoriesService` which already + uppercases the abbreviation, protects system rows from + abbreviation edits, and rejects duplicate abbreviations + (`backend/src/modules/admin/categories.service.ts:39-78`). + - User-supplied persistent state is stored under `/data/hipctf` + (created by `setup.sh`); this job reads no project-shared data. +- **Test Framework & Structure:** + - Jest 29 (`ts-jest`). Two projects configured in + `tests/jest.config.js`. Frontend specs in `tests/frontend/`, + backend specs in `tests/backend/`. Single `npm test` (also wired + into `package.json` at the repo root) executes everything. + - No DOM harness library is configured; existing frontend specs + query the rendered DOM through `fixture.nativeElement.querySelector` + on the rendered template (see e.g. + `tests/frontend/admin-general-pure.spec.ts`). +- **Required Tools & Dependencies:** No new dependencies are needed. + All tooling required to fix and verify this job is already + installed by `setup.sh` and the root `package.json` / + `tests/package.json`. The implementer does **not** need to touch + `setup.sh`. + +### Component wiring already present in the repo + +| Layer | File | Status | +|-------|------|--------| +| Edit modal | `frontend/src/app/features/admin/categories/category-form-modal.component.ts` | **buggy** — edit pre-fill does not reach the DOM | +| Categories list + edit dispatcher | `frontend/src/app/features/admin/categories/categories.component.ts:109-114` | correct (`openEdit(c)` sets `editing` and `modalMode`) | +| General page (embeds categories) | `frontend/src/app/features/admin/general.component.ts:195` | already imports `AdminCategoriesComponent` | +| Admin service | `frontend/src/app/core/services/admin.service.ts:96-122` | correct (`listCategories`, `updateCategory`) | +| Backend controller | `backend/src/modules/admin/admin-categories.controller.ts` | correct | +| Backend service + DTO | `backend/src/modules/admin/categories.service.ts`, `backend/src/modules/admin/dto/categories.dto.ts` | correct | +| Backend tests | `tests/backend/admin-categories-service.spec.ts` | correct (system-row, dup, etc.) | +| Guide | `docs/guides/admin-categories.md` | correct (states the modal must pre-fill) | + +--- + +## 2. Impacted Files + +### To Modify (1 file) + +- `frontend/src/app/features/admin/categories/category-form-modal.component.ts` + — replace the broken constructor `effect()` with a robust Angular 17 + reactive-form prefill that always populates `name`, + `abbreviation`, `description`, sets `abbreviationReadonly` from + `c.isSystem`, and resets `iconPreview`/`iconFile` whenever the + parent changes the `category` or `mode` input. Must keep the + existing `[data-testid]` selectors intact. + +### To Create (1 file) + +- `tests/frontend/admin-categories-form-modal.spec.ts` + — Jest spec that mounts `CategoryFormModalComponent` with + `TestBed`, simulates the parent passing `open=true`, `mode='edit'`, + and a fully-populated `AdminCategory` (including `isSystem: true`), + then asserts that: + 1. `input[data-testid="cf-name"]` value equals the row's `name`, + 2. `input[data-testid="cf-abbr"]` value equals the row's + `abbreviation` *and* has the `readonly` attribute when + `isSystem === true`, + 3. `textarea[data-testid="cf-desc"]` value equals the row's + `description`, + 4. switching the inputs to a different category (a second + `setInput('category', ...)` / `setInput('mode', 'edit')` call) + re-populates the form to the new row's values, + 5. switching back to `mode='create'` clears the form. + +--- + +## 3. Proposed Changes + +### 1. Fix the edit-prefill in the modal (`category-form-modal.component.ts`) + +The bug: the existing constructor `effect()` reads +`this.category()` and `this.mode()` and calls +`this.form.patchValue(...)` inside an OnPush component. In Angular 17 +the value-update flow works, but the *display* of the patched value +inside the `` element is not reaching the +DOM because the change originates from a constructor +`effect()` while change-detection is sitting idle, and the +`formControlName` directive's write-back into the native `` +relies on the OnPush view being marked dirty. The same code path also +fails to mark `abbreviationReadonly`, which is exactly what the bug +report observes (abbreviation is not `readonly` and the three fields +read `''`). + +Replace the constructor body so that: + +1. We keep reading both inputs inside the effect: + `const c = this.category(); const m = this.mode();`. +2. We synchronise the form using **individual + `FormControl.setValue` calls** (not `patchValue` on the group), so + the directive chain inside each `FormControlName` runs the + `DefaultValueAccessor.writeValue` path that reaches the native + input. +3. After every prefill (edit or create-reset), we explicitly call + `inject(ChangeDetectorRef).markForCheck()` so the OnPush view + repaints. +4. We still set `abbreviationReadonly.set(c.isSystem)` and + `iconPreview.set(c.iconPath || null)` inside the `edit` branch, and + `abbreviationReadonly.set(false)` / + `iconPreview.set(null)` in the `else` branch. +5. We also clear `iconFile` (already done in current code) and reset + the `description` control in the create branch. +6. We add `allowSignalWrites: true` to the `effect()` options so that + the signal writes inside it are tolerated. + +Pseudocode for the replacement constructor: + +```ts +private readonly cdr = inject(ChangeDetectorRef); + +constructor() { + effect( + () => { + const c = this.category(); + const m = this.mode(); + if (m === 'edit' && c) { + this.form.controls.name.setValue(c.name, { emitEvent: false }); + this.form.controls.abbreviation.setValue(c.abbreviation, { emitEvent: false }); + this.form.controls.description.setValue(c.description ?? '', { emitEvent: false }); + this.abbreviationReadonly.set(!!c.isSystem); + this.iconPreview.set(c.iconPath || null); + } else { + this.form.controls.name.setValue('', { emitEvent: false }); + this.form.controls.abbreviation.setValue('', { emitEvent: false }); + this.form.controls.description.setValue('', { emitEvent: false }); + this.abbreviationReadonly.set(false); + this.iconPreview.set(null); + } + this.iconFile.set(null); + this.form.controls.name.markAsUntouched(); + this.form.controls.name.markAsPristine(); + this.form.controls.abbreviation.markAsUntouched(); + this.form.controls.abbreviation.markAsPristine(); + this.form.controls.description.markAsUntouched(); + this.form.controls.description.markAsPristine(); + this.cdr.markForCheck(); + }, + { allowSignalWrites: true }, + ); +} +``` + +All other parts of the modal (template selectors `cf-name`, +`cf-abbr`, `cf-desc`, `cf-icon`, `cf-cancel`, `cf-ok`, +`cat-form-modal`, `cat-form-backdrop`; the `CategoryFormSubmit` +output; the icon FileReader; the readonly binding on the abbreviation +input) stay exactly the same, so the parent +`AdminCategoriesComponent` and its `submit` handler do not need to +change. + +### 2. Add a frontend Jest spec (`admin-categories-form-modal.spec.ts`) + +The spec file lives under `tests/frontend/`. It runs with the existing +`tests/jest.config.js` `frontend` project (jest + jsdom), is picked up +by `npm test` from the repo root, requires no UI, and asserts the +behaviour the bug report says is broken. + +Mocking strategy: +* No HTTP mocks are required — the modal is rendered in isolation with + its three inputs (`open`, `mode`, `category`) bound directly via + `fixture.componentRef.setInput(...)`. `AdminService` is unused by + the modal. +* Construct `CategoryFormModalComponent` through `TestBed` and target + by querying the existing `data-testid` attributes + (`cf-name`, `cf-abbr`, `cf-desc`). No CSS selectors are introduced. +* Provide `FormBuilder` and `DestroyRef` via DI (already provided + via `inject(FormBuilder)` and `inject(DestroyRef)` because + `TestBed.createComponent` resolves Angular's DI tree). + +Spec assertions (single `describe('edit prefill')`, four `it`s): + +1. `populates name, abbreviation, description for a system row` — + set `open=true`, `mode='edit'`, + `category = { id:'x', name:'Cryptography', abbreviation:'CRY', + description:'Cryptographic challenges', iconPath:'', + isSystem:true }`, call `fixture.detectChanges()`, + read each input's `.value`, expect the row's values, and expect + `cf-abbr` to carry the `readonly` attribute. +2. `populates name, abbreviation, description for a user row` — same + shape but `isSystem: false`; expect `cf-abbr` to NOT be `readonly`. +3. `re-populates when the parent switches to another row` — + `setInput('mode','edit')` with row A, `detectChanges()`, assert + values, then `setInput('category', rowB)`, `detectChanges()`, + assert new values. +4. `clears the form when the parent switches back to create` — + after the edit-prefill assertions, `setInput('mode','create')`, + `setInput('category', null)`, `detectChanges()`, assert empty + values. + +The spec is intentionally minimal (5 assertions per case, no async +timers, no DOM harnesses) so it compiles and runs instantly. + +--- + +## 4. Test Strategy + +- **Target spec file:** + `tests/frontend/admin-categories-form-modal.spec.ts` (new). +- **Mocking strategy:** no HTTP mocks, no service mocks. The modal is + component-only; `fixture.componentRef.setInput(...)` simulates the + parent setting the `open`, `mode`, `category` signals, and + `fixture.detectChanges()` flushes the patched values to the DOM + via the OnPush view marker we add. +- **Coverage scope:** the four bullets above plus a smoke check that + `submit` and `cancel` outputs are not emitted by the modal alone + (the parent owns the network call). +- **Run command:** `npm test` from the repository root + (`tests/jest.config.js` is the active config). The new spec runs + inside the `frontend` project and finishes in well under a second + with no network or DB activity. +- **Existing tests:** re-run `tests/backend/admin-categories-service.spec.ts` + to confirm the system-row / dup / CRUD rules still pass; the + frontend modal change has no backend impact. + +--- + +## 5. Files Touched — Summary + +| Path | Action | +|------|--------| +| `frontend/src/app/features/admin/categories/category-form-modal.component.ts` | edit — replace constructor `effect()` body with the explicit-`setValue` + `markForCheck` version above; add `inject(ChangeDetectorRef)` and `{ allowSignalWrites: true }` option | +| `tests/frontend/admin-categories-form-modal.spec.ts` | create — the four `it()`s described in §3.2 | + +No backend, schema, DTO, controller, service, route, migration, or +dependency change is required, and `setup.sh` does not need to be +modified. diff --git a/frontend/src/app/features/admin/categories/category-form-modal.component.ts b/frontend/src/app/features/admin/categories/category-form-modal.component.ts index 654e862..1326dad 100644 --- a/frontend/src/app/features/admin/categories/category-form-modal.component.ts +++ b/frontend/src/app/features/admin/categories/category-form-modal.component.ts @@ -1,6 +1,6 @@ -import { ChangeDetectionStrategy, Component, DestroyRef, effect, inject, input, output, signal } from '@angular/core'; +import { ChangeDetectionStrategy, ChangeDetectorRef, Component, DestroyRef, effect, inject, input, output, signal } from '@angular/core'; import { CommonModule } from '@angular/common'; -import { FormBuilder, ReactiveFormsModule, Validators } from '@angular/forms'; +import { FormBuilder, FormControl, FormGroup, ReactiveFormsModule, Validators } from '@angular/forms'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { AdminCategory } from '../../../core/services/admin.service'; @@ -15,6 +15,41 @@ export interface CategoryFormSubmit { iconFile?: File; } +export type CategoryFormGroup = FormGroup<{ + name: FormControl; + abbreviation: FormControl; + description: FormControl; +}>; + +export function syncCategoryForm( + form: CategoryFormGroup, + mode: CategoryFormMode, + category: AdminCategory | null, +): { abbreviationReadonly: boolean; iconPreview: string | null } { + if (mode === 'edit' && category) { + form.controls.name.setValue(category.name ?? '', { emitEvent: false }); + form.controls.abbreviation.setValue(category.abbreviation ?? '', { emitEvent: false }); + form.controls.description.setValue(category.description ?? '', { emitEvent: false }); + form.controls.name.markAsUntouched(); + form.controls.name.markAsPristine(); + form.controls.abbreviation.markAsUntouched(); + form.controls.abbreviation.markAsPristine(); + form.controls.description.markAsUntouched(); + form.controls.description.markAsPristine(); + return { abbreviationReadonly: !!category.isSystem, iconPreview: category.iconPath || null }; + } + form.controls.name.setValue('', { emitEvent: false }); + form.controls.abbreviation.setValue('', { emitEvent: false }); + form.controls.description.setValue('', { emitEvent: false }); + form.controls.name.markAsUntouched(); + form.controls.name.markAsPristine(); + form.controls.abbreviation.markAsUntouched(); + form.controls.abbreviation.markAsPristine(); + form.controls.description.markAsUntouched(); + form.controls.description.markAsPristine(); + return { abbreviationReadonly: false, iconPreview: null }; +} + @Component({ selector: 'app-category-form-modal', standalone: true, @@ -69,6 +104,7 @@ export interface CategoryFormSubmit { export class CategoryFormModalComponent { private readonly fb = inject(FormBuilder); private readonly destroyRef = inject(DestroyRef); + private readonly cdr = inject(ChangeDetectorRef); readonly open = input(false); readonly mode = input('create'); @@ -82,31 +118,23 @@ export class CategoryFormModalComponent { readonly submitting = signal(false); readonly abbreviationReadonly = signal(false); - readonly form = this.fb.nonNullable.group({ + readonly form: CategoryFormGroup = this.fb.nonNullable.group({ name: this.fb.nonNullable.control('', [Validators.required, Validators.maxLength(120)]), abbreviation: this.fb.nonNullable.control('', [Validators.required, Validators.minLength(2), Validators.maxLength(6)]), description: this.fb.nonNullable.control(''), }); constructor() { - effect(() => { - const c = this.category(); - const m = this.mode(); - if (m === 'edit' && c) { - this.form.patchValue({ - name: c.name, - abbreviation: c.abbreviation, - description: c.description, - }); - this.abbreviationReadonly.set(c.isSystem); - this.iconPreview.set(c.iconPath || null); - } else { - this.form.patchValue({ name: '', abbreviation: '', description: '' }); - this.abbreviationReadonly.set(false); - this.iconPreview.set(null); - } - this.iconFile.set(null); - }); + effect( + () => { + const result = syncCategoryForm(this.form, this.mode(), this.category()); + this.abbreviationReadonly.set(result.abbreviationReadonly); + this.iconPreview.set(result.iconPreview); + this.iconFile.set(null); + this.cdr.markForCheck(); + }, + { allowSignalWrites: true }, + ); } onFile(ev: Event): void { diff --git a/tests/frontend/admin-categories-form-modal.spec.ts b/tests/frontend/admin-categories-form-modal.spec.ts new file mode 100644 index 0000000..4df51ea --- /dev/null +++ b/tests/frontend/admin-categories-form-modal.spec.ts @@ -0,0 +1,125 @@ +import '@angular/compiler'; +import { FormBuilder, FormControl, FormGroup } from '@angular/forms'; +import { + syncCategoryForm, + CategoryFormMode, +} from '../../frontend/src/app/features/admin/categories/category-form-modal.component'; +import { AdminCategory } from '../../frontend/src/app/core/services/admin.service'; + +function buildForm(): FormGroup<{ + name: FormControl; + abbreviation: FormControl; + description: FormControl; +}> { + const fb = new FormBuilder(); + return fb.nonNullable.group({ + name: fb.nonNullable.control('', []), + abbreviation: fb.nonNullable.control('', []), + description: fb.nonNullable.control(''), + }); +} + +function makeRow(overrides: Partial = {}): AdminCategory { + return { + id: 'row-1', + name: 'Cryptography', + abbreviation: 'CRY', + description: 'Cryptographic challenges', + iconPath: '', + isSystem: true, + systemKey: 'CRY', + createdAt: '', + updatedAt: '', + ...overrides, + }; +} + +describe('syncCategoryForm - edit prefill', () => { + it('populates name, abbreviation, description for a system row and locks abbreviation', () => { + const form = buildForm(); + const result = syncCategoryForm(form, 'edit', makeRow()); + + expect(form.controls.name.value).toBe('Cryptography'); + expect(form.controls.abbreviation.value).toBe('CRY'); + expect(form.controls.description.value).toBe('Cryptographic challenges'); + expect(result.abbreviationReadonly).toBe(true); + expect(result.iconPreview).toBeNull(); + }); + + it('does not lock abbreviation for a user (non-system) row', () => { + const form = buildForm(); + const result = syncCategoryForm( + form, + 'edit', + makeRow({ + name: 'Misc', + abbreviation: 'MSC', + description: 'Other', + isSystem: false, + systemKey: null, + }), + ); + + expect(form.controls.name.value).toBe('Misc'); + expect(form.controls.abbreviation.value).toBe('MSC'); + expect(form.controls.description.value).toBe('Other'); + expect(result.abbreviationReadonly).toBe(false); + }); + + it('re-populates when invoked a second time with a different category', () => { + const form = buildForm(); + syncCategoryForm( + form, + 'edit', + makeRow({ name: 'Alpha', abbreviation: 'AAA', description: 'first', isSystem: false, systemKey: null }), + ); + expect(form.controls.name.value).toBe('Alpha'); + expect(form.controls.abbreviation.value).toBe('AAA'); + + const result = syncCategoryForm( + form, + 'edit', + makeRow({ name: 'Bravo', abbreviation: 'BBB', description: 'second', isSystem: false, systemKey: null }), + ); + + expect(form.controls.name.value).toBe('Bravo'); + expect(form.controls.abbreviation.value).toBe('BBB'); + expect(form.controls.description.value).toBe('second'); + expect(result.abbreviationReadonly).toBe(false); + }); + + it('clears the form when invoked with mode="create" and a null category', () => { + const form = buildForm(); + syncCategoryForm(form, 'edit', makeRow()); + expect(form.controls.name.value).toBe('Cryptography'); + + const result = syncCategoryForm(form, 'create', null); + + expect(form.controls.name.value).toBe(''); + expect(form.controls.abbreviation.value).toBe(''); + expect(form.controls.description.value).toBe(''); + expect(result.abbreviationReadonly).toBe(false); + expect(result.iconPreview).toBeNull(); + }); + + it('marks controls as pristine and untouched after prefill', () => { + const form = buildForm(); + syncCategoryForm(form, 'edit', makeRow()); + expect(form.controls.name.pristine).toBe(true); + expect(form.controls.name.touched).toBe(false); + expect(form.controls.abbreviation.pristine).toBe(true); + expect(form.controls.abbreviation.touched).toBe(false); + expect(form.controls.description.pristine).toBe(true); + expect(form.controls.description.touched).toBe(false); + }); + + it('returns iconPreview from category.iconPath when set', () => { + const form = buildForm(); + const result = syncCategoryForm( + form, + 'edit', + makeRow({ iconPath: '/uploads/icons/cry.png' }), + ); + expect(result.iconPreview).toBe('/uploads/icons/cry.png'); + }); +}); \ No newline at end of file diff --git a/tests/jest.config.js b/tests/jest.config.js index 46bbb3c..d843500 100644 --- a/tests/jest.config.js +++ b/tests/jest.config.js @@ -17,7 +17,13 @@ module.exports = { roots: [__dirname], testMatch: ['/frontend/**/*.spec.ts'], transform: { - '^.+\\.ts$': ['ts-jest', { tsconfig: '/frontend.tsconfig.json' }], + '^.+\\.(ts|js|mjs)$': ['ts-jest', { tsconfig: '/frontend.tsconfig.json', useESM: false }], + }, + transformIgnorePatterns: [ + '/node_modules/(?!(@angular|rxjs|tslib)/)', + ], + moduleNameMapper: { + '^(\\.{1,2}/.*)\\.mjs$': '$1', }, setupFiles: ['/frontend/jest.setup.ts'], },