docs(20-01): complete member-profile update plan — SUMMARY + all CI gates green

2026-06-18 17:21:30 -04:00
parent bc48632756
commit 4a179a943d
1 changed files with 120 additions and 0 deletions
@@ -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) |