HIPCTF2/.kilo/plans/888.md

Implementation Plan: Job 888 — Admin Area General Settings (event window validation)

Status

NOT YET IMPLEMENTED. Two negative-case defects remain in the admin General settings form:

1. Backend (backend/src/modules/admin/dto/general.dto.ts:10-11) — the eventStartUtc / eventEndUtc fields are typed z.union([z.literal(''), z.string().datetime({...})]). Empty strings bypass validation and overwrite a previously valid event schedule with ''. The docs at docs/api/admin.md:48-53 and docs/guides/admin-general-settings.md describe this empty-allowed behaviour, but the Job explicitly requires the empty case to be rejected so the prior schedule is preserved.
2. Frontend (frontend/src/app/features/admin/general.component.ts:109-125) — the cross-field endBeforeStart error and the per-field general-eventEnd-error are both gated on form.controls.eventEndUtc.touched. When the user simply types a value the field-level @if may not render visibly, and the End input has no aria-invalid, aria-describedby, title, or validity.validationMessage binding, so a disabled Save button leaves the user with no visible cue.

The plan below fixes both defects and re-enables existing positive paths. No schema migration is required.

1. Architectural Reconnaissance

• Codebase style & conventions:

  • Backend: NestJS 10, TypeScript strict, zod schemas under backend/src/modules/<module>/dto/<module>.dto.ts consumed via ZodValidationPipe (backend/src/common/pipes/zod-validation.pipe.ts). Errors are raised through ApiError.validation(...) returning { code: 'VALIDATION_FAILED', message, details: [{path, message}] } with HTTP 400 (backend/src/common/errors/api-error.ts).
  • Frontend: Angular 17+ standalone components with ChangeDetectionStrategy.OnPush, reactive forms (ReactiveFormsModule), and explicit signal wrappers (Value, Invalid, TouchedOrDirty) for inline error messaging (see general.component.ts:200-257). Templates use @if blocks with data-testid hooks tested in tests/frontend/admin-general-pure.spec.ts.
  • Pure helpers: frontend/src/app/features/admin/general.pure.ts exports all validation/message helpers and is the only place to mutate when changing field-level rules; the component imports them.

• Data Layer: SQLite (better-sqlite3) with a setting key/value table (docs/database/auth-settings.md). AdminGeneralService.updateSettings (backend/src/modules/admin/general.service.ts:61-74) writes each eventStartUtc / eventEndUtc value through SettingsService.set. No schema migration is needed for this Job.

• Test Framework & Structure: Jest (ts-jest) with two projects (backend / frontend) configured in tests/jest.config.js. Tests live under tests/backend/** and tests/frontend/**. The repo-root npm test (alias of jest --config tests/jest.config.js) runs everything. Backend tests boot the full Nest app via Test.createTestingModule({ imports: [AppModule] }) and use request.agent + CSRF (tests/backend/csrf-client.ts); frontend tests are pure-function specs under tests/frontend/admin-general-pure.spec.ts — no DOM, no UI. New tests must follow the same single-command layout.

• Required Tools & Dependencies: No new packages. The required tools (Node 20+, npm workspaces, jest, ts-jest, better-sqlite3) are already declared in package.json and bootstrapped by setup.sh. The implementer does not need to install anything new.

2. Impacted Files

• To Modify:

  • backend/src/modules/admin/dto/general.dto.ts — tighten the eventStartUtc / eventEndUtc zod union so empty strings are rejected with the per-field message; leave superRefine end-after-start check in place.
  • frontend/src/app/features/admin/general.component.ts — extend the Event end <input> 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 <input> 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 <span class="sr-only"> 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.