AI Implementation feature(865): Scoreboard: Ranking, Matrix, Event Log and Score Graph (#52)
This commit was merged in pull request #52.
This commit is contained in:
@@ -0,0 +1,162 @@
|
||||
# Plan: Scoreboard Matrix — Distinguish "solved (not top-3)" from "not solved"
|
||||
|
||||
## Goal
|
||||
In the Matrix view, a cell must render a green checkmark for *any* solve
|
||||
(positions 1, 2, 3, **and** 4+) and stay empty only when the player
|
||||
genuinely has not solved the challenge. Today the backend stores
|
||||
positions > 3 as `null` (skipped at
|
||||
`backend/src/modules/challenges/scoreboard.service.ts:87`), so the
|
||||
frontend can't tell "solved but not top-3" apart from "never solved".
|
||||
We introduce a `'solved'` sentinel so the frontend can always render
|
||||
✓ for a real solve.
|
||||
|
||||
## 1. Type changes
|
||||
|
||||
### 1.1 Backend
|
||||
`backend/src/modules/challenges/dto/scoreboard.dto.ts:21`:
|
||||
```ts
|
||||
export type MatrixCellRank = 1 | 2 | 3 | null;
|
||||
```
|
||||
→
|
||||
```ts
|
||||
export type MatrixCellRank = 1 | 2 | 3 | 'solved' | null;
|
||||
```
|
||||
|
||||
### 1.2 Frontend
|
||||
`frontend/src/app/features/scoreboard/scoreboard.pure.ts:27`:
|
||||
```ts
|
||||
export type MatrixCellRank = 1 | 2 | 3 | null;
|
||||
```
|
||||
→ identical widening to `1 | 2 | 3 | 'solved' | null`.
|
||||
|
||||
Both types must stay in lock-step so the controller JSON, the store,
|
||||
and the matrix component all agree on the new sentinel.
|
||||
|
||||
## 2. Backend projection
|
||||
|
||||
`backend/src/modules/challenges/scoreboard.service.ts:83-96` — the
|
||||
per-challenge solver walk currently does:
|
||||
```ts
|
||||
for (let i = 0; i < list.length; i += 1) {
|
||||
const position = i + 1;
|
||||
if (position > 3) continue; // <- drops positions > 3
|
||||
...
|
||||
perPlayer.set(challengeId, position as MatrixCellRank);
|
||||
}
|
||||
```
|
||||
Replace the `if (position > 3) continue;` with:
|
||||
```ts
|
||||
const value: MatrixCellRank = position <= 3 ? (position as 1|2|3) : 'solved';
|
||||
perPlayer.set(challengeId, value);
|
||||
```
|
||||
Everything else in this loop (player-row creation, map writes) stays
|
||||
unchanged. The cells map therefore now contains a row for every
|
||||
solver of every challenge, with `1|2|3|'solved'|null`.
|
||||
|
||||
## 3. Frontend live mutation
|
||||
|
||||
`frontend/src/app/features/scoreboard/scoreboard.pure.ts:246-248`
|
||||
(`mutateMatrixFromSolve`) — currently only writes for positions 1-3:
|
||||
```ts
|
||||
if (payload.position >= 1 && payload.position <= 3) {
|
||||
cellRow[payload.challengeId] = payload.position as MatrixCellRank;
|
||||
}
|
||||
```
|
||||
Replace with:
|
||||
```ts
|
||||
if (payload.position >= 1 && payload.position <= 3) {
|
||||
cellRow[payload.challengeId] = payload.position as 1 | 2 | 3;
|
||||
} else if (payload.position >= 4) {
|
||||
cellRow[payload.challengeId] = 'solved';
|
||||
}
|
||||
```
|
||||
(no else branch — for `position <= 0` we leave the cell alone, same
|
||||
as today.) With this, a 4th-or-later live solve correctly flips a
|
||||
blank cell to the green ✓.
|
||||
|
||||
## 4. Frontend rendering
|
||||
|
||||
`frontend/src/app/features/scoreboard/matrix.component.ts:86-95` —
|
||||
currently the template uses `*ngSwitchDefault` for non-top-3 cells
|
||||
and re-derives "is this player solved" via a helper method. Now the
|
||||
sentinel tells us directly:
|
||||
```html
|
||||
<ng-container [ngSwitch]="view.cells[p.playerId]?.[ch.id]">
|
||||
<span *ngSwitchCase="1" class="cell-gold" title="1st solver">★</span>
|
||||
<span *ngSwitchCase="2" class="cell-silver" title="2nd solver">★</span>
|
||||
<span *ngSwitchCase="3" class="cell-bronze" title="3rd solver">★</span>
|
||||
<span *ngSwitchCase="'solved'" class="cell-check" title="solved">✓</span>
|
||||
<span *ngSwitchDefault></span>
|
||||
</ng-container>
|
||||
```
|
||||
Changes vs today:
|
||||
- Add an explicit `*ngSwitchCase="'solved'"` branch that renders ✓
|
||||
with `class="cell-check"` (the green color already defined at
|
||||
`matrix.component.ts:32`).
|
||||
- Remove the `*ngSwitchDefault` body (which used `cellSolved()` and a
|
||||
nested `<ng-template #blank>`). With the sentinel there is no
|
||||
ambiguity — `null` means empty, `'solved'` means checkmark.
|
||||
- The `cellSolved(playerId, challengeId)` helper method on
|
||||
`MatrixComponent` becomes dead code (its only caller is the
|
||||
template branch we removed) → delete it.
|
||||
|
||||
## 5. Test updates
|
||||
|
||||
### 5.1 `tests/backend/scoreboard.service.spec.ts`
|
||||
Update the matrix assertion at the "marks top-3 positions" case —
|
||||
currently:
|
||||
```ts
|
||||
expect(matrix.cells['u-d']?.['c1']).toBeNull();
|
||||
```
|
||||
Change to:
|
||||
```ts
|
||||
expect(matrix.cells['u-d']?.['c1']).toBe('solved');
|
||||
```
|
||||
(u-d is the 4th solver of c1 in that test, so this validates the
|
||||
new sentinel.) u-b and u-c stay as `2` and `3` respectively. The
|
||||
matrix cell-shape test for the controller
|
||||
(`tests/backend/scoreboard-controller.spec.ts`) does not assert on
|
||||
the >3 sentinel and needs no change.
|
||||
|
||||
### 5.2 `tests/frontend/scoreboard.pure.spec.ts`
|
||||
Update `mutateMatrixFromSolve` test:
|
||||
```ts
|
||||
const next = mutateMatrixFromSolve(matrix, {
|
||||
...
|
||||
position: 5,
|
||||
});
|
||||
expect(next!.cells.p2.c1).toBe('solved');
|
||||
```
|
||||
(Was `position: 2` expecting `2`; now uses `position: 5` to prove the
|
||||
new branch fires.)
|
||||
|
||||
## 6. Files touched
|
||||
|
||||
- `backend/src/modules/challenges/dto/scoreboard.dto.ts` (type only)
|
||||
- `backend/src/modules/challenges/scoreboard.service.ts` (one block, ~3 lines)
|
||||
- `frontend/src/app/features/scoreboard/scoreboard.pure.ts` (type + `mutateMatrixFromSolve`)
|
||||
- `frontend/src/app/features/scoreboard/matrix.component.ts` (template + remove `cellSolved` helper)
|
||||
- `tests/backend/scoreboard.service.spec.ts` (one assertion)
|
||||
- `tests/frontend/scoreboard.pure.spec.ts` (one assertion)
|
||||
|
||||
## 7. Implementation order
|
||||
|
||||
1. Widen `MatrixCellRank` in backend DTO and frontend pure types.
|
||||
2. Update backend `getMatrix` to store `'solved'` for positions > 3.
|
||||
3. Update `mutateMatrixFromSolve` to write `'solved'` for live 4+ solves.
|
||||
4. Update `matrix.component.ts` template (add `'solved'` case, drop default helper branch) and delete `cellSolved()`.
|
||||
5. Update the two test assertions.
|
||||
6. Run `npm test` (all 590+ tests must pass) and `npm run build`.
|
||||
|
||||
## 8. Compatibility & risk
|
||||
|
||||
- Public API contract: `cells` JSON gains a new string literal value
|
||||
`'solved'`. Older clients that already only branch on `1|2|3|null`
|
||||
would render an unknown cell as empty (because of `ngSwitchDefault`
|
||||
falling back). To keep current behaviour while migrating, the
|
||||
template change MUST land in the same commit as the backend change
|
||||
(single-tenant SPA, no external consumers — safe).
|
||||
- No DB or migration change; the sentinel is purely a projection
|
||||
change over the existing `solve` table.
|
||||
- No impact on Ranking, Event Log, or Score Graph — they do not use
|
||||
`MatrixCellRank`.
|
||||
@@ -1,26 +0,0 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user