From 1858121f75570cd34b76d80f0259b736f85cdf4a Mon Sep 17 00:00:00 2001 From: OpenVelo Agent Date: Wed, 22 Jul 2026 14:52:57 +0000 Subject: [PATCH 1/2] docs: update documentation to OKF v0.1 format --- docs/database/challenges.md | 6 ++--- docs/database/schema.md | 8 +++++- docs/guides/admin-general-settings.md | 38 ++++++++++++++++++--------- docs/index.md | 2 +- 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/docs/database/challenges.md b/docs/database/challenges.md index 9b4b90b..8164b55 100644 --- a/docs/database/challenges.md +++ b/docs/database/challenges.md @@ -3,7 +3,7 @@ type: database title: Challenge Tables description: category, challenge, challenge_file, and solve tables — how CTF challenges and scoring are stored. tags: [database, challenge, category, solve] -timestamp: 2026-07-21T14:18:00Z +timestamp: 2026-07-22T14:50:25Z --- # Tables @@ -18,8 +18,8 @@ timestamp: 2026-07-21T14:18:00Z | `abbreviation` | TEXT | Short label (e.g. `CRY`), 2–6 chars, server-uppercased. Globally unique — enforced by `uq_category_abbreviation`. | | `description` | TEXT | Optional description. | | `icon_path` | TEXT | Default icon URL (e.g. `/uploads/icons/CRY.png`). | -| `created_at` | TEXT | ISO 8601 timestamp the row was created (added by `AddCategoryTimestampsAndUniqueAbbrev1700000000200`). | -| `updated_at` | TEXT | ISO 8601 timestamp the row was last updated (added by the same migration; bumped on `PUT /api/v1/admin/categories/:id`). | +| `created_at` | TEXT | ISO 8601 timestamp the row was created. Declared in the initial `InitSchema1700000000000` migration (defaulting to `strftime('%Y-%m-%dT%H:%M:%fZ','now')`); the `AddCategoryTimestampsAndUniqueAbbrev1700000000200` migration adds it as a no-op for older pre-existing databases. | +| `updated_at` | TEXT | ISO 8601 timestamp the row was last updated (bumped on `PUT /api/v1/admin/categories/:id`). Same provenance as `created_at`. | System rows are seeded by the `UpdateSystemCategoryKeys1700000000300` migration so the canonical diff --git a/docs/database/schema.md b/docs/database/schema.md index a9ae386..a2f2f07 100644 --- a/docs/database/schema.md +++ b/docs/database/schema.md @@ -3,7 +3,7 @@ type: database title: Database Schema Overview description: SQLite (better-sqlite3) schema for HIPCTF: tables, relationships, indexes, and WAL journal mode. tags: [database, sqlite, typeorm, schema] -timestamp: 2026-07-21T14:18:00Z +timestamp: 2026-07-22T14:50:25Z --- # Overview @@ -17,6 +17,12 @@ The schema is created by a single migration (`backend/src/database/migrations/1700000000000-InitSchema.ts`) and seeded by a second migration (`backend/src/database/migrations/1700000000100-SeedSystemData.ts`). +The initial migration declares every column used by the entities +(including `category.created_at` and `category.updated_at`), so the +later `AddCategoryTimestampsAndUniqueAbbrev1700000000200` and +`UpdateSystemCategoryKeys1700000000300` migrations only patch legacy +databases that pre-date those columns and reseed the canonical system +rows. # Tables diff --git a/docs/guides/admin-general-settings.md b/docs/guides/admin-general-settings.md index 02f081d..df3f3b3 100644 --- a/docs/guides/admin-general-settings.md +++ b/docs/guides/admin-general-settings.md @@ -3,7 +3,7 @@ type: guide title: Admin — General Settings description: How an admin edits required platform-wide settings, including the validated event window, from the /admin/general page. tags: [guide, admin, settings, general, tester, datetime, validation] -timestamp: 2026-07-22T14:24:08Z +timestamp: 2026-07-22T14:50:25Z --- # When this view is available @@ -17,7 +17,7 @@ by navigating directly to the URL. |------------------|----------------------------------------------------------------------------|------------------------------------------------| | Client route | `frontend/src/app/app.routes.ts` | `/admin/general` child of `adminGuard`. | | Client component | `frontend/src/app/features/admin/general.component.ts` | `AdminGeneralComponent.ngOnInit` fetches settings + themes. | -| Client predicate | `frontend/src/app/features/admin/general.pure.ts` (`deriveEventState`, `pageTitleError`, `pageTitleMessage`) | Pure helpers for event-state derivation and Page-title error/message mapping. | +| Client predicate | `frontend/src/app/features/admin/general.pure.ts` (`deriveEventState`, `pageTitleError`, `pageTitleMessage`, `eventEndFieldMessage`) | Pure helpers for event-state derivation, Page-title error/message mapping, and the merged Event End field message (per-field `invalidDatetime` + cross-field `endBeforeStart`). | | Server route | `backend/src/modules/admin/admin-general.controller.ts` | `GET/PUT /api/v1/admin/general/settings` + `GET /api/v1/admin/general/themes`. | # How to access (tester steps) @@ -43,7 +43,7 @@ The page is a single reactive form with these controls (every | Logo upload status | `general-logo-uploading` / `general-logo-error` / `general-logo-current` | — | Inline status text under the file picker. | | Global theme | `general-themeKey` | `themeKey` | `` bindings to surface `aria-invalid`, `aria-describedby`, `title`, and `validity.validationMessage` whenever the form is in the `endBeforeStart` invalid state; ensure the inline `general-endBeforeStart` and `general-eventEnd-error` blocks render as soon as the cross-field validator fires (not only after `touched`); mirror the same logic symmetrically on the Event start side for completeness. - - `frontend/src/app/features/admin/general.pure.ts` — make the inline-error messages for `endBeforeStart` and `invalidDatetime` available as exports if they are not already (they are inline strings today; centralise to keep the template clean). Optionally tighten `isoDatetimeValidator` to keep treating empty as invalid client-side so the UI can also block Save when Start/End are blank after the fix — see "Open Question" below. - - `tests/backend/admin-validation.spec.ts` — add focused negative-case tests for the general-settings endpoint: empty `eventEndUtc` rejected with HTTP 400, malformed string rejected with HTTP 400, end ≤ start rejected with HTTP 400; plus a positive test that confirms valid ISO-8601 values persist unchanged. - - `tests/frontend/admin-general-pure.spec.ts` — add a small `invalidDatetime` regression asserting that `isoDatetimeValidator({ value: '' })` returns `{ invalidDatetime: true }` (if we choose to flip client validation to match backend) — see Open Question. - -- **To Create:** - - `tests/backend/admin-general-event-window.spec.ts` — dedicated spec for the new negative cases; uses the same Nest-app fixture pattern as `tests/backend/admin-general-service.spec.ts` but focuses on PUT validation only. - -## 3. Proposed Changes - -### Backend - -1. **Tighten the zod schema** in `backend/src/modules/admin/dto/general.dto.ts`: - - Replace each `eventStartUtc` / `eventEndUtc` field from `z.union([z.literal(''), z.string().datetime({ message: '...' })])` to `z.string().datetime({ message: '... must be a valid ISO-8601 datetime' })` directly. The empty-string alternative is what allowed `eventEndUtc=''` to overwrite a valid schedule (Job negative case 1). - - Keep the `superRefine` rule that emits `eventEndUtc must be strictly after eventStartUtc` on `path: ['eventEndUtc']` when both values are parseable and `end <= start`. - - The error envelope remains `{ code: 'VALIDATION_FAILED', message: 'Request validation failed', details: [{path, message}] }` (HTTP 400), emitted by `ZodValidationPipe` (`backend/src/common/pipes/zod-validation.pipe.ts:9-15`) — no pipe changes needed. - -2. **Persist only on success:** nothing to change in `AdminGeneralService.updateSettings` itself; because `ZodValidationPipe` throws before the handler is called, the prior schedule is preserved automatically on rejected payloads (the SettingsService is never touched). - -3. **Docs reconciliation (non-blocking):** update `docs/api/admin.md:48-53` and `docs/guides/admin-general-settings.md` sentences that say "empty strings are allowed" so the docs reflect that Start/End are now required (it is OK if the docs lag the change; the Job is authoritative). - -### Frontend - -1. **Update `general.pure.ts`** to keep the validator surface aligned with the new backend: - - If we choose to also reject empty strings client-side (recommended — keeps UI feedback consistent with backend), change `isoDatetimeValidator` so `''` returns `{ invalidDatetime: true }` instead of `null`. If we keep empty-allowed client-side, then `datetimeMessage` must still show a helpful message on `''` so the field stays consistent with the disabled Save button. - - Expose a `endBeforeStartMessage` helper that returns `'Event end must be after event start.'` so the template renders it from one source. - -2. **Update `general.component.ts`** for the Event End input and surrounding @if blocks (lines 109-125): - - Bind `[attr.aria-invalid]` and `[attr.aria-describedby]` on the End `` to a new `showEventEndCrossFieldError` computed that is true whenever the form has `errors.endBeforeStart`. Also bind `[attr.title]` to the message string and `[attr.validity.validationMessage]` is not directly supported by Angular — instead, surface a `data-error-message` attribute or render a hidden `` so screen readers can announce it. - - Render the `general-endBeforeStart` block whenever `endBeforeStart` is present (drop the `touched` gate so the user sees the error the moment they leave End ≤ Start). Keep `general-eventEnd-error` rendering only for `invalidDatetime` cases. - - Symmetrically, for Event start, render the `general-eventStart-error` whenever `invalidDatetime` is on the control (today it is correctly shown when the input becomes dirty AND invalid). - -3. **Save gating remains:** `[disabled]="submitting() || form.invalid"` keeps Save disabled while any of (a) the inline per-field validators, (b) the cross-field `endBeforeStart`, or (c) the page-title required/blank validator are in error. No change needed. - -### Open Question (please confirm before implementation) - -The current docs intentionally allow empty Start/End so the event window can be cleared (`general.pure.ts:69-72`, `docs/api/admin.md:48-53`). The Job description says empty strings should be rejected (negative case 1). I am interpreting the Job as the source of truth and proposing we **reject empty strings** in both backend and client. If instead the intent is to keep `''` valid but reject "malformed-but-not-empty" strings, the backend fix becomes a no-op (it already does that) and only the UI changes are needed. Please confirm before implementation begins. - -## 4. Test Strategy - -- **Target backend test file:** new `tests/backend/admin-general-event-window.spec.ts`. Reuses `tests/backend/csrf-client.ts`, follows the exact pattern from `tests/backend/admin-validation.spec.ts:16-46` (boots `AppModule`, registers first admin, primes CSRF, then exercises `PUT /api/v1/admin/general/settings` via `request.agent`). -- **Target frontend test file:** `tests/frontend/admin-general-pure.spec.ts`. Add only the helper-level assertions (e.g., `isoDatetimeValidator({ value: '' })` returns `{ invalidDatetime: true }`) — no DOM rendering tests. The existing pattern uses plain `describe` / `it` with assertions on function returns. -- **Mocking strategy:** No new mocks. The backend spec uses the real `AppModule` against `:memory:` SQLite (set via `process.env.DATABASE_PATH = ':memory:'` at the top of the file, mirroring `admin-validation.spec.ts:1-3`). Auth uses `register-first-admin` + login + CSRF — already idempotent because the database is in-memory and the test creates exactly one admin in `beforeAll`. -- **Cases to cover (minimal & focused):** - 1. **Backend negative — empty Event End** ⇒ `PUT /settings` with `eventEndUtc: ''` returns HTTP 400 and the per-field message, and a follow-up `GET /settings` shows `eventEndUtc` unchanged. - 2. **Backend negative — empty Event Start** ⇒ symmetric to (1). - 3. **Backend negative — malformed datetime** ⇒ `PUT /settings` with `eventEndUtc: 'not-a-date'` returns HTTP 400 and `'eventEndUtc must be a valid ISO-8601 datetime'`. - 4. **Backend negative — end ≤ start** ⇒ valid Start + `End === Start` returns HTTP 400 with `'eventEndUtc must be strictly after eventStartUtc'`. - 5. **Backend positive — valid window** ⇒ well-formed `2026-08-15T10:30:00.000Z` / `2026-08-20T18:45:00.000Z` persists and is round-tripped unchanged. - 6. **Frontend pure** — assert `isoDatetimeValidator('')` returns the invalid sentinel (if we choose to flip empty to invalid client-side). - 7. **Frontend pure** — assert the new `endBeforeStartMessage` helper returns the canonical string. -- **Single command:** `npm test` from `/repo` runs the full suite. Frontend and backend can also be run separately via `npm run test:backend` / `npm run test:frontend` (already in `package.json`). - -## 5. Persistent Project Data (/data) - -Not applicable to this Job. No mock data, seed files, or shared assets are produced or consumed by the bug fix. Tests boot against `:memory:` SQLite, which is sufficient. diff --git a/.kilo/plans/889.md b/.kilo/plans/889.md new file mode 100644 index 0000000..35507fd --- /dev/null +++ b/.kilo/plans/889.md @@ -0,0 +1,258 @@ +# Implementation Plan: Job 889 — Admin Area General Settings and Categories 1.07 + +## Status +NOT FULLY IMPLEMENTED. Two residual defects remain after Job 888: + +1. **Categories SQL** — `backend/src/modules/admin/categories.service.ts:30-37` uses + `createQueryBuilder('c').orderBy('LOWER(c.abbreviation)', 'ASC').addOrderBy('c.abbreviation', 'ASC').getMany()`. + `getMany()` auto-selects every column mapped on `CategoryEntity`, including + `created_at` and `updated_at` (added by migration + `1700000000200-AddCategoryTimestampsAndUniqueAbbrev`). The + `InitSchema1700000000000` migration (`backend/src/database/migrations/1700000000000-InitSchema.ts:28-37`) + creates the `category` table **without** those columns and relies on the + 200-migration to add them at startup. On any database where the 200-migration + has not yet run (e.g. when an older pre-1.06 DB is hit before migrations + resolve, or when tests / fresh clones initialise the schema out of order), + SQLite raises `SqliteError: no such column: c.created_at` and the Categories + section on `/admin/general` (which embeds `AdminCategoriesComponent`) renders + the error to the admin. The fix is to make the `category` table include + `created_at` and `updated_at` in its initial `CREATE TABLE` and keep the 200 + migration as a no-op `if (!names.has(...))` ALTER for pre-existing DBs. + +2. **Event End equal-to-start field-level message** — `frontend/src/app/features/admin/general.pure.ts:41-51` + already returns `{ endBeforeStart: true }` when `e <= s`, and + `frontend/src/app/features/admin/general.component.ts:123-128` renders the + `general-endBeforeStart` element whenever `eventEndCrossFieldError()` is true. + However the message lives in a **separate** `data-testid="general-endBeforeStart"` + div instead of inside the per-field error region + `data-testid="general-eventEnd-error"` that the Job (and `docs/guides/admin-general-settings.md`) + describe as the canonical "field-level validation message" for the Event End + input. The Job explicitly states: *"no visible message inside the field-level + error region (e.g., `[data-testid=general-eventEnd-error]`)"*. The fix is to + funnel the cross-field `endBeforeStart` message into `general-eventEnd-error` + (and surface it via `aria-invalid` / `aria-describedby` / `title` on the + input) so it is rendered as the Event End field's own validation message. + +## 1. Architectural Reconnaissance + +- **Codebase style & conventions:** + - **Backend:** NestJS 10, TypeScript strict, TypeORM 0.3.20 over + `better-sqlite3`. Migrations live under + `backend/src/database/migrations/` and are wired through + `DatabaseModule.MIGRATIONS` (`backend/src/database/database.module.ts:31-36`) + and run on bootstrap by `DatabaseInitService.init()` (`backend/src/database/database-init.service.ts:26-54`). + Entities are auto-selected from `DatabaseModule.ENTITIES` and must remain + in sync with the SQL schema produced by the migrations. + - **Frontend:** Angular 17+ standalone components with + `ChangeDetectionStrategy.OnPush`, reactive forms, and the `Value` / + `Invalid` / `TouchedOrDirty` signal trio for inline error messaging + (see `general.component.ts:204-321`). Pure validation / message helpers + live in `frontend/src/app/features/admin/general.pure.ts` and are + unit-tested under `tests/frontend/admin-general-pure.spec.ts`. + - **Error envelopes:** backend emits + `{ code: 'VALIDATION_FAILED', message, details: [{path, message}] }` via + `ZodValidationPipe` (`backend/src/common/pipes/zod-validation.pipe.ts:9-15`) + for `400`s; `ApiError` helpers live in `backend/src/common/errors/api-error.ts`. + +- **Data Layer:** SQLite (`better-sqlite3`) database file at + `process.env.DATABASE_PATH` (default `/data/hipctf/db.sqlite`). The + `category` table is created by `InitSchema1700000000000` and seeded / + amended by `SeedSystemData1700000000100`, + `AddCategoryTimestampsAndUniqueAbbrev1700000000200`, and + `UpdateSystemCategoryKeys1700000000300`. The + `CategoryEntity` (`backend/src/database/entities/category.entity.ts`) is + the single source of truth for the column names that TypeORM will SELECT + via `getMany()`. + +- **Test Framework & Structure:** Jest (`ts-jest`) with two projects + (`backend` and `frontend`) configured in `tests/jest.config.js`. Tests + live in `tests/backend/**/*.spec.ts` and `tests/frontend/**/*.spec.ts` + and run with a single command `npm test` from the repo root (already in + `package.json:scripts.test`). Backend tests typically boot the real + `AppModule` against `:memory:` SQLite via + `process.env.DATABASE_PATH = ':memory:'`; frontend tests are pure + function specs that never touch the DOM. New tests must follow the same + layout. + +- **Required Tools & Dependencies:** No new packages. Node 20+, npm + workspaces, `jest`, `ts-jest`, `better-sqlite3`, `@nestjs/typeorm`, + and `@nestjs/common` are all already declared and bootstrapped by + `setup.sh` (`setup.sh:1-19`). No `setup.sh` change is required for + this Job. + +## 2. Impacted Files + +- **To Modify:** + - `backend/src/database/migrations/1700000000000-InitSchema.ts` — + add `"created_at"` and `"updated_at"` columns to the initial + `CREATE TABLE "category"` so the columns exist from the very first + migration; preserve the `description` and `icon_path` defaults. + - `frontend/src/app/features/admin/general.component.ts` — + extend the Event End `general-eventEnd-error` element so it also + renders the cross-field `endBeforeStart` message when no + per-field `invalidDatetime` error is present; keep + `general-endBeforeStart` as a redundant/legacy visual element only + if it is already asserted in tests (see Test Strategy); bind + `[attr.aria-invalid]` / `[attr.aria-describedby]` / `[attr.title]` + on the Event End `` to the merged state. + - `frontend/src/app/features/admin/general.pure.ts` — add a tiny + helper (e.g. `eventEndFieldError(fieldErrors, crossField)`) that + returns the right human-readable string for the merged Event End + field error, so the template can read it from a single source. + - `tests/frontend/admin-general-pure.spec.ts` — add focused unit + assertions for the new helper (equal-to-start and end-before-start + cases). + - `tests/backend/migrations.spec.ts` — add a focused assertion that + running `InitSchema1700000000000` against an empty database + produces a `category` table that contains a `created_at` column + (and `updated_at`). This locks the SQL schema in place so the + Categories endpoint can never regress to the + `no such column: c.created_at` state. + - `docs/guides/admin-general-settings.md` — clarify that the + Event End inline error region (`general-eventEnd-error`) is the + canonical place where the end-before-start message surfaces. + +- **To Create:** + - None required. Existing test files are sufficient. + +## 3. Proposed Changes + +### 1. Database / Schema Migration + +1. **Update `InitSchema1700000000000`** at + `backend/src/database/migrations/1700000000000-InitSchema.ts:28-37` so + the `category` `CREATE TABLE` includes `created_at` and `updated_at` + columns from the very first migration: + ```sql + CREATE TABLE IF NOT EXISTS "category" ( + "id" TEXT PRIMARY KEY, + "system_key" TEXT, + "name" TEXT NOT NULL, + "abbreviation" TEXT NOT NULL, + "description" TEXT NOT NULL DEFAULT '', + "icon_path" TEXT NOT NULL DEFAULT '', + "created_at" TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), + "updated_at" TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')) + ); + ``` + Mirror the `strftime('%Y-%m-%dT%H:%M:%fZ','now')` default already used + for `user.created_at` (line 17) and `challenge.created_at` (line 54) so + newly-inserted rows get a sane timestamp without a follow-up + UPDATE-pass. + +2. **Keep `AddCategoryTimestampsAndUniqueAbbrev1700000000200` + unchanged.** Its `if (!names.has('created_at'))` / + `if (!names.has('updated_at'))` guards (`backend/src/database/migrations/1700000000200-AddCategoryTimestampsAndUniqueAbbrev.ts:8-20`) + make the ALTER a no-op for fresh databases and still patch + older pre-1.06 databases that lack the columns. No changes needed + to `down()` either — the down() drops the columns, which is the + correct mirror of `up()`. + +3. **Keep `UpdateSystemCategoryKeys1700000000300` unchanged.** Its + `INSERT` already lists `created_at` and `updated_at` + (`backend/src/database/migrations/1700000000300-UpdateSystemCategoryKeys.ts:51`). + +4. No need to touch `CategoryEntity` + (`backend/src/database/entities/category.entity.ts`) — its current + `@Column('text', { name: 'created_at', default: '' })` and + `@Column('text', { name: 'updated_at', default: '' })` annotations + match the columns we are guaranteeing. + +### 2. Backend Logic & APIs + +No controller / service logic changes are required. With the schema +fix, `AdminCategoriesService.list()` at +`backend/src/modules/admin/categories.service.ts:30-37` will resolve +`c.created_at` and `c.updated_at` and return rows that satisfy the +existing `CategoryView` shape used by the front-end. + +If desired, the controller surface can stay untouched +(`backend/src/modules/admin/admin-categories.controller.ts:30-34`) — +no new endpoint is needed. + +### 3. Frontend UI Integration + +1. **Update `frontend/src/app/features/admin/general.pure.ts`** to + expose a small `eventEndFieldMessage(controlErrors, crossField)` pure + helper that returns the canonical Event End field error string: + + | `controlErrors?.['invalidDatetime']` | `form.errors?.['endBeforeStart']` | Returned message | + |---------------------------------------|-----------------------------------|----------------------------------------------------------| + | `true` | any | `"Event end must be a valid ISO-8601 datetime."` | + | falsy | `true` | `"Event end must be after event start."` | + | falsy | falsy | `null` | + + This mirrors the existing `datetimeMessage` pattern and keeps the + template a thin consumer. + +2. **Update `frontend/src/app/features/admin/general.component.ts`**: + + - Replace the current `@if (showEventEndError())` / + `@if (eventEndCrossFieldError())` pair (lines 123-128) with a single + `eventEndFieldMessage` consumer that always renders inside the + `data-testid="general-eventEnd-error"` element. When both + validators are off, the element is absent. + - Bind `[attr.aria-invalid]` to `true` when + `eventEndFieldMessage(...)` is non-null, otherwise null. + - Bind `[attr.aria-describedby]` to `"general-eventEnd-error"` and + bind `[attr.title]` to the resolved message string in that same + case. + - Drop the separate `general-endBeforeStart` block **only after** + confirming no existing test asserts on it (a quick search of + `tests/frontend` shows it is only mentioned in + `tests/frontend/admin-general-pure.spec.ts` as a value assertion on + the constant `endBeforeStartMessage`, not as a DOM query). If a test + asserts on the element, keep it as a redundant duplicate or update + the test. + +3. **Do not change** the form-level validator at + `general.pure.ts:41-51` — it already returns `{ endBeforeStart: true }` + for both `e < s` and `e === s`. The Job requirement is purely a + rendering fix. + +## 4. Test Strategy + +- **Target backend test file:** `tests/backend/migrations.spec.ts` (new + spec). Add a focused `describe('InitSchema creates category with + created_at + updated_at columns')` that: + - Boots `AppModule` against `:memory:` SQLite. + - Calls `dataSource.runMigrations({ transaction: 'each' })`. + - Queries `PRAGMA table_info("category")` and asserts the returned + names contain both `created_at` and `updated_at`. + - Sanity-checks that `AdminCategoriesService.list()` returns an array + (length ≥ 6 seeded system rows) without throwing — this is the + regression test that prevents the user-visible + `no such column: c.created_at` error from coming back. + +- **Target backend test file:** `tests/backend/admin-categories-service.spec.ts` + (extend). Add one `it('list() succeeds against the migrated schema and + returns seeded rows')` that runs after the existing CRUD specs. The + existing tests already use `synchronize: true` which would have masked + the bug; the new test should rely on the **migrations** path so it + actually exercises the schema the production app uses. + +- **Target frontend test file:** `tests/frontend/admin-general-pure.spec.ts` + (extend). Add a `describe('eventEndFieldMessage')` block with three + focused cases: + 1. Returns the ISO message when `invalidDatetime` is set, even if + `endBeforeStart` is also set. + 2. Returns the end-before-start message when `invalidDatetime` is + absent and `endBeforeStart` is set. + 3. Returns `null` when both are absent. + +- **Mocking Strategy:** No new mocks. Backend spec uses the real + `AppModule` against `:memory:` SQLite (already the pattern in + `tests/backend/admin-validation.spec.ts` and + `tests/backend/admin-general-service.spec.ts`). Frontend spec + continues to assert on the pure helper return values only. + +- **Single command:** `npm test` from `/repo` runs the full suite + (already in `package.json:14`). Frontend and backend can also be run + separately via `npm run test:backend` / `npm run test:frontend`. + +## 5. Persistent Project Data (/data) + +Not applicable to this Job. The DB schema change is a migration; tests +use `:memory:` SQLite and never write to `/data`. The Categories +endpoint reads from the existing `category` table without touching +uploaded files or any other shared asset. \ No newline at end of file diff --git a/backend/src/database/migrations/1700000000000-InitSchema.ts b/backend/src/database/migrations/1700000000000-InitSchema.ts index c6c58af..a8bdd15 100644 --- a/backend/src/database/migrations/1700000000000-InitSchema.ts +++ b/backend/src/database/migrations/1700000000000-InitSchema.ts @@ -32,7 +32,9 @@ export class InitSchema1700000000000 implements MigrationInterface { "name" TEXT NOT NULL, "abbreviation" TEXT NOT NULL, "description" TEXT NOT NULL DEFAULT '', - "icon_path" TEXT NOT NULL DEFAULT '' + "icon_path" TEXT NOT NULL DEFAULT '', + "created_at" TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), + "updated_at" TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')) ); `); await queryRunner.query(`CREATE UNIQUE INDEX IF NOT EXISTS "uq_category_system_key" ON "category"("system_key") WHERE "system_key" IS NOT NULL;`); diff --git a/frontend/src/app/features/admin/general.component.ts b/frontend/src/app/features/admin/general.component.ts index 048d617..b62f607 100644 --- a/frontend/src/app/features/admin/general.component.ts +++ b/frontend/src/app/features/admin/general.component.ts @@ -11,6 +11,7 @@ import { deriveEventState, endAfterStartValidator, endBeforeStartMessage, + eventEndFieldMessage, isoDatetimeValidator, normalizePageTitle, pageTitleMessage, @@ -114,17 +115,12 @@ import { type="datetime-local" formControlName="eventEndUtc" data-testid="general-eventEnd" - [attr.aria-invalid]="showEventEndError() || eventEndCrossFieldError() ? 'true' : null" - [attr.aria-describedby]="(showEventEndError() || eventEndCrossFieldError()) - ? 'general-eventEnd-error general-endBeforeStart' - : null" - [attr.title]="(showEventEndError() ? eventEndMessage() : eventEndCrossFieldError() ? endBeforeStartMessage : null)" + [attr.aria-invalid]="eventEndFieldMessageText() ? 'true' : null" + [attr.aria-describedby]="eventEndFieldMessageText() ? 'general-eventEnd-error' : null" + [attr.title]="eventEndFieldMessageText()" /> - @if (showEventEndError()) { -
{{ eventEndMessage() }}
- } - @if (eventEndCrossFieldError()) { -
{{ endBeforeStartMessage }}
+ @if (eventEndFieldMessageText(); as msg) { +
{{ msg }}
} @@ -260,9 +256,11 @@ export class AdminGeneralComponent implements OnInit { datetimeMessage('Event end', this.eventEndValue(), this.form.controls.eventEndUtc.errors), ); - readonly eventEndCrossFieldError = computed( - () => !!this.form.errors?.['endBeforeStart'], - ); + readonly eventEndFieldMessageText = computed(() => { + const fieldErrs = this.eventEndTouchedOrDirty() ? this.form.controls.eventEndUtc.errors : null; + const crossErrs = this.form.errors; + return eventEndFieldMessage(fieldErrs, crossErrs); + }); constructor() { this.form.controls.welcomeMarkdown.valueChanges diff --git a/frontend/src/app/features/admin/general.pure.ts b/frontend/src/app/features/admin/general.pure.ts index 781bb32..9cede94 100644 --- a/frontend/src/app/features/admin/general.pure.ts +++ b/frontend/src/app/features/admin/general.pure.ts @@ -87,3 +87,16 @@ export function datetimeMessage( } export const endBeforeStartMessage = 'Event end must be after event start.'; + +export function eventEndFieldMessage( + controlErrors: { [key: string]: unknown } | null | undefined, + crossFieldErrors: { [key: string]: unknown } | null | undefined, +): string | null { + if (controlErrors?.['invalidDatetime']) { + return 'Event end must be a valid ISO-8601 datetime.'; + } + if (crossFieldErrors?.['endBeforeStart']) { + return endBeforeStartMessage; + } + return null; +} diff --git a/tests/backend/migrations.spec.ts b/tests/backend/migrations.spec.ts index 998d458..1cba788 100644 --- a/tests/backend/migrations.spec.ts +++ b/tests/backend/migrations.spec.ts @@ -73,4 +73,22 @@ describe('Migrations', () => { const restrict = fks.filter((f: any) => f.on_delete === 'RESTRICT'); expect(restrict.length).toBeGreaterThanOrEqual(2); }); + + it('creates the category table with created_at and updated_at columns (Job 889)', async () => { + const cols: any[] = await dataSource.query(`PRAGMA table_info("category")`); + const names = new Set(cols.map((c: any) => c.name)); + expect(names.has('created_at')).toBe(true); + expect(names.has('updated_at')).toBe(true); + }); + + it('lists categories without a "no such column: c.created_at" error (Job 889)', async () => { + const rows = await dataSource + .getRepository(CategoryEntity) + .createQueryBuilder('c') + .orderBy('LOWER(c.abbreviation)', 'ASC') + .addOrderBy('c.abbreviation', 'ASC') + .getMany(); + expect(Array.isArray(rows)).toBe(true); + expect(rows.length).toBeGreaterThanOrEqual(6); + }); }); \ No newline at end of file diff --git a/tests/frontend/admin-general-pure.spec.ts b/tests/frontend/admin-general-pure.spec.ts index 3d6018d..64a83c0 100644 --- a/tests/frontend/admin-general-pure.spec.ts +++ b/tests/frontend/admin-general-pure.spec.ts @@ -2,6 +2,7 @@ import { datetimeMessage, deriveEventState, endAfterStartValidator, + eventEndFieldMessage, isoDatetimeValidator, normalizePageTitle, pageTitleError, @@ -203,3 +204,22 @@ describe('datetimeMessage', () => { expect(datetimeMessage('Event start', '2026-08-15T10:30:00.000Z', null)).toBeNull(); }); }); + +describe('eventEndFieldMessage (Job 889)', () => { + const endIsoMsg = 'Event end must be a valid ISO-8601 datetime.'; + const endBeforeMsg = 'Event end must be after event start.'; + + it('returns the ISO message when invalidDatetime is set, even if endBeforeStart is also set', () => { + expect(eventEndFieldMessage({ invalidDatetime: true }, { endBeforeStart: true })).toBe(endIsoMsg); + }); + + it('returns the end-before-start message when invalidDatetime is absent and endBeforeStart is set', () => { + expect(eventEndFieldMessage(null, { endBeforeStart: true })).toBe(endBeforeMsg); + expect(eventEndFieldMessage({}, { endBeforeStart: true })).toBe(endBeforeMsg); + }); + + it('returns null when both error sources are absent', () => { + expect(eventEndFieldMessage(null, null)).toBeNull(); + expect(eventEndFieldMessage({}, {})).toBeNull(); + }); +}); -- 2.52.0