Skip to content

feat(admin): Users page to look up and deactivate a user - #1998

Open
DavidCockerill wants to merge 22 commits into
stagefrom
admin-users-deactivate
Open

DavidCockerill wants to merge 22 commits into
stagefrom
admin-users-deactivate

Conversation

@DavidCockerill

@DavidCockerill DavidCockerill commented Oct 22, 2026 •

Copy link
Copy Markdown
Member
A duck receptionist watches a nervous duck write a long scroll beside a big red button

⊙ Problem

Studio has no page for staff to look a user up, see their organizations, or deactivate them; that takes hand-written requests to the central manager.

🤔 Who should see the page?

Only accounts holding user:status, which among staff roles only fabric_admin has, plus the two reads its details lookup needs. Read-only staff can read users, but the page was asked for as fabric_admin-only, so user:read alone doesn't show it.

💡 Solution

Admin → Users:

Needs HarperFast/central-manager#1021 deployed first.

⚖️ Alternatives

  • Compose clusters in Studio from /Admin/Organization/{id} — gated on org:read rather than cluster:read, and one more request per organization.
  • Ask central manager whether it supports removeMemberships — not needed: central-manager#1021 squash-merges, so the details view this page requires and the flag land in one commit.

🤔 Skip the details and only deactivate?

A lookup plus a Deactivate button that reports what happened would be about half the code. The details show staff, before they act, which organizations keep a sole admin and which clusters those organizations run, and they were asked for.

🔧 Changes

✅ Verification

  • index.test.tsx (37 cases) drives the real nested routes: lookups by email and id and every way one is dropped, tiles and links, folding, the cache collision, the menu's states, the typed-email gate, status-only, cleanup and reactivation dialogs, and failed or unreported results. Removing each guard fails its case.
  • admin-users.anon.spec.ts runs the page in Chromium against a mocked central manager and screenshots search, details, menu, confirm (both choices), result and reactivation in light, dark and at 390 px with no horizontal scroll.
  • Route, rail-visibility and entity-id tests extended. Full vitest run: 6,077 pass; tsc -p tsconfig.app.json, oxlint and dprint are clean.
  • Deployed to the local rig against central-manager#1021; not clicked through there with a real sign-in yet.

Refs #1997

🤖 Generated with Claude Code

Related PRs: #1988 overlaps (both add an admin route), #1992 overlaps (both change the admin rail's expected labels)
Complexity: medium

Deferred findings

  • 8b0955349454f051 nit maintainability — src/features/admin/users/index.tsx — Project-local imports omit file extensions — NOT FILED
  • 12f52d66f6797fa5 nit maintainability — src/features/admin/users/userAdminApi.ts — Project-local imports omit file extensions — NOT FILED

Review-Coverage: authored=claude; ran=codex,gemini; adjudicated=domain; blocked=muse(failed); declined=cursor-grok,cursor-composer; rounds=15; full=2 @ 56c345d

Review-Attention: study ~9m (decisions: do-less-alternative, strict-details-shape, users-page-needs-three-permissions, reactivate-plain-confirm, reactivate-leaves-memberships-removed, ignore-not-abort, removal-is-the-default) @ 56c345d

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new 'Users' admin section to Harper Fabric, allowing administrators with the user:status permission to look up users by email, view their organization memberships, and deactivate accounts. The changes include new routing, UI components, API integrations, and comprehensive test coverage. Feedback on the changes suggests improving type safety by validating the shape of parsed API responses instead of using unsafe type assertions, and adding a guard against empty or whitespace-only inputs in the email lookup form to avoid unnecessary refetching.

Comment thread src/features/admin/users/index.tsx Resolved
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 82.29% 16881 / 20514
🔵 Statements 82.18% 17923 / 21807
🔵 Functions 77.74% 4610 / 5930
🔵 Branches 77.25% 13500 / 17475
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/features/admin/routes.ts 64.86% 100% 50% 64.86% 33-34, 40-41, 47-48, 54-55, 61, 68-69, 75-76
src/features/admin/components/AdminShell.tsx 36.84% 66.66% 66.66% 46.66% 65-87
src/features/admin/users/UserDetails.tsx 100% 93.75% 100% 100%
src/features/admin/users/index.tsx 97.56% 100% 87.5% 97.43% 140-143
src/features/admin/users/userAdminApi.ts 100% 100% 100% 100%
src/features/admin/users/components/Deactivation.tsx 96.29% 90.19% 91.66% 95.65% 48
src/features/admin/users/components/EmptyState.tsx 100% 100% 100% 100%
src/features/admin/users/components/OrganizationCard.tsx 100% 76.19% 100% 100%
src/features/admin/users/components/Reactivation.tsx 85.71% 28.57% 75% 85.71% 27
src/features/admin/users/components/UserActionsMenu.tsx 100% 100% 100% 100%
src/features/admin/users/components/UserProfileCard.tsx 100% 54.54% 100% 100%
src/lib/string/entityId.ts 100% 100% 100% 100%
Generated in workflow #2486 for commit 56c345d by the Vitest Coverage Report Action

@dawsontoth dawsontoth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool beans

