feat: Admin Area General Settings and Categories 1.00

2026-07-22 12:05:12 +00:00
parent 94c08489f1
commit 590850fbe7
2 changed files with 81 additions and 118 deletions
@@ -1,118 +0,0 @@
 # Implementation Plan: Repair `better-sqlite3` native binding to unblock test suite
 ## 0. Problem Statement
 21 of 48 test suites fail with `TypeError: this.sqlite is not a constructor` thrown from `TypeOrm BetterSqlite3Driver.createDatabaseConnection` (`node_modules/typeorm/driver/better-sqlite3/BetterSqlite3Driver.js:88` — `new this.sqlite(database, ...)`). The remaining suites timeout with `Exceeded timeout of 5000 ms` because TypeORM silently retries (10 retries × 1s) on the same error and the test eventually times out. The visible symptoms in Jest's output are:
 - Direct-DB suites (`tests/backend/theme-required.spec.ts`, `database-init.spec.ts`, `admin-categories-service.spec.ts`, `uploads-logo.spec.ts`, plus any other suite that imports `DatabaseModule`, `CommonModule`, or builds its own `TypeOrmModule.forRoot`) all hang/timeout with the same root error.
 - Suites that use `AppModule` (bootstrap, auth, event-stream, uploads integration tests, etc.) cascade the failure the first time `TypeOrmModule.forRootAsync` is loaded — 16 of 30 backend suites in total trigger the failure mode.
 ## 1. Root Cause (verified by direct read + REPL probe)
 `/repo/node_modules/better-sqlite3` directory exists but **its compiled native binding (`better_sqlite3.node`) is missing**.
 - `ls /repo/node_modules/better-sqlite3/build` → does not exist.
 - `find /repo -name 'better_sqlite3.node'` → zero results anywhere on disk.
 - `cat /repo/node_modules/better-sqlite3/package.json` shows `"install": "prebuild-install || node-gyp rebuild --release"` but the install step never ran for this install. The result is `require('better-sqlite3')` returns a plain function that throws when called as a constructor (`TypeError: this.sqlite is not a constructor` is what TypeORM sees at its `new this.sqlite(...)` call because its `loadDependencies()` reads the package via `PlatformTools.load("better-sqlite3")`; the require "succeeds" returning the stub function, `this.sqlite = sqlite` is set, then `new this.sqlite(...)` blows up because there is no native binding).
 The trigger: an earlier `npm install sharp --no-audit --no-fund --prefix /repo --save-dev` step (used to add `sharp` so the icon-normalization tests could load it) re-mounted the workspace's dependency tree. The output of that command reported `added 7 packages, and removed 345 packages`, and `removed 345` includes the previously-compiled native bindings for `better-sqlite3` (and likely nuked the per-workspace `node_modules` too — `ls /repo/backend/node_modules` shows only `uuid` left, where before install there were ~50 packages). Other native packages (`argon2`, `sharp` itself) still work because they ship prebuilds that survive reinstall, but `better-sqlite3@11` historically relied on a `node-gyp` build that did not run during the reinstall.
 This is **environmental / build-harness damage**, not a regression in any `.ts` source file. None of the seven files I edited this session (Job 859 + logo-upload changes) need to be reverted.
 ## 2. Constraints
 - Plan Mode is active — I will not run install commands or modify files during planning. The full fix is the implementation turn's responsibility.
 - The container runs Node v20.20.2 on `linux-x64` (confirmed via `node -v` and `os.platform()`).
 - `better-sqlite3@11.10.0` was resolved and present in `/repo/node_modules/better-sqlite3/package.json` from the failed install.
 - We must keep `sharp` (added for the icon-normalize feature) and `uuid` (added at backend level) — both were intentional and unrelated to the breakage.
 - The fix must not require a network install if avoidable; it should rely on re-running the existing install scripts.
 - The fix must not require any source-code changes.
 ## 3. Architectural Reconnaissance
 - **Codebase style & conventions:** NestJS 10 backend + Angular 17 frontend. Workspace uses npm workspaces (`package.json` declares `"workspaces": ["backend", "frontend"]`). Native modules resolve via npm's hoisted `node_modules/`.
 - **Native-module recovery precedent in this repo:** None — `setup.sh` simply runs `npm ci --no-audit --no-fund || npm install --no-audit --no-fund` at the repo root, then `npm --workspace frontend run build` and `npm --workspace backend run build`. There is no explicit `npm rebuild` step.
 - **Test framework:** Jest + ts-jest. Two projects: `tests/backend` (node env) and `tests/frontend` (jsdom env). All tests can be re-run with a single `npm test`.
 - **Required Tools & Dependencies:** No new system tools. The fix needs `npm` (already on PATH) and a working C/C++ toolchain in the container (Node was built natively here, so `node-gyp`/prebuilds should work).
 ## 4. Impacted Files (non-source)
 ### To Modify (none — no source code edits required for this fix)
 ### To Update (operational only)
 - `setup.sh` — add an explicit `npm rebuild better-sqlite3` step (see §5) so a fresh container / CI run never has this regression again.
 - `package.json` scripts (optional, low-risk) — add `"rebuild-native": "npm rebuild better-sqlite3"` for future debugging.
 ### No changes needed
 - `backend/package.json` (already declares `better-sqlite3: ^11.0.0` correctly).
 - `backend/src/**` (no source regression).
 - `frontend/src/**` (no source regression).
 - Any test file (the existing tests are correct).
 ## 5. Proposed Changes (ordered, safe, idempotent)
 The fix is two commands + one documentation tweak. All steps are read-only wrt source code.
 1. **Recompile the native binding.** From `/repo`, run:
   ```
   npm rebuild better-sqlite3 --build-from-source
   ```
   This forces `npm` to re-run better-sqlite3's `install` script (prebuild-install, falling back to `node-gyp rebuild --release`) against the current Node 20.20.2 binary. Acceptable runtime: ~30–90 seconds. Verifier: `find /repo -name 'better_sqlite3.node'` should return exactly one path under `node_modules/better-sqlite3/build/Release/...`.
 2. **Verify TypeORM can construct the connection.** Run a quick smoke test:
   ```
   node -e "const db = require('/repo/node_modules/better-sqlite3'); const inst = new db(':memory:'); console.log('memory:', inst.memory);"
   ```
   This must print `memory: <an object>`. If `inst.memory` is an empty object, the binding is correctly loaded.
 3. **Re-run the entire test suite.** Single command from the repo root:
   ```
   npm test
   ```
   Expected: `Test Suites: 48 passed, 48 total / Tests: 271 passed, 271 total` (matching the pre-regression baseline). If the count differs by exactly the new tests added in the prior logo-upload step (still 48 suites, 275 tests), that is the desired outcome.
 4. **Hard-wire the recovery into `setup.sh`.** After the existing `npm ci --no-audit --no-fund || npm install --no-audit --no-fund` step, insert:
   ```
   # Repro native modules that depend on the host Node ABI.
   npm rebuild better-sqlite3 --build-from-source || npm rebuild better-sqlite3
   ```
   This guarantees a fresh clone / CI container that runs `setup.sh` always ends up with a working binding, even if the upstream npm resolution order accidentally drops it again.
 5. **(Optional) Add a maintenance npm script.** In `package.json`'s top-level `scripts`, add:
   ```
   "rebuild-native": "npm rebuild better-sqlite3 --build-from-source"
   ```
   so a future implementer can recover with `npm run rebuild-native` when this surfaces again.
 ## 6. Test Strategy (verification only — no new test files)
 We will not add a new test for this issue because the test suite itself is the canary. After applying steps 1–3 above, the suite must return to its previous green state.
 Verification checklist executed in the implementation turn:
 | Check | How | Expected result |
 |---|---|---|
 | Native binary exists | `find /repo -name 'better_sqlite3.node'` | exactly one file under `build/Release/better_sqlite3.node` |
 | Node can construct | smoke-test one-liner above | `inst.memory` is an object literal, not `undefined`/throw |
 | Backend unit tests green | `npx jest --selectProjects backend --testPathPattern=theme-required` | all 6 tests pass within 30s |
 | Backend integration tests green | `npx jest --selectProjects backend --testPathPattern=uploads-logo` | all 4 tests pass |
 | Full suite green | `npm test` | 48 / 48 suites, 271+ / 271+ tests pass |
 Mocking strategy: not applicable (this is an environmental fix; no new code paths).
 ## 7. Order of Operations (recommended)
 1. Run `npm rebuild better-sqlite3 --build-from-source` from `/repo`.
 2. Run the smoke-test one-liner to confirm binding loads.
 3. Run `npx jest --selectProjects backend --testPathPattern=theme-required` — expect green.
 4. Run the full `npm test` — expect green.
 5. Edit `setup.sh` (and optionally root `package.json`) to wire the rebuild into the canonical setup flow.
 6. Re-run `npm test` one final time to confirm the wired step works on a clean install path (skip if we can't fully simulate a clean install inside plan mode).
 ## 8. Risk and Rollback
 - Risk is minimal: `npm rebuild` on a single package does not touch other dependencies, and `setup.sh` is only modified to add a recovery step (additive only).
 - Rollback: if `npm rebuild better-sqlite3 --build-from-source` fails on this image (no compiler toolchain), the fallback `npm rebuild better-sqlite3` (without `--build-from-source`) will attempt to download a precompiled binary via `prebuild-install`. Either path restores the binding; if both fail the issue is environmental and out of scope of the implementation turn.
 - We are NOT reverting any prior fix per the user's instruction — the seven files changed for Jobs 859 + logo-upload stay exactly as they are.
@@ -0,0 +1,81 @@
 # Implementation Plan: Admin Area — General Settings and Categories (Job 882)
 **[ALREADY_IMPLEMENTED]**
 This Job describes the Admin Area shell, the General settings page, the Categories management list/create/edit/delete, the username-menu "Admin area" entry, the admin navigation guard, and the "Invalid username or password" login feedback. Every functional surface and acceptance criterion listed in the Job Description is already present in the repository on the `main` branch. No code, schema, configuration, or test changes are required.
 ## 1. Why this Job is already implemented
 I walked the data flow end-to-end (browser → frontend `AuthService` → `POST /api/v1/auth/login` → backend `AuthService.login` → `ApiError` envelope → frontend `LandingComponent` → username menu → admin nav guard → admin shell routes) and confirmed every piece the Job calls out is wired up and unit/integration tested.
 ## 2. Architectural Reconnaissance
 - **Codebase style & conventions:**
  - Backend: NestJS 10, TypeORM with `better-sqlite3`, controllers/services/modules, zod-validated DTOs via `ZodValidationPipe`, errors funneled through `ApiError` / `GlobalExceptionFilter`, Argon2id for password hashing.
  - Frontend: Angular 17 standalone components, OnPush change detection, signal-based state, reactive forms, route-level guards (`CanActivateFn`), CSRF + auth HTTP interceptors.
  - Auth: `JwtAuthGuard` (global) + `AdminGuard` (per-handler) + `@Roles('admin')` metadata → `RolesGuard`.
  - Cross-tab session invalidation: `BroadcastChannel` + `localStorage` fallback in `AuthService`.
 - **Data Layer:** TypeORM entities on SQLite (`backend/src/database/entities/user.entity.ts`, `category.entity.ts`, `challenge.entity.ts`, `setting.entity.ts`, `refresh-token.entity.ts`). Settings live in the `setting` table by key; the `category` table has `id`, `name`, `abbreviation`, `description`, `iconPath`, `systemKey`, `createdAt`, `updatedAt`. Users have `role` (`'admin' | 'player'`) and `status` (`'enabled' | 'disabled'`).
 - **Test Framework & Structure:** Jest via `npm test` (root), configured at `tests/jest.config.js` with two projects (`backend`, `frontend`). Tests live exclusively under `tests/backend/*.spec.ts` and `tests/frontend/*.spec.ts`. Backend tests use `supertest` against an in-memory Nest app; frontend tests are pure-TS spec files against the libraries/services.
 - **Required Tools & Dependencies:** None new — backend `argon2`, `zod`, `@nestjs/typeorm`, `better-sqlite3`, `better-sqlite3` native rebuild in `setup.sh`. Frontend: `@angular/core`, `@angular/router`, `@angular/forms`, `@angular/common/http`. Everything is already pinned in `package.json` / workspaces.
 ## 3. Impacted Files (existing — all already implemented)
 ### Backend
 - `backend/src/modules/admin/admin.module.ts` — registers `AdminController`, `AdminGeneralController`, `AdminCategoriesController`, `AdminService`, `AdminGeneralService`, `AdminCategoriesService`; imports `AuthModule`, `UsersModule`, `SettingsModule`, `CommonModule`, and `TypeOrmModule.forFeature([UserEntity, CategoryEntity, ChallengeEntity])`.
 - `backend/src/modules/admin/admin-general.controller.ts` — `@Controller('api/v1/admin/general')`, `@UseGuards(AdminGuard)` + `@Roles('admin')`; exposes `GET/PUT /settings`, `GET /themes`.
 - `backend/src/modules/admin/general.service.ts` — `AdminGeneralService.getSettings / updateSettings / listThemes`. Reads/writes all general settings via `SettingsService` (`PAGE_TITLE`, `LOGO`, `WELCOME_MARKDOWN`, `THEME_KEY`, `EVENT_START_UTC`, `EVENT_END_UTC`, `DEFAULT_CHALLENGE_IP`, `REGISTRATIONS_ENABLED`); emits an SSE `general` event after update via `SseHubService`.
 - `backend/src/modules/admin/dto/general.dto.ts` — `GeneralSettingsSchema` (zod) with `superRefine` enforcing `eventEndUtc > eventStartUtc` and `THEME_IDS` enum on `themeKey`.
 - `backend/src/modules/admin/admin-categories.controller.ts` — `@Controller('api/v1/admin/categories')`, admin-only; `GET / POST / PUT :id / DELETE :id`.
 - `backend/src/modules/admin/categories.service.ts` — `AdminCategoriesService.list / create / update / remove` with **uppercased abbreviation**, **duplicate-abbreviation → 409**, **system-category abbreviation immutable → 409 SYSTEM_PROTECTED**, **delete system category → 403 SYSTEM_PROTECTED**, **delete category with challenges → 409 CATEGORY_HAS_CHALLENGES**.
 - `backend/src/modules/admin/dto/categories.dto.ts` — `CreateCategorySchema`, `UpdateCategorySchema`, `CategoryIdParamSchema`.
 - `backend/src/modules/auth/auth.service.ts` — `login()` throws `ApiError(ERROR_CODES.INVALID_CREDENTIALS, 'Invalid credentials', 401)` on missing/disabled user *or* argon2 verify mismatch. The controller (`auth.controller.ts`) emits the standard envelope to the SPA.
 ### Frontend
 - `frontend/src/app/app.routes.ts` — `/admin` mounted under the auth-guarded `''` (home) parent with child paths `general` (default) and `categories`. `adminGuard` protects the whole subtree.
 - `frontend/src/app/core/guards/admin.guard.ts` + `admin.guard.decision.ts` — Pure `decideAdminGuard({ initialized, isAuthenticated, role })`: not-init → `/bootstrap`, not-auth → `/login`, auth-but-not-admin → `/`, admin → `allow`.
 - `frontend/src/app/core/services/auth.service.ts` — `login()` returns a discriminated `LoginResult`; errors flow through `mapAuthError()`.
 - `frontend/src/app/features/landing/login-modal.service.ts` — `buildLoginFailureMessage()` maps `INVALID_CREDENTIALS` → `'Invalid username or password.'` (the exact text the Job cites).
 - `frontend/src/app/features/landing/landing.component.ts` + `landing.component.html` — surfaces `[data-testid="login-server-error"]` with the mapped text and a `RATE_LIMITED` warn variant; closes modal + navigates to `/` on success.
 - `frontend/src/app/features/shell/header/shell-header.component.ts` — username `user-menu-trigger` toggles `[data-testid="user-menu"]`. When `canAccessAdmin` is true (computed by `shouldShowAdminNav({ isAuthenticated, role })`), the "Admin area" `[data-testid="user-menu-admin"]` entry is rendered.
 - `frontend/src/app/features/home/home.component.ts` — wires the `adminClick` output to `goAdmin()` → `router.navigateByUrl('/admin')`.
 - `frontend/src/app/features/home/home.shell.ts` — exports `shouldShowAdminNav` (pure, unit-tested).
 - `frontend/src/app/features/admin/admin-shell.component.ts` — admin shell template with the side nav `ENTRIES = [General, Challenges, Players, Blog, System]`. `data-testid="admin-aside"`, `data-testid="admin-nav"`, `data-testid="admin-nav-general"`, `<router-outlet />` for child pages. General is `enabled: true`; Challenges/Players/Blog/System render greyed-out (placeholders, per the Job wording — the Job requires the menu items to be present).
 - `frontend/src/app/features/admin/general.component.ts` + `general.pure.ts` — full General Settings page: page title, logo (file upload), global theme (select), event start/end, default challenge IP, registrations toggle, welcome Markdown with live preview, event-state derived display, save with success/error states. All `data-testid`s match the existing tests.
 - `frontend/src/app/features/admin/categories/categories.component.ts` + `category-form-modal.component.ts` + `category-delete-modal.component.ts` — list sorted alphabetically by abbreviation, create/edit/delete modals, icon upload, deletion shows the friendly "Cannot delete: category has N challenge(s) attached" message and a "System categories cannot be deleted" message.
 - `frontend/src/app/core/services/admin.service.ts` — typed wrappers for all the admin endpoints used by the components above.
 ### Tests (existing — all passing)
 - `tests/backend/admin-general-service.spec.ts` — covers `getSettings`, `updateSettings` (settings persisted, SSE `general` event emitted), theme listing intersection.
 - `tests/backend/admin-categories-service.spec.ts` — abbreviation uppercasing, duplicate-abbreviation 409, system abbreviation immutable, system name/description editable, system delete blocked, delete with attached challenges 409, sort by lowercase abbreviation.
 - `tests/backend/admin-guard.spec.ts`, `tests/backend/admin-validation.spec.ts` — guard chain + zod validation.
 - `tests/frontend/admin-shell.spec.ts`, `tests/frontend/admin-navigation.spec.ts` — pure predicate and guard-decision coverage.
 - `tests/frontend/admin-general-pure.spec.ts` — `deriveEventState`, `endAfterStartValidator`, datetime helpers.
 - `tests/frontend/landing-modal.spec.ts` — covers `buildLoginFailureMessage({ code: 'INVALID_CREDENTIALS', ... })` → `'Invalid username or password.'`.
 - `tests/backend/login-shell-smoke.spec.ts` — exercise `POST /api/v1/auth/register-first-admin` → login → `/me` end-to-end.
 ## 4. Job requirement → existing implementation map
 | Job-stated requirement | Where it lives today |
 |---|---|
 | Admin login can be authenticated (POST /api/v1/auth/login admin-creds 201) | `backend/src/modules/auth/auth.service.ts:51-73` + `auth.controller.ts`; covered by `tests/backend/login-shell-smoke.spec.ts`. |
 | Newly-registered non-admin player username menu exposes no "Admin area" entry | `frontend/src/app/features/shell/header/shell-header.component.ts:76-85` (gated by `canAccessAdmin()`); predicate at `frontend/src/app/features/home/home.shell.ts:9-13`; covered by `tests/frontend/admin-shell.spec.ts`. |
 | Direct navigation to /admin by non-admin redirects | `frontend/src/app/core/guards/admin.guard.ts` + `admin.guard.decision.ts:11-13`; covered by `tests/frontend/admin-navigation.spec.ts`. |
 | Admin opens username menu → "Admin area" → /admin reachable | `home.component.ts:154-157 goAdmin()` + `app.routes.ts` `/admin` lazy-loads `AdminShellComponent` which redirects to `/admin/general`. |
 | Admin area shows side menu: General, Challenges, Players, Blog, System | `frontend/src/app/features/admin/admin-shell.component.ts:12-18 ENTRIES`. General enabled, others greyed-out placeholders (matches Job: "with the General, Challenges, Players, Blog, and System menu"). |
 | General section loads + saves settings | `AdminGeneralComponent.ngOnInit / onSubmit` + `AdminService.getGeneralSettings / updateGeneralSettings / listAdminThemes / uploadLogo`. |
 | Categories management data exposed (list/create/update/delete with validation + system protection + challenge-attached protection) | `AdminCategoriesComponent` + `CategoryFormModal` + `CategoryDeleteModal` consuming `AdminService.listCategories / createCategory / updateCategory / deleteCategory / uploadCategoryIcon` against `AdminCategoriesService` + DTOs. All error codes (`SYSTEM_PROTECTED`, `CATEGORY_HAS_CHALLENGES`, conflict) rendered into user-facing messages. |
 | Login with admin credentials returning HTTP 401 with visible "Invalid username or password" | Backend returns `401 { code: 'INVALID_CREDENTIALS', message: 'Invalid credentials' }` from `auth.service.ts:61,67`; frontend maps via `buildLoginFailureMessage` (`login-modal.service.ts:21-22`) → `'Invalid username or password.'`, rendered at `[data-testid="login-server-error"]`. |
 ## 5. Conclusion
 Per the project's "Already Implemented Check" rule, since the entire Job scope — admin shell + General + Categories + admin guard + admin nav predicate + login error mapping — is present and exercised by the existing test suite, the implementation phase should perform **no application edits and no test creation**. The only legitimate action items, if any successor job wanted to extend coverage, would be out of scope for this Job:
 - Optionally enable the still-disabled `Challenges`, `Players`, `Blog`, `System` nav entries (separate future jobs).
 - Optionally surface a more verbose login-failure message (e.g. account-locked distinct from invalid credentials) — also a separate concern.
 ### Files NOT to Modify
 None — the Job is fully complete.