diff --git a/.planning/phases/20-admin-member-editor-form-declutter/20-01-SUMMARY.md b/.planning/phases/20-admin-member-editor-form-declutter/20-01-SUMMARY.md new file mode 100644 index 0000000..3e36069 --- /dev/null +++ b/.planning/phases/20-admin-member-editor-form-declutter/20-01-SUMMARY.md @@ -0,0 +1,120 @@ +--- +phase: 20-admin-member-editor-form-declutter +plan: "01" +subsystem: api/admin +status: complete +tags: [tdd, backend, admin, member-profile, last-admin-guard] +dependency_graph: + requires: [] + provides: + - "PATCH /api/admin/members/:id (member-profile update: displayName and/or isAdmin)" + - "isAdmin field on GET /api/admin/members response" + affects: + - apps/api/src/routes/admin.ts + - apps/api/tests/routes/admin.test.ts +tech_stack: + added: [] + patterns: + - "Last-admin guard via COUNT(*) query before demoting the only admin (D-03)" + - "Partial update via whichever fields are present in updateMemberSchema" + - "noEchoHook + parsePositiveIntParam reuse for new PATCH route" +key_files: + created: [] + modified: + - apps/api/src/routes/admin.ts + - apps/api/tests/routes/admin.test.ts +decisions: + - "Use PATCH verb for the member-profile update route (idiomatic REST for partial update)" + - "D-03 guard uses COUNT(*) on users.isAdmin — adapted from auth/user.ts:151-156 pattern" + - "noEchoHook applied to PATCH route for consistency even though body has no sensitive data" + - "Test D: switch currentDevUserId to adminId2 for GET verification after self-demotion (adminId1 is no longer admin post-PATCH)" +metrics: + duration: "4m" + completed: "2026-06-18" + tasks_completed: 3 + files_changed: 2 +--- + +# Phase 20 Plan 01: Member-profile update route + isAdmin read Summary + +PATCH /api/admin/members/:id with displayName/isAdmin partial update, D-03 last-admin guard (409), and isAdmin added to GET /members — implemented test-first. + +## Tasks Completed + +| Task | Name | Commit | Files | +|------|------|--------|-------| +| 1 | RED — failing tests for member-profile route + isAdmin read | a0a82ac | apps/api/tests/routes/admin.test.ts | +| 2 | GREEN — implement PATCH /members/:id + isAdmin in GET /members | bc48632 | apps/api/src/routes/admin.ts, apps/api/tests/routes/admin.test.ts | +| 3 | REFACTOR — tidy + pass CI gates | (no changes needed) | — | + +## What Was Built + +- **`PATCH /api/admin/members/:id`** route in `apps/api/src/routes/admin.ts`: + - Accepts `{ displayName?: string; isAdmin?: boolean }` via `updateMemberSchema` + - Protected by router-wide `requireAdmin` (no second guard — D-02) + - `parsePositiveIntParam` rejects malformed ids → 400 + - Existence check → 404 for unknown member ids + - D-03 last-admin guard: when demoting the only admin → 409 `{ error: 'Cannot remove the last admin' }` + - Self-demotion with a second admin present → 200 + - Partial `set()` from whichever fields are present; try/catch 503 fallback + - `noEchoHook` applied per T-20-04 consistency posture +- **`isAdmin` field** added to `GET /api/admin/members` select and mapped response object + +## Test Coverage (8 scenarios, all passing) + +| Test | Scenario | Status | +|------|----------|--------| +| A | displayName update → 200; GET reflects change | GREEN | +| B | isAdmin promote → 200; GET shows isAdmin true | GREEN | +| C | Last-admin demotion → 409; member stays admin | GREEN | +| D | Self-demotion with second admin → 200; one admin remains | GREEN | +| E | Non-admin PATCH → 403 (requireAdmin boundary) | GREEN | +| F | Wrong-type body → 400 Invalid request; malformed :id → 400 | GREEN | +| G | Non-existent member id → 404 | GREEN | +| H | GET /members includes boolean isAdmin per member | GREEN | + +Full suite: **44 tests passed, 0 failed**. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Test D GET called with demoted user** +- **Found during:** Task 2 (GREEN run) +- **Issue:** Test D called `GET /members` while `currentDevUserId` was still `adminId1`, who had just been demoted — resulting in 403 instead of 200 for the verification GET +- **Fix:** Switched `currentDevUserId = adminId2` before the GET call so the verification uses the remaining admin's session +- **Files modified:** apps/api/tests/routes/admin.test.ts +- **Commit:** bc48632 + +## CI Gates + +All gates pass for modified files: +- `tsc --noEmit`: pass +- `eslint src/routes/admin.ts`: pass +- `prettier --check`: pass (both files) + +## TDD Gate Compliance + +- RED gate commit: `a0a82ac` (`test(20-01): ...`) — 7 tests failing for right reasons +- GREEN gate commit: `bc48632` (`feat(20-01): ...`) — all 44 tests passing +- REFACTOR: no code changes needed — code was already clean from GREEN + +## Known Stubs + +None. + +## Threat Flags + +None — no new network surfaces beyond the planned PATCH route. All T-20-xx mitigations applied as specified. + +## Self-Check: PASSED + +| Check | Result | +|-------|--------| +| apps/api/src/routes/admin.ts | FOUND | +| apps/api/tests/routes/admin.test.ts | FOUND | +| 20-01-SUMMARY.md | FOUND | +| Commit a0a82ac (RED) | FOUND | +| Commit bc48632 (GREEN) | FOUND | +| PATCH route registered | FOUND (line 230) | +| isAdmin in GET /members select | FOUND (line 109) |