DavidCockerill and others added 7 commits October 26, 2026 17:08
Staff holding user:status can find an account by email, see each of its
organizations marked "will be removed" or "kept: last admin" (central
manager's GET /Admin/User/{id}?previewMembershipRemoval=true), and deactivate
it behind a confirm dialog that then shows what the PATCH actually did.

Refs #1997

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A failed refetch no longer replaces the loaded account, so a deactivation
result stays open. Looking up the same address again refetches. The lookup
refuses a central manager that answers without the membership preview, the
duplicate-account error no longer carries the address, and the calls use
the SDK's /Admin/User paths.

Refs #1997

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…orted result

A failed PATCH closes the confirm dialog: the toast reports the outcome and
the refetch shows the account as it now is. A successful answer that does
not list the organizations shows "Not reported" instead of breaking the
dialog.

Refs #1997

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The dialog keeps the account as it was when Deactivate was pressed, so the
refetch after the PATCH can no longer unmount the result, whatever it finds.

Refs #1997

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Admin → Users is now a search ("Email or user ID") above a details route,
/admin/users/$userId: the account's dates, its organizations with the roles held and
the deactivation marking, and each organization's live clusters, linked into Studio.
Deactivate runs from there. Reads GET /Admin/User/{id}?details=true; only the id ever
reaches the URL.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… ones

A failed or duplicate lookup now clears the open account like not-found does, a failed
one reaches RUM with central manager's reason, and a lookup answering after the page was
left no longer navigates. Deleted accounts offer no deactivation. The lookup refuses a
blank address and a non-list answer itself, and the result no longer waits on the refetch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DavidCockerill and others added 14 commits October 26, 2026 17:22
A profile header with initials, status, copyable email and id, and tiles for organizations,
live clusters, last activity and membership age; organizations as collapsible cards with role
chips, the deactivation marking and status-dotted cluster rows; a danger-zone panel and a
two-column confirm/result dialog; an empty state for the search and for an unknown id.
Laid out to hold on a phone. An anon e2e spec screenshots it in light, dark and mobile against
a mocked central manager.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…user cache

Both cached under ['fabric-admin', 'users', id], so opening a token owner on the Users page
within five minutes served a name summary where the details belonged, and the page crashed.
Details now live under ['fabric-admin', 'users', 'details', id]. Drops three comments that
narrated the code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The details page no longer marks each organization as kept or removed, or counts them; it
shows the account, its organizations and clusters, and a Deactivate button. The confirm
dialog still lists what a deactivation would remove and keep, and the result what it did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A gear at the card's top right opens a menu whose one item is Deactivate…, with a line
saying what it does, or why it is unavailable (a deleted account, or a deactivated one in
no organization it could leave). The panel at the bottom of the page is gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deactivate stays disabled until the address typed into the dialog matches the account's,
compared the way central manager stores addresses (trimmed, lowercased). Enter confirms
once it matches.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The confirm dialog asks whether to deactivate and leave organizations (the default, with
its preview) or only deactivate, which sends removeMemberships: false and keeps every
membership. The result still lists anything the server reports removed. An account that is
already deactivated gets no choice: deactivating it again only exists to remove leftovers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s its own action

A deactivated account may have kept its memberships on purpose, so its menu item and dialog
now read "Remove from organizations…" (with a Remove button and a "Memberships removed"
result) instead of offering to deactivate it again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…at the server said

The result now reads the submitted choice rather than the live radio, which is locked while
the request is out, and only claims memberships were kept when the server reports nothing
removed; an answer without the lists says it was not reported.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Users layout stays mounted while the next page loads, so the unmount bump missed a
lookup answering mid-navigation, which then replaced the navigation the operator chose.
The lookup now compares the router's latest location with the one it started from.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ee the form

Comparing locations let a round trip through and left the form searching. The lookup
counter is now bumped on every onBeforeNavigate and on unmount, and a dropped lookup
resets the form.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
router.invalidate() runs on every auth change and emits onBeforeNavigate with the URL
unchanged; only a changed href now drops the lookup.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A deactivated account's gear menu now leads with Reactivate…, which confirms and then
sets the status back to ACTIVE. Memberships removed at deactivation are not restored, and
the dialog says so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@DavidCockerill
DavidCockerill marked this pull request as ready for review October 28, 2026 16:00
@DavidCockerill
DavidCockerill requested a review from a team as a code owner October 28, 2026 16:00

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Wish we had preview deployments.

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Reviewed with Muse

…removals by outcome

- Refuse a details answer whose memberships do not say where the account is the last admin,
  rather than previewing those memberships as removable.
- Show Users only to staff who also hold user:read and cluster:read, the permissions its
  details lookup needs, instead of relying on every user:status holder having them.
- Title the remove-from-organizations result from what the server removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@DavidCockerill

Copy link
Copy Markdown
Member Author

Thanks for the review, @kriszyp! All three points were right. They're fixed in 56c345d with a test for each, and each test fails on the previous commit. Replies are in the threads.

🤖 Generated by Claude (Anthropic); posted via @DavidCockerill.

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

🤖 Reviewed with Cursor-grok

: (
<div className="grid gap-3 sm:grid-cols-2">
<MembershipList tone="leave" title="Removed from" memberships={result.removedFrom} />
<MembershipList tone="stay" title="Kept in, as the last admin" memberships={result.retainedIn} />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] A non-last-admin entry in retainedIn is labelled “last admin”; use neutral wording for results that do not establish that distinction

if (
!Array.isArray(organizations)
|| organizations.some((membership) =>
!Array.isArray(membership.clusters)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] organizations containing null produce a raw TypeError instead of the intended compatibility refusal; guard each membership before reading its fields

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants