HIPCTF2/.kilo/plans/889.md

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 400s; 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:

   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.