feat: Challenges Page and Challenge Solve Modal 1.05

This commit is contained in:
OpenVelo Agent
2026-07-23 04:06:47 +00:00
parent c874e96879
commit 8abb633a64
4 changed files with 105 additions and 122 deletions
-119
View File
@@ -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<void> {
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<void> {
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 `<span *ngIf="card.solvedByMe" class="check" aria-label="Solved">✓</span>`. 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.
+26
View File
@@ -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 `<div>` 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 `<button type="button">`, preserving the existing `data-testid`, `(click)` output, solved CSS class, layout, and visual styling. Normalize button typography/alignment/background as needed so the UI does not regress while gaining native keyboard activation and button semantics.
2. Bind an accessible name or state description on the root card that includes the challenge name and an explicit `Solved` versus `Not solved` phrase derived from `card.solvedByMe`. This puts the state on the card node inspected by accessibility-tree tooling instead of relying on a nested decorative glyph.
3. Keep the checkmark conditionally rendered only when `card.solvedByMe` is true, add a stable `data-testid` tied to the challenge id, and mark the glyph `aria-hidden="true"` because the root card now announces the solved state and duplicate speech should be avoided.
4. Do not infer solved state from solve count, points, global solver data, or another player's SSE event. The template must bind only to `card.solvedByMe`, preserving PlayerA/PlayerB isolation already enforced by `ChallengesStore`.
## 4. Test Strategy
- **Target Unit Test File:** `tests/frontend/challenge-card-accessibility.spec.ts`.
- **Mocking Strategy:** No HTTP, backend, database, SSE, filesystem, or UI-browser mocks. Read the component source as an isolated contract test, following the repository's existing lightweight pattern in `tests/frontend/shell-led.spec.ts`. Assert that the card is a native button, its accessible label/state distinguishes solved from unsolved using `card.solvedByMe`, and the visible checkmark remains conditionally tied to the same field while being hidden from assistive technology to prevent duplicate announcements. Run from the repository root with `npm run test:frontend` (and the full `npm test` verification command during implementation); also run the existing frontend production build because no dedicated lint or typecheck scripts are defined.
@@ -15,6 +15,9 @@ import { BoardCard } from './challenges.pure';
border-radius: var(--radius-sm, 4px); background: var(--color-surface, #1c1f26);
cursor: pointer; transition: transform 0.1s ease;
min-height: 96px;
text-align: left;
color: inherit;
font: inherit;
}
.card:hover { transform: translateY(-1px); border-color: var(--color-primary, #3b82f6); }
.card.solved { border-color: var(--color-success, #22c55e); }
@@ -32,16 +35,25 @@ import { BoardCard } from './challenges.pure';
.diff-HIGH { background: rgba(239,68,68,0.18); color: #dc2626; }
`],
template: `
<div
<button
type="button"
class="card"
[class.solved]="card.solvedByMe"
[attr.data-testid]="'challenge-card-' + card.id"
[attr.data-solved]="card.solvedByMe ? 'true' : 'false'"
[attr.aria-label]="'Challenge ' + card.name + (card.solvedByMe ? ', solved' : ', not solved')"
[attr.aria-pressed]="card.solvedByMe"
(click)="open.emit(card.id)"
>
<div class="card-head">
<img *ngIf="card.categoryIconPath" class="icon" [src]="card.categoryIconPath" [alt]="card.categoryAbbreviation" />
<span class="name">{{ card.name }}</span>
<span *ngIf="card.solvedByMe" class="check" aria-label="Solved">✓</span>
<span
*ngIf="card.solvedByMe"
class="check"
[attr.data-testid]="'challenge-check-' + card.id"
aria-hidden="true"
>✓</span>
</div>
<div class="meta">
<span class="diff" [class]="'diff-' + card.difficulty">{{ card.difficulty }}</span>
@@ -49,7 +61,7 @@ import { BoardCard } from './challenges.pure';
<span>·</span>
<span>{{ card.solveCount }} solve<span *ngIf="card.solveCount !== 1">s</span></span>
</div>
</div>
</button>
`,
})
export class ChallengeCardComponent {
@@ -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 <button type="button"> so it is keyboard-activatable', () => {
expect(template).toMatch(/<button[^>]*type="button"[^>]*>/);
expect(template).not.toMatch(/class="card"[^>]*role=/);
});
it('binds the click output on the button root and preserves open.emit semantics', () => {
expect(template).toMatch(/\(click\)\s*=\s*"open\.emit\(card\.id\)"/);
});
it('preserves the challenge-card-<uuid> data-testid on the new button root', () => {
expect(template).toMatch(/\[attr\.data-testid\]="'challenge-card-' \+ card\.id"/);
});
it('exposes a player-specific solved state on the accessibility tree via aria-label', () => {
expect(template).toMatch(
/\[attr\.aria-label\]="'Challenge ' \+ card\.name \+ \(card\.solvedByMe \? ', solved' : ', not solved'\)"/,
);
});
it('exposes an aria-pressed value derived solely from card.solvedByMe', () => {
const matches = template.match(/\[attr\.aria-pressed\]="card\.solvedByMe"/g) ?? [];
expect(matches.length).toBe(1);
});
it('keeps the conditional checkmark glyph tied to card.solvedByMe and hidden from assistive tech', () => {
expect(template).toMatch(/\*ngIf="card\.solvedByMe"/);
expect(template).toMatch(/class="check"/);
expect(template).toMatch(/aria-hidden="true"/);
});
it('does not derive solved state from solve count, points, or any global field', () => {
expect(template).not.toMatch(/solvedByMe\s*\|\|/);
expect(template).not.toMatch(/solvedByMe\s*&&/);
expect(template).not.toMatch(/solveCount\s*>\s*0/);
expect(template).not.toMatch(/livePoints\s*[<>]=?\s*0/);
expect(template).toMatch(/card\.solvedByMe\s*\?\s*'true'\s*:\s*'false'/);
expect(template).toMatch(/card\.solvedByMe\s*\?\s*', solved'\s*:\s*', not solved'/);
});
it('still binds the solved CSS class solely to card.solvedByMe for visual feedback', () => {
expect(template).toMatch(/\[class\.solved\]="card\.solvedByMe"/);
});
});