From 49d0930780baf1b852a92e7e5fdc7793fe01243d Mon Sep 17 00:00:00 2001 From: m0rph3us1987 Date: Thu, 23 Jul 2026 04:06:49 +0000 Subject: [PATCH] AI Implementation feature(909): Challenges Page and Challenge Solve Modal 1.05 (#51) --- .kilo/plans/908.md | 119 ------------------ .kilo/plans/909.md | 26 ++++ docs/architecture/key-files.md | 5 +- docs/guides/challenges-board.md | 9 +- docs/guides/challenges-per-user-state.md | 4 +- docs/index.md | 2 +- .../challenges/challenge-card.component.ts | 18 ++- .../challenge-card-accessibility.spec.ts | 64 ++++++++++ 8 files changed, 116 insertions(+), 131 deletions(-) delete mode 100644 .kilo/plans/908.md create mode 100644 .kilo/plans/909.md create mode 100644 tests/frontend/challenge-card-accessibility.spec.ts diff --git a/.kilo/plans/908.md b/.kilo/plans/908.md deleted file mode 100644 index eb58f72..0000000 --- a/.kilo/plans/908.md +++ /dev/null @@ -1,119 +0,0 @@ -# Implementation Plan: Job 908 — Clear challenge state at the session boundary - -## 1. Architectural Reconnaissance - -- **Codebase style & conventions:** TypeScript monorepo using NestJS (backend) + Angular 20 (frontend SPA), `providedIn: 'root'` signal-based stores, `ChangeDetectionStrategy.OnPush`, standalone components, RxJS for HTTP and SSE; backend uses TypeORM with SQLite. The repo follows the docs in `/repo/docs/` (esp. `architecture/frontend-structure.md`, `guides/challenges-board.md`, `api/challenges.md`). -- **Data Layer:** SQLite via TypeORM. The board endpoint already filters `solvedByMe` per caller's `currentUserId` (`backend/src/modules/challenges/challenges.service.ts:75-125`). **The backend is correct** — `solvedByMe` is computed from the JWT-authenticated user. The bug is purely on the frontend. -- **Test Framework & Structure:** Jest 29 via `ts-jest` (configs in `tests/jest.config.js`). Tests live in `tests/frontend/` (jsdom) and `tests/backend/` (node). Run with `npm test` from repo root. Frontend tests favor plain TS classes / stores with manual fakes (no TestBed harness needed for the unit in question — see `tests/frontend/challenges.store.spec.ts` pattern). -- **Required Tools & Dependencies:** No new packages. The fix is purely a code change inside existing modules. - -## 2. Impacted Files - -- **To Modify:** - - `frontend/src/app/features/challenges/challenges.store.ts` — `setMyUserId()` resets whenever the cached state belongs to an unknown user (i.e. when `_board !== null` and `prev === null` or `prev !== id`). Add a small helper `clearIfStaleOrDifferent(id)` so the rule is consistent. - - `frontend/src/app/features/home/home.component.ts` — inject `ChallengesStore` and call `challengesStore.reset()` in `onLogout()` (after `auth.logout()`) and in `handleSessionInvalidated()` (before navigating to `/login`). - - `tests/frontend/challenges.store.reset.spec.ts` — add a regression test covering the new rule: a preloaded board with **no recorded user id** is cleared when PlayerB's id is set. Also keep the existing user-switch test. -- **To Create:** (none) - -## 3. Proposed Changes - -### 3.1 Root cause - -`ChallengesStore` is a `providedIn: 'root'` singleton that survives logout/login. The current `setMyUserId()` only resets when `prev !== null && prev !== id`, so two real-world cases still leak PlayerA's per-user state into PlayerB's view: - -1. **PlayerB logs in fresh on a server that has never seen them before `setMyUserId` ran.** `prev === null` (no user id has been recorded yet) so the guard skips `reset()`. If the previous user (PlayerA) had loaded a board (now stale in memory), PlayerB may flash the PlayerA board during the moment before their `load()` resolves. In an SSR or hot-reload scenario where `ChallengesPage` is constructed before any user has been associated with the singleton, the very first `setMyUserId` call must clear any pre-existing board. -2. **Session invalidation (logout, SSE 401/403, peer tab logout) clears the auth state but leaves the singleton cache.** `HomeComponent.handleSessionInvalidated()` and `HomeComponent.onLogout()` navigate to `/login` without telling `ChallengesStore` to drop the cached board. - -### 3.2 Backend changes - -None. - -### 3.3 Frontend changes — `frontend/src/app/features/challenges/challenges.store.ts` - -1. **Update `setMyUserId(id)`** so it resets whenever the cached state is for an unknown or different user. Concretely: - - ```ts - setMyUserId(id: string | null): string | null { - const prev = this._myUserId(); - const hasCachedUserData = this._board() !== null || this._selectedDetail() !== null; - // Reset whenever the cached state belongs to a different or unknown user. - if (hasCachedUserData && prev !== id) { - this.reset(); - } - this._myUserId.set(id); - return prev; - } - ``` - - Notes: - - When `prev === id` (same user re-mounts), we keep the cached board — this preserves the existing tests' expectation that a re-mount does not refetch. - - When `prev === null && id !== null` and no board is cached, we just record the id without reset (matches the singleton's first-use path). - - When `prev === null && id !== null` and a board IS cached (orphan state), we reset — this is the new case the test must cover. - - When `prev !== null && prev !== id`, we reset — already covered today. - - When `id === null` (logout path) and the store has cached data, we also reset. - -2. Keep `reset()` as is — it already stops SSE, clears `_board`, `_selectedDetail`, `_myUserId`, `_error`, `_loading`, drops `countdownZeroHandler`, and empties `solveListeners`. - -### 3.4 Frontend changes — `frontend/src/app/features/home/home.component.ts` - -1. **Inject `ChallengesStore`** alongside the existing `userStore` and `auth` injections in `HomeComponent`. - -2. **In `onLogout()`** — after `await this.auth.logout()` and before navigation, call `this.challengesStore.reset()`: - - ```ts - async onLogout(): Promise { - await this.auth.logout(); - this.userMenuOpen.set(false); - this.userStore.reset(); - this.challengesStore.reset(); - await this.router.navigateByUrl('/login'); - } - ``` - -3. **In `handleSessionInvalidated()`** — reset the challenges store alongside `userStore.reset()`, before the navigation guard: - - ```ts - private async handleSessionInvalidated( - _reason: 'peer-logout' | 'sse-unauthorized', - ): Promise { - this.userStore.reset(); - this.challengesStore.reset(); - this.changePasswordOpen.set(false); - this.userMenuOpen.set(false); - if (this.navigatingToLogin) return; - if (this.router.url.startsWith('/login')) return; - this.navigatingToLogin = true; - try { - await this.router.navigateByUrl('/login'); - } finally { - this.navigatingToLogin = false; - } - } - ``` - -This double-coverage (HomeComponent at the session boundary + `setMyUserId` at the page mount) means the store is always cleared at least once on the way out, regardless of whether logout went through the GUI or a peer-tab broadcast. - -### 3.5 Accessibility / UX confirmation - -- `ChallengeCardComponent` binds `[class.solved]="card.solvedByMe"` and the `✓`. After the fix, PlayerB's `solvedByMe` is per-user and falsy for Alpha Cipher. The generic "Solved ✓" therefore disappears from PlayerB's accessibility snapshot. The card retains `120 pts`, `4 solves`, and the difficulty pill. - -## 4. Test Strategy - -- **Target Unit Test File:** `tests/frontend/challenges.store.reset.spec.ts` (update existing spec). Existing tests cover `reset()` and the user-switch path; we add the new "unknown-user" case. -- **Mocking Strategy:** Pure store tests using the hand-rolled API fake pattern from `tests/frontend/challenges.store.spec.ts` (`new ChallengesStore(api as any, { onDestroy: () => undefined } as any)`). No HTTP, no Angular TestBed; no DOM visuals. -- **Key assertions (minimal — only the core success path and one regression per change):** - 1. **NEW: preloaded board with no recorded user id is cleared when PlayerB's id is set.** - - Load a PlayerA-shaped board (`solvedByMe: true`) with **no prior** `setMyUserId()` call. - - Call `setMyUserId('playerB')`. - - Assert `store.board() === null`, `findCard('alpha') === null`, and `store.selectedChallengeDetail() === null`. - - This is the regression that closes the original bug. - 2. **Existing: `setMyUserId` with a different id flushes the previous board** — keep as is. - 3. **Existing: `reset()` clears board, selectedDetail, myUserId and stops any SSE** — keep as is. - 4. **Existing: `applySubmitResponse` always takes the server authoritative `solvedByMe`** — keep as is. -- **Single command from root:** `npm test -- --testPathPattern=challenges.store` runs the updated spec via the existing frontend Jest project; the existing `npm test` continues to run the whole suite. - -## 5. Required Tools & Dependencies - -- **System tools:** Node 20+ (already required by `setup.sh`), SQLite via `better-sqlite3` (already installed via root workspaces). -- **Package dependencies:** None new. The fix uses only existing `@angular/core` (`signal`, `DestroyRef`) and TS types. -- **`setup.sh` impact:** None — no new script, no new env var, no new migration needed. \ No newline at end of file diff --git a/.kilo/plans/909.md b/.kilo/plans/909.md new file mode 100644 index 0000000..8319da1 --- /dev/null +++ b/.kilo/plans/909.md @@ -0,0 +1,26 @@ +# Implementation Plan: Challenges Page and Challenge Solve Modal 1.05 + +## 1. Architectural Reconnaissance +- **Codebase style & conventions:** TypeScript monorepo with a NestJS backend and Angular 17 standalone frontend. The challenges feature uses OnPush presentational components with inline templates/styles, a root-provided signal store, RxJS HTTP services, and authenticated SSE updates. The backend already returns authoritative per-user `solvedByMe` values, and the store already preserves that isolation across load, submit, SSE, logout, and user-switch boundaries. The remaining defect is accessibility semantics: the visible `✓` is nested inside a click-only `
` card and does not reliably appear as an independently exposed solved state in the card's accessibility tree. +- **Data Layer:** SQLite through TypeORM. No schema or migration change is required; `solve` rows and the board API already compute player-specific solve state correctly. +- **Test Framework & Structure:** Root Jest 29 configuration with dedicated suites under `tests/frontend` and `tests/backend`; `npm test` runs all projects and `npm run test:frontend` runs frontend tests. Add one focused, CLI-only source-contract regression test under `tests/frontend` rather than a browser or visual test. The test should verify both solved and unsolved accessibility bindings are derived directly from `card.solvedByMe` and that the solved glyph remains conditionally rendered. +- **Required Tools & Dependencies:** No new system tools, global CLIs, npm packages, persistent `/data` assets, or `setup.sh` changes are needed. Existing Angular, TypeScript, Jest, ts-jest, and jsdom dependencies are sufficient. + +## 2. Impacted Files +- **To Modify:** + - `frontend/src/app/features/challenges/challenge-card.component.ts` — make each card a semantic keyboard-operable control and expose its player-specific solved status on the card accessibility node while retaining the visible conditional checkmark and solved border. +- **To Create:** + - `tests/frontend/challenge-card-accessibility.spec.ts` — minimal regression coverage for the card's accessible solved/unsolved state and conditional checkmark wiring. + +## 3. Proposed Changes +1. **Database / Schema Migration:** No change. Preserve the existing `solve` uniqueness rules and backend `solvedByMe` calculation; the reported scoring, concurrency, and cross-player API behavior already pass. +2. **Backend Logic & APIs:** No change. Continue consuming `BoardCard.solvedByMe` from `GET /api/v1/challenges/board`, challenge detail, submit responses, and player-matched SSE updates as the sole source of player-specific state. +3. **Frontend UI Integration:** + 1. Update the challenge card root in `ChallengeCardComponent` from a generic click target to a native ` `, }) export class ChallengeCardComponent { diff --git a/tests/frontend/challenge-card-accessibility.spec.ts b/tests/frontend/challenge-card-accessibility.spec.ts new file mode 100644 index 0000000..d31ff12 --- /dev/null +++ b/tests/frontend/challenge-card-accessibility.spec.ts @@ -0,0 +1,64 @@ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; + +const sourcePath = resolve( + __dirname, + '../../frontend/src/app/features/challenges/challenge-card.component.ts', +); +const templatePath = resolve( + __dirname, + '../../frontend/src/app/features/challenges/challenge-card.component.ts', +); +const source = readFileSync(sourcePath, 'utf8'); + +describe('ChallengeCardComponent — per-card solved accessibility contract', () => { + function extractTemplate(src: string): string { + const match = src.match(/template:\s*`([\s\S]*?)`,?\s*\n\s*\}/); + return match ? match[1] : ''; + } + + const template = extractTemplate(source); + + it('wraps the card in a native