.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) |

apps/api/src/routes/admin.ts

@@ -11,12 +11,13 @@
  *   - No console.log of request bodies or passwords in any handler (T-10-10).
  *
  * Routes:
- *   GET  /api/admin/members                    → list members + credential status (UI-SPEC Surface 2)
+ *   GET   /api/admin/members                    → list members + credential status + isAdmin (UI-SPEC Surface 2)
- *   POST /api/admin/members                    → create local member: users row + local_credentials (AUTH-LOCAL-07)
+ *   POST  /api/admin/members                    → create local member: users row + local_credentials (AUTH-LOCAL-07)
- *   POST /api/admin/members/:id/password       → admin reset local member password (AUTH-LOCAL-08)
+ *   PATCH /api/admin/members/:id               → update member profile: displayName and/or isAdmin (Plan 20-01)
- *   POST /api/admin/credentials                → validate+encrypt+store for any member (ADMIN-01)
+ *   POST  /api/admin/members/:id/password       → admin reset local member password (AUTH-LOCAL-08)
- *   GET  /api/admin/calendars                  → list synced calendars (UI-SPEC Surface 5)
+ *   POST  /api/admin/credentials                → validate+encrypt+store for any member (ADMIN-01)
- *   PUT  /api/admin/calendars/:id/shared       → exclusive is_shared designation (ADMIN-02)
+ *   GET   /api/admin/calendars                  → list synced calendars (UI-SPEC Surface 5)
+ *   PUT   /api/admin/calendars/:id/shared       → exclusive is_shared designation (ADMIN-02)
  *
  * Mounted in index.ts: app.route('/api/admin', adminRouter)
  */
@@ -105,6 +106,7 @@ adminRouter.get('/members', async (c) => {
       id: users.id,
       displayName: users.displayName,
       color: users.color,
+      isAdmin: users.isAdmin, // Plan 20-01: feeds editor admin toggle initial state (D-02)
       credentialId: memberCredentials.id,
       localCredId: localCredentials.id, // LEFT JOIN — null when no local_credentials row
     })
@@ -116,6 +118,7 @@ adminRouter.get('/members', async (c) => {
     id: row.id,
     displayName: row.displayName,
     color: row.color,
+    isAdmin: row.isAdmin, // Plan 20-01
     hasCredential: row.credentialId !== null,
     hasLocalCredential: row.localCredId !== null, // AUTH-LOCAL-17
   }));
@@ -208,6 +211,74 @@ adminRouter.post('/members', zValidator('json', createMemberSchema, noEchoHook),
   }
 });
 
+// ---------------------------------------------------------------------------
+// PATCH /api/admin/members/:id
+//
+// Updates a member's displayName and/or isAdmin flag (Plan 20-01, D-02, D-03).
+// Security:
+//   - requireAdmin: inherited from adminRouter.use('*', requireAdmin) (D-02 — no second guard)
+//   - noEchoHook: applied for consistency with other admin write routes (T-20-04)
+//   - D-03 last-admin guard: rejects isAdmin=false when target is the sole remaining admin (T-20-02)
+//   - parsePositiveIntParam: rejects malformed ids (T-20-03)
+// ---------------------------------------------------------------------------
+
+const updateMemberSchema = z.object({
+  displayName: z.string().min(1).max(256).optional(),
+  isAdmin: z.boolean().optional(),
+});
+
+adminRouter.patch(
+  '/members/:id',
+  zValidator('json', updateMemberSchema, noEchoHook),
+  async (c) => {
+    const targetId = parsePositiveIntParam(c.req.param('id'));
+    if (targetId === null) {
+      return c.json({ error: 'Invalid member id' }, 400);
+    }
+
+    const { displayName, isAdmin } = c.req.valid('json');
+    // T-20-04: NEVER log request body
+
+    // Verify the target user exists (404 if not)
+    const [target] = await db
+      .select({ id: users.id, isAdmin: users.isAdmin })
+      .from(users)
+      .where(eq(users.id, targetId))
+      .limit(1);
+
+    if (!target) {
+      return c.json({ error: 'Member not found' }, 404);
+    }
+
+    // D-03 last-admin guard: reject demotion of the only remaining admin (T-20-02)
+    if (isAdmin === false && target.isAdmin) {
+      const [{ count }] = await db
+        .select({ count: sql<number>`COUNT(*)` })
+        .from(users)
+        .where(eq(users.isAdmin, true));
+      if (Number(count) <= 1) {
+        return c.json({ error: 'Cannot remove the last admin' }, 409);
+      }
+    }
+
+    // Build a partial set() from whichever fields are present
+    const updates: { displayName?: string; isAdmin?: boolean } = {};
+    if (displayName !== undefined) updates.displayName = displayName;
+    if (isAdmin !== undefined) updates.isAdmin = isAdmin;
+
+    try {
+      await db.update(users).set(updates).where(eq(users.id, targetId));
+      return c.json({ ok: true }, 200);
+    } catch (err) {
+      console.error(
+        '[admin/PATCH /members/:id] Unexpected error:',
+        err instanceof Error ? err.message : String(err),
+      );
+      return c.json({ error: 'Service unavailable' }, 503);
+    }
+  },
+);
+
 // ---------------------------------------------------------------------------
 // POST /api/admin/members/:id/password
 //

