From 4007e40c7dd7c8eec2e7debe1dcecc2b507cb291 Mon Sep 17 00:00:00 2001 From: OpenVelo Agent Date: Wed, 22 Jul 2026 17:34:53 +0000 Subject: [PATCH] feat: Admin Area General Settings and Categories 1.12 --- .kilo/plans/893.md | 37 --------- .kilo/plans/894.md | 82 +++++++++++++++++++ backend/src/modules/admin/general.service.ts | 8 +- .../backend/admin-general-list-themes.spec.ts | 54 ++++++++++++ 4 files changed, 143 insertions(+), 38 deletions(-) delete mode 100644 .kilo/plans/893.md create mode 100644 .kilo/plans/894.md create mode 100644 tests/backend/admin-general-list-themes.spec.ts diff --git a/.kilo/plans/893.md b/.kilo/plans/893.md deleted file mode 100644 index c0589a9..0000000 --- a/.kilo/plans/893.md +++ /dev/null @@ -1,37 +0,0 @@ -# Implementation Plan: Admin Area General Settings and Categories 1.11 - -## 1. Architectural Reconnaissance -- **Codebase style & conventions:** Node.js npm-workspace monorepo with strict TypeScript. The backend is NestJS 10 with thin controllers and repository-backed services; the frontend is Angular 17 using standalone, OnPush components, signals, typed `HttpClient` calls, and embedded `AdminCategoriesComponent` rendering on both `/admin/categories` and `/admin/general`. Category API responses are mapped from entities into `CategoryView`, and list ordering is `LOWER(abbreviation)` plus abbreviation as a deterministic tie-breaker. -- **Data Layer:** TypeORM 0.3 over `better-sqlite3`, `synchronize: false` in the application, explicit migrations registered in `backend/src/database/database.module.ts`, and startup migration execution through `DatabaseInitService.init()` before listening. The persistent production database defaults to `/data/hipctf/db.sqlite`. The current entity and list service require `created_at`/`updated_at`, but legacy databases can have the earlier six-column `category` table. Although migration `1700000000200` attempts to repair timestamps and `1700000000300` defines the canonical CRY/MSC/PWN/REV/WEB/HW rows, the observed persistent database proves a new forward migration is required: already-recorded migrations must never be edited or relied on to rerun. -- **Test Framework & Structure:** Root Jest 29 multi-project configuration with `ts-jest`; backend tests live in `tests/backend/` and run via `npm test` (or focused `npm run test:backend`). Existing migration tests use an isolated in-memory SQLite database. Extend this dedicated test area with a focused legacy-schema migration regression rather than UI/visual tests; the Angular component and service wiring already render icon, abbreviation, description, edit, and delete actions from a successful response. -- **Required Tools & Dependencies:** No new system tools, global CLIs, npm packages, or `/data` fixtures are required. Existing Node/npm, TypeScript, TypeORM, `better-sqlite3`, NestJS testing, and Jest dependencies are sufficient; `setup.sh` should remain unchanged. Verification should use the root single-command suite (`npm test`) and existing build command (`npm run build`). - -## 2. Impacted Files -- **To Modify:** - - `backend/src/database/database.module.ts` — register the new forward repair migration after `UpdateSystemCategoryKeys1700000000300` so existing `/data` databases receive it on restart. - - `tests/backend/migrations.spec.ts` — register the migration in the fresh-schema migration list and strengthen canonical system-category assertions to require exactly CRY, HW, MSC, PWN, REV, WEB once each with populated metadata. - - `tests/backend/database-init.spec.ts` — assert startup initialization yields the exact canonical set and remains duplicate-free across repeated initialization/startup behavior. - - `backend/src/config/env.schema.ts` — replace the obsolete crypto/forensics/pwn/web/misc/osint seed constants with the canonical system-key metadata, keeping seed definitions consistent with the repaired database model for fresh installs and future reuse. -- **To Create:** - - `backend/src/database/migrations/1700000000400-RepairCategorySchemaAndSystemCategories.ts` — forward-only, idempotent compatibility migration for legacy category schemas and stale system-category rows. - - `tests/backend/category-repair-migration.spec.ts` — focused in-memory regression starting from the exact legacy six-column schema and CRY/FOR/MSC/OSI/PWN/WEB data described by the Job. - -## 3. Proposed Changes -1. **Database / Schema Migration:** - - Add a new migration with a timestamp/name greater than `1700000000300`; do not modify the semantics of migrations that may already be recorded in persistent databases. - - Inspect `PRAGMA table_info("category")`; add `created_at` and `updated_at` only when absent, then backfill blank/null values with an ISO UTC SQLite timestamp. This makes TypeORM `SELECT c.*` valid before any category rows are read through `CategoryEntity`. - - Reconcile system rows transactionally against one canonical definition ordered by the required sort key: CRY (Cryptography), HW (Hardware), MSC (Misc), PWN (Pwn), REV (Reverse Engineering), WEB (Web), each with non-empty description and icon path. - - Preserve user-created rows (`system_key IS NULL`). For system rows, remove obsolete FOR/OSI records, update existing canonical-key rows to canonical name/abbreviation/description/icon metadata, and insert only missing canonical keys. Handle abbreviation collisions deterministically: reuse an existing row with the canonical abbreviation only when it can safely become that system row; otherwise avoid destructive changes to unrelated user data and fail migration with a clear conflict rather than silently corrupting references. - - Ensure uniqueness indexes for non-null `system_key` and `abbreviation` exist after reconciliation. Make the SQL guarded/upsert-like so rerunning the migration logic in a test, or restarting after it has been recorded, leaves exactly one row per canonical system key and no duplicates. - - Keep rollback conservative: reverse only schema/index additions that can be safely removed, or explicitly make `down` non-destructive for canonical data so rollback cannot delete user/challenge relationships. -2. **Backend Logic & APIs:** - - Keep `AdminCategoriesService.list()` and `CategoryEntity` timestamp fields intact; once startup repair aligns the physical schema, TypeORM can select all declared columns and map the existing `CategoryView` without the `c.created_at` 500. - - Keep `GET /api/v1/admin/categories` controller/service contracts unchanged. Confirm the service returns repaired rows alphabetically by lowercased abbreviation with deterministic tie-breaking and exposes `iconPath`, `abbreviation`, `description`, `isSystem`, and timestamps. - - Align `SYSTEM_CATEGORY_KEYS`/`SYSTEM_CATEGORY_META` with CRY/HW/MSC/PWN/REV/WEB so a fresh database and any future seed consumer cannot recreate FOR/OSI. Use one canonical metadata representation where practical to prevent the seed and repair migration from drifting. - - Tighten startup verification to validate the canonical six system keys rather than only logging a total category count; retain user-created categories in the total and report missing/duplicate canonical rows explicitly. -3. **Frontend UI Integration:** - - No frontend code change is planned. `AdminGeneralComponent` already embeds `AdminCategoriesComponent`; `AdminService.listCategories()` already calls the correct endpoint; and the category template already renders icon, abbreviation, description, pencil, and trash controls for every returned row. Repairing the API restores both `/admin/general` and `/admin/categories` without introducing a competing Angular effect or touching the reported NG0600 symptom. - -## 4. Test Strategy -- **Target Unit Test File:** `tests/backend/category-repair-migration.spec.ts`, plus minimal assertion updates in `tests/backend/migrations.spec.ts` and `tests/backend/database-init.spec.ts`. -- **Mocking Strategy:** Use a real isolated `better-sqlite3` in-memory `DataSource`/`QueryRunner`; do not mock TypeORM or use the persistent `/data` database. Construct the exact legacy `category` table with only `id`, `system_key`, `name`, `abbreviation`, `description`, and `icon_path`, seed CRY/FOR/MSC/OSI/PWN/WEB, run the new migration, and assert: timestamp columns exist and are populated; a TypeORM repository/list query no longer throws; the system abbreviations are exactly `[CRY, HW, MSC, PWN, REV, WEB]` in API sort order; each row has icon and description metadata; and invoking the repair twice leaves six unique system rows. Keep the migration suite's fresh-database assertion equally exact, and close every DataSource after the tests. No browser, visual confirmation, network service, or heavyweight frontend fixture is needed. diff --git a/.kilo/plans/894.md b/.kilo/plans/894.md new file mode 100644 index 0000000..eaeb74f --- /dev/null +++ b/.kilo/plans/894.md @@ -0,0 +1,82 @@ +# Implementation Plan: Admin Area General Settings and Categories 1.12 (Job 894) + +## 1. Architectural Reconnaissance + +- **Codebase style & conventions:** + - Backend: NestJS 10 with TypeORM + better-sqlite3, TypeScript, async/await throughout, zod-validated DTOs. + - Frontend: Angular 17 standalone components, signals + reactive forms, OnPush change detection, jest-jsdom for tests. + - Tests live under `/repo/tests/backend` and `/repo/tests/frontend` (a single root-level `tests/` directory — never alongside source). They are executed with `npm test` from the repository root via `tests/jest.config.js` (multi-project Jest config, two projects: `backend` and `frontend`). +- **Data Layer:** SQLite via TypeORM (`/data/hipctf/db.sqlite` by default). Theme catalog is file-based (`backend/themes/*.json`), loaded at module-init by `ThemeLoaderService` and filtered at request-time by `AdminGeneralService.listThemes()`. +- **Test Framework & Structure:** Jest 29 with `ts-jest`. Two projects under `tests/jest.config.js`. Backend project uses `node` environment, frontend uses `jsdom` with `tests/frontend/jest.setup.ts`. New tests must be placed under `tests/backend/` (or `tests/frontend/`) and be discoverable by the existing globs (`/backend/**/*.spec.ts` / `/frontend/**/*.spec.ts`). +- **Required Tools & Dependencies:** No new dependencies are required. The fix is a path-resolution change in the existing service. No `setup.sh` change is required (the canonical themes already ship in the repo at `backend/themes/`). + +## 2. Impacted Files + +- **To Modify:** + - `backend/src/modules/admin/general.service.ts` — change the `THEMES_DIR` resolution so the default `./themes` resolves to the backend project's `themes/` directory regardless of process CWD. This is the only behavioural change needed. +- **To Create:** + - `tests/backend/admin-general-list-themes.spec.ts` — minimal unit test that asserts `listThemes()` returns all 10 canonical themes for the default configuration (the current `admin-general-service.spec.ts` already has a `listThemes` suite but it uses `process.env.THEMES_DIR` and a temp dir, so we add a focused new spec that exercises the **default** path-resolution without any env override, which is the regression case). + +## 3. Proposed Changes + +### Root-cause analysis + +`backend/src/modules/admin/general.service.ts:80-103` (`listThemes`) reads +```ts +const themesDir = path.resolve(this.config.get('THEMES_DIR', './themes')); +``` +`path.resolve('./themes', ...)` is resolved against `process.cwd()`. The backend is started with `node /repo/backend/dist/main.js` from `/repo` (per `package.json` `start` script), so `./themes` resolves to `/repo/themes`, which does not exist. The 10 canonical JSON files live at `/repo/backend/themes/`. Because the directory does not exist, `present` stays empty and every theme is filtered out, so `GET /api/v1/admin/general/themes` returns `[]` and the Admin → General "Global theme" `` (frontend tests focus on logic), custom `THEMES_DIR` overrides (already covered by the existing `admin-general-service.spec.ts` "filter to on-disk themes" suite), `PUT /settings` happy-path (already covered), the unknown-`themeKey` 400 (already covered by the existing `rejects unknown themeKey` case in `admin-general-service.spec.ts`), and the runtime NG0600 warning (a downstream symptom that goes away once the list is non-empty — not a separate contract). + +The single test must run with `npm test -- tests/backend/admin-general-list-themes.spec.ts` (or simply `npm test`) from the repository root. diff --git a/backend/src/modules/admin/general.service.ts b/backend/src/modules/admin/general.service.ts index 7b660cc..4aef703 100644 --- a/backend/src/modules/admin/general.service.ts +++ b/backend/src/modules/admin/general.service.ts @@ -78,7 +78,7 @@ export class AdminGeneralService { } listThemes(): ThemeView[] { - const themesDir = path.resolve(this.config.get('THEMES_DIR', './themes')); + const themesDir = this.resolveThemesDir(this.config.get('THEMES_DIR', './themes')); let present = new Set(); try { if (fs.existsSync(themesDir)) { @@ -101,4 +101,10 @@ export class AdminGeneralService { .filter((t) => present.has(t.id)) .map((t) => ({ id: t.id, key: t.id, name: t.name })); } + + private resolveThemesDir(raw: string): string { + if (path.isAbsolute(raw)) return raw; + const backendRoot = path.resolve(__dirname, '..', '..', '..'); + return path.resolve(backendRoot, raw); + } } diff --git a/tests/backend/admin-general-list-themes.spec.ts b/tests/backend/admin-general-list-themes.spec.ts new file mode 100644 index 0000000..67e2ca1 --- /dev/null +++ b/tests/backend/admin-general-list-themes.spec.ts @@ -0,0 +1,54 @@ +process.env.DATABASE_PATH = ':memory:'; +process.env.FRONTEND_DIST = './frontend/dist'; +process.env.NODE_ENV = 'test'; +// Intentionally do NOT set process.env.THEMES_DIR — this spec exercises the +// default path-resolution behaviour so the test fails if the default ever +// regresses to a CWD-relative path. + +import { AdminGeneralService } from '../../backend/src/modules/admin/general.service'; +import { THEME_IDS } from '../../backend/src/common/types/theme-ids'; + +function makeFakeThemeLoader() { + return THEME_IDS.map((id) => ({ + id, + name: id.charAt(0).toUpperCase() + id.slice(1), + tokens: {} as any, + })); +} + +describe('AdminGeneralService.listThemes - default THEMES_DIR resolution (Job 894)', () => { + const fakeSettings = { get: jest.fn(), set: jest.fn() } as any; + const fakeHub = { emitEvent: jest.fn() } as any; + const fakeThemes = { listThemes: () => makeFakeThemeLoader() } as any; + // Mimic the production Nest ConfigService: .get('THEMES_DIR', './themes') + // returns the literal default string when no env override is present. + const fakeConfig = { + get: jest.fn().mockImplementation((key: string, fallback?: any) => fallback), + } as any; + + it('returns all 10 canonical themes when THEMES_DIR is left at its default', () => { + const svc = new AdminGeneralService(fakeSettings, fakeThemes, fakeHub, fakeConfig); + const themes = svc.listThemes(); + expect(themes.length).toBe(10); + const ids = themes.map((t) => t.id).sort(); + expect(ids).toEqual([...THEME_IDS].sort()); + }); + + it('maps every entry to { id, key, name } with id === key and a non-empty name', () => { + const svc = new AdminGeneralService(fakeSettings, fakeThemes, fakeHub, fakeConfig); + const themes = svc.listThemes(); + for (const t of themes) { + expect(t.id).toBe(t.key); + expect(typeof t.name).toBe('string'); + expect(t.name.length).toBeGreaterThan(0); + } + }); + + it('produces the same 10-entry list on repeated calls (no I/O ordering surprises)', () => { + const svc = new AdminGeneralService(fakeSettings, fakeThemes, fakeHub, fakeConfig); + const a = svc.listThemes().map((t) => t.id).sort(); + const b = svc.listThemes().map((t) => t.id).sort(); + expect(b).toEqual(a); + expect(a.length).toBe(10); + }); +}); \ No newline at end of file -- 2.52.0