From 5c27b509f54503f03aa67bc36d101f2475e75eb0 Mon Sep 17 00:00:00 2001 From: OpenVelo Agent Date: Wed, 22 Jul 2026 20:50:05 +0000 Subject: [PATCH] feat: Admin Area Challenges: List, Import/Export and Add/Edit Modal 1.00 --- .kilo/plans/861.md | 89 ------------------- .kilo/plans/900.md | 62 +++++++++++++ .../admin/challenges/challenges.component.ts | 9 +- tests/frontend/admin-challenges-form.spec.ts | 19 ++++ 4 files changed, 87 insertions(+), 92 deletions(-) delete mode 100644 .kilo/plans/861.md create mode 100644 .kilo/plans/900.md diff --git a/.kilo/plans/861.md b/.kilo/plans/861.md deleted file mode 100644 index 8fa9478..0000000 --- a/.kilo/plans/861.md +++ /dev/null @@ -1,89 +0,0 @@ -# Implementation Plan: Admin Area Challenges: List, Import/Export and Add/Edit Modal - -## 1. Architectural Reconnaissance -- **Status:** Not already implemented. The repository only has the base `ChallengeEntity`, `ChallengeFileEntity`, and `SolveEntity`, plus a direct generic challenge upload route. There is no `/api/v1/admin/challenges` controller/service/DTO layer, the admin Challenges navigation entry is disabled, no Angular challenges admin route or components exist, and the current schema does not meet the Job contract. -- **Codebase style & conventions:** Node.js npm-workspace monorepo; NestJS 10 backend and Angular 17 standalone frontend, both in strict TypeScript. Backend routes use thin controllers, injected services/repositories, Zod schemas through `ZodValidationPipe`, `ApiError`/`GlobalExceptionFilter`, global JWT auth, and controller-level `AdminGuard` plus `@Roles('admin')`. Frontend routes are lazy-loaded; components are standalone, OnPush, signal-driven containers with typed non-nullable reactive forms and input/output-based presentational modals. HTTP calls are centralized in `AdminService`. Reuse `MarkdownService` (`marked` + DOMPurify) for sanitized preview rather than adding a renderer. -- **Data Layer:** TypeORM 0.3 with `better-sqlite3`, explicit forward-only migrations, `synchronize: false` at runtime, UUID text primary keys, snake_case columns, and `DataSource.transaction(...)` for atomic multi-row mutations. Existing challenge tables are incomplete: `challenge` lacks case-insensitive uniqueness, `enabled`, and `updated_at`, uses lowercase `low|med|high` and `nc|web`, and has zero-valued scoring defaults; `challenge_file` only has `stored_path`; challenge-to-file/solve FKs are `ON DELETE RESTRICT`; `solve` has no first/second/third fields. Preserve the existing `(challenge_id,user_id)` unique index and user FK while migrating/rebuilding affected SQLite tables. Implement file staging under `${UPLOAD_DIR}/challenges/.staging` and final files under `${UPLOAD_DIR}/challenges`. -- **Test Framework & Structure:** Root Jest 29 multi-project config with `ts-jest`; backend tests run in Node with Nest testing helpers/Supertest and in-memory SQLite, frontend tests run in jsdom and favor exported pure helpers. All new tests belong under `tests/backend/` and `tests/frontend/`, never beside source, and run together via root `npm test` (or individually via `npm run test:backend` / `npm run test:frontend`). Keep coverage focused on core success paths and key validation/transaction failures without browser or visual checks. -- **Required Tools & Dependencies:** No new package or global CLI is required: TypeORM, Zod, Multer, UUID, RxJS, Angular reactive forms, `marked`, DOMPurify, Jest, Supertest, and jsdom already exist. Node filesystem APIs are sufficient for staging/export. Update `setup.sh` only to ensure `./data/uploads/challenges/.staging` exists (alongside the current upload root); retain npm install/native `better-sqlite3` rebuild/build behavior. Runtime persistence remains under configured `UPLOAD_DIR` (default `./data/uploads`); tests should use generated temporary directories, not `/data`, because no cross-job fixture is needed. - -## 2. Impacted Files -- **To Modify:** - - `backend/src/database/entities/challenge.entity.ts` — align fields/defaults/enums, timestamps, enabled flag, relations, and case-insensitive uniqueness contract. - - `backend/src/database/entities/challenge-file.entity.ts` — replace path-only metadata with stored filename, MIME type, byte size, timestamp, and cascade relation. - - `backend/src/database/entities/solve.entity.ts` — model cascade relation and first/second/third award flags while preserving solve uniqueness and current scoring columns. - - `backend/src/database/database.module.ts` — register the new challenge-schema migration. - - `backend/src/modules/admin/admin.module.ts` — register challenge/file/solve repositories, challenge controller, and challenge/file services. - - `backend/src/common/errors/error-codes.ts` — add stable challenge/import codes (`CHALLENGE_NAME_TAKEN`, `CHALLENGE_INVALID_CATEGORY`, `IMPORT_VALIDATION_FAILED`, `IMPORT_PARTIAL_FAILURE`). - - `backend/src/modules/uploads/uploads.controller.ts` — remove or redirect the current direct-to-final challenge upload behavior so challenge files use the new staged-upload lifecycle and cannot overwrite by original name. - - `backend/src/modules/uploads/uploads.module.ts` — import/provide the challenge staging dependency if the staged endpoint remains in the uploads module; otherwise retire the obsolete challenge route registration. - - `frontend/src/app/app.routes.ts` — lazy-load `/admin/challenges` under the existing protected admin shell. - - `frontend/src/app/features/admin/admin-shell.component.ts` — enable the Challenges navigation entry and render it as a semantic keyboard-operable navigation control. - - `frontend/src/app/core/services/admin.service.ts` — add typed list/detail/CRUD, staging/cleanup, import preview/confirm, and export download API methods and contracts. - - `setup.sh` — create challenge and staging upload directories idempotently. - - `tests/backend/migrations.spec.ts` — register/assert the migration, new columns/defaults/indexes, cascade FKs, and retained solve uniqueness. - - `tests/backend/uploads.spec.ts` — replace the legacy immediate challenge-file assertion with staged upload/limit behavior, or remove it if coverage moves to challenge endpoint tests. - - `docs/architecture/backend-modules.md`, `docs/architecture/frontend-structure.md`, `docs/database/challenges.md`, `docs/api/admin.md`, `docs/api/uploads.md`, `docs/guides/admin-shell.md` — update generated project documentation to describe the completed feature and schema/API/navigation changes. -- **To Create:** - - `backend/src/database/migrations/1700000000500-UpgradeChallengeAdminSchema.ts` — rebuild/upgrade challenge-related SQLite tables and indexes safely. - - `backend/src/modules/admin/admin-challenges.controller.ts` — admin-only REST surface for list/detail/create/update/delete, stage/discard files, import preview/confirm, and export. - - `backend/src/modules/admin/challenges.service.ts` — challenge CRUD, list aggregates/filtering, validation/business rules, transactional deletion, DTO mapping, export, and import orchestration. - - `backend/src/modules/admin/challenge-files.service.ts` — collision-safe staging, limit enforcement, commit/discard/delete helpers, base64 export/import file handling, and best-effort filesystem cleanup logging. - - `backend/src/modules/admin/dto/challenges.dto.ts` — strict Zod schemas/types for IDs, list queries, create/update payloads, staged-file manifests, and import format/version validation with field paths. - - `frontend/src/app/features/admin/challenges/challenges.component.ts` — smart admin page for debounced queries, sorting, modal state, request loading locks, refreshes, downloads, and status messages/toasts. - - `frontend/src/app/features/admin/challenges/challenge-form-modal.component.ts` — reusable accessible Add/Edit dialog with General/Connection/Files sections, typed form, sanitized Markdown preview, flag reveal, staged uploads, and attached-file removal. - - `frontend/src/app/features/admin/challenges/challenge-form.pure.ts` — pure form defaults/prefill, points/port validation, minimum-points synchronization, DTO mapping, and server-field-error helpers. - - `frontend/src/app/features/admin/challenges/challenge-delete-modal.component.ts` — non-backdrop-dismissible destructive confirmation dialog. - - `frontend/src/app/features/admin/challenges/challenge-import-modal.component.ts` — non-backdrop-dismissible preview/overwrite confirmation with counts/conflicts/warnings. - - `frontend/src/app/features/admin/challenges/challenges.pure.ts` — pure query normalization/sorting, file-size formatting, import/export filename, and error-summary helpers. - - `tests/backend/admin-challenges-service.spec.ts` — focused CRUD/list/delete service coverage with in-memory SQLite and temporary uploads. - - `tests/backend/admin-challenges-import-export.spec.ts` — focused version-1 export/import preview/overwrite and rollback/staging behavior. - - `tests/backend/admin-challenges-api.spec.ts` — minimal guard and structured validation/error contract coverage for the new controller. - - `tests/frontend/admin-challenges-form.spec.ts` — pure form defaults/prefill, points/NC-port rules, flag omission/presence, and staged-file payload mapping. - - `tests/frontend/admin-challenges-list.spec.ts` — pure debounce-query/sort/empty-state decision and import/export helper coverage. - - `docs/guides/admin-challenges.md` — focused admin Challenges user/architecture guide. - -## 3. Proposed Changes -1. **Database / Schema Migration:** - - Add a forward-only SQLite migration after `1700000000400` that disables FK checks only for the controlled rebuild, creates replacement `challenge`, `challenge_file`, and `solve` tables, copies compatible legacy data, swaps tables, recreates indexes, and re-enables/verifies FKs. - - Store canonical API enum values `LOW|MEDIUM|HIGH` and `NC|WEB`; map legacy `low|med|high` (`med` → `MEDIUM`) and lowercase protocols during copy. Add `enabled INTEGER NOT NULL DEFAULT 1`, `updated_at`, challenge defaults (`initial_points=100`, valid minimum/decay fallback), and a unique index on `LOWER(name)` for case-insensitive uniqueness. - - Keep `category_id NOT NULL` with `ON DELETE RESTRICT`. Change `challenge_file.challenge_id` and `solve.challenge_id` to `ON DELETE CASCADE`; retain `solve.user_id ON DELETE RESTRICT` and `uq_solve_challenge_user`. - - Upgrade `challenge_file` to `stored_filename UNIQUE`, `mime_type`, `size_bytes` (SQLite INTEGER mapped through a safe TypeORM bigint transformer/string boundary if necessary), and `created_at`; derive a basename for legacy `stored_path`, stat legacy files when available, and use safe defaults when metadata cannot be recovered. - - Add `is_first`, `is_second`, and `is_third` boolean columns to `solve` without removing existing `base_points`/`rank_bonus`, avoiding regressions in rank/scoring code. - - Update entity relations to mirror cascade behavior and timestamps, but use the explicit migration rather than runtime synchronization. - -2. **Backend Logic & APIs:** - - Add `AdminChallengesController` at `/api/v1/admin/challenges`, guarded at controller level by `AdminGuard` and `@Roles('admin')`; leave global JWT and CSRF enforcement intact. Keep handlers thin and validate every query/param/body with strict Zod schemas and `ZodValidationPipe`. - - Implement `GET /api/v1/admin/challenges` with optional trimmed `q` and `categoryId`, parameterized `LOWER(name) LIKE LOWER(:q)`, category join, aggregate `COUNT(DISTINCT file.id)`/`COUNT(DISTINCT solve.id)`, and deterministic `LOWER(name), name` ordering. Return only the specified summary DTO (including category abbreviation/icon, enabled/timestamps/counts), never `flag`. - - Add `GET /api/v1/admin/challenges/:id` for edit prefill because list rows intentionally omit description, connection/scoring details, secret, and file metadata. This admin-only DTO may include plaintext `flag` and attached-file metadata; make the mapping explicit so ORM entities are never returned directly. Create/update responses may use this admin detail DTO. - - Implement `POST` create and `PUT /:id` update with trimmed 1–128 name, required Markdown/category/difficulty/flag, integer point ranges, `minimumPoints <= initialPoints`, decay range, protocol-dependent port rules, optional validated IP/IPv6/hostname string, and enabled default. Resolve categories before writes; translate the lower-name unique index violation to `409 CHALLENGE_NAME_TAKEN` with `{ path: 'name' }`, and invalid category to `CHALLENGE_INVALID_CATEGORY`. For updates, retain the existing flag only if the client deliberately uses an explicit unchanged contract; otherwise require the admin detail value and never use an empty sentinel ambiguously. - - Define staged-file endpoints beneath the required prefix, e.g. `POST /api/v1/admin/challenges/files/stage` (multipart, multiple calls or `FilesInterceptor`) and `DELETE /api/v1/admin/challenges/files/stage/:token`, plus request fields on create/update for staged tokens and existing file IDs to remove. Use UUID-based stored names, `path.basename`/safe path joins, global `UPLOAD_SIZE_LIMIT`, no type restriction, and preserve original filename/MIME/size metadata. Staging responses contain opaque tokens and metadata, not arbitrary filesystem paths. - - On create/update, perform DB changes and `ChallengeFile` metadata changes in one transaction; commit selected staging records by atomically moving staged files to final UUID filenames and associate them with the challenge. On modal cancellation, delete each staged token. Add stale staging cleanup (age-based on use/startup or opportunistically) so abandoned browser sessions do not leak indefinitely. - - Implement `DELETE /:id` by first loading attached stored filenames, deleting the challenge inside one TypeORM transaction so file and solve rows cascade, then unlinking final files after successful commit. Catch each unlink failure and call `console.log` with context without reverting the committed DB deletion. - - Implement `GET /export` before the parameterized `/:id` route. Load all challenges/categories/files in alphabetical order, read each file from `${UPLOAD_DIR}/challenges/`, and emit `{format:'hipctf-challenges',version:1,exportedAt,challenges}` with canonical enums, category `systemKey`, plaintext flags, numeric `sizeBytes`, and base64 data. Set JSON attachment headers with `challenges-export-.json`; an empty DB yields an empty array. Treat missing/unreadable source files as a structured server failure rather than silently producing corrupt export data. - - Implement `POST /import` as a two-phase API. Without `confirm=overwrite`, validate top-level format/version/array, every challenge field/rule/category reference, and every file's strict base64/decoded length/limit; return structured `IMPORT_VALIDATION_FAILED` details with stable paths plus preview counts/conflicts. To reconcile the stated preview warning with validation ordering, classify unknown category keys as preview/confirm `skipped` warnings rather than accepting invalid references silently; malformed/empty keys remain validation errors. - - Do not trust a second client submission to be identical: confirm must re-parse/revalidate the same document (or an opaque server-side preview token tied to staged content). On `confirm=overwrite`, decode and write every incoming file to `.staging/` before DB replacement. Then open one transaction, resolve categories by immutable `systemKey`, skip missing categories with warnings, find conflicts case-insensitively, delete conflicting challenges (cascading files/solves), recreate challenges, and insert file metadata referencing predetermined final stored filenames. Commit only after all DB operations and staging writes succeed; on pre-commit failure roll back and delete all new staging files. - - After import DB commit, rename staged files to final paths and best-effort delete superseded files. The Job's phrase “inserts ChallengeFile rows after commit” conflicts with the requirement that the DB remain consistent if a post-commit rename fails; preserve consistency by inserting metadata in the transaction and using predetermined final names, then log post-commit rename/delete failures with `console.log` without changing the successful response. Return `{imported, skipped:[{name,reason}], warnings}` and use `IMPORT_PARTIAL_FAILURE` when the contract requires a stable code for a completed import with skipped/warning items; use structured `500` for pre-commit staging failures. - - Extend error constants/types and, where necessary, `ApiError` helpers so Zod/import issues use stable codes and `details` paths while `GlobalExceptionFilter` keeps the existing envelope. Audit current and future player-facing serializers: no non-admin challenge endpoint currently exists, so establish separate admin summary/detail types and ensure any later public DTO is an explicit allowlist without `flag`. - -3. **Frontend UI Integration:** - - Enable the Challenges side-nav item and add the lazy `/admin/challenges` child route, inheriting the existing `adminGuard`. Keep the shell as layout-only and put feature files under `features/admin/challenges`. - - Extend `AdminService` with strict request/response interfaces and methods for list filters, edit detail, CRUD, stage/discard, import preview/confirm, and export as `Blob`. Encode query/path values, keep credentials/interceptors, and avoid direct `HttpClient` use in components. - - Build `AdminChallengesComponent` as the smart container. Maintain signals for rows/loading/errors/search/category filter/sort direction/modals/in-flight actions/status. Use a `Subject` or search control `valueChanges` with `debounceTime(250)`, `distinctUntilChanged()`, and `takeUntilDestroyed()` to reload server-filtered results; guard responses against stale searches. Render a semantic table with sortable Name (and deterministic initial name order), 32×32 category image with an `error` fallback to abbreviation, textual difficulty/protocol badges, and labeled pencil/trash buttons. Show separate “No challenges yet” and search-no-results states; only the unfiltered empty state has the single Add button specified. - - Add toolbar controls in required order: search, Add challenge, Import challenges, Export challenges. Disable each relevant control and show an inline spinner/text while its request is active. Export obtains the Blob, creates a temporary object URL/anchor using the server or helper filename, clicks it, and always revokes the URL. - - Build a reusable typed Add/Edit modal with accessible `role="dialog"`, labels, focus management/return, Escape and backdrop cancellation enabled, and General/Connection/Files tabs or accordion in the required order. Prefill edit mode from the detail endpoint; defaults are initial 100, minimum 50, enabled true, difficulty/protocol defaults, and valid decay. Expose all validation messages inline and map `CHALLENGE_NAME_TAKEN` to Name without closing the dialog. - - Implement minimum-points live defaulting only while it remains auto-derived/pristine: changing initial points recomputes `floor(initialPoints/2)`; once the admin edits minimum explicitly, preserve that value and only validate `<= initialPoints`. Apply required/range validators, dynamically require/clear port for NC/WEB, omit an empty WEB port, and submit numeric values rather than strings. - - Render description preview beside/toggled from the textarea using the existing `MarkdownService`; bind only its DOMPurify-sanitized output and never call `bypassSecurityTrustHtml`. Use a password input with an in-field labeled eye button that toggles only the input type. Do not persist flag or import documents in browser storage/logging. - - In Files, list existing metadata and mark removals locally until Save. Support multiple picker/drag-drop files and stage them immediately, displaying per-file progress/error and enforcing the known global limit when available while treating backend enforcement as authoritative. Save sends staged tokens and removal IDs; Cancel/backdrop/Escape calls discard for all unsaved tokens before closing. Prevent Save while staging is active or invalid. - - Build destructive delete confirmation with exact copy, disabled backdrop dismissal, Cancel/Delete, request lock/spinner, then refresh and announce a success status/toast. Build import file selection for `.json`, parse text client-side only for immediate malformed-file feedback, send the document to preview, and show a non-backdrop-dismissible overwrite modal containing counts/conflicts/skips and required message. Confirm reposts the same document, refreshes, and displays imported/skipped/warning summary; all actions prevent double submission. - - Since no shared toast primitive exists, implement a small page-local signal status banner with `role="status"`/`aria-live="polite"` rather than introducing a new UI dependency. Use native buttons/table/form controls, labels and alt text, visible focus states, keyboard operation, and focus trapping/return for dialogs. - -## 4. Test Strategy -- **Target Unit Test File:** - - `tests/backend/migrations.spec.ts`: assert canonical columns/defaults, lower-name unique index, file/solve cascade FKs, and unchanged `(challenge_id,user_id)` uniqueness. - - `tests/backend/admin-challenges-service.spec.ts`: create/list/search/category filter success, secret omission from list, duplicate-name/category/point/NC-port rejection, and delete cascading DB rows while tolerating a mocked unlink failure. - - `tests/backend/admin-challenges-import-export.spec.ts`: version-1 empty/non-empty export shape with base64, preview conflicts/warnings, confirmed overwrite by case-insensitive name, and one staged-write failure rolling back DB changes and cleaning staging. - - `tests/backend/admin-challenges-api.spec.ts`: unauthenticated/player denial and one structured Zod/import validation response with stable code/path; avoid duplicating all service cases through Supertest. - - `tests/frontend/admin-challenges-form.spec.ts`: test exported pure helpers for create/edit prefill, half-point synchronization, minimum/initial and NC-port validation, WEB port omission, file-removal/staged-token mapping, and duplicate-name field error mapping. - - `tests/frontend/admin-challenges-list.spec.ts`: test pure query normalization/sorting/empty-state classification and import/export summary/filename logic; no visual assertions or browser automation. -- **Mocking Strategy:** Use real in-memory `better-sqlite3` repositories for backend service/migration behavior and a per-test temporary upload directory generated at runtime, with Node `fs` methods spied only for deterministic write/rename/unlink failure branches; restore spies and remove temp trees after each suite. Mock no DB in the small API guard test—use the existing `AppModule`, `initDb`, JWT, CSRF, and Supertest pattern. Frontend tests should import pure helpers and use lightweight typed fake `AdminService` methods only if container orchestration needs one focused test; stub `FileReader`, object URL, and anchor click rather than requiring a browser. No persistent `/data` fixture is needed; if future shared fixtures become necessary, tests must detect/create them under `/data` dynamically. diff --git a/.kilo/plans/900.md b/.kilo/plans/900.md new file mode 100644 index 0000000..c5a85f3 --- /dev/null +++ b/.kilo/plans/900.md @@ -0,0 +1,62 @@ +# Implementation Plan: Fix Admin Challenge Create Validation (All Fields "Required") + +## 1. Architectural Reconnaissance +- **Codebase style & conventions:** TypeScript end-to-end, NestJS 10 REST API with `@Controller` / `@UseGuards` / `@Body` parameter pipes, Angular 17+ standalone components using signals + `ReactiveFormsModule`. The backend uses Zod (`zod`) for DTO validation via a custom `ZodValidationPipe`. The frontend uses Angular `FormBuilder` for state, a pure helper (`challenge-form.pure.ts`) for payload shaping, and an Angular `HttpClient`-based `AdminService`. +- **Data Layer:** TypeORM + `better-sqlite3` (`./data/` SQLite file). Entities under `backend/src/database/entities/`. Schema is migration-driven. +- **Test Framework & Structure:** Jest multi-project (`tests/jest.config.js`) with two projects — `backend` (Node env, `tests/backend/**/*.spec.ts`) and `frontend` (jsdom env, `tests/frontend/**/*.spec.ts`). Run with `npm test` from the repo root. Frontend tests already cover the pure helpers in `challenge-form.pure.ts` (`tests/frontend/admin-challenges-form.spec.ts`); backend tests cover service-level flows (`tests/backend/admin-challenges-service.spec.ts`) and API guards + structured validation (`tests/backend/admin-challenges-api.spec.ts`). +- **Required Tools & Dependencies:** No new dependencies. The fix is contained in the frontend. Backend, schemas, and entity are unchanged. + +## 2. Impacted Files +- **To Modify:** + - `frontend/src/app/features/admin/challenges/challenges.component.ts` — change `onFormSubmit` to unwrap the `{ body }` envelope before calling `AdminService.createChallenge` / `updateChallenge`. The modal already emits `{ body }` (see `challenge-form-modal.component.ts:528`), so the parent must extract `body` first. This is the root cause of the bug. + - `tests/frontend/admin-challenges-form.spec.ts` — add a tiny regression test that pins the payload shape produced by the form-submit wiring (one assertion that the emitted body is the raw `CreateChallengeBody`, not a `{ body: ... }` wrapper). +- **To Create:** + - None. The backend DTOs (`backend/src/modules/admin/dto/challenges.dto.ts`), service (`backend/src/modules/admin/challenges.service.ts`), and controller (`backend/src/modules/admin/admin-challenges.controller.ts`) are already correct and need no changes. + +## 3. Proposed Changes +1. **Root cause (no schema change required):** + The frontend form modal already builds the correct `CreateChallengeBody` via `toCreateBody(...)` (`challenge-form.pure.ts:158`) and emits it wrapped: `this.submit.emit({ body })` (`challenge-form-modal.component.ts:528`). + The smart container at `challenges.component.ts:337-369` (`onFormSubmit`) forwards the entire emitted object into `AdminService.createChallenge(payload)` / `updateChallenge(id, payload)` (`admin.service.ts:268` and `:276`). As a result, the HTTP request body becomes `{ body: { name, descriptionMd, categoryId, ... } }` — every field is one level too deep. + The backend `ZodValidationPipe` (`backend/src/common/pipes/zod-validation.pipe.ts`) runs `CreateChallengeSchema.safeParse(value)` where `value` is that wrapped object. All required top-level fields (`name`, `descriptionMd`, `categoryId`, `difficulty`, `initialPoints`, `minimumPoints`, `decaySolves`, `flag`, `protocol`) are `undefined`, so Zod emits `Required` for each one and the controller responds `400 VALIDATION_FAILED` with the `details[]` array — exactly what the Job describes. + +2. **Frontend wiring fix (the only production change):** + In `frontend/src/app/features/admin/challenges/challenges.component.ts`, change the body of `onFormSubmit(payload)` so the wrapped envelope is unwrapped before hitting the HTTP layer: + ```ts + async onFormSubmit(payload: { body: CreateChallengeBody | UpdateChallengeBody }): Promise { + this.saving.set(true); + this.formError.set(null); + this.fieldErrors.set(null); + const body = payload.body; + try { + if (this.formMode() === 'create') { + await this.admin.createChallenge(body as CreateChallengeBody); + } else { + const detail = this.formDetail(); + if (!detail) throw new Error('Missing challenge detail'); + await this.admin.updateChallenge(detail.id, body as UpdateChallengeBody); + } + // ...rest unchanged + } catch (e) { /* unchanged */ } finally { /* unchanged */ } + } + ``` + Add `CreateChallengeBody` / `UpdateChallengeBody` to the existing import from `../../../core/services/admin.service` at the top of the file (they are already exported there). + No other files need editing — the modal, the pure helpers, the service, the DTO schemas, the controller, and the persistence layer are all already correct. + +3. **Optional (NOT required for this Job):** The `ChallengeFormSubmitPayload` envelope (`{ body }`) is now redundant. Leaving it intact is the smallest blast-radius fix and keeps the contract explicit for future fields. **No change here.** + +4. **Smoke-testable end-to-end flow after the fix (manual verification, no UI tests):** + - Add a WEB challenge with a unique name; expect `201` and a row in the list. + - Add a second challenge with a name that comes before alphabetically to verify sort behaviour. + - Type in the search box to verify `q=` debounce filters case-insensitively. + - Toggle the Name sort header to verify asc/desc. + +## 4. Test Strategy +- **Target Test File:** `tests/frontend/admin-challenges-form.spec.ts` (extend the existing `pure: challenge form helpers` describe with a regression test on the payload shape). +- **Mocking Strategy:** No new mocking needed. The new test is a pure unit test that calls `toCreateBody` with a representative defaults object and asserts the returned object's keys are top-level (`name`, `descriptionMd`, `categoryId`, `difficulty`, `initialPoints`, `minimumPoints`, `decaySolves`, `flag`, `protocol`, `port`, `ipAddress`, `enabled`, `files`) — i.e. there is no nested `body` property. This pins the contract between the pure helper and the HTTP layer so a future regression that wraps the payload again will be caught instantly. +- **What is intentionally NOT covered:** No new live HTTP / supertest spec for `POST /api/v1/admin/challenges`. The existing `tests/backend/admin-challenges-api.spec.ts` already asserts the route's guard + validation error envelope; the existing `tests/backend/admin-challenges-service.spec.ts` already exercises `AdminChallengesService.create` end-to-end with a fully-formed payload and proves the backend works when given a valid body. Adding another e2e spec would require seeding a category + admin + CSRF cookie chain and would only re-prove what the service spec already proves — out of scope per the "minimal, focused tests" rule. +- **Run:** `npm test` (or `npm run test:frontend` for the targeted project). + +## Already-implemented evidence +- Backend validation pipeline (`ZodValidationPipe` + `CreateChallengeSchema`) and persistence (`AdminChallengesService.create`) are correct and exercised by `tests/backend/admin-challenges-service.spec.ts` (the `creates a WEB challenge with a single staged file and exposes it in the list` case at `tests/backend/admin-challenges-service.spec.ts:88`). +- Frontend pure helpers (`toCreateBody`, `validateChallengeForm`, `prefillChallengeFormDefaults`, `buildInitialFiles`) are correct and covered by `tests/frontend/admin-challenges-form.spec.ts`. +- The defect is exclusively the missing unwrap in `challenges.component.ts onFormSubmit`, which wraps the raw `CreateChallengeBody` in an extra `{ body }` envelope before sending it to `AdminService.createChallenge`. The fix is one site, one new regression assertion, no schema or API change. diff --git a/frontend/src/app/features/admin/challenges/challenges.component.ts b/frontend/src/app/features/admin/challenges/challenges.component.ts index 87ecf84..0734834 100644 --- a/frontend/src/app/features/admin/challenges/challenges.component.ts +++ b/frontend/src/app/features/admin/challenges/challenges.component.ts @@ -15,8 +15,10 @@ import { AdminService, AdminCategory, AdminChallengeListItem, + CreateChallengeBody, ImportPreviewResponse, ImportResultResponse, + UpdateChallengeBody, } from '../../../core/services/admin.service'; import { buildExportFilename, @@ -334,17 +336,18 @@ export class AdminChallengesComponent implements OnInit, OnDestroy { this.fieldErrors.set(null); } - async onFormSubmit(payload: any): Promise { + async onFormSubmit(payload: { body: CreateChallengeBody | UpdateChallengeBody }): Promise { this.saving.set(true); this.formError.set(null); this.fieldErrors.set(null); + const body = payload.body; try { if (this.formMode() === 'create') { - await this.admin.createChallenge(payload); + await this.admin.createChallenge(body as CreateChallengeBody); } else { const detail = this.formDetail(); if (!detail) throw new Error('Missing challenge detail'); - await this.admin.updateChallenge(detail.id, payload); + await this.admin.updateChallenge(detail.id, body as UpdateChallengeBody); } this.closeForm(); await this.load(); diff --git a/tests/frontend/admin-challenges-form.spec.ts b/tests/frontend/admin-challenges-form.spec.ts index e58d8d9..143caf1 100644 --- a/tests/frontend/admin-challenges-form.spec.ts +++ b/tests/frontend/admin-challenges-form.spec.ts @@ -165,6 +165,25 @@ describe('pure: challenge form helpers', () => { expect(out.files?.[1].stagedToken).toBe('tok'); }); + it('toCreateBody produces a top-level payload (no nested body envelope) for HTTP transport', () => { + const out = toCreateBody(makeDefaults(), []); + const requiredTopLevel = [ + 'name', + 'descriptionMd', + 'categoryId', + 'difficulty', + 'initialPoints', + 'minimumPoints', + 'decaySolves', + 'flag', + 'protocol', + ]; + for (const k of requiredTopLevel) { + expect((out as any)[k]).toBeDefined(); + expect((out as any).body).toBeUndefined(); + } + }); + it('buildInitialFiles hydrates a UI list from detail.files', () => { const list: ChallengeStagedFileUi[] = buildInitialFiles(makeDetail()); expect(list.length).toBe(1);