# 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 `<input>` 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.