xTr1m/GameList
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Updated review

Browse Source
This commit is contained in:
Frank Tovar
2026-02-07 00:24:30 +01:00
parent 7ecf2b4a59
commit fcd44de4e4
1 changed files with 6 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
REVIEW.md
View File

@@ -65,7 +65,7 @@ Worst coupling points:
- `Endpoints/EndpointHelpers.cs` (84 call sites): de facto god module.
- `wwwroot/js/ui.js` + global `state` object (131 direct state references across JS modules).
- Suggestion and phase workflows span endpoint functions, helper functions, filters, and duplicated client-side checks.
- Operational behavior is split across `API.md`, `scripts/smoke.ps1`, and runtime filters with contradictory contracts.
- Operational behavior is split across `API.md` and runtime filters with contradictory contracts.
## C) Critical task list
@@ -78,10 +78,10 @@ Worst coupling points:
- Dependencies (if any): none.
[P0] Repair admin auth contract drift across code, docs, and smoke automation
- Problem: Severity `High`, Category `Documentation/Tooling`. Documentation and smoke script specify header-based admin auth, runtime requires authenticated admin cookie. Impact: smoke checks and runbooks are misleading during incidents/releases.
- Evidence: `API.md:3`, `scripts/smoke.ps1:46`, `scripts/smoke.ps1:48`, `scripts/smoke.ps1:51`, `Infrastructure/AdminOnlyFilter.cs:12`.
- Recommendation: Make one contract authoritative (cookie-based admin role), update docs and scripts to follow it, and add one integration smoke test path that validates real auth flow.
- Acceptance criteria (testable): `scripts/smoke.ps1` passes against local app without manual edits; `API.md` no longer references `X-Admin-Key` unless implemented; one automated test verifies admin route rejection/acceptance behavior.
- Problem: Severity `High`, Category `Documentation/Tooling`. Documentation and smoke script specify header-based admin auth, runtime requires authenticated admin cookie. Impact: runbooks are misleading during incidents/releases.
- Evidence: `API.md:3`, `Infrastructure/AdminOnlyFilter.cs:12`.
- Recommendation: Make one contract authoritative (account-based admin role), update docs to follow it, and add one integration smoke test path that validates real auth flow.
- Acceptance criteria (testable): `API.md` no longer references `X-Admin-Key`; one automated test verifies admin route rejection/acceptance behavior.
- Effort / Risk: `S / Low`.
- Dependencies (if any): none.
@@ -136,7 +136,7 @@ Worst coupling points:
[P1] Remove legacy/dead paths to reduce cognitive load
- Problem: Severity `Medium`, Category `Other`. Legacy `Reveal` phase and dead UI hooks remain in active code, increasing confusion.
- Evidence: `Domain/Phase.cs:6`, `Endpoints/StateEndpoints.cs:107`, `wwwroot/js/data.js:30`, `wwwroot/js/ui.js:156`, `wwwroot/js/ui.js:1191`.
- Recommendation: Remove obsolete phase enum/value handling and dead UI references (`all-suggestions`, `nav-vote-next`) or reintroduce missing elements intentionally.
- Recommendation: Remove obsolete phase enum/value handling and dead UI references (`all-suggestions`, `nav-vote-next`).
- Acceptance criteria (testable): no references to removed phase/UI ids remain; tests validate expected phase transitions only (`Suggest`, `Vote`, `Results`).
- Effort / Risk: `S / Low`.
- Dependencies (if any): P0 phase cleanup.
@@ -177,7 +177,6 @@ Worst coupling points:
Quick wins (hours to 1 day each):
- Remove stale admin key wording in `API.md` and align with actual auth behavior.
- Fix `scripts/smoke.ps1` to authenticate via register/login and remove duplicate factory reset call.
- Add `Content Remove="GameList.Tests\**\*"` (or equivalent) and verify clean build output.
- Add a build check script that fails on warnings and run it locally/CI.
- Delete dead `DeletePlayerRequest` in `Contracts/Dtos.cs:23` if unused.