apps/api/tests/routes/admin.test.ts

@@ -1071,3 +1071,185 @@ describe('POST /api/admin/members', () => {
     expect(parsed.error).toBe('Invalid request');
   });
 });
+
+// ===========================================================================
+// PATCH /api/admin/members/:id — member-profile update + last-admin guard (Plan 20-01)
+// ===========================================================================
+
+describe('PATCH /api/admin/members/:id', () => {
+  // Test A: happy path — update displayName only
+  it('Test A (happy path displayName): PATCH with { displayName } as admin returns 200; GET reflects new name', async () => {
+    const adminId = await seedUser('admin-patch-name', true);
+    const memberId = await seedUser('member-patch-target', false);
+    currentDevUserId = adminId;
+    const app = await getApp();
+
+    const res = await app.fetch(
+      jsonRequest('PATCH', `/api/admin/members/${memberId}`, { displayName: 'New Name' }),
+    );
+    expect(res.status).toBe(200);
+    const body = (await res.json()) as { ok: boolean };
+    expect(body.ok).toBe(true);
+
+    // GET /members should reflect the updated displayName
+    const getRes = await app.fetch(jsonRequest('GET', '/api/admin/members'));
+    expect(getRes.status).toBe(200);
+    const getBody = (await getRes.json()) as { members: Array<{ id: number; displayName: string }> };
+    const updated = getBody.members.find((m) => m.id === memberId);
+    expect(updated).toBeDefined();
+    expect(updated!.displayName).toBe('New Name');
+  });
+
+  // Test B: happy path — promote non-admin to admin
+  it('Test B (happy path isAdmin promote): PATCH with { isAdmin: true } returns 200; GET shows isAdmin true', async () => {
+    const adminId = await seedUser('admin-patch-promote', true);
+    const memberId = await seedUser('member-patch-promote', false);
+    currentDevUserId = adminId;
+    const app = await getApp();
+
+    const res = await app.fetch(
+      jsonRequest('PATCH', `/api/admin/members/${memberId}`, { isAdmin: true }),
+    );
+    expect(res.status).toBe(200);
+
+    const getRes = await app.fetch(jsonRequest('GET', '/api/admin/members'));
+    expect(getRes.status).toBe(200);
+    const getBody = (await getRes.json()) as {
+      members: Array<{ id: number; isAdmin: boolean }>;
+    };
+    const promoted = getBody.members.find((m) => m.id === memberId);
+    expect(promoted).toBeDefined();
+    expect(promoted!.isAdmin).toBe(true);
+  });
+
+  // Test C: last-admin guard — only admin cannot demote themselves
+  it('Test C (last-admin guard): with exactly one admin, PATCH { isAdmin: false } returns 409; member stays admin', async () => {
+    const adminId = await seedUser('admin-last-admin', true);
+    currentDevUserId = adminId;
+    const app = await getApp();
+
+    const res = await app.fetch(
+      jsonRequest('PATCH', `/api/admin/members/${adminId}`, { isAdmin: false }),
+    );
+    expect(res.status).toBe(409);
+    const body = (await res.json()) as { error: string };
+    expect(typeof body.error).toBe('string');
+    expect(body.error.length).toBeGreaterThan(0);
+
+    // The admin flag must still be true after the rejected demotion
+    const getRes = await app.fetch(jsonRequest('GET', '/api/admin/members'));
+    expect(getRes.status).toBe(200);
+    const getBody = (await getRes.json()) as {
+      members: Array<{ id: number; isAdmin: boolean }>;
+    };
+    const adminRow = getBody.members.find((m) => m.id === adminId);
+    expect(adminRow).toBeDefined();
+    expect(adminRow!.isAdmin).toBe(true);
+  });
+
+  // Test D: self-demotion allowed when another admin exists
+  it('Test D (self-demotion allowed): with two admins, PATCH { isAdmin: false } returns 200; one admin remains', async () => {
+    const adminId1 = await seedUser('admin-demote-1', true);
+    const adminId2 = await seedUser('admin-demote-2', true);
+    // Log in as adminId1 to perform the self-demotion
+    currentDevUserId = adminId1;
+    const app = await getApp();
+
+    const res = await app.fetch(
+      jsonRequest('PATCH', `/api/admin/members/${adminId1}`, { isAdmin: false }),
+    );
+    expect(res.status).toBe(200);
+
+    // Switch to adminId2 to verify the outcome — adminId1 is now non-admin
+    // and can no longer call GET /members (would 403).
+    currentDevUserId = adminId2;
+    const getRes = await app.fetch(jsonRequest('GET', '/api/admin/members'));
+    expect(getRes.status).toBe(200);
+    const getBody = (await getRes.json()) as {
+      members: Array<{ id: number; isAdmin: boolean }>;
+    };
+    const row1 = getBody.members.find((m) => m.id === adminId1);
+    const row2 = getBody.members.find((m) => m.id === adminId2);
+    expect(row1!.isAdmin).toBe(false);
+    expect(row2!.isAdmin).toBe(true);
+  });
+
+  // Test E: auth boundary — non-admin gets 403
+  it('Test E (auth boundary): non-admin PATCH returns 403', async () => {
+    const adminId = await seedUser('admin-patch-auth', true);
+    const nonAdminId = await seedUser('non-admin-patch', false);
+    currentDevUserId = nonAdminId;
+    const app = await getApp();
+
+    const res = await app.fetch(
+      jsonRequest('PATCH', `/api/admin/members/${adminId}`, { displayName: 'Hacked' }),
+    );
+    expect(res.status).toBe(403);
+  });
+
+  // Test F: validation — wrong type and malformed id
+  it('Test F (validation): PATCH with { isAdmin: "yes" } returns 400 { error: "Invalid request" }', async () => {
+    const adminId = await seedUser('admin-patch-validation', true);
+    const memberId = await seedUser('member-patch-validation', false);
+    currentDevUserId = adminId;
+    const app = await getApp();
+
+    const res = await app.fetch(
+      jsonRequest('PATCH', `/api/admin/members/${memberId}`, { isAdmin: 'yes' }),
+    );
+    expect(res.status).toBe(400);
+    const body = (await res.json()) as { error: string };
+    expect(body.error).toBe('Invalid request');
+  });
+
+  it('Test F (malformed id): PATCH with malformed :id (e.g. "1abc") returns 400', async () => {
+    const adminId = await seedUser('admin-patch-badid', true);
+    currentDevUserId = adminId;
+    const app = await getApp();
+
+    const res = await app.fetch(
+      jsonRequest('PATCH', '/api/admin/members/1abc', { displayName: 'Test' }),
+    );
+    expect(res.status).toBe(400);
+  });
+
+  // Test G: not found — non-existent member id
+  it('Test G (not found): PATCH non-existent member id returns 404', async () => {
+    const adminId = await seedUser('admin-patch-notfound', true);
+    currentDevUserId = adminId;
+    const app = await getApp();
+
+    const res = await app.fetch(
+      jsonRequest('PATCH', '/api/admin/members/99999999', { displayName: 'Ghost' }),
+    );
+    expect(res.status).toBe(404);
+  });
+});
+
+// ===========================================================================
+// GET /api/admin/members — isAdmin field (Plan 20-01)
+// ===========================================================================
+
+describe('GET /api/admin/members — isAdmin field', () => {
+  it('Test H (GET isAdmin field): each member object includes a boolean isAdmin field', async () => {
+    const adminId = await seedUser('admin-isadmin-field', true);
+    const memberId = await seedUser('member-isadmin-field', false);
+    currentDevUserId = adminId;
+    const app = await getApp();
+
+    const res = await app.fetch(jsonRequest('GET', '/api/admin/members'));
+    expect(res.status).toBe(200);
+    const body = (await res.json()) as {
+      members: Array<{ id: number; isAdmin: boolean }>;
+    };
+    // Both seeded users should have a boolean isAdmin field
+    const adminRow = body.members.find((m) => m.id === adminId);
+    const memberRow = body.members.find((m) => m.id === memberId);
+    expect(adminRow).toBeDefined();
+    expect(typeof adminRow!.isAdmin).toBe('boolean');
+    expect(adminRow!.isAdmin).toBe(true);
+    expect(memberRow).toBeDefined();
+    expect(typeof memberRow!.isAdmin).toBe('boolean');
+    expect(memberRow!.isAdmin).toBe(false);
+  });
+});