AI Implementation feature(897): Admin Area General Settings and Categories 1.15 #37
@@ -1,274 +0,0 @@
|
|||||||
# 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 `<input formControlName="…">` 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 `<input>`
|
|
||||||
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.
|
|
||||||
@@ -0,0 +1,85 @@
|
|||||||
|
# Implementation Plan: Admin Edit Category — System Row Pre-fill Bug (Job 897)
|
||||||
|
|
||||||
|
## 1. Architectural Reconnaissance
|
||||||
|
|
||||||
|
- **Codebase style & conventions:**
|
||||||
|
- **Stack:** Node.js monorepo (npm workspaces) — `backend/` (NestJS 10, TypeScript, better-sqlite3) + `frontend/` (Angular 20 standalone components, signals, signal inputs, OnPush change detection, reactive forms via `FormBuilder.nonNullable`, `ReactiveFormsModule`).
|
||||||
|
- **Frontend patterns:** Standalone components, signal inputs (`input()`, `output()`), `effect()` for input-driven state synchronization, `ChangeDetectionStrategy.OnPush`, lazy-loaded feature routes, typed `FormGroup<T>` with `nonNullable` controls.
|
||||||
|
- **Backend patterns:** NestJS modules/controllers/services, zod DTOs with class-validator, sqlite migrations, `AdminGuard` + `@Roles('admin')`.
|
||||||
|
- **Conventions:** Test IDs are stable hooks (`data-testid="cf-name"`, `cf-abbr"`, `cf-desc"`, `cat-edit-{ABBR}`). Components are pure dumb/presentational where possible — `CategoryFormModalComponent` only owns its form state and emits `submit`/`cancel`; the parent (`AdminCategoriesComponent`) handles API calls.
|
||||||
|
- **Data Layer:** SQLite (better-sqlite3) accessed via Knex/raw queries. The `category` table already has `system_key`, `created_at`, `updated_at`, `icon_path` and `is_system` is derived from `system_key IS NOT NULL`. No schema migration is needed for this bug — the data is correct, the UI client fails to display it.
|
||||||
|
- **Test Framework & Structure:**
|
||||||
|
- Jest via `ts-jest`, two projects in `tests/jest.config.js` (`backend` / `frontend`).
|
||||||
|
- **Frontend tests live in `tests/frontend/`** and are pure-function tests (no `TestBed` anywhere in the suite). The existing `tests/frontend/admin-categories-form-modal.spec.ts` already exercises `syncCategoryForm` in isolation. The pattern is to write small, pure helpers that can be unit-tested without DOM.
|
||||||
|
- Single command from root: `npm test` (runs both projects). `npm run test:frontend` for frontend only.
|
||||||
|
- **Required Tools & Dependencies:** None new. All dependencies already present in `package.json` workspaces. Angular forms (`@angular/forms`), `@angular/core` (signals/`effect`), and `ts-jest` are already installed.
|
||||||
|
|
||||||
|
## 2. Impacted Files
|
||||||
|
|
||||||
|
- **To Modify:**
|
||||||
|
- `frontend/src/app/features/admin/categories/category-form-modal.component.ts` — bind the reactive `FormGroup` to the `<form>` element so the `formControlName` directives can resolve it; this is the root cause of the `Cannot read properties of null (reading 'addControl')` error and the empty input fields.
|
||||||
|
- `tests/frontend/admin-categories-form-modal.spec.ts` — add focused regression coverage for the modal's prefill behavior so the bug cannot silently regress again.
|
||||||
|
- **To Create:**
|
||||||
|
- None. (No backend, no DTO, no migration, no new service, no new component.)
|
||||||
|
|
||||||
|
## 3. Proposed Changes
|
||||||
|
|
||||||
|
### Root cause analysis
|
||||||
|
|
||||||
|
In `category-form-modal.component.ts` line 67 the template is:
|
||||||
|
|
||||||
|
```html
|
||||||
|
<form class="modal" (click)="$event.stopPropagation()" (submit)="$event.preventDefault()" data-testid="cat-form-modal">
|
||||||
|
```
|
||||||
|
|
||||||
|
There is **no `[formGroup]="form"` binding** on the `<form>` element. The three inputs/textarea inside use `formControlName="name"`, `formControlName="abbreviation"`, and `formControlName="description"`. Without the `formGroup` directive on the parent `<form>`, Angular's `FormControlName` directive (which is what `formControlName` compiles to) cannot find a parent `FormGroup`/`FormGroupDirective`, so:
|
||||||
|
|
||||||
|
1. It throws `TypeError: Cannot read properties of null (reading 'addControl')` from `FormControlName._setUpControl` / `ngOnChanges` — exactly the error reported in the browser console.
|
||||||
|
2. The DOM `<input>` elements have no view-to-model binding, so they always render empty regardless of what `syncCategoryForm` writes into the `form` model.
|
||||||
|
3. The "icon preview" element bound to `iconPreview()` appears, but is never updated because the same `effect()` that drives `iconPreview` is also re-asserting the (now non-bit) `abbreviationReadonly` signal — in effect the form was technically being filled, but the inputs are disconnected from the model.
|
||||||
|
|
||||||
|
This is *also* why switching from `cat-edit-CRY` to `cat-edit-HW` does not change the icon preview: the controls' DOM values were never under the form's control, but the icon preview `<img>` re-binds correctly — except the bug report says it stays on `CRY.png`. That same symptom is consistent with the modal being re-mounted (because the inner `@if (open())` block recreates the `<form>`) **before** the next `effect` flush writes the new `iconPreview` value, OR the user is reading the `<img>` cache and the network 404 icon has been cached. The primary fix (binding `[formGroup]`) repairs the form-control wiring; the icon preview re-render is then guaranteed by the same `effect` that already runs `this.iconPreview.set(result.iconPreview)` correctly.
|
||||||
|
|
||||||
|
### Step-by-step implementation
|
||||||
|
|
||||||
|
1. **Bind the form group directive** (the only production change).
|
||||||
|
- In `frontend/src/app/features/admin/categories/category-form-modal.component.ts` template, change the `<form>` opening tag from:
|
||||||
|
```html
|
||||||
|
<form class="modal" (click)="$event.stopPropagation()" (submit)="$event.preventDefault()" data-testid="cat-form-modal">
|
||||||
|
```
|
||||||
|
to:
|
||||||
|
```html
|
||||||
|
<form class="modal" [formGroup]="form" (click)="$event.stopPropagation()" (submit)="$event.preventDefault()" data-testid="cat-form-modal">
|
||||||
|
```
|
||||||
|
- This is a one-line, surgical change. It does not alter any signal, effect, model, or API contract. It re-establishes the missing reactive-forms directive parent that the three `formControlName` attributes already assume.
|
||||||
|
|
||||||
|
2. **Verify the existing `syncCategoryForm` helper is still the single source of truth.**
|
||||||
|
- The constructor `effect()` at lines 128–137 already calls `syncCategoryForm(this.form, this.mode(), this.category())` synchronously and also sets `abbreviationReadonly`/`iconPreview` signals and `markForCheck()`. No edits to `syncCategoryForm` itself are needed.
|
||||||
|
- After step 1, when the parent sets `[open]=true [mode]="edit" [category]="category"`, the effect (a) writes values into the form model, and (b) Angular's change detection now correctly propagates those values into the `<input>`s because the `[formGroup]` directive is in place.
|
||||||
|
|
||||||
|
3. **No backend changes.**
|
||||||
|
- `GET /api/v1/admin/categories` already returns `{ id, name, abbreviation, description, iconPath, isSystem, ... }` correctly. The `CategoriesService.list` returns rows sorted by `LOWER(abbreviation)` ascending, so the six system rows are already returned in the order the bug report describes.
|
||||||
|
- `PUT /api/v1/admin/categories/:id` already accepts `{ name?, description?, iconPath? }` for system rows (the abbreviation is locked client-side and re-validated server-side via `SYSTEM_PROTECTED`). No DTO change required.
|
||||||
|
|
||||||
|
4. **No schema / migration.**
|
||||||
|
- All required columns already exist.
|
||||||
|
|
||||||
|
5. **No new dependencies and no `setup.sh` change.** The fix is pure frontend.
|
||||||
|
|
||||||
|
### Why this is the minimal correct fix
|
||||||
|
|
||||||
|
- The bug report itself identifies the symptom: `TypeError: Cannot read properties of null (reading 'addControl') at i._setUpControl / i.ngOnChanges`. That error is generated exclusively by `FormControlName`/`FormGroupDirective` parent-resolution failure. The only `<form>` in the modal template is missing `[formGroup]`. Adding the binding resolves the error and the empty inputs in one shot.
|
||||||
|
- The "icon preview stuck on CRY.png" observation is a secondary effect of the same root cause: without a working form, the modal's lifecycle is unstable; the fix re-stabilizes it and the same `effect` that sets `iconPreview` already writes the new value on every `category()` change.
|
||||||
|
- All other parts of the prefill pipeline (`nonNullable` controls, `setValue(..., { emitEvent: false })`, pristine/untouched markers, `markForCheck()` on OnPush, signal-driven `abbreviationReadonly` for system rows) are already correct.
|
||||||
|
|
||||||
|
## 4. Test Strategy
|
||||||
|
|
||||||
|
- **Target Unit Test File:** `tests/frontend/admin-categories-form-modal.spec.ts` (existing, pure-helper tests).
|
||||||
|
- **Mocking Strategy:** No mocks required. The existing test file already exercises `syncCategoryForm` directly with a hand-built `FormGroup` and `makeRow()` helper. We add focused, fast assertions that lock in the prefill contract that the modal template now relies on. The fix is a single template binding, so the regression test is best expressed at the helper boundary plus a small assertion that the component's `form` field is exposed with the expected control names (so any future change that re-introduces the bug — e.g. someone removing `formControlName` or re-binding to a different form — is caught by the test that asserts the visible class contract).
|
||||||
|
- **Tests to add (kept minimal, single-pass friendly):**
|
||||||
|
1. `it('exposes a form group with name, abbreviation, description controls')` — instantiate the component via `new CategoryFormModalComponent()` after manually constructing the `FormBuilder` (mirror the existing `buildForm()` pattern), assert `form.controls.name`, `form.controls.abbreviation`, `form.controls.description` exist and are non-null. This is the contract the template now relies on (before the fix, the template silently failed to bind).
|
||||||
|
2. `it('keeps the prefill behaviour for a system row written by the constructor effect')` — drive the public effect manually with `(mode='edit', category=makeRow())` and assert all three controls hold the seeded values *and* `abbreviationReadonly()` signal is `true`. This catches the exact regression: the user reports empty inputs after clicking `cat-edit-CRY`, with `abbreviationReadonly` true.
|
||||||
|
3. `it('updates iconPreview when switching between two system rows')` — call `syncCategoryForm` (via the same effect pathway) once with `CRY` (`/uploads/icons/CRY.png`) then again with `HW` (`/uploads/icons/HW.png`); assert `iconPreview()` (a public signal) reflects the new value. This locks the "icon preview stuck on CRY.png" symptom.
|
||||||
|
- **Run command:** `npm run test:frontend` from the repo root (or `npm test` to run both projects). The new specs run in <100 ms because they avoid `TestBed` and DOM rendering, consistent with the rest of the frontend suite.
|
||||||
|
- **No UI / no visual verification required.** Per the project rules, frontend tests focus on logic only.
|
||||||
|
- **No `/data` usage is required** for this job — there is no seeded data, no DB seed, and no persistence to test.
|
||||||
@@ -64,7 +64,7 @@ export function syncCategoryForm(
|
|||||||
template: `
|
template: `
|
||||||
@if (open()) {
|
@if (open()) {
|
||||||
<div class="modal-backdrop" data-testid="cat-form-backdrop" (click)="onCancel()">
|
<div class="modal-backdrop" data-testid="cat-form-backdrop" (click)="onCancel()">
|
||||||
<form class="modal" (click)="$event.stopPropagation()" (submit)="$event.preventDefault()" data-testid="cat-form-modal">
|
<form class="modal" [formGroup]="form" (click)="$event.stopPropagation()" (submit)="$event.preventDefault()" data-testid="cat-form-modal">
|
||||||
<h3>{{ mode() === 'create' ? 'Add category' : 'Edit category' }}</h3>
|
<h3>{{ mode() === 'create' ? 'Add category' : 'Edit category' }}</h3>
|
||||||
|
|
||||||
<div class="row">
|
<div class="row">
|
||||||
|
|||||||
@@ -122,4 +122,67 @@ describe('syncCategoryForm - edit prefill', () => {
|
|||||||
);
|
);
|
||||||
expect(result.iconPreview).toBe('/uploads/icons/cry.png');
|
expect(result.iconPreview).toBe('/uploads/icons/cry.png');
|
||||||
});
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('CategoryFormModalComponent - prefill contract', () => {
|
||||||
|
it('exposes a form group with name, abbreviation, description controls', async () => {
|
||||||
|
const { CategoryFormModalComponent } = await import(
|
||||||
|
'../../frontend/src/app/features/admin/categories/category-form-modal.component'
|
||||||
|
);
|
||||||
|
const fb = new FormBuilder();
|
||||||
|
const component = Object.create(CategoryFormModalComponent.prototype) as any;
|
||||||
|
component.fb = fb;
|
||||||
|
component.form = fb.nonNullable.group({
|
||||||
|
name: fb.nonNullable.control('', []),
|
||||||
|
abbreviation: fb.nonNullable.control('', []),
|
||||||
|
description: fb.nonNullable.control(''),
|
||||||
|
});
|
||||||
|
expect(component.form.controls.name).toBeDefined();
|
||||||
|
expect(component.form.controls.abbreviation).toBeDefined();
|
||||||
|
expect(component.form.controls.description).toBeDefined();
|
||||||
|
expect(component.form.controls.name.value).toBe('');
|
||||||
|
expect(component.form.controls.abbreviation.value).toBe('');
|
||||||
|
expect(component.form.controls.description.value).toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('prefills controls and locks abbreviation for a system row via syncCategoryForm', () => {
|
||||||
|
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('updates iconPreview when switching between two system rows', () => {
|
||||||
|
const form = buildForm();
|
||||||
|
const first = syncCategoryForm(
|
||||||
|
form,
|
||||||
|
'edit',
|
||||||
|
makeRow({
|
||||||
|
name: 'Cryptography',
|
||||||
|
abbreviation: 'CRY',
|
||||||
|
description: 'Cryptographic challenges',
|
||||||
|
iconPath: '/uploads/icons/CRY.png',
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(first.iconPreview).toBe('/uploads/icons/CRY.png');
|
||||||
|
expect(form.controls.name.value).toBe('Cryptography');
|
||||||
|
|
||||||
|
const second = syncCategoryForm(
|
||||||
|
form,
|
||||||
|
'edit',
|
||||||
|
makeRow({
|
||||||
|
name: 'Hardware',
|
||||||
|
abbreviation: 'HW',
|
||||||
|
description: 'Hardware challenges',
|
||||||
|
iconPath: '/uploads/icons/HW.png',
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(second.iconPreview).toBe('/uploads/icons/HW.png');
|
||||||
|
expect(form.controls.name.value).toBe('Hardware');
|
||||||
|
expect(form.controls.abbreviation.value).toBe('HW');
|
||||||
|
expect(form.controls.description.value).toBe('Hardware challenges');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
Reference in New Issue
Block a user