From ee25d199514058f550a061871bc5a61ec8016fd0 Mon Sep 17 00:00:00 2001 From: David C Date: Thu, 22 Oct 2026 19:32:25 -0400 Subject: [PATCH 01/23] feat(admin): Users page to look up and deactivate a user 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 HarperFast/studio#1997 Co-Authored-By: Claude Opus 5.5 --- .../admin/__tests__/adminRoutes.test.ts | 1 + .../__tests__/adminShellVisibility.test.ts | 10 +- src/features/admin/components/AdminShell.tsx | 4 +- src/features/admin/routes.ts | 8 + src/features/admin/users/index.test.tsx | 149 ++++++++++++ src/features/admin/users/index.tsx | 223 ++++++++++++++++++ src/features/admin/users/userAdminApi.ts | 77 ++++++ src/integrations/api/api.patch.d.ts | 1 + 8 files changed, 470 insertions(+), 3 deletions(-) create mode 100644 src/features/admin/users/index.test.tsx create mode 100644 src/features/admin/users/index.tsx create mode 100644 src/features/admin/users/userAdminApi.ts diff --git a/src/features/admin/__tests__/adminRoutes.test.ts b/src/features/admin/__tests__/adminRoutes.test.ts index a15556282..713684044 100644 --- a/src/features/admin/__tests__/adminRoutes.test.ts +++ b/src/features/admin/__tests__/adminRoutes.test.ts @@ -28,6 +28,7 @@ describe('admin section routes', () => { ['/admin/regions', '/regions'], ['/admin/hosts', '/hosts'], ['/admin/monitoring', '/monitoring'], + ['/admin/users', '/users'], ['/admin/api-token', '/api-token'], ])('%s resolves to its own route', (path, expectedId) => { expect(matchLeaf(path).routeId).toContain(expectedId); diff --git a/src/features/admin/__tests__/adminShellVisibility.test.ts b/src/features/admin/__tests__/adminShellVisibility.test.ts index 2bdaaea00..b996b7a75 100644 --- a/src/features/admin/__tests__/adminShellVisibility.test.ts +++ b/src/features/admin/__tests__/adminShellVisibility.test.ts @@ -60,10 +60,16 @@ describe('admin section visibility', () => { ]); expect(labels(withField)).toEqual(['Notifications', 'Regions', 'Hosts']); // Legacy API without staffPermissions: same carve-out via the fallback. - expect(labels(user('super_user'))).toEqual(['Notifications', 'Regions', 'Hosts', 'Monitoring']); + expect(labels(user('super_user'))).toEqual(['Notifications', 'Regions', 'Hosts', 'Monitoring', 'Users']); }); it('keeps the older pages, not API Tokens, for a legacy-API fabric_admin', () => { - expect(labels(user('fabric_admin'))).toEqual(['Notifications', 'Regions', 'Hosts', 'Monitoring']); + expect(labels(user('fabric_admin'))).toEqual(['Notifications', 'Regions', 'Hosts', 'Monitoring', 'Users']); + }); + + // Reading users (every staff role) is not enough to see the Users page. + it('shows Users only to holders of user:status', () => { + expect(labels(user('fabric_admin', ['user:read', 'user:status']))).toEqual(['Users']); + expect(labels(user('fabric_readonly', ['user:read']))).toEqual([]); }); }); diff --git a/src/features/admin/components/AdminShell.tsx b/src/features/admin/components/AdminShell.tsx index 058e5d01b..6e66e9160 100644 --- a/src/features/admin/components/AdminShell.tsx +++ b/src/features/admin/components/AdminShell.tsx @@ -2,7 +2,7 @@ import { SubNavItem, SubNavRail } from '@/components/SubNavRail'; import { hasStaffPermission, useCloudAuth } from '@/hooks/useAuth'; import { LocalUser, StaffPermission, User } from '@/integrations/api/api.patch'; import { Navigate, Outlet, useLocation } from '@tanstack/react-router'; -import { BellIcon, GlobeIcon, KeyRoundIcon, ServerIcon, SirenIcon } from 'lucide-react'; +import { BellIcon, GlobeIcon, KeyRoundIcon, ServerIcon, SirenIcon, UsersIcon } from 'lucide-react'; /** * Shell for the Admin section: a responsive sub-nav rail (so future admin @@ -20,6 +20,8 @@ const items: Array< { to: '/admin/regions', label: 'Regions', icon: GlobeIcon, permission: 'region:read' }, { to: '/admin/hosts', label: 'Hosts', icon: ServerIcon, permission: 'host:read' }, { to: '/admin/monitoring', label: 'Monitoring', icon: SirenIcon, permission: 'cluster:read' }, + // Gated on deactivation's permission rather than user:read, which keeps the page fabric_admin-only. + { to: '/admin/users', label: 'Users', icon: UsersIcon, permission: 'user:status' }, // Only an API that sends apiToken:self has the token endpoints this page drives (DESIGN.md); // super_user cannot mint a staff credential. { diff --git a/src/features/admin/routes.ts b/src/features/admin/routes.ts index 0fd3cdaf1..b69cfc1ee 100644 --- a/src/features/admin/routes.ts +++ b/src/features/admin/routes.ts @@ -48,6 +48,13 @@ const monitoringRoute = createRoute({ component: lazyRouteComponent(async () => import('@/features/admin/monitoring/index'), 'MonitoringPolicyAdminIndex'), }); +const usersRoute = createRoute({ + getParentRoute: () => adminLayoutRoute, + path: 'users', + head: () => ({ meta: [{ title: 'Users — Harper Fabric' }] }), + component: lazyRouteComponent(async () => import('@/features/admin/users/index'), 'UsersAdminIndex'), +}); + const apiTokenRoute = createRoute({ getParentRoute: () => adminLayoutRoute, path: 'api-token', @@ -62,5 +69,6 @@ export const adminRoutes = [ regionsRoute, hostsRoute, monitoringRoute, + usersRoute, apiTokenRoute, ]; diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx new file mode 100644 index 000000000..520808ce3 --- /dev/null +++ b/src/features/admin/users/index.test.tsx @@ -0,0 +1,149 @@ +/** + * @vitest-environment jsdom + */ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react'; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { get, patch } = vi.hoisted(() => ({ get: vi.fn(), patch: vi.fn() })); + +vi.mock('@/config/apiClient', () => ({ apiClient: { get, patch } })); + +import { UsersAdminIndex } from './index'; + +const USER = { + id: 'usr-1', + email: 'pat@example.com', + firstname: 'Pat', + lastname: 'Lee', + status: 'ACTIVE', + organizations: [ + { organizationId: 'org-solo', organizationName: 'Solo Co', roleNames: ['admin'], lastAdmin: true }, + { + organizationId: 'org-shared', + organizationName: 'Shared Co', + roleNames: ['admin', 'developer'], + lastAdmin: false, + }, + ], +}; + +beforeAll(() => { + // Radix Dialog relies on DOM APIs jsdom doesn't implement. + Element.prototype.hasPointerCapture ??= () => false; + Element.prototype.setPointerCapture ??= () => undefined; + Element.prototype.releasePointerCapture ??= () => undefined; + Element.prototype.scrollIntoView ??= () => undefined; +}); + +let user: typeof USER; + +beforeEach(() => { + user = structuredClone(USER); + get.mockReset(); + patch.mockReset(); + get.mockImplementation(async (url: string) => { + if (url === '/Admin/User/?email=pat%40example.com') { + return { data: [{ id: 'usr-old', status: 'DELETED' }, { id: 'usr-1', status: user.status }] }; + } + if (url === '/Admin/User/usr-1?previewMembershipRemoval=true') { return { data: user }; } + if (url.startsWith('/Admin/User/?email=')) { return { data: [] }; } + throw new Error(`unexpected GET ${url}`); + }); + patch.mockImplementation(async () => { + user = { ...user, status: 'DEACTIVATED', organizations: [user.organizations[0]] }; + return { + data: { + id: 'usr-1', + status: 'DEACTIVATED', + removedFrom: [{ + organizationId: 'org-shared', + organizationName: 'Shared Co', + roleNames: ['admin', 'developer'], + }], + retainedIn: [{ organizationId: 'org-solo', organizationName: 'Solo Co', roleNames: ['admin'] }], + }, + }; + }); +}); +afterEach(cleanup); + +function renderPage() { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + + , + ); +} + +async function lookUp(address: string) { + await act(async () => { + fireEvent.change(screen.getByLabelText('Email'), { target: { value: address } }); + }); + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: 'Look up' })); + }); +} + +describe('UsersAdminIndex', () => { + it('looks up the live account by normalized address and previews each organization', async () => { + renderPage(); + await lookUp(' Pat@Example.com '); + + expect(await screen.findByText('Pat Lee')).toBeTruthy(); + const solo = screen.getByText('Solo Co').closest('li')!; + expect(within(solo).getByText('Kept: last admin')).toBeTruthy(); + const shared = screen.getByText('Shared Co').closest('li')!; + expect(within(shared).getByText('Will be removed')).toBeTruthy(); + expect(within(shared).getByText('admin, developer')).toBeTruthy(); + expect(get).toHaveBeenCalledWith('/Admin/User/usr-1?previewMembershipRemoval=true'); + }); + + it('says so when no account uses the address', async () => { + renderPage(); + await lookUp('nobody@example.com'); + + expect(await screen.findByText('No account uses nobody@example.com.')).toBeTruthy(); + }); + + it('confirms, deactivates, and shows what actually happened', async () => { + renderPage(); + await lookUp('pat@example.com'); + await act(async () => { + fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); + }); + + const confirm = screen.getByRole('dialog'); + expect(within(confirm).getByText('Deactivate pat@example.com?')).toBeTruthy(); + expect(within(within(confirm).getByText('Will be removed from').parentElement!).getByText('Shared Co')) + .toBeTruthy(); + expect(within(within(confirm).getByText('Will stay in, as the last admin').parentElement!).getByText('Solo Co')) + .toBeTruthy(); + + await act(async () => { + fireEvent.click(within(confirm).getByRole('button', { name: 'Deactivate' })); + }); + + expect(patch).toHaveBeenCalledWith('/Admin/User/usr-1', { status: 'DEACTIVATED' }); + const done = await screen.findByRole('dialog'); + expect(within(done).getByText('User deactivated')).toBeTruthy(); + expect(within(within(done).getByText('Removed from').parentElement!).getByText('Shared Co')).toBeTruthy(); + expect(within(within(done).getByText('Kept in, as the last admin').parentElement!).getByText('Solo Co')) + .toBeTruthy(); + + await act(async () => { + fireEvent.click(within(done).getByRole('button', { name: 'Done' })); + }); + expect(await screen.findByText('Deactivated, and in no organization it could leave.')).toBeTruthy(); + }); + + it('still offers deactivation to an already-deactivated account that has memberships to remove', async () => { + user.status = 'DEACTIVATED'; + renderPage(); + await lookUp('pat@example.com'); + + expect(await screen.findByText(/Already deactivated/)).toBeTruthy(); + expect(screen.getByRole('button', { name: 'Deactivate' })).toBeTruthy(); + }); +}); diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx new file mode 100644 index 000000000..1503e0847 --- /dev/null +++ b/src/features/admin/users/index.tsx @@ -0,0 +1,223 @@ +import { Badge } from '@/components/ui/badge'; +import { Button } from '@/components/ui/button'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog'; +import { Input } from '@/components/ui/input'; +import { Label } from '@/components/ui/label'; +import { Skeleton } from '@/components/ui/skeleton'; +import { + AdminUser, + adminUserQueryOptions, + DeactivateUserResult, + MembershipPreview, + normalizeEmail, + OrganizationMembership, + useDeactivateUserMutation, +} from '@/features/admin/users/userAdminApi'; +import { useQuery } from '@tanstack/react-query'; +import { FormEvent, useState } from 'react'; + +export function UsersAdminIndex() { + const [input, setInput] = useState(''); + // Held in state, never in the URL: addresses must stay out of the page URL that RUM and third-party pixels read. + const [email, setEmail] = useState(''); + const { data: user, isFetching, isError, error } = useQuery(adminUserQueryOptions(email)); + + const lookUp = (event: FormEvent) => { + event.preventDefault(); + setEmail(normalizeEmail(input)); + }; + + return ( +
+

Users

+

+ Deactivating an account ends its sessions and blocks sign-in. It also leaves every organization that keeps + another admin who can sign in; where it is the last one, its membership stays so the organization is never left + without an admin. +

+ +
+
+ + setInput(event.target.value)} + /> +
+ +
+ +
+ {!email + ? null + : isFetching && user === undefined + ? + : isError + ?

{error?.message || 'Could not look up that address.'}

+ : user + ? + :

No account uses {email}.

} +
+
+ ); +} + +function UserDetails({ user }: { user: AdminUser }) { + const [confirming, setConfirming] = useState(false); + const name = [user.firstname, user.lastname].filter(Boolean).join(' '); + const removable = user.organizations.filter((membership) => !membership.lastAdmin); + const deactivated = user.status === 'DEACTIVATED'; + + return ( +
+
+
+ {name || user.email} + {user.email} + {user.id} +
+ {user.status ?? 'unknown'} +
+ +
+

Organizations

+ {user.organizations.length + ? ( +
    + {user.organizations.map((membership) => ( + + ))} +
+ ) + :

Not a member of any organization.

} +
+ + {deactivated && !removable.length + ?

Deactivated, and in no organization it could leave.

+ : ( +
+ {deactivated && ( + + Already deactivated: deactivating again removes the memberships marked for removal. + + )} + +
+ )} + + {confirming && setConfirming(false)} />} +
+ ); +} + +function MembershipRow({ membership }: { membership: MembershipPreview }) { + return ( +
  • + + {organizationLabel(membership)} + {membership.roleNames.join(', ')} + + {membership.lastAdmin + ? Kept: last admin + : Will be removed} +
    • + ); +} + +const organizationLabel = (membership: OrganizationMembership) => + membership.organizationName || membership.organizationId; + +/** Mount only while open, so a result never outlives the session that produced it. */ +function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => void }) { + const mutation = useDeactivateUserMutation(); + const [result, setResult] = useState(null); + const removed = user.organizations.filter((membership) => !membership.lastAdmin); + const kept = user.organizations.filter((membership) => membership.lastAdmin); + + return ( + !open && !mutation.isPending && onClose()}> + + + {result ? 'User deactivated' : `Deactivate ${user.email}?`} + + {result + ? 'Sessions ended and sign-in is blocked.' + : 'Their sessions end now and they can no longer sign in.'} + + + + {result + ? ( +
    + + +
    + ) + : ( +
    + + +

    + This preview can change before you confirm; the result lists what actually happened. +

    +
    + )} + + + {result + ? + : ( + <> + + + + )} + +     +     + ); +} + +function MembershipList({ title, memberships, empty }: { + title: string; + memberships: OrganizationMembership[]; + empty: string; +}) { + return ( +
    +

    {title}

    + {memberships.length + ? ( +
      + {memberships.map((membership) =>
    • {organizationLabel(membership)}
      • )} +
    + ) + :

    {empty}

    } +
    + ); +} diff --git a/src/features/admin/users/userAdminApi.ts b/src/features/admin/users/userAdminApi.ts new file mode 100644 index 000000000..88946d9e7 --- /dev/null +++ b/src/features/admin/users/userAdminApi.ts @@ -0,0 +1,77 @@ +// Central manager's staff-only `/Admin/User` endpoint, hand-typed: the generated SDK carries neither the membership +// preview nor the deactivation result, so URLs are cast as elsewhere. The endpoint enforces every permission the page +// gates on. +import { apiClient } from '@/config/apiClient'; +import { queryOptions, useMutation, useQueryClient } from '@tanstack/react-query'; + +export interface OrganizationMembership { + organizationId: string; + organizationName: string | null; + roleNames: string[]; +} + +/** `lastAdmin`: the user is the organization's only admin who could sign in, so a deactivation keeps them there. */ +export interface MembershipPreview extends OrganizationMembership { + lastAdmin: boolean; +} + +export interface AdminUser { + id: string; + email: string; + firstname?: string | null; + lastname?: string | null; + status?: string | null; + organizations: MembershipPreview[]; +} + +export interface DeactivateUserResult { + id: string; + status: string; + removedFrom: OrganizationMembership[]; + retainedIn: OrganizationMembership[]; +} + +const BASE = '/Admin/User/'; + +const userAdminQueryKey = ['fabric-admin', 'users'] as const; + +/** Matches how central manager stores addresses, so the exact-match lookup finds mixed-case input. */ +export const normalizeEmail = (email: string) => email.trim().toLowerCase(); + +async function findUser(email: string): Promise { + const { data } = await apiClient.get(`${BASE}?email=${encodeURIComponent(email)}` as '/User/'); + const live = (data as unknown as Array<{ id: string; status?: string }>).filter((user) => user.status !== 'DELETED'); + if (live.length > 1) { throw new Error(`More than one account uses ${email}.`); } + if (!live.length) { return null; } + + const { data: user } = await apiClient.get( + `${BASE}${encodeURIComponent(live[0].id)}?previewMembershipRemoval=true` as '/User/{id}', + ); + return user as unknown as AdminUser; +} + +/** Callers never send a blank address: the endpoint would answer every user instead of a match. */ +export function adminUserQueryOptions(email: string) { + return queryOptions({ + queryKey: [...userAdminQueryKey, email], + queryFn: () => findUser(email), + enabled: email.length > 0, + }); +} + +/** + * PATCH the status to DEACTIVATED. Failures toast through the global MutationCache handler, which already words an + * unanswered write as "outcome unknown". Every settle refetches, because a failed answer can still be a committed write. + */ +export function useDeactivateUserMutation() { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: async (userId: string) => { + const { data } = await apiClient.patch(`${BASE}${encodeURIComponent(userId)}` as '/User/{id}', { + status: 'DEACTIVATED', + }); + return data as unknown as DeactivateUserResult; + }, + onSettled: () => queryClient.invalidateQueries({ queryKey: userAdminQueryKey }), + }); +} diff --git a/src/integrations/api/api.patch.d.ts b/src/integrations/api/api.patch.d.ts index 1d73d19c4..be986c2c5 100644 --- a/src/integrations/api/api.patch.d.ts +++ b/src/integrations/api/api.patch.d.ts @@ -57,6 +57,7 @@ export type StaffPermission = | 'region:read' | 'region:write' | 'monitoringPolicy:write' + | 'user:status' | 'systemStatus:write' | 'apiToken:create' | 'apiToken:self'; From 027e427793e4333ce33e34397db92b1621321468 Mon Sep 17 00:00:00 2001 From: David C Date: Thu, 22 Oct 2026 19:39:55 -0400 Subject: [PATCH 02/23] fix(admin): keep the Users page and its result on a failed refresh 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 HarperFast/studio#1997 Co-Authored-By: Claude Opus 5.5 --- .../__tests__/adminShellVisibility.test.ts | 3 +- src/features/admin/users/index.test.tsx | 50 +++++++++++++++++++ src/features/admin/users/index.tsx | 21 ++++++-- src/features/admin/users/userAdminApi.ts | 23 +++++---- 4 files changed, 82 insertions(+), 15 deletions(-) diff --git a/src/features/admin/__tests__/adminShellVisibility.test.ts b/src/features/admin/__tests__/adminShellVisibility.test.ts index b996b7a75..bc4ae740a 100644 --- a/src/features/admin/__tests__/adminShellVisibility.test.ts +++ b/src/features/admin/__tests__/adminShellVisibility.test.ts @@ -67,8 +67,7 @@ describe('admin section visibility', () => { expect(labels(user('fabric_admin'))).toEqual(['Notifications', 'Regions', 'Hosts', 'Monitoring', 'Users']); }); - // Reading users (every staff role) is not enough to see the Users page. - it('shows Users only to holders of user:status', () => { + it('shows Users only to holders of user:status, not to every role that can read users', () => { expect(labels(user('fabric_admin', ['user:read', 'user:status']))).toEqual(['Users']); expect(labels(user('fabric_readonly', ['user:read']))).toEqual([]); }); diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 520808ce3..34a476e12 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -146,4 +146,54 @@ describe('UsersAdminIndex', () => { expect(await screen.findByText(/Already deactivated/)).toBeTruthy(); expect(screen.getByRole('button', { name: 'Deactivate' })).toBeTruthy(); }); + + it('keeps the result open when the refetch after deactivating fails', async () => { + renderPage(); + await lookUp('pat@example.com'); + await act(async () => { + fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); + }); + get.mockRejectedValue(new Error('network down')); + await act(async () => { + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); + }); + + expect(await screen.findByText(/Could not refresh this account/)).toBeTruthy(); + const done = screen.getByRole('dialog'); + expect(within(done).getByText('User deactivated')).toBeTruthy(); + expect(within(within(done).getByText('Removed from').parentElement!).getByText('Shared Co')).toBeTruthy(); + }); + + it('looks the same address up again when asked', async () => { + const answer = get.getMockImplementation()!; + get.mockRejectedValueOnce(new Error('timeout')); + renderPage(); + await lookUp('pat@example.com'); + expect(await screen.findByText('timeout')).toBeTruthy(); + + get.mockImplementation(answer); + await lookUp('pat@example.com'); + expect(await screen.findByText('Pat Lee')).toBeTruthy(); + }); + + it('refuses a central manager that answers without the preview', async () => { + const { organizations: _omitted, ...plainRecord } = user; + get.mockImplementation(async (url: string) => + url.includes('?email=') ? { data: [{ id: 'usr-1', status: 'ACTIVE' }] } : { data: plainRecord } + ); + renderPage(); + await lookUp('pat@example.com'); + + expect(await screen.findByText(/cannot preview organization memberships/)).toBeTruthy(); + expect(screen.queryByRole('button', { name: 'Deactivate' })).toBeNull(); + }); + + it('keeps the address out of the duplicate-account error', async () => { + get.mockResolvedValueOnce({ data: [{ id: 'usr-1', status: 'ACTIVE' }, { id: 'usr-2', status: 'ACTIVE' }] }); + renderPage(); + await lookUp('pat@example.com'); + + const message = await screen.findByText(/More than one live account/); + expect(message.textContent).not.toContain('pat@example.com'); + }); }); diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index 1503e0847..850566c6c 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -27,11 +27,14 @@ export function UsersAdminIndex() { const [input, setInput] = useState(''); // Held in state, never in the URL: addresses must stay out of the page URL that RUM and third-party pixels read. const [email, setEmail] = useState(''); - const { data: user, isFetching, isError, error } = useQuery(adminUserQueryOptions(email)); + const { data: user, isFetching, isError, error, refetch } = useQuery(adminUserQueryOptions(email)); const lookUp = (event: FormEvent) => { event.preventDefault(); - setEmail(normalizeEmail(input)); + const next = normalizeEmail(input); + // The same address keeps the same query key, so it needs an explicit refetch to retry or refresh the preview. + if (next === email) { void refetch(); } + else { setEmail(next); } }; return ( @@ -63,10 +66,20 @@ export function UsersAdminIndex() { ? null : isFetching && user === undefined ? + : user + ? ( + // A failed refetch keeps the loaded account, so an open deactivation result stays mounted. + <> + {isError && ( +

    + Could not refresh this account; showing the last one loaded. +

    + )} + + + ) : isError ?

    {error?.message || 'Could not look up that address.'}

    - : user - ? :

    No account uses {email}.

    } diff --git a/src/features/admin/users/userAdminApi.ts b/src/features/admin/users/userAdminApi.ts index 88946d9e7..00c62d5a0 100644 --- a/src/features/admin/users/userAdminApi.ts +++ b/src/features/admin/users/userAdminApi.ts @@ -1,6 +1,5 @@ -// Central manager's staff-only `/Admin/User` endpoint, hand-typed: the generated SDK carries neither the membership -// preview nor the deactivation result, so URLs are cast as elsewhere. The endpoint enforces every permission the page -// gates on. +// Central manager's staff-only `/Admin/User` endpoint. The generated SDK types the paths but not the membership preview +// or the deactivation result, so those are hand-typed. The endpoint enforces every permission the page gates on. import { apiClient } from '@/config/apiClient'; import { queryOptions, useMutation, useQueryClient } from '@tanstack/react-query'; @@ -39,14 +38,20 @@ const userAdminQueryKey = ['fabric-admin', 'users'] as const; export const normalizeEmail = (email: string) => email.trim().toLowerCase(); async function findUser(email: string): Promise { - const { data } = await apiClient.get(`${BASE}?email=${encodeURIComponent(email)}` as '/User/'); + const { data } = await apiClient.get(`${BASE}?email=${encodeURIComponent(email)}` as '/Admin/User/'); const live = (data as unknown as Array<{ id: string; status?: string }>).filter((user) => user.status !== 'DELETED'); - if (live.length > 1) { throw new Error(`More than one account uses ${email}.`); } + // The address stays out of error text: query errors reach the console, and from there RUM. + if (live.length > 1) { throw new Error('More than one live account uses this address.'); } if (!live.length) { return null; } const { data: user } = await apiClient.get( - `${BASE}${encodeURIComponent(live[0].id)}?previewMembershipRemoval=true` as '/User/{id}', + `${BASE}${encodeURIComponent(live[0].id)}?previewMembershipRemoval=true` as '/Admin/User/{id}', ); + // A central manager without the preview answers the plain record; refuse it rather than offer a deactivation + // whose effect can't be shown. + if (!Array.isArray((user as unknown as Partial).organizations)) { + throw new Error('This central manager cannot preview organization memberships yet.'); + } return user as unknown as AdminUser; } @@ -60,14 +65,14 @@ export function adminUserQueryOptions(email: string) { } /** - * PATCH the status to DEACTIVATED. Failures toast through the global MutationCache handler, which already words an - * unanswered write as "outcome unknown". Every settle refetches, because a failed answer can still be a committed write. + * Failures toast through the global MutationCache handler, which already words an unanswered write as "outcome + * unknown". Every settle refetches, because a failed answer can still be a committed write. */ export function useDeactivateUserMutation() { const queryClient = useQueryClient(); return useMutation({ mutationFn: async (userId: string) => { - const { data } = await apiClient.patch(`${BASE}${encodeURIComponent(userId)}` as '/User/{id}', { + const { data } = await apiClient.patch(`${BASE}${encodeURIComponent(userId)}` as '/Admin/User/{id}', { status: 'DEACTIVATED', }); return data as unknown as DeactivateUserResult; From 2bd93bbfedd5991bcac3016858b687b97f6e87ba Mon Sep 17 00:00:00 2001 From: David C Date: Thu, 22 Oct 2026 19:44:05 -0400 Subject: [PATCH 03/23] fix(admin): close the deactivate dialog on failure, and show an unreported 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 HarperFast/studio#1997 Co-Authored-By: Claude Opus 5.5 --- src/features/admin/users/index.test.tsx | 32 +++++++++++++++++++++++- src/features/admin/users/index.tsx | 10 +++++--- src/features/admin/users/userAdminApi.ts | 16 ++++++------ 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 34a476e12..3d0696895 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -9,7 +9,7 @@ const { get, patch } = vi.hoisted(() => ({ get: vi.fn(), patch: vi.fn() })); vi.mock('@/config/apiClient', () => ({ apiClient: { get, patch } })); -import { UsersAdminIndex } from './index'; +import { UsersAdminIndex } from '@/features/admin/users/index'; const USER = { id: 'usr-1', @@ -196,4 +196,34 @@ describe('UsersAdminIndex', () => { const message = await screen.findByText(/More than one live account/); expect(message.textContent).not.toContain('pat@example.com'); }); + + it('closes the confirm dialog when the PATCH fails', async () => { + patch.mockRejectedValueOnce(new Error('no answer')); + renderPage(); + await lookUp('pat@example.com'); + await act(async () => { + fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); + }); + await act(async () => { + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); + }); + + expect(screen.queryByRole('dialog')).toBeNull(); + }); + + it('says so when a successful PATCH does not report the organizations', async () => { + patch.mockResolvedValueOnce({ data: '' }); + renderPage(); + await lookUp('pat@example.com'); + await act(async () => { + fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); + }); + await act(async () => { + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); + }); + + const done = await screen.findByRole('dialog'); + expect(within(done).getByText('User deactivated')).toBeTruthy(); + expect(within(done).getAllByText('Not reported by the server.')).toHaveLength(2); + }); }); diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index 850566c6c..037cdc4e1 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -32,7 +32,6 @@ export function UsersAdminIndex() { const lookUp = (event: FormEvent) => { event.preventDefault(); const next = normalizeEmail(input); - // The same address keeps the same query key, so it needs an explicit refetch to retry or refresh the preview. if (next === email) { void refetch(); } else { setEmail(next); } }; @@ -204,7 +203,8 @@ function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => v @@ -218,13 +218,15 @@ function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => v function MembershipList({ title, memberships, empty }: { title: string; - memberships: OrganizationMembership[]; + memberships: OrganizationMembership[] | null; empty: string; }) { return (

    {title}

    - {memberships.length + {memberships === null + ?

    Not reported by the server.

    + : memberships.length ? (
      {memberships.map((membership) =>
    • {organizationLabel(membership)}
      • )} diff --git a/src/features/admin/users/userAdminApi.ts b/src/features/admin/users/userAdminApi.ts index 00c62d5a0..0646a89a9 100644 --- a/src/features/admin/users/userAdminApi.ts +++ b/src/features/admin/users/userAdminApi.ts @@ -1,5 +1,4 @@ -// Central manager's staff-only `/Admin/User` endpoint. The generated SDK types the paths but not the membership preview -// or the deactivation result, so those are hand-typed. The endpoint enforces every permission the page gates on. +// The generated SDK types these paths but not the membership preview or the deactivation result, so those are typed here. import { apiClient } from '@/config/apiClient'; import { queryOptions, useMutation, useQueryClient } from '@tanstack/react-query'; @@ -23,11 +22,10 @@ export interface AdminUser { organizations: MembershipPreview[]; } +/** Null when the server's answer did not say, though the write went through. */ export interface DeactivateUserResult { - id: string; - status: string; - removedFrom: OrganizationMembership[]; - retainedIn: OrganizationMembership[]; + removedFrom: OrganizationMembership[] | null; + retainedIn: OrganizationMembership[] | null; } const BASE = '/Admin/User/'; @@ -75,7 +73,11 @@ export function useDeactivateUserMutation() { const { data } = await apiClient.patch(`${BASE}${encodeURIComponent(userId)}` as '/Admin/User/{id}', { status: 'DEACTIVATED', }); - return data as unknown as DeactivateUserResult; + const result = data as unknown as Partial | undefined; + return { + removedFrom: Array.isArray(result?.removedFrom) ? result.removedFrom : null, + retainedIn: Array.isArray(result?.retainedIn) ? result.retainedIn : null, + }; }, onSettled: () => queryClient.invalidateQueries({ queryKey: userAdminQueryKey }), }); From f0d359701fa2b2ea5ee8885507829880007b867f Mon Sep 17 00:00:00 2001 From: David C Date: Thu, 22 Oct 2026 19:47:14 -0400 Subject: [PATCH 04/23] fix(admin): hold the deactivation dialog at page level 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 HarperFast/studio#1997 Co-Authored-By: Claude Opus 5.5 --- src/features/admin/users/index.test.tsx | 16 ++++++++++++++++ src/features/admin/users/index.tsx | 20 +++++++++++--------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 3d0696895..c958632f6 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -226,4 +226,20 @@ describe('UsersAdminIndex', () => { expect(within(done).getByText('User deactivated')).toBeTruthy(); expect(within(done).getAllByText('Not reported by the server.')).toHaveLength(2); }); + + it('keeps the result open when the refetch finds no account any more', async () => { + renderPage(); + await lookUp('pat@example.com'); + await act(async () => { + fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); + }); + get.mockResolvedValue({ data: [] }); + await act(async () => { + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); + }); + + expect(await screen.findByText('No account uses pat@example.com.')).toBeTruthy(); + const done = screen.getByRole('dialog'); + expect(within(within(done).getByText('Removed from').parentElement!).getByText('Shared Co')).toBeTruthy(); + }); }); diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index 037cdc4e1..b1c525acd 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -28,6 +28,7 @@ export function UsersAdminIndex() { // Held in state, never in the URL: addresses must stay out of the page URL that RUM and third-party pixels read. const [email, setEmail] = useState(''); const { data: user, isFetching, isError, error, refetch } = useQuery(adminUserQueryOptions(email)); + const [deactivating, setDeactivating] = useState(null); const lookUp = (event: FormEvent) => { event.preventDefault(); @@ -67,26 +68,26 @@ export function UsersAdminIndex() { ? : user ? ( - // A failed refetch keeps the loaded account, so an open deactivation result stays mounted. <> {isError && (

      Could not refresh this account; showing the last one loaded.

      )} - + setDeactivating(user)} /> ) : isError ?

      {error?.message || 'Could not look up that address.'}

      :

      No account uses {email}.

      }
    + + {deactivating && setDeactivating(null)} />} ); } -function UserDetails({ user }: { user: AdminUser }) { - const [confirming, setConfirming] = useState(false); +function UserDetails({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { const name = [user.firstname, user.lastname].filter(Boolean).join(' '); const removable = user.organizations.filter((membership) => !membership.lastAdmin); const deactivated = user.status === 'DEACTIVATED'; @@ -124,11 +125,9 @@ function UserDetails({ user }: { user: AdminUser }) { Already deactivated: deactivating again removes the memberships marked for removal. )} - + )} - - {confirming && setConfirming(false)} />} ); } @@ -150,7 +149,10 @@ function MembershipRow({ membership }: { membership: MembershipPreview }) { const organizationLabel = (membership: OrganizationMembership) => membership.organizationName || membership.organizationId; -/** Mount only while open, so a result never outlives the session that produced it. */ +/** + * Mount only while open. It holds the account as it was when Deactivate was pressed, so the refetch that follows the + * PATCH can't change or unmount it, whatever that refetch finds. + */ function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => void }) { const mutation = useDeactivateUserMutation(); const [result, setResult] = useState(null); @@ -203,7 +205,7 @@ function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => v + + )} + + + ); +} + +function Fact({ label, value }: { label: string; value: string }) { + return ( +
    +
    {label}
    +
    {value}
    +
    + ); +} + +const clusterStatusVariant = (status: string | null) => + status === 'RUNNING' ? 'success' : status === 'FAILED' ? 'destructive' : 'outline'; + +function MembershipCard({ membership }: { membership: MembershipDetails }) { + const { organizationId } = membership; + return ( +
  • +
    + + + {organizationLabel(membership)} + + {membership.roleNames.join(', ')} + + {membership.lastAdmin + ? Kept: last admin + : Removed on deactivation} +
    + {membership.clusters.length + ? ( +
      + {membership.clusters.map((cluster) => ( +
    • + + {cluster.name} + + {cluster.status ?? 'unknown'} +
      • + ))} +
    + ) + :

    No clusters.

    } +
    • + ); +} + +const organizationLabel = (membership: OrganizationMembership) => + membership.organizationName || membership.organizationId; + +/** + * Mount only while open. It holds the account as it was when Deactivate was pressed, so the refetch that follows the + * PATCH can't change or unmount it, whatever that refetch finds. + */ +function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => void }) { + const mutation = useDeactivateUserMutation(); + const [result, setResult] = useState(null); + const removed = user.organizations.filter((membership) => !membership.lastAdmin); + const kept = user.organizations.filter((membership) => membership.lastAdmin); + + return ( + !open && !mutation.isPending && onClose()}> + + + {result ? 'User deactivated' : `Deactivate ${user.email}?`} + + {result + ? 'Sessions ended and sign-in is blocked.' + : 'Their sessions end now and they can no longer sign in.'} + + + + {result + ? ( +
    + + +
    + ) + : ( +
    + + +

    + This preview can change before you confirm; the result lists what actually happened. +

    +
    + )} + + + {result + ? + : ( + <> + + + + )} + +     +     + ); +} + +function MembershipList({ title, memberships, empty }: { + title: string; + memberships: OrganizationMembership[] | null; + empty: string; +}) { + return ( +
    +

    {title}

    + {memberships === null + ?

    Not reported by the server.

    + : memberships.length + ? ( +
      + {memberships.map((membership) =>
    • {organizationLabel(membership)}
      • )} +
    + ) + :

    {empty}

    } +
    + ); +} diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index c958632f6..3e9c96449 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -2,14 +2,24 @@ * @vitest-environment jsdom */ import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, + Outlet, + RouterProvider, +} from '@tanstack/react-router'; import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react'; +import { AxiosError } from 'axios'; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; const { get, patch } = vi.hoisted(() => ({ get: vi.fn(), patch: vi.fn() })); vi.mock('@/config/apiClient', () => ({ apiClient: { get, patch } })); -import { UsersAdminIndex } from '@/features/admin/users/index'; +import { UsersAdminLayout } from '@/features/admin/users/index'; +import { UserDetailsPage } from '@/features/admin/users/UserDetails'; const USER = { id: 'usr-1', @@ -17,17 +27,31 @@ const USER = { firstname: 'Pat', lastname: 'Lee', status: 'ACTIVE', + isVerified: true, organizations: [ - { organizationId: 'org-solo', organizationName: 'Solo Co', roleNames: ['admin'], lastAdmin: true }, + { organizationId: 'org-solo', organizationName: 'Solo Co', roleNames: ['admin'], lastAdmin: true, clusters: [] }, { organizationId: 'org-shared', organizationName: 'Shared Co', roleNames: ['admin', 'developer'], lastAdmin: false, + clusters: [ + { id: 'clu-a', name: 'alpha', status: 'RUNNING' }, + { id: 'clu-b', name: 'beta', status: 'FAILED' }, + ], }, ], }; +const OTHER = { + id: 'usr-2', + email: 'sam@example.com', + firstname: 'Sam', + lastname: 'Ray', + status: 'ACTIVE', + organizations: [], +}; + beforeAll(() => { // Radix Dialog relies on DOM APIs jsdom doesn't implement. Element.prototype.hasPointerCapture ??= () => false; @@ -36,6 +60,8 @@ beforeAll(() => { Element.prototype.scrollIntoView ??= () => undefined; }); +const notFound = () => new AxiosError('Not Found', '404', undefined, undefined, { status: 404 } as never); + let user: typeof USER; beforeEach(() => { @@ -46,8 +72,10 @@ beforeEach(() => { if (url === '/Admin/User/?email=pat%40example.com') { return { data: [{ id: 'usr-old', status: 'DELETED' }, { id: 'usr-1', status: user.status }] }; } - if (url === '/Admin/User/usr-1?previewMembershipRemoval=true') { return { data: user }; } if (url.startsWith('/Admin/User/?email=')) { return { data: [] }; } + if (url === '/Admin/User/usr-1?details=true') { return { data: user }; } + if (url === '/Admin/User/usr-2?details=true') { return { data: OTHER }; } + if (url.endsWith('?details=true')) { throw notFound(); } throw new Error(`unexpected GET ${url}`); }); patch.mockImplementation(async () => { @@ -68,48 +96,143 @@ beforeEach(() => { }); afterEach(cleanup); -function renderPage() { +/** The admin users routes as the app nests them, without the admin shell's auth around them. */ +async function renderAt(path = '/admin/users') { + const rootRoute = createRootRoute({ component: Outlet }); + const usersRoute = createRoute({ getParentRoute: () => rootRoute, path: 'admin/users', component: UsersAdminLayout }); + const routeTree = rootRoute.addChildren([ + usersRoute.addChildren([ + createRoute({ getParentRoute: () => usersRoute, path: '/' }), + createRoute({ getParentRoute: () => usersRoute, path: '$userId', component: UserDetailsPage }), + ]), + ]); + const router = createRouter({ routeTree, history: createMemoryHistory({ initialEntries: [path] }) }); const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); - return render( + render( - + , ); + await screen.findByLabelText('Email or user ID'); + return router; } -async function lookUp(address: string) { +async function lookUp(value: string) { await act(async () => { - fireEvent.change(screen.getByLabelText('Email'), { target: { value: address } }); + fireEvent.change(screen.getByLabelText('Email or user ID'), { target: { value } }); }); await act(async () => { fireEvent.click(screen.getByRole('button', { name: 'Look up' })); }); } -describe('UsersAdminIndex', () => { - it('looks up the live account by normalized address and previews each organization', async () => { - renderPage(); +async function confirmDeactivation() { + await act(async () => { + fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); + }); + await act(async () => { + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); + }); +} + +describe('admin users', () => { + it('looks up the live account by normalized address and opens it by id, keeping the address out of the URL', async () => { + const router = await renderAt(); await lookUp(' Pat@Example.com '); expect(await screen.findByText('Pat Lee')).toBeTruthy(); + expect(router.state.location.href).toBe('/admin/users/usr-1'); + expect(get).toHaveBeenCalledWith('/Admin/User/?email=pat%40example.com'); + expect(get).toHaveBeenCalledWith('/Admin/User/usr-1?details=true'); + }); + + it('shows each organization with its roles, its deactivation marking and its live clusters', async () => { + await renderAt('/admin/users/usr-1'); + + expect(await screen.findByText('2 organizations · 2 clusters')).toBeTruthy(); const solo = screen.getByText('Solo Co').closest('li')!; expect(within(solo).getByText('Kept: last admin')).toBeTruthy(); + expect(within(solo).getByText('No clusters.')).toBeTruthy(); + expect(screen.getByRole('link', { name: 'Solo Co' }).getAttribute('href')).toBe('/org-solo'); + const shared = screen.getByText('Shared Co').closest('li')!; - expect(within(shared).getByText('Will be removed')).toBeTruthy(); + expect(within(shared).getByText('Removed on deactivation')).toBeTruthy(); expect(within(shared).getByText('admin, developer')).toBeTruthy(); - expect(get).toHaveBeenCalledWith('/Admin/User/usr-1?previewMembershipRemoval=true'); + expect(within(shared).getByRole('link', { name: 'alpha' }).getAttribute('href')).toBe('/org-shared/clu-a'); + expect(within(shared).getByText('FAILED')).toBeTruthy(); + }); + + it('opens a pasted user id without looking up an address', async () => { + const router = await renderAt(); + await lookUp(' usr-2 '); + + expect(await screen.findByText('Sam Ray')).toBeTruthy(); + expect(router.state.location.href).toBe('/admin/users/usr-2'); + expect(get.mock.calls.some(([url]) => url.includes('?email='))).toBe(false); + }); + + it('encodes the address it looks up', async () => { + await renderAt(); + await lookUp('pat+ops@example.com'); + + expect(await screen.findByText('No account uses pat+ops@example.com.')).toBeTruthy(); + expect(get).toHaveBeenCalledWith('/Admin/User/?email=pat%2Bops%40example.com'); }); - it('says so when no account uses the address', async () => { - renderPage(); + it('leaves the open account when no account uses the address', async () => { + const router = await renderAt('/admin/users/usr-1'); + await screen.findByText('Pat Lee'); await lookUp('nobody@example.com'); expect(await screen.findByText('No account uses nobody@example.com.')).toBeTruthy(); + expect(router.state.location.href).toBe('/admin/users'); + expect(screen.queryByText('Pat Lee')).toBeNull(); }); - it('confirms, deactivates, and shows what actually happened', async () => { - renderPage(); + it('says so when no account has the ID', async () => { + await renderAt('/admin/users/usr-missing'); + + expect(await screen.findByText('No account has the ID usr-missing.')).toBeTruthy(); + expect(screen.queryByRole('button', { name: 'Deactivate' })).toBeNull(); + }); + + it('keeps the address out of the duplicate-account error and the URL', async () => { + get.mockResolvedValueOnce({ data: [{ id: 'usr-1', status: 'ACTIVE' }, { id: 'usr-2', status: 'ACTIVE' }] }); + const router = await renderAt(); + await lookUp('pat@example.com'); + + const message = await screen.findByText(/More than one live account/); + expect(message.textContent).not.toContain('pat@example.com'); + expect(router.state.location.href).toBe('/admin/users'); + }); + + it('looks the same address up again when asked, refreshing the open account', async () => { + const answer = get.getMockImplementation()!; + get.mockRejectedValueOnce(new Error('timeout')); + await renderAt(); + await lookUp('pat@example.com'); + expect(await screen.findByText('timeout')).toBeTruthy(); + + get.mockImplementation(answer); await lookUp('pat@example.com'); + expect(await screen.findByText('Pat Lee')).toBeTruthy(); + + user = { ...user, firstname: 'Patricia' }; + await lookUp('pat@example.com'); + expect(await screen.findByText('Patricia Lee')).toBeTruthy(); + }); + + it('refuses a central manager that answers without the details view', async () => { + const { organizations: _omitted, ...plainRecord } = user; + get.mockResolvedValue({ data: plainRecord }); + await renderAt('/admin/users/usr-1'); + + expect(await screen.findByText(/cannot show user details/)).toBeTruthy(); + expect(screen.queryByRole('button', { name: 'Deactivate' })).toBeNull(); + }); + + it('confirms, deactivates, and shows what actually happened', async () => { + await renderAt('/admin/users/usr-1'); await act(async () => { fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); }); @@ -140,16 +263,14 @@ describe('UsersAdminIndex', () => { it('still offers deactivation to an already-deactivated account that has memberships to remove', async () => { user.status = 'DEACTIVATED'; - renderPage(); - await lookUp('pat@example.com'); + await renderAt('/admin/users/usr-1'); expect(await screen.findByText(/Already deactivated/)).toBeTruthy(); expect(screen.getByRole('button', { name: 'Deactivate' })).toBeTruthy(); }); it('keeps the result open when the refetch after deactivating fails', async () => { - renderPage(); - await lookUp('pat@example.com'); + await renderAt('/admin/users/usr-1'); await act(async () => { fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); }); @@ -164,82 +285,52 @@ describe('UsersAdminIndex', () => { expect(within(within(done).getByText('Removed from').parentElement!).getByText('Shared Co')).toBeTruthy(); }); - it('looks the same address up again when asked', async () => { - const answer = get.getMockImplementation()!; - get.mockRejectedValueOnce(new Error('timeout')); - renderPage(); - await lookUp('pat@example.com'); - expect(await screen.findByText('timeout')).toBeTruthy(); - - get.mockImplementation(answer); - await lookUp('pat@example.com'); - expect(await screen.findByText('Pat Lee')).toBeTruthy(); - }); - - it('refuses a central manager that answers without the preview', async () => { - const { organizations: _omitted, ...plainRecord } = user; - get.mockImplementation(async (url: string) => - url.includes('?email=') ? { data: [{ id: 'usr-1', status: 'ACTIVE' }] } : { data: plainRecord } - ); - renderPage(); - await lookUp('pat@example.com'); - - expect(await screen.findByText(/cannot preview organization memberships/)).toBeTruthy(); - expect(screen.queryByRole('button', { name: 'Deactivate' })).toBeNull(); - }); - - it('keeps the address out of the duplicate-account error', async () => { - get.mockResolvedValueOnce({ data: [{ id: 'usr-1', status: 'ACTIVE' }, { id: 'usr-2', status: 'ACTIVE' }] }); - renderPage(); - await lookUp('pat@example.com'); - - const message = await screen.findByText(/More than one live account/); - expect(message.textContent).not.toContain('pat@example.com'); - }); - - it('closes the confirm dialog when the PATCH fails', async () => { - patch.mockRejectedValueOnce(new Error('no answer')); - renderPage(); - await lookUp('pat@example.com'); + it('keeps the result open when the refetch finds no account any more', async () => { + await renderAt('/admin/users/usr-1'); await act(async () => { fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); }); + get.mockRejectedValue(notFound()); await act(async () => { fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); }); + expect(await screen.findByText('No account has the ID usr-1.')).toBeTruthy(); + const done = screen.getByRole('dialog'); + expect(within(within(done).getByText('Removed from').parentElement!).getByText('Shared Co')).toBeTruthy(); + }); + + it('closes the confirm dialog when the PATCH fails', async () => { + patch.mockRejectedValueOnce(new Error('no answer')); + await renderAt('/admin/users/usr-1'); + await confirmDeactivation(); + expect(screen.queryByRole('dialog')).toBeNull(); }); it('says so when a successful PATCH does not report the organizations', async () => { patch.mockResolvedValueOnce({ data: '' }); - renderPage(); - await lookUp('pat@example.com'); - await act(async () => { - fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); - }); - await act(async () => { - fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); - }); + await renderAt('/admin/users/usr-1'); + await confirmDeactivation(); const done = await screen.findByRole('dialog'); expect(within(done).getByText('User deactivated')).toBeTruthy(); expect(within(done).getAllByText('Not reported by the server.')).toHaveLength(2); }); - it('keeps the result open when the refetch finds no account any more', async () => { - renderPage(); - await lookUp('pat@example.com'); + it('drops an open confirmation when moving to another account', async () => { + const router = await renderAt('/admin/users/usr-1'); await act(async () => { fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); }); - get.mockResolvedValue({ data: [] }); + expect(screen.getByRole('dialog')).toBeTruthy(); + await act(async () => { - fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); + await router.navigate({ to: '/admin/users/$userId', params: { userId: 'usr-2' } } as never); }); - expect(await screen.findByText('No account uses pat@example.com.')).toBeTruthy(); - const done = screen.getByRole('dialog'); - expect(within(within(done).getByText('Removed from').parentElement!).getByText('Shared Co')).toBeTruthy(); + expect(await screen.findByText('Sam Ray')).toBeTruthy(); + expect(screen.queryByRole('dialog')).toBeNull(); + expect(patch).not.toHaveBeenCalled(); }); }); diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index b1c525acd..8fa370578 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -1,240 +1,82 @@ -import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; -import { - Dialog, - DialogContent, - DialogDescription, - DialogFooter, - DialogHeader, - DialogTitle, -} from '@/components/ui/dialog'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; -import { Skeleton } from '@/components/ui/skeleton'; -import { - AdminUser, - adminUserQueryOptions, - DeactivateUserResult, - MembershipPreview, - normalizeEmail, - OrganizationMembership, - useDeactivateUserMutation, -} from '@/features/admin/users/userAdminApi'; -import { useQuery } from '@tanstack/react-query'; +import { findUserIdByEmail, normalizeEmail, userDetailsQueryKey } from '@/features/admin/users/userAdminApi'; +import { detectEntityId } from '@/lib/string/entityId'; +import { useQueryClient } from '@tanstack/react-query'; +import { Outlet, useNavigate } from '@tanstack/react-router'; import { FormEvent, useState } from 'react'; -export function UsersAdminIndex() { +type Lookup = { state: 'idle' | 'searching' } | { state: 'notFound'; email: string } | { + state: 'failed'; + message: string; +}; + +/** The search stays above whichever account is open, so the next lookup is one step away. */ +export function UsersAdminLayout() { + const navigate = useNavigate(); + const queryClient = useQueryClient(); const [input, setInput] = useState(''); - // Held in state, never in the URL: addresses must stay out of the page URL that RUM and third-party pixels read. - const [email, setEmail] = useState(''); - const { data: user, isFetching, isError, error, refetch } = useQuery(adminUserQueryOptions(email)); - const [deactivating, setDeactivating] = useState(null); + const [lookup, setLookup] = useState({ state: 'idle' }); - const lookUp = (event: FormEvent) => { + const lookUp = async (event: FormEvent) => { event.preventDefault(); - const next = normalizeEmail(input); - if (next === email) { void refetch(); } - else { setEmail(next); } + const pasted = detectEntityId(input); + let userId = pasted?.kind === 'user' ? pasted.id : null; + if (!userId) { + const email = normalizeEmail(input); + setLookup({ state: 'searching' }); + try { + userId = await findUserIdByEmail(email); + } catch (error) { + setLookup({ state: 'failed', message: (error as Error)?.message || 'Could not look up that address.' }); + return; + } + if (!userId) { + setLookup({ state: 'notFound', email }); + void navigate({ to: '/admin/users' }); + return; + } + } + setLookup({ state: 'idle' }); + // Looking up the account already open refreshes it rather than doing nothing. + void queryClient.invalidateQueries({ queryKey: userDetailsQueryKey(userId) }); + // Only the id reaches the URL: addresses must stay out of the page URL that RUM and third-party pixels read. + void navigate({ to: '/admin/users/$userId', params: { userId } }); }; return (

    Users

    - Deactivating an account ends its sessions and blocks sign-in. It also leaves every organization that keeps - another admin who can sign in; where it is the last one, its membership stays so the organization is never left - without an admin. + Look up an account to see its organizations and their clusters, and to deactivate it.

    - + setInput(event.target.value)} />
    - +
    -
    - {!email - ? null - : isFetching && user === undefined - ? - : user - ? ( - <> - {isError && ( -

    - Could not refresh this account; showing the last one loaded. -

    - )} - setDeactivating(user)} /> - - ) - : isError - ?

    {error?.message || 'Could not look up that address.'}

    - :

    No account uses {email}.

    } -
    - - {deactivating && setDeactivating(null)} />} -
    - ); -} - -function UserDetails({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { - const name = [user.firstname, user.lastname].filter(Boolean).join(' '); - const removable = user.organizations.filter((membership) => !membership.lastAdmin); - const deactivated = user.status === 'DEACTIVATED'; + {lookup.state === 'notFound' && ( +

    No account uses {lookup.email}.

    + )} + {lookup.state === 'failed' &&

    {lookup.message}

    } - return ( -
    -
    -
    - {name || user.email} - {user.email} - {user.id} -
    - {user.status ?? 'unknown'} -
    - -
    -

    Organizations

    - {user.organizations.length - ? ( -
      - {user.organizations.map((membership) => ( - - ))} -
    - ) - :

    Not a member of any organization.

    } +
    +
    - - {deactivated && !removable.length - ?

    Deactivated, and in no organization it could leave.

    - : ( -
    - {deactivated && ( - - Already deactivated: deactivating again removes the memberships marked for removal. - - )} - -
    - )} -
    - ); -} - -function MembershipRow({ membership }: { membership: MembershipPreview }) { - return ( -
  • - - {organizationLabel(membership)} - {membership.roleNames.join(', ')} - - {membership.lastAdmin - ? Kept: last admin - : Will be removed} -
    • - ); -} - -const organizationLabel = (membership: OrganizationMembership) => - membership.organizationName || membership.organizationId; - -/** - * Mount only while open. It holds the account as it was when Deactivate was pressed, so the refetch that follows the - * PATCH can't change or unmount it, whatever that refetch finds. - */ -function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => void }) { - const mutation = useDeactivateUserMutation(); - const [result, setResult] = useState(null); - const removed = user.organizations.filter((membership) => !membership.lastAdmin); - const kept = user.organizations.filter((membership) => membership.lastAdmin); - - return ( - !open && !mutation.isPending && onClose()}> - - - {result ? 'User deactivated' : `Deactivate ${user.email}?`} - - {result - ? 'Sessions ended and sign-in is blocked.' - : 'Their sessions end now and they can no longer sign in.'} - - - - {result - ? ( -
    - - -
    - ) - : ( -
    - - -

    - This preview can change before you confirm; the result lists what actually happened. -

    -
    - )} - - - {result - ? - : ( - <> - - - - )} - -     -     - ); -} - -function MembershipList({ title, memberships, empty }: { - title: string; - memberships: OrganizationMembership[] | null; - empty: string; -}) { - return ( -
    -

    {title}

    - {memberships === null - ?

    Not reported by the server.

    - : memberships.length - ? ( -
      - {memberships.map((membership) =>
    • {organizationLabel(membership)}
      • )} -
    - ) - :

    {empty}

    }
    ); } diff --git a/src/features/admin/users/userAdminApi.ts b/src/features/admin/users/userAdminApi.ts index 0646a89a9..7a2b8a350 100644 --- a/src/features/admin/users/userAdminApi.ts +++ b/src/features/admin/users/userAdminApi.ts @@ -1,6 +1,7 @@ -// The generated SDK types these paths but not the membership preview or the deactivation result, so those are typed here. +// The generated SDK types these paths but not the details view or the deactivation result, so those are typed here. import { apiClient } from '@/config/apiClient'; import { queryOptions, useMutation, useQueryClient } from '@tanstack/react-query'; +import { AxiosError } from 'axios'; export interface OrganizationMembership { organizationId: string; @@ -8,9 +9,19 @@ export interface OrganizationMembership { roleNames: string[]; } -/** `lastAdmin`: the user is the organization's only admin who could sign in, so a deactivation keeps them there. */ -export interface MembershipPreview extends OrganizationMembership { +export interface AdminCluster { + id: string; + name: string; + status: string | null; +} + +/** + * `lastAdmin`: the user is the organization's only admin who could sign in, so a deactivation keeps them there. + * `clusters`: the organization's clusters that have not been torn down. + */ +export interface MembershipDetails extends OrganizationMembership { lastAdmin: boolean; + clusters: AdminCluster[]; } export interface AdminUser { @@ -19,7 +30,11 @@ export interface AdminUser { firstname?: string | null; lastname?: string | null; status?: string | null; - organizations: MembershipPreview[]; + isVerified?: boolean | null; + createdAt?: string | null; + lastLoginDate?: string | null; + lastAccessedAt?: string | null; + organizations: MembershipDetails[]; } /** Null when the server's answer did not say, though the write went through. */ @@ -32,33 +47,44 @@ const BASE = '/Admin/User/'; const userAdminQueryKey = ['fabric-admin', 'users'] as const; +export const userDetailsQueryKey = (userId: string) => [...userAdminQueryKey, userId] as const; + /** Matches how central manager stores addresses, so the exact-match lookup finds mixed-case input. */ export const normalizeEmail = (email: string) => email.trim().toLowerCase(); -async function findUser(email: string): Promise { +/** + * The id of the one live account using `email`, or null when none does. Callers never send a blank address: the + * endpoint would answer every user instead of a match. + */ +export async function findUserIdByEmail(email: string): Promise { const { data } = await apiClient.get(`${BASE}?email=${encodeURIComponent(email)}` as '/Admin/User/'); const live = (data as unknown as Array<{ id: string; status?: string }>).filter((user) => user.status !== 'DELETED'); - // The address stays out of error text: query errors reach the console, and from there RUM. + // The address stays out of error text: errors reach the console, and from there RUM. if (live.length > 1) { throw new Error('More than one live account uses this address.'); } - if (!live.length) { return null; } + return live[0]?.id ?? null; +} - const { data: user } = await apiClient.get( - `${BASE}${encodeURIComponent(live[0].id)}?previewMembershipRemoval=true` as '/Admin/User/{id}', - ); - // A central manager without the preview answers the plain record; refuse it rather than offer a deactivation +async function getUserDetails(userId: string): Promise { + let data: unknown; + try { + ({ data } = await apiClient.get(`${BASE}${encodeURIComponent(userId)}?details=true` as '/Admin/User/{id}')); + } catch (error) { + if (error instanceof AxiosError && error.response?.status === 404) { return null; } + throw error; + } + // A central manager without the details view answers the plain record; refuse it rather than offer a deactivation // whose effect can't be shown. - if (!Array.isArray((user as unknown as Partial).organizations)) { - throw new Error('This central manager cannot preview organization memberships yet.'); + const organizations = (data as Partial | undefined)?.organizations; + if (!Array.isArray(organizations) || organizations.some((membership) => !Array.isArray(membership.clusters))) { + throw new Error('This central manager cannot show user details yet.'); } - return user as unknown as AdminUser; + return data as AdminUser; } -/** Callers never send a blank address: the endpoint would answer every user instead of a match. */ -export function adminUserQueryOptions(email: string) { +export function userDetailsQueryOptions(userId: string) { return queryOptions({ - queryKey: [...userAdminQueryKey, email], - queryFn: () => findUser(email), - enabled: email.length > 0, + queryKey: userDetailsQueryKey(userId), + queryFn: () => getUserDetails(userId), }); } diff --git a/src/lib/string/entityId.test.ts b/src/lib/string/entityId.test.ts index 0b262c9d7..d00e8b612 100644 --- a/src/lib/string/entityId.test.ts +++ b/src/lib/string/entityId.test.ts @@ -16,6 +16,10 @@ describe('detectEntityId', () => { expect(detectEntityId('ins-abc123')).toEqual({ kind: 'instance', id: 'ins-abc123' }); }); + it('detects user ids', () => { + expect(detectEntityId('usr-0f8gtsvjqed6xwy9')).toEqual({ kind: 'user', id: 'usr-0f8gtsvjqed6xwy9' }); + }); + it('trims surrounding whitespace before matching', () => { expect(detectEntityId(' org-1 ')).toEqual({ kind: 'organization', id: 'org-1' }); }); diff --git a/src/lib/string/entityId.ts b/src/lib/string/entityId.ts index 7bde440fa..030cf5709 100644 --- a/src/lib/string/entityId.ts +++ b/src/lib/string/entityId.ts @@ -1,20 +1,21 @@ // Harper resources are addressed by IDs that carry a distinct, lowercase prefix // per resource type (e.g. `org-qpz5akmyrp1d0opj`, `clu-tc9pqw20vrks2zik`, -// `ins-…`). The lowercase casing is what lets us tell a pasted ID apart from a +// `ins-…`, `usr-…`). The lowercase casing is what lets us tell a pasted ID apart from a // human-typed title — a title like "Org Chart" or "Cluster West" never matches. -export type EntityIdKind = 'organization' | 'cluster' | 'instance'; +export type EntityIdKind = 'organization' | 'cluster' | 'instance' | 'user'; const PREFIX_TO_KIND: Record = { org: 'organization', clu: 'cluster', ins: 'instance', + usr: 'user', }; // Lowercase prefix + lowercase alphanumeric body, anchored end-to-end so // partial/looser matches (mixed case, spaces, trailing junk) fall through to // the normal name search instead of being treated as an ID. -const ENTITY_ID_PATTERN = /^(org|clu|ins)-[a-z0-9]+$/; +const ENTITY_ID_PATTERN = /^(org|clu|ins|usr)-[a-z0-9]+$/; /** * Detects whether a string is a Harper resource ID and, if so, which kind of From 8a181e3320d315265b65aba915665f6ddc644a91 Mon Sep 17 00:00:00 2001 From: David C Date: Mon, 26 Oct 2026 16:55:19 -0400 Subject: [PATCH 06/23] fix(admin): close the open account on a failed lookup, and drop stale 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 --- src/features/admin/users/UserDetails.tsx | 17 +++--- src/features/admin/users/index.test.tsx | 71 +++++++++++++++++++++++- src/features/admin/users/index.tsx | 64 +++++++++++++-------- src/features/admin/users/userAdminApi.ts | 30 ++++++---- 4 files changed, 139 insertions(+), 43 deletions(-) diff --git a/src/features/admin/users/UserDetails.tsx b/src/features/admin/users/UserDetails.tsx index a8367010f..5488e4d22 100644 --- a/src/features/admin/users/UserDetails.tsx +++ b/src/features/admin/users/UserDetails.tsx @@ -64,6 +64,8 @@ function UserDetails({ user, onDeactivate }: { user: AdminUser; onDeactivate: () const name = [user.firstname, user.lastname].filter(Boolean).join(' '); const removable = user.organizations.filter((membership) => !membership.lastAdmin); const deactivated = user.status === 'DEACTIVATED'; + // Central manager answers a status change on a deleted account with 404. + const deleted = user.status === 'DELETED'; const clusterCount = user.organizations.reduce((total, membership) => total + membership.clusters.length, 0); return ( @@ -75,7 +77,7 @@ function UserDetails({ user, onDeactivate }: { user: AdminUser; onDeactivate: () {user.email} {user.id}
    - {user.status ?? 'unknown'} + {user.status ?? 'unknown'}
    @@ -93,7 +95,7 @@ function UserDetails({ user, onDeactivate }: { user: AdminUser; onDeactivate: () ? (
      {user.organizations.map((membership) => ( - + ))}
    ) @@ -107,7 +109,9 @@ function UserDetails({ user, onDeactivate }: { user: AdminUser; onDeactivate: () another admin who can sign in; where it is the last one, its membership stays so the organization is never left without an admin.

    - {deactivated && !removable.length + {deleted + ?

    This account is deleted, so it cannot be deactivated.

    + : deactivated && !removable.length ?

    Deactivated, and in no organization it could leave.

    : (
    @@ -136,7 +140,7 @@ function Fact({ label, value }: { label: string; value: string }) { const clusterStatusVariant = (status: string | null) => status === 'RUNNING' ? 'success' : status === 'FAILED' ? 'destructive' : 'outline'; -function MembershipCard({ membership }: { membership: MembershipDetails }) { +function MembershipCard({ membership, marked }: { membership: MembershipDetails; marked: boolean }) { const { organizationId } = membership; return (
  • @@ -147,9 +151,9 @@ function MembershipCard({ membership }: { membership: MembershipDetails }) { {membership.roleNames.join(', ')} - {membership.lastAdmin + {marked && (membership.lastAdmin ? Kept: last admin - : Removed on deactivation} + : Removed on deactivation)}
    • {membership.clusters.length ? ( @@ -231,7 +235,6 @@ function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => v - + } title="No organizations"> + Not a member of any organization. + )} - - ); -} -function Fact({ label, value }: { label: string; value: string }) { - return ( -
    -
    {label}
    -
    {value}
    +
    ); } -const clusterStatusVariant = (status: string | null) => - status === 'RUNNING' ? 'success' : status === 'FAILED' ? 'destructive' : 'outline'; - -function MembershipCard({ membership, marked }: { membership: MembershipDetails; marked: boolean }) { - const { organizationId } = membership; +function Callout({ children }: { children: string }) { return ( -
  • -
    - - - {organizationLabel(membership)} - - {membership.roleNames.join(', ')} - - {marked && (membership.lastAdmin - ? Kept: last admin - : Removed on deactivation)} -
    - {membership.clusters.length - ? ( -
      - {membership.clusters.map((cluster) => ( -
    • - - {cluster.name} - - {cluster.status ?? 'unknown'} -
      • - ))} -
    - ) - :

    No clusters.

    } -
    • +

    + + {children} +

    ); } -const organizationLabel = (membership: OrganizationMembership) => - membership.organizationName || membership.organizationId; - -/** - * Mount only while open. It holds the account as it was when Deactivate was pressed, so the refetch that follows the - * PATCH can't change or unmount it, whatever that refetch finds. - */ -function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => void }) { - const mutation = useDeactivateUserMutation(); - const [result, setResult] = useState(null); - const removed = user.organizations.filter((membership) => !membership.lastAdmin); - const kept = user.organizations.filter((membership) => membership.lastAdmin); - +function DetailsSkeleton() { return ( - !open && !mutation.isPending && onClose()}> - - - {result ? 'User deactivated' : `Deactivate ${user.email}?`} - - {result - ? 'Sessions ended and sign-in is blocked.' - : 'Their sessions end now and they can no longer sign in.'} - - - - {result - ? ( -
    - - -
    - ) - : ( -
    - - -

    - This preview can change before you confirm; the result lists what actually happened. -

    -
    - )} - - - {result - ? - : ( - <> - - - - )} - -     -     - ); -} - -function MembershipList({ title, memberships, empty }: { - title: string; - memberships: OrganizationMembership[] | null; - empty: string; -}) { - return ( -
    -

    {title}

    - {memberships === null - ?

    Not reported by the server.

    - : memberships.length - ? ( -
      - {memberships.map((membership) =>
    • {organizationLabel(membership)}
      • )} -
    - ) - :

    {empty}

    } +
    + +
    + + + +
    ); } diff --git a/src/features/admin/users/components/Deactivation.tsx b/src/features/admin/users/components/Deactivation.tsx new file mode 100644 index 000000000..5b1948103 --- /dev/null +++ b/src/features/admin/users/components/Deactivation.tsx @@ -0,0 +1,151 @@ +import { Button } from '@/components/ui/button'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog'; +import { organizationLabel } from '@/features/admin/users/components/OrganizationCard'; +import { + AdminUser, + DeactivateUserResult, + OrganizationMembership, + useDeactivateUserMutation, +} from '@/features/admin/users/userAdminApi'; +import { cn } from '@/lib/cn'; +import { CircleSlashIcon, Loader2Icon, LogOutIcon, ShieldCheckIcon, UserXIcon } from 'lucide-react'; +import { ReactNode, useState } from 'react'; + +const organizations = (count: number) => (count === 1 ? '1 organization' : `${count} organizations`); + +export function DeactivatePanel({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { + const removable = user.organizations.filter((membership) => !membership.lastAdmin).length; + const kept = user.organizations.length - removable; + const deactivated = user.status === 'DEACTIVATED'; + // Central manager answers a status change on a deleted account with 404. + const deleted = user.status === 'DELETED'; + + if (deleted || (deactivated && !removable)) { + return ( +
    + +

    + {deleted + ? 'This account is deleted, so it cannot be deactivated.' + : 'Deactivated, and in no organization it could leave.'} +

    +
    + ); + } + + return ( +
    +
    + + + +
    +

    {deactivated ? 'Finish deactivating' : 'Deactivate account'}

    +

    + {deactivated + ? 'Already deactivated: deactivating again removes the memberships marked for removal.' + : 'Ends every session and blocks sign-in.'} Leaves {organizations(removable)} + {kept > 0 && `, and stays in ${organizations(kept)} as the last admin who can sign in`}. +

    +
    + +
    +
    + ); +} + +/** + * Mount only while open. It holds the account as it was when Deactivate was pressed, so the refetch that follows the + * PATCH can't change or unmount it, whatever that refetch finds. + */ +export function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => void }) { + const mutation = useDeactivateUserMutation(); + const [result, setResult] = useState(null); + const removed = user.organizations.filter((membership) => !membership.lastAdmin); + const kept = user.organizations.filter((membership) => membership.lastAdmin); + + return ( + !open && !mutation.isPending && onClose()}> + + + {result ? 'User deactivated' : `Deactivate ${user.email}?`} + + {result + ? 'Sessions ended and sign-in is blocked.' + : 'Their sessions end now and they can no longer sign in.'} + + + + {result + ? ( +
    + + +
    + ) + : ( +
    +
    + + +
    +

    + This preview can change before you confirm; the result lists what actually happened. +

    +
    + )} + + + {result + ? + : ( + <> + + + + )} + +     +     + ); +} + +const TONES: Record<'leave' | 'stay', { icon: ReactNode; box: string }> = { + leave: { icon: , box: 'bg-muted/40' }, + stay: { icon: , box: 'border-yellow/40 bg-yellow/10' }, +}; + +function MembershipList({ tone, title, memberships }: { + tone: 'leave' | 'stay'; + title: string; + memberships: OrganizationMembership[] | null; +}) { + return ( +
    +

    {TONES[tone].icon}{title}

    + {memberships === null + ?

    Not reported by the server.

    + : memberships.length + ? ( +
      + {memberships.map((membership) =>
    • {organizationLabel(membership)}
      • )} +
    + ) + :

    No organization.

    } +
    + ); +} diff --git a/src/features/admin/users/components/EmptyState.tsx b/src/features/admin/users/components/EmptyState.tsx new file mode 100644 index 000000000..bf7b8a989 --- /dev/null +++ b/src/features/admin/users/components/EmptyState.tsx @@ -0,0 +1,13 @@ +import { ReactNode } from 'react'; + +export function EmptyState({ icon, title, children }: { icon: ReactNode; title: string; children: ReactNode }) { + return ( +
    + + {icon} + +

    {title}

    +

    {children}

    +
    + ); +} diff --git a/src/features/admin/users/components/OrganizationCard.tsx b/src/features/admin/users/components/OrganizationCard.tsx new file mode 100644 index 000000000..aa03953c1 --- /dev/null +++ b/src/features/admin/users/components/OrganizationCard.tsx @@ -0,0 +1,112 @@ +import { MembershipDetails } from '@/features/admin/users/userAdminApi'; +import { cn } from '@/lib/cn'; +import { buildAbsoluteLinkToPage } from '@/lib/urls/buildAbsoluteLinkToPage'; +import { Link } from '@tanstack/react-router'; +import { Building2Icon, ChevronRightIcon, LogOutIcon, ShieldCheckIcon } from 'lucide-react'; +import { useState } from 'react'; + +const STATUS_DOT: Record = { + RUNNING: 'bg-green', + FAILED: 'bg-red-600', + STOPPED: 'bg-muted-foreground/40', +}; + +/** Anything not settled (provisioning, updating, restarting…) pulses. */ +const statusDot = (status: string | null) => STATUS_DOT[status ?? ''] ?? 'animate-pulse bg-yellow'; + +const statusLabel = (status: string | null) => + status ? status.charAt(0) + status.slice(1).toLowerCase().replaceAll('_', ' ') : 'Unknown'; + +export const organizationLabel = (membership: { organizationName: string | null; organizationId: string }) => + membership.organizationName || membership.organizationId; + +export function OrganizationCard({ membership, marked }: { membership: MembershipDetails; marked: boolean }) { + const [open, setOpen] = useState(true); + const { organizationId, clusters } = membership; + const running = clusters.filter((cluster) => cluster.status === 'RUNNING').length; + const label = organizationLabel(membership); + + return ( +
  • +
    + {clusters.length + ? ( + + ) + : } + + + +
    +
    + + {label} + + {membership.roleNames.map((role) => ( + + {role} + + ))} +
    +

    + {clusters.length === 1 ? '1 live cluster' : `${clusters.length} live clusters`} + {clusters.length > 0 && ` · ${running} running`} + {' · '}{organizationId} +

    +
    + {marked && ( + // Its own line on a phone, under the name, so the name keeps the width. +
    + +
    + )} +
    + {open && clusters.length > 0 && ( +
      + {clusters.map((cluster) => ( +
    • + + + {cluster.name} + + {cluster.id} + + {statusLabel(cluster.status)} + +
      • + ))} +
    + )} +
    • + ); +} + +function DeactivationFate({ lastAdmin }: { lastAdmin: boolean }) { + return lastAdmin + ? ( + + + Kept: last admin + + ) + : ( + + + Removed on deactivation + + ); +} diff --git a/src/features/admin/users/components/UserProfileCard.tsx b/src/features/admin/users/components/UserProfileCard.tsx new file mode 100644 index 000000000..8403a4e4c --- /dev/null +++ b/src/features/admin/users/components/UserProfileCard.tsx @@ -0,0 +1,100 @@ +import { Badge } from '@/components/ui/badge'; +import { Button } from '@/components/ui/button'; +import { initialsOf } from '@/features/admin/apiToken/components/StaffUserCell'; +import { AdminUser } from '@/features/admin/users/userAdminApi'; +import { Timestamp } from '@/features/organization/serviceAccounts/components/Timestamp'; +import { useCopyToClipboard } from '@/hooks/useCopyToClipboard'; +import { cn } from '@/lib/cn'; +import { CopyIcon, FingerprintIcon, MailIcon } from 'lucide-react'; +import { ReactNode } from 'react'; + +type StatusStyle = { label: string; variant: 'success' | 'destructive' | 'secondary' | 'warning' }; + +const USER_STATUS: Record = { + ACTIVE: { label: 'Active', variant: 'success' }, + DEACTIVATED: { label: 'Deactivated', variant: 'destructive' }, + DELETED: { label: 'Deleted', variant: 'secondary' }, + PENDING: { label: 'Pending', variant: 'warning' }, +}; + +export function UserProfileCard({ user }: { user: AdminUser }) { + const name = [user.firstname, user.lastname].filter(Boolean).join(' ') || user.email; + const status = USER_STATUS[user.status ?? ''] ?? { label: user.status ?? 'Unknown', variant: 'secondary' }; + const [copyEmail, copyId] = useCopyToClipboard(user.email, user.id); + const clusters = user.organizations.flatMap((membership) => membership.clusters); + const adminOf = user.organizations.filter((membership) => membership.roleNames.includes('admin')).length; + const running = clusters.filter((cluster) => cluster.status === 'RUNNING').length; + + return ( +
    +
    + + {initialsOf({ ...user, firstname: user.firstname ?? '', lastname: user.lastname ?? '' })} + +
    +
    +

    {name}

    + {status.label} +
    +
    + } copyLabel="Copy email" onCopy={copyEmail}>{user.email} + } copyLabel="Copy user ID" onCopy={copyId} mono> + {user.id} + +
    +
    +
    +
    + + + : 'Never'} + detail={user.lastLoginDate + ? ( + <> + Signed in + + ) + : 'Never signed in'} + /> + : '—'} + detail={user.isVerified ? 'Email verified' : 'Email not verified'} + /> +
    +
    + ); +} + +function CopyableValue({ icon, copyLabel, onCopy, mono, children }: { + icon: ReactNode; + copyLabel: string; + onCopy: () => void; + mono?: boolean; + children: string; +}) { + return ( + + {icon} + {children} + + + ); +} + +function Stat({ label, value, detail }: { label: string; value: ReactNode; detail: ReactNode }) { + return ( +
    +
    {label}
    +
    {value}
    +
    {detail}
    +
    + ); +} diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index fa7d6b300..8926d8b88 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -154,17 +154,39 @@ describe('admin users', () => { it('shows each organization with its roles, its deactivation marking and its live clusters', async () => { await renderAt('/admin/users/usr-1'); - expect(await screen.findByText('2 organizations · 2 clusters')).toBeTruthy(); + const stat = (label: string) => screen.getByText(label, { selector: 'dt' }).parentElement!; + await screen.findByText('Pat Lee'); + expect(within(stat('Organizations')).getByText('2')).toBeTruthy(); + expect(within(stat('Organizations')).getByText('Admin of 2')).toBeTruthy(); + expect(within(stat('Live clusters')).getByText('2')).toBeTruthy(); + expect(within(stat('Live clusters')).getByText('1 running')).toBeTruthy(); + const solo = screen.getByText('Solo Co').closest('li')!; expect(within(solo).getByText('Kept: last admin')).toBeTruthy(); - expect(within(solo).getByText('No clusters.')).toBeTruthy(); + expect(within(solo).getByText('0 live clusters')).toBeTruthy(); expect(screen.getByRole('link', { name: 'Solo Co' }).getAttribute('href')).toBe('/org-solo'); const shared = screen.getByText('Shared Co').closest('li')!; expect(within(shared).getByText('Removed on deactivation')).toBeTruthy(); - expect(within(shared).getByText('admin, developer')).toBeTruthy(); + expect(within(shared).getByText('admin')).toBeTruthy(); + expect(within(shared).getByText('developer')).toBeTruthy(); expect(within(shared).getByRole('link', { name: 'alpha' }).getAttribute('href')).toBe('/org-shared/clu-a'); - expect(within(shared).getByText('FAILED')).toBeTruthy(); + expect(within(shared).getByText('Failed')).toBeTruthy(); + }); + + it("folds an organization's clusters away and back", async () => { + await renderAt('/admin/users/usr-1'); + const toggle = await screen.findByRole('button', { name: 'Hide the clusters of Shared Co' }); + + await act(async () => { + fireEvent.click(toggle); + }); + expect(screen.queryByRole('link', { name: 'alpha' })).toBeNull(); + + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: 'Show the clusters of Shared Co' })); + }); + expect(screen.getByRole('link', { name: 'alpha' })).toBeTruthy(); }); it('opens a pasted user id without looking up an address', async () => { diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index fb1ed4f18..06a106c67 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -1,6 +1,7 @@ import { Button } from '@/components/ui/button'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; +import { EmptyState } from '@/features/admin/users/components/EmptyState'; import { DuplicateAccountError, findUserIdByEmail, @@ -11,6 +12,7 @@ import { detectEntityId } from '@/lib/string/entityId'; import { describeError } from '@/react-query/queryClient'; import { useQueryClient } from '@tanstack/react-query'; import { Outlet, useNavigate } from '@tanstack/react-router'; +import { Loader2Icon, SearchIcon, TriangleAlertIcon, UserSearchIcon, UserXIcon } from 'lucide-react'; import { FormEvent, useEffect, useRef, useState } from 'react'; type Lookup = { state: 'idle' | 'searching' } | { state: 'notFound'; email: string } | { @@ -64,37 +66,69 @@ export function UsersAdminLayout() { }; return ( -
    +

    Users

    Look up an account to see its organizations and their clusters, and to deactivate it.

    -
    -
    - - setInput(event.target.value)} - /> + + +
    +
    + + setInput(event.target.value)} + /> +
    +
    - + {lookup.state === 'notFound' + ? ( +

    + + No account uses {lookup.email}. +

    + ) + : lookup.state === 'failed' + ? ( +

    + + {lookup.message} +

    + ) + : ( +

    + A user ID opens the account directly; an email address finds the one live account that uses it. +

    + )} - {lookup.state === 'notFound' && ( -

    No account uses {lookup.email}.

    - )} - {lookup.state === 'failed' &&

    {lookup.message}

    } - -
    +
    ); } + +export function UsersSearchPrompt() { + return ( + } title="Find an account"> + You will see its organizations, the clusters in each, and which memberships a deactivation would remove. + + ); +} From 1df814f55de10d7ec382b55922a6cd1e45a4c999 Mon Sep 17 00:00:00 2001 From: David C Date: Mon, 26 Oct 2026 17:35:33 -0400 Subject: [PATCH 09/23] fix(admin): keep user details apart from the API Tokens page's staff-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 --- .../users/components/OrganizationCard.tsx | 2 -- src/features/admin/users/index.test.tsx | 18 ++++++++++++++++-- src/features/admin/users/index.tsx | 1 - src/features/admin/users/userAdminApi.ts | 7 ++++--- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/features/admin/users/components/OrganizationCard.tsx b/src/features/admin/users/components/OrganizationCard.tsx index aa03953c1..d7947ba24 100644 --- a/src/features/admin/users/components/OrganizationCard.tsx +++ b/src/features/admin/users/components/OrganizationCard.tsx @@ -11,7 +11,6 @@ const STATUS_DOT: Record = { STOPPED: 'bg-muted-foreground/40', }; -/** Anything not settled (provisioning, updating, restarting…) pulses. */ const statusDot = (status: string | null) => STATUS_DOT[status ?? ''] ?? 'animate-pulse bg-yellow'; const statusLabel = (status: string | null) => @@ -63,7 +62,6 @@ export function OrganizationCard({ membership, marked }: { membership: Membershi

    {marked && ( - // Its own line on a phone, under the name, so the name keeps the width.
    diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 8926d8b88..0e7b914bf 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -18,6 +18,7 @@ const { get, patch } = vi.hoisted(() => ({ get: vi.fn(), patch: vi.fn() })); vi.mock('@/config/apiClient', () => ({ apiClient: { get, patch } })); +import { getStaffUserQueryOptions, StaffUserSummary } from '@/features/admin/apiToken/queries/staffUsers'; import { UsersAdminLayout } from '@/features/admin/users/index'; import { DuplicateAccountError, findUserIdByEmail } from '@/features/admin/users/userAdminApi'; import { UserDetailsPage } from '@/features/admin/users/UserDetails'; @@ -100,8 +101,7 @@ afterEach(() => { vi.restoreAllMocks(); }); -/** The admin users routes as the app nests them, without the admin shell's auth around them. */ -async function renderAt(path = '/admin/users') { +async function renderAt(path = '/admin/users', seed?: (client: QueryClient) => void) { const rootRoute = createRootRoute({ component: Outlet }); const usersRoute = createRoute({ getParentRoute: () => rootRoute, path: 'admin/users', component: UsersAdminLayout }); const routeTree = rootRoute.addChildren([ @@ -113,6 +113,7 @@ async function renderAt(path = '/admin/users') { ]); const router = createRouter({ routeTree, history: createMemoryHistory({ initialEntries: [path] }) }); const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + seed?.(queryClient); render( @@ -189,6 +190,19 @@ describe('admin users', () => { expect(screen.getByRole('link', { name: 'alpha' })).toBeTruthy(); }); + it('keeps its details apart from the staff-user names the API Tokens page caches', async () => { + await renderAt('/admin/users/usr-1', (client) => { + client.setQueryData(getStaffUserQueryOptions('usr-1').queryKey, { + id: 'usr-1', + email: 'pat@example.com', + firstname: 'Pat', + lastname: 'Lee', + } as StaffUserSummary); + }); + + expect(await screen.findByText('Solo Co')).toBeTruthy(); + }); + it('opens a pasted user id without looking up an address', async () => { const router = await renderAt(); await lookUp(' usr-2 '); diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index 06a106c67..a997231f1 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -33,7 +33,6 @@ export function UsersAdminLayout() { const open = (userId: string) => { setLookup({ state: 'idle' }); - // Looking up the account already open refreshes it rather than doing nothing. void queryClient.invalidateQueries({ queryKey: userDetailsQueryKey(userId) }); void navigate({ to: '/admin/users/$userId', params: { userId } }); }; diff --git a/src/features/admin/users/userAdminApi.ts b/src/features/admin/users/userAdminApi.ts index bcd4776b5..6e151b863 100644 --- a/src/features/admin/users/userAdminApi.ts +++ b/src/features/admin/users/userAdminApi.ts @@ -45,9 +45,10 @@ export interface DeactivateUserResult { const BASE = '/Admin/User/'; -const userAdminQueryKey = ['fabric-admin', 'users'] as const; +// Under its own segment: the API Tokens page caches staff-user summaries at ['fabric-admin', 'users', id]. +const userDetailsQueryPrefix = ['fabric-admin', 'users', 'details'] as const; -export const userDetailsQueryKey = (userId: string) => [...userAdminQueryKey, userId] as const; +export const userDetailsQueryKey = (userId: string) => [...userDetailsQueryPrefix, userId] as const; /** Matches how central manager stores addresses, so the exact-match lookup finds mixed-case input. */ export const normalizeEmail = (email: string) => email.trim().toLowerCase(); @@ -112,7 +113,7 @@ export function useDeactivateUserMutation() { }, // Not returned: the dialog would wait on the refetch before showing what the write did. onSettled: () => { - void queryClient.invalidateQueries({ queryKey: userAdminQueryKey }); + void queryClient.invalidateQueries({ queryKey: userDetailsQueryPrefix }); }, }); } From 15034dca2bccde61ca538a7a3ef97d4431295a87 Mon Sep 17 00:00:00 2001 From: David C Date: Mon, 26 Oct 2026 18:33:36 -0400 Subject: [PATCH 10/23] refactor(admin): leave the deactivation split to the confirm dialog 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 --- src/features/admin/users/UserDetails.tsx | 14 ++-------- .../admin/users/components/Deactivation.tsx | 8 ++---- .../users/components/OrganizationCard.tsx | 27 +++---------------- src/features/admin/users/index.test.tsx | 9 +++---- 4 files changed, 10 insertions(+), 48 deletions(-) diff --git a/src/features/admin/users/UserDetails.tsx b/src/features/admin/users/UserDetails.tsx index c8e1c65d7..c152cac90 100644 --- a/src/features/admin/users/UserDetails.tsx +++ b/src/features/admin/users/UserDetails.tsx @@ -44,27 +44,17 @@ function UserDetailsView({ userId }: { userId: string }) { } function UserDetails({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { - const deleted = user.status === 'DELETED'; - const removable = user.organizations.filter((membership) => !membership.lastAdmin).length; - return (
    -
    -

    Organizations

    - {!deleted && user.organizations.length > 0 && ( -

    - {removable} removed on deactivation · {user.organizations.length - removable} kept as the last admin -

    - )} -
    +

    Organizations

    {user.organizations.length ? (
      {user.organizations.map((membership) => ( - + ))}
    ) diff --git a/src/features/admin/users/components/Deactivation.tsx b/src/features/admin/users/components/Deactivation.tsx index 5b1948103..03e5c9a15 100644 --- a/src/features/admin/users/components/Deactivation.tsx +++ b/src/features/admin/users/components/Deactivation.tsx @@ -18,11 +18,8 @@ import { cn } from '@/lib/cn'; import { CircleSlashIcon, Loader2Icon, LogOutIcon, ShieldCheckIcon, UserXIcon } from 'lucide-react'; import { ReactNode, useState } from 'react'; -const organizations = (count: number) => (count === 1 ? '1 organization' : `${count} organizations`); - export function DeactivatePanel({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { const removable = user.organizations.filter((membership) => !membership.lastAdmin).length; - const kept = user.organizations.length - removable; const deactivated = user.status === 'DEACTIVATED'; // Central manager answers a status change on a deleted account with 404. const deleted = user.status === 'DELETED'; @@ -50,9 +47,8 @@ export function DeactivatePanel({ user, onDeactivate }: { user: AdminUser; onDea

    {deactivated ? 'Finish deactivating' : 'Deactivate account'}

    {deactivated - ? 'Already deactivated: deactivating again removes the memberships marked for removal.' - : 'Ends every session and blocks sign-in.'} Leaves {organizations(removable)} - {kept > 0 && `, and stays in ${organizations(kept)} as the last admin who can sign in`}. + ? 'Already deactivated: deactivating again removes it from the organizations it can still leave.' + : 'Ends every session and blocks sign-in.'} You'll see which organizations it leaves before you confirm.

    diff --git a/src/features/admin/users/components/OrganizationCard.tsx b/src/features/admin/users/components/OrganizationCard.tsx index d7947ba24..3dbc82ddc 100644 --- a/src/features/admin/users/components/OrganizationCard.tsx +++ b/src/features/admin/users/components/OrganizationCard.tsx @@ -2,7 +2,7 @@ import { MembershipDetails } from '@/features/admin/users/userAdminApi'; import { cn } from '@/lib/cn'; import { buildAbsoluteLinkToPage } from '@/lib/urls/buildAbsoluteLinkToPage'; import { Link } from '@tanstack/react-router'; -import { Building2Icon, ChevronRightIcon, LogOutIcon, ShieldCheckIcon } from 'lucide-react'; +import { Building2Icon, ChevronRightIcon } from 'lucide-react'; import { useState } from 'react'; const STATUS_DOT: Record = { @@ -19,7 +19,7 @@ const statusLabel = (status: string | null) => export const organizationLabel = (membership: { organizationName: string | null; organizationId: string }) => membership.organizationName || membership.organizationId; -export function OrganizationCard({ membership, marked }: { membership: MembershipDetails; marked: boolean }) { +export function OrganizationCard({ membership }: { membership: MembershipDetails }) { const [open, setOpen] = useState(true); const { organizationId, clusters } = membership; const running = clusters.filter((cluster) => cluster.status === 'RUNNING').length; @@ -27,7 +27,7 @@ export function OrganizationCard({ membership, marked }: { membership: Membershi return (
  • -
    +
    {clusters.length ? (
    - {marked && ( -
    - -
    - )}
    {open && clusters.length > 0 && (
      @@ -92,19 +87,3 @@ export function OrganizationCard({ membership, marked }: { membership: Membershi ); } - -function DeactivationFate({ lastAdmin }: { lastAdmin: boolean }) { - return lastAdmin - ? ( - - - Kept: last admin - - ) - : ( - - - Removed on deactivation - - ); -} diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 0e7b914bf..75f832264 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -152,7 +152,7 @@ describe('admin users', () => { expect(get).toHaveBeenCalledWith('/Admin/User/usr-1?details=true'); }); - it('shows each organization with its roles, its deactivation marking and its live clusters', async () => { + it('shows each organization with its roles and live clusters, leaving the deactivation split to the dialog', async () => { await renderAt('/admin/users/usr-1'); const stat = (label: string) => screen.getByText(label, { selector: 'dt' }).parentElement!; @@ -163,16 +163,15 @@ describe('admin users', () => { expect(within(stat('Live clusters')).getByText('1 running')).toBeTruthy(); const solo = screen.getByText('Solo Co').closest('li')!; - expect(within(solo).getByText('Kept: last admin')).toBeTruthy(); expect(within(solo).getByText('0 live clusters')).toBeTruthy(); expect(screen.getByRole('link', { name: 'Solo Co' }).getAttribute('href')).toBe('/org-solo'); const shared = screen.getByText('Shared Co').closest('li')!; - expect(within(shared).getByText('Removed on deactivation')).toBeTruthy(); expect(within(shared).getByText('admin')).toBeTruthy(); expect(within(shared).getByText('developer')).toBeTruthy(); expect(within(shared).getByRole('link', { name: 'alpha' }).getAttribute('href')).toBe('/org-shared/clu-a'); expect(within(shared).getByText('Failed')).toBeTruthy(); + expect(screen.queryByText(/last admin|on deactivation/)).toBeNull(); }); it("folds an organization's clusters away and back", async () => { @@ -285,14 +284,12 @@ describe('admin users', () => { expect(screen.getByText('Elsewhere')).toBeTruthy(); }); - it('offers no deactivation, and no membership marking, for a deleted account', async () => { + it('offers no deactivation for a deleted account', async () => { user.status = 'DELETED'; await renderAt('/admin/users/usr-1'); expect(await screen.findByText('This account is deleted, so it cannot be deactivated.')).toBeTruthy(); expect(screen.queryByRole('button', { name: 'Deactivate' })).toBeNull(); - expect(screen.queryByText('Removed on deactivation')).toBeNull(); - expect(screen.queryByText('Kept: last admin')).toBeNull(); }); it('looks the same address up again when asked, refreshing the open account', async () => { From 5479dde6bdc9863da2d3e7618112f3d395ba2996 Mon Sep 17 00:00:00 2001 From: David C Date: Mon, 26 Oct 2026 18:44:54 -0400 Subject: [PATCH 11/23] feat(admin): move Deactivate into a settings menu on the user card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- e2e/tests/admin-users.anon.spec.ts | 7 ++- src/features/admin/users/UserDetails.tsx | 6 +-- .../admin/users/components/Deactivation.tsx | 41 +------------- .../users/components/UserActionsMenu.tsx | 35 ++++++++++++ .../users/components/UserProfileCard.tsx | 8 ++- src/features/admin/users/index.test.tsx | 54 +++++++++++-------- 6 files changed, 82 insertions(+), 69 deletions(-) create mode 100644 src/features/admin/users/components/UserActionsMenu.tsx diff --git a/e2e/tests/admin-users.anon.spec.ts b/e2e/tests/admin-users.anon.spec.ts index d235c8c48..da0f6c32a 100644 --- a/e2e/tests/admin-users.anon.spec.ts +++ b/e2e/tests/admin-users.anon.spec.ts @@ -110,10 +110,13 @@ test('deactivating confirms the split, then shows what the server did', async ({ await page.setViewportSize({ width: 1440, height: 1000 }); await page.goto(`/#/admin/users/${pat.id}`); - await page.getByRole('button', { name: 'Deactivate' }).click(); + await page.getByRole('button', { name: 'Account settings' }).click(); + await expect(page.getByRole('menuitem', { name: /Deactivate/ })).toBeVisible(); + await page.screenshot({ path: testInfo.outputPath('users-menu.png'), animations: 'disabled' }); + await page.getByRole('menuitem', { name: /Deactivate/ }).click(); const dialog = page.getByRole('dialog'); await expect(dialog.getByText('Will stay in, as the last admin')).toBeVisible(); - await page.screenshot({ path: testInfo.outputPath('users-confirm.png') }); + await page.screenshot({ path: testInfo.outputPath('users-confirm.png'), animations: 'disabled' }); await dialog.getByRole('button', { name: 'Deactivate' }).click(); await expect(dialog.getByText('User deactivated')).toBeVisible(); diff --git a/src/features/admin/users/UserDetails.tsx b/src/features/admin/users/UserDetails.tsx index c152cac90..077e0346b 100644 --- a/src/features/admin/users/UserDetails.tsx +++ b/src/features/admin/users/UserDetails.tsx @@ -1,5 +1,5 @@ import { Skeleton } from '@/components/ui/skeleton'; -import { DeactivateDialog, DeactivatePanel } from '@/features/admin/users/components/Deactivation'; +import { DeactivateDialog } from '@/features/admin/users/components/Deactivation'; import { EmptyState } from '@/features/admin/users/components/EmptyState'; import { OrganizationCard } from '@/features/admin/users/components/OrganizationCard'; import { UserProfileCard } from '@/features/admin/users/components/UserProfileCard'; @@ -46,7 +46,7 @@ function UserDetailsView({ userId }: { userId: string }) { function UserDetails({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { return (
      - +

      Organizations

      @@ -64,8 +64,6 @@ function UserDetails({ user, onDeactivate }: { user: AdminUser; onDeactivate: () )}
      - -
      ); } diff --git a/src/features/admin/users/components/Deactivation.tsx b/src/features/admin/users/components/Deactivation.tsx index 03e5c9a15..952766cbe 100644 --- a/src/features/admin/users/components/Deactivation.tsx +++ b/src/features/admin/users/components/Deactivation.tsx @@ -15,48 +15,9 @@ import { useDeactivateUserMutation, } from '@/features/admin/users/userAdminApi'; import { cn } from '@/lib/cn'; -import { CircleSlashIcon, Loader2Icon, LogOutIcon, ShieldCheckIcon, UserXIcon } from 'lucide-react'; +import { Loader2Icon, LogOutIcon, ShieldCheckIcon } from 'lucide-react'; import { ReactNode, useState } from 'react'; -export function DeactivatePanel({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { - const removable = user.organizations.filter((membership) => !membership.lastAdmin).length; - const deactivated = user.status === 'DEACTIVATED'; - // Central manager answers a status change on a deleted account with 404. - const deleted = user.status === 'DELETED'; - - if (deleted || (deactivated && !removable)) { - return ( -
      - -

      - {deleted - ? 'This account is deleted, so it cannot be deactivated.' - : 'Deactivated, and in no organization it could leave.'} -

      -
      - ); - } - - return ( -
      -
      - - - -
      -

      {deactivated ? 'Finish deactivating' : 'Deactivate account'}

      -

      - {deactivated - ? 'Already deactivated: deactivating again removes it from the organizations it can still leave.' - : 'Ends every session and blocks sign-in.'} You'll see which organizations it leaves before you confirm. -

      -
      - -
      -
      - ); -} - /** * Mount only while open. It holds the account as it was when Deactivate was pressed, so the refetch that follows the * PATCH can't change or unmount it, whatever that refetch finds. diff --git a/src/features/admin/users/components/UserActionsMenu.tsx b/src/features/admin/users/components/UserActionsMenu.tsx new file mode 100644 index 000000000..903d9f86f --- /dev/null +++ b/src/features/admin/users/components/UserActionsMenu.tsx @@ -0,0 +1,35 @@ +import { Button } from '@/components/ui/button'; +import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger } from '@/components/ui/dropdownMenu'; +import { AdminUser } from '@/features/admin/users/userAdminApi'; +import { SettingsIcon, UserXIcon } from 'lucide-react'; + +function deactivation(user: AdminUser): { available: boolean; detail: string } { + // Central manager answers a status change on a deleted account with 404. + if (user.status === 'DELETED') { return { available: false, detail: 'Deleted accounts cannot be deactivated' }; } + if (user.status !== 'DEACTIVATED') { return { available: true, detail: 'Ends sessions and blocks sign-in' }; } + return user.organizations.some((membership) => !membership.lastAdmin) + ? { available: true, detail: 'Already deactivated; leaves the organizations it still can' } + : { available: false, detail: 'Already deactivated, and in no organization it could leave' }; +} + +export function UserActionsMenu({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { + const { available, detail } = deactivation(user); + return ( + + + + + + + + + Deactivate… + {detail} + + + + + ); +} diff --git a/src/features/admin/users/components/UserProfileCard.tsx b/src/features/admin/users/components/UserProfileCard.tsx index 8403a4e4c..914085723 100644 --- a/src/features/admin/users/components/UserProfileCard.tsx +++ b/src/features/admin/users/components/UserProfileCard.tsx @@ -1,6 +1,7 @@ import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; import { initialsOf } from '@/features/admin/apiToken/components/StaffUserCell'; +import { UserActionsMenu } from '@/features/admin/users/components/UserActionsMenu'; import { AdminUser } from '@/features/admin/users/userAdminApi'; import { Timestamp } from '@/features/organization/serviceAccounts/components/Timestamp'; import { useCopyToClipboard } from '@/hooks/useCopyToClipboard'; @@ -17,7 +18,7 @@ const USER_STATUS: Record = { PENDING: { label: 'Pending', variant: 'warning' }, }; -export function UserProfileCard({ user }: { user: AdminUser }) { +export function UserProfileCard({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { const name = [user.firstname, user.lastname].filter(Boolean).join(' ') || user.email; const status = USER_STATUS[user.status ?? ''] ?? { label: user.status ?? 'Unknown', variant: 'secondary' }; const [copyEmail, copyId] = useCopyToClipboard(user.email, user.id); @@ -27,7 +28,10 @@ export function UserProfileCard({ user }: { user: AdminUser }) { return (
      -
      +
      +
      + +
      { Element.prototype.setPointerCapture ??= () => undefined; Element.prototype.releasePointerCapture ??= () => undefined; Element.prototype.scrollIntoView ??= () => undefined; + window.PointerEvent ??= class extends MouseEvent {} as typeof PointerEvent; }); const notFound = () => new AxiosError('Not Found', '404', undefined, undefined, { status: 404 } as never); @@ -132,10 +133,24 @@ async function lookUp(value: string) { }); } -async function confirmDeactivation() { +// Radix opens its menu on pointerdown, not click. +async function openSettings() { + const trigger = await screen.findByRole('button', { name: 'Account settings' }); + await act(async () => { + fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false }); + }); + return within(screen.getByRole('menu')).getByRole('menuitem', { name: /Deactivate/ }); +} + +async function openDeactivate() { + const item = await openSettings(); await act(async () => { - fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); + fireEvent.click(item); }); +} + +async function confirmDeactivation() { + await openDeactivate(); await act(async () => { fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); }); @@ -233,7 +248,7 @@ describe('admin users', () => { await renderAt('/admin/users/usr-missing'); expect(await screen.findByText('No account has the ID usr-missing.')).toBeTruthy(); - expect(screen.queryByRole('button', { name: 'Deactivate' })).toBeNull(); + expect(screen.queryByRole('button', { name: 'Account settings' })).toBeNull(); }); it('closes the open account on a duplicate address, keeping the address out of the error and unreported', async () => { @@ -288,8 +303,9 @@ describe('admin users', () => { user.status = 'DELETED'; await renderAt('/admin/users/usr-1'); - expect(await screen.findByText('This account is deleted, so it cannot be deactivated.')).toBeTruthy(); - expect(screen.queryByRole('button', { name: 'Deactivate' })).toBeNull(); + const item = await openSettings(); + expect(item.getAttribute('aria-disabled')).toBe('true'); + expect(within(item).getByText('Deleted accounts cannot be deactivated')).toBeTruthy(); }); it('looks the same address up again when asked, refreshing the open account', async () => { @@ -314,14 +330,12 @@ describe('admin users', () => { await renderAt('/admin/users/usr-1'); expect(await screen.findByText(/cannot show user details/)).toBeTruthy(); - expect(screen.queryByRole('button', { name: 'Deactivate' })).toBeNull(); + expect(screen.queryByRole('button', { name: 'Account settings' })).toBeNull(); }); it('confirms, deactivates, and shows what actually happened', async () => { await renderAt('/admin/users/usr-1'); - await act(async () => { - fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); - }); + await openDeactivate(); const confirm = screen.getByRole('dialog'); expect(within(confirm).getByText('Deactivate pat@example.com?')).toBeTruthy(); @@ -344,22 +358,24 @@ describe('admin users', () => { await act(async () => { fireEvent.click(within(done).getByRole('button', { name: 'Done' })); }); - expect(await screen.findByText('Deactivated, and in no organization it could leave.')).toBeTruthy(); + await screen.findByText('Deactivated'); + const item = await openSettings(); + expect(item.getAttribute('aria-disabled')).toBe('true'); + expect(within(item).getByText('Already deactivated, and in no organization it could leave')).toBeTruthy(); }); it('still offers deactivation to an already-deactivated account that has memberships to remove', async () => { user.status = 'DEACTIVATED'; await renderAt('/admin/users/usr-1'); - expect(await screen.findByText(/Already deactivated/)).toBeTruthy(); - expect(screen.getByRole('button', { name: 'Deactivate' })).toBeTruthy(); + const item = await openSettings(); + expect(item.getAttribute('aria-disabled')).toBeNull(); + expect(within(item).getByText(/Already deactivated; leaves/)).toBeTruthy(); }); it('keeps the result open when the refetch after deactivating fails', async () => { await renderAt('/admin/users/usr-1'); - await act(async () => { - fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); - }); + await openDeactivate(); get.mockRejectedValue(new Error('network down')); await act(async () => { fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); @@ -373,9 +389,7 @@ describe('admin users', () => { it('keeps the result open when the refetch finds no account any more', async () => { await renderAt('/admin/users/usr-1'); - await act(async () => { - fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); - }); + await openDeactivate(); get.mockRejectedValue(notFound()); await act(async () => { fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Deactivate' })); @@ -406,9 +420,7 @@ describe('admin users', () => { it('drops an open confirmation when moving to another account', async () => { const router = await renderAt('/admin/users/usr-1'); - await act(async () => { - fireEvent.click(await screen.findByRole('button', { name: 'Deactivate' })); - }); + await openDeactivate(); expect(screen.getByRole('dialog')).toBeTruthy(); await act(async () => { From f257668d0f0e6389885b51d92ef1b6bb9d1b81cc Mon Sep 17 00:00:00 2001 From: David C Date: Mon, 26 Oct 2026 18:46:34 -0400 Subject: [PATCH 12/23] style(admin): enlarge the user card's settings gear Co-Authored-By: Claude Opus 5.5 --- src/features/admin/users/components/UserActionsMenu.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/features/admin/users/components/UserActionsMenu.tsx b/src/features/admin/users/components/UserActionsMenu.tsx index 903d9f86f..2a781a1b5 100644 --- a/src/features/admin/users/components/UserActionsMenu.tsx +++ b/src/features/admin/users/components/UserActionsMenu.tsx @@ -17,8 +17,8 @@ export function UserActionsMenu({ user, onDeactivate }: { user: AdminUser; onDea return ( - From 65d3667665b857247cf06b23d74d5884ed91648d Mon Sep 17 00:00:00 2001 From: David C Date: Mon, 26 Oct 2026 18:50:03 -0400 Subject: [PATCH 13/23] feat(admin): type the account's email to confirm a deactivation 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 --- e2e/tests/admin-users.anon.spec.ts | 3 ++ .../admin/users/components/Deactivation.tsx | 29 ++++++++++- src/features/admin/users/index.test.tsx | 49 ++++++++++++++----- 3 files changed, 68 insertions(+), 13 deletions(-) diff --git a/e2e/tests/admin-users.anon.spec.ts b/e2e/tests/admin-users.anon.spec.ts index da0f6c32a..9f19dc381 100644 --- a/e2e/tests/admin-users.anon.spec.ts +++ b/e2e/tests/admin-users.anon.spec.ts @@ -116,7 +116,10 @@ test('deactivating confirms the split, then shows what the server did', async ({ await page.getByRole('menuitem', { name: /Deactivate/ }).click(); const dialog = page.getByRole('dialog'); await expect(dialog.getByText('Will stay in, as the last admin')).toBeVisible(); + await expect(dialog.getByRole('button', { name: 'Deactivate' })).toBeDisabled(); await page.screenshot({ path: testInfo.outputPath('users-confirm.png'), animations: 'disabled' }); + await dialog.getByLabel(`Type ${pat.email} to confirm`).fill(pat.email); + await expect(dialog.getByRole('button', { name: 'Deactivate' })).toBeEnabled(); await dialog.getByRole('button', { name: 'Deactivate' }).click(); await expect(dialog.getByText('User deactivated')).toBeVisible(); diff --git a/src/features/admin/users/components/Deactivation.tsx b/src/features/admin/users/components/Deactivation.tsx index 952766cbe..92454bd00 100644 --- a/src/features/admin/users/components/Deactivation.tsx +++ b/src/features/admin/users/components/Deactivation.tsx @@ -7,10 +7,13 @@ import { DialogHeader, DialogTitle, } from '@/components/ui/dialog'; +import { Input } from '@/components/ui/input'; +import { Label } from '@/components/ui/label'; import { organizationLabel } from '@/features/admin/users/components/OrganizationCard'; import { AdminUser, DeactivateUserResult, + normalizeEmail, OrganizationMembership, useDeactivateUserMutation, } from '@/features/admin/users/userAdminApi'; @@ -27,6 +30,13 @@ export function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: const [result, setResult] = useState(null); const removed = user.organizations.filter((membership) => !membership.lastAdmin); const kept = user.organizations.filter((membership) => membership.lastAdmin); + const [typedEmail, setTypedEmail] = useState(''); + const confirmed = normalizeEmail(typedEmail) === normalizeEmail(user.email); + + const deactivate = () => { + if (!confirmed || mutation.isPending) { return; } + mutation.mutate(user.id, { onSuccess: setResult, onError: onClose }); + }; return ( !open && !mutation.isPending && onClose()}> @@ -56,6 +66,21 @@ export function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose:

      This preview can change before you confirm; the result lists what actually happened.

      +
      + + setTypedEmail(event.target.value)} + onKeyDown={(event) => event.key === 'Enter' && deactivate()} + /> +
      )} @@ -67,8 +92,8 @@ export function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: )} diff --git a/src/features/admin/users/components/UserActionsMenu.tsx b/src/features/admin/users/components/UserActionsMenu.tsx index 2a781a1b5..3b867d087 100644 --- a/src/features/admin/users/components/UserActionsMenu.tsx +++ b/src/features/admin/users/components/UserActionsMenu.tsx @@ -3,17 +3,31 @@ import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigge import { AdminUser } from '@/features/admin/users/userAdminApi'; import { SettingsIcon, UserXIcon } from 'lucide-react'; -function deactivation(user: AdminUser): { available: boolean; detail: string } { +// A deactivated account may have kept its memberships on purpose, so removing them is named as its own action +// rather than offered as deactivating again. +function deactivation(user: AdminUser): { label: string; available: boolean; detail: string } { // Central manager answers a status change on a deleted account with 404. - if (user.status === 'DELETED') { return { available: false, detail: 'Deleted accounts cannot be deactivated' }; } - if (user.status !== 'DEACTIVATED') { return { available: true, detail: 'Ends sessions and blocks sign-in' }; } + if (user.status === 'DELETED') { + return { label: 'Deactivate…', available: false, detail: 'Deleted accounts cannot be deactivated' }; + } + if (user.status !== 'DEACTIVATED') { + return { label: 'Deactivate…', available: true, detail: 'Ends sessions and blocks sign-in' }; + } return user.organizations.some((membership) => !membership.lastAdmin) - ? { available: true, detail: 'Already deactivated; leaves the organizations it still can' } - : { available: false, detail: 'Already deactivated, and in no organization it could leave' }; + ? { + label: 'Remove from organizations…', + available: true, + detail: 'Already deactivated; leaves every organization that keeps another admin', + } + : { + label: 'Remove from organizations…', + available: false, + detail: 'Already deactivated, and in no organization it could leave', + }; } export function UserActionsMenu({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { - const { available, detail } = deactivation(user); + const { label, available, detail } = deactivation(user); return ( @@ -25,7 +39,7 @@ export function UserActionsMenu({ user, onDeactivate }: { user: AdminUser; onDea - Deactivate… + {label} {detail} diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 6847ac74d..01c47dbc3 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -139,7 +139,7 @@ async function openSettings() { await act(async () => { fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false }); }); - return within(screen.getByRole('menu')).getByRole('menuitem', { name: /Deactivate/ }); + return within(screen.getByRole('menu')).getByRole('menuitem'); } async function openDeactivate() { @@ -373,12 +373,13 @@ describe('admin users', () => { expect(within(item).getByText('Already deactivated, and in no organization it could leave')).toBeTruthy(); }); - it('still offers deactivation to an already-deactivated account that has memberships to remove', async () => { + it('names removing an already-deactivated account from organizations as its own action', async () => { user.status = 'DEACTIVATED'; await renderAt('/admin/users/usr-1'); const item = await openSettings(); expect(item.getAttribute('aria-disabled')).toBeNull(); + expect(within(item).getByText('Remove from organizations…')).toBeTruthy(); expect(within(item).getByText(/Already deactivated; leaves/)).toBeTruthy(); }); @@ -460,13 +461,22 @@ describe('admin users', () => { expect(within(within(done).getByText('Removed from').parentElement!).getByText('Shared Co')).toBeTruthy(); }); - it('offers no status-only choice for an account that is already deactivated', async () => { + it('removes an already-deactivated account from organizations without offering to only deactivate', async () => { user.status = 'DEACTIVATED'; await renderAt('/admin/users/usr-1'); await openDeactivate(); + const dialog = screen.getByRole('dialog'); + + expect(within(dialog).getByText('Remove pat@example.com from organizations?')).toBeTruthy(); + expect(within(dialog).queryByRole('radio')).toBeNull(); + expect(within(dialog).getByText('Will be removed from')).toBeTruthy(); - expect(within(screen.getByRole('dialog')).queryByRole('radio')).toBeNull(); - expect(within(screen.getByRole('dialog')).getByText('Will be removed from')).toBeTruthy(); + await typeConfirmation(); + await act(async () => { + fireEvent.click(within(dialog).getByRole('button', { name: 'Remove' })); + }); + expect(patch).toHaveBeenCalledWith('/Admin/User/usr-1', { status: 'DEACTIVATED' }); + expect(await screen.findByText('Memberships removed')).toBeTruthy(); }); it('closes the confirm dialog when the PATCH fails', async () => { From 2ba212486218eac30d110b22d55ec209c88d1815 Mon Sep 17 00:00:00 2001 From: David C Date: Mon, 26 Oct 2026 19:06:30 -0400 Subject: [PATCH 16/23] fix(admin): report a status-only deactivation by what was sent and what 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 --- .../admin/users/components/Deactivation.tsx | 3 +- src/features/admin/users/index.test.tsx | 36 ++++++++++++++++++- src/features/admin/users/index.tsx | 2 +- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/features/admin/users/components/Deactivation.tsx b/src/features/admin/users/components/Deactivation.tsx index 6d452352f..e5b470e6c 100644 --- a/src/features/admin/users/components/Deactivation.tsx +++ b/src/features/admin/users/components/Deactivation.tsx @@ -65,7 +65,7 @@ export function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: {result - ? mode === 'statusOnly' && !result.removedFrom?.length + ? mutation.variables?.removeMemberships === false && result.removedFrom?.length === 0 ? (

      Every membership was left in place. @@ -81,6 +81,7 @@ export function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose:

      {!cleanup && ( setMode(value as typeof mode)} aria-label="What to deactivate" diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 01c47dbc3..2c468ee10 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -10,7 +10,7 @@ import { Outlet, RouterProvider, } from '@tanstack/react-router'; -import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react'; +import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; import { AxiosError } from 'axios'; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -310,6 +310,40 @@ describe('admin users', () => { expect(screen.getByText('Elsewhere')).toBeTruthy(); }); + it('does not claim memberships were kept when a status-only answer leaves out what it removed', async () => { + patch.mockResolvedValueOnce({ data: { id: 'usr-1', status: 'DEACTIVATED' } }); + await renderAt('/admin/users/usr-1'); + await openDeactivate(); + await act(async () => { + fireEvent.click(within(screen.getByRole('dialog')).getByRole('radio', { name: /Only deactivate/ })); + }); + await confirmInDialog(); + + const done = await screen.findByRole('dialog'); + expect(within(done).queryByText('Every membership was left in place.')).toBeNull(); + expect(within(done).getAllByText('Not reported by the server.')).toHaveLength(2); + }); + + it('locks the choice while the request is out, and reports on the choice that was sent', async () => { + let answer!: (value: unknown) => void; + patch.mockReturnValueOnce(new Promise((resolve) => (answer = resolve))); + await renderAt('/admin/users/usr-1'); + await openDeactivate(); + const dialog = screen.getByRole('dialog'); + await act(async () => { + fireEvent.click(within(dialog).getByRole('radio', { name: /Only deactivate/ })); + }); + await confirmInDialog(); + + await waitFor(() => + expect(within(dialog).getByRole('radio', { name: /leave organizations/ })).toHaveProperty('disabled', true) + ); + await act(async () => { + answer({ data: { id: 'usr-1', status: 'DEACTIVATED', removedFrom: [], retainedIn: [] } }); + }); + expect(await within(dialog).findByText('Every membership was left in place.')).toBeTruthy(); + }); + it('offers no deactivation for a deleted account', async () => { user.status = 'DELETED'; await renderAt('/admin/users/usr-1'); diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index a997231f1..74efe11f7 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -60,7 +60,7 @@ export function UsersAdminLayout() { return; } setLookup(result); - // Close the open account, so the message never sits above someone else's Deactivate button. + // Close the open account, so the message never sits above someone else's details. void navigate({ to: '/admin/users' }); }; From dfc77239f7135dc0382de136554727a39726f398 Mon Sep 17 00:00:00 2001 From: David C Date: Mon, 26 Oct 2026 19:21:19 -0400 Subject: [PATCH 17/23] fix(admin): drop an email lookup once any navigation has started 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 --- src/features/admin/users/index.test.tsx | 32 +++++++++++++++++++++++++ src/features/admin/users/index.tsx | 14 +++++------ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 2c468ee10..9d77e36e5 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -66,9 +66,11 @@ beforeAll(() => { const notFound = () => new AxiosError('Not Found', '404', undefined, undefined, { status: 404 } as never); let user: typeof USER; +let slowGate: Promise; beforeEach(() => { user = structuredClone(USER); + slowGate = Promise.resolve(); get.mockReset(); patch.mockReset(); get.mockImplementation(async (url: string) => { @@ -111,6 +113,12 @@ async function renderAt(path = '/admin/users', seed?: (client: QueryClient) => v createRoute({ getParentRoute: () => usersRoute, path: '$userId', component: UserDetailsPage }), ]), createRoute({ getParentRoute: () => rootRoute, path: 'elsewhere', component: () =>

      Elsewhere

      }), + createRoute({ + getParentRoute: () => rootRoute, + path: 'slow', + beforeLoad: () => slowGate, + component: () =>

      Slow

      , + }), ]); const router = createRouter({ routeTree, history: createMemoryHistory({ initialEntries: [path] }) }); const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); @@ -344,6 +352,30 @@ describe('admin users', () => { expect(await within(dialog).findByText('Every membership was left in place.')).toBeTruthy(); }); + it('drops an email lookup that answers while another page is still loading', async () => { + let release!: () => void; + slowGate = new Promise((resolve) => (release = resolve)); + let answer!: (value: unknown) => void; + const router = await renderAt(); + get.mockReturnValueOnce(new Promise((resolve) => (answer = resolve))); + await lookUp('pat@example.com'); + + let arriving!: Promise; + await act(async () => { + arriving = router.navigate({ to: '/slow' } as never); + }); + await act(async () => { + answer({ data: [{ id: 'usr-1', status: 'ACTIVE' }] }); + }); + await act(async () => { + release(); + await arriving; + }); + + expect(router.state.location.href).toBe('/slow'); + expect(screen.getByText('Slow')).toBeTruthy(); + }); + it('offers no deactivation for a deleted account', async () => { user.status = 'DELETED'; await renderAt('/admin/users/usr-1'); diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index 74efe11f7..9b9a5bbbd 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -11,9 +11,9 @@ import { import { detectEntityId } from '@/lib/string/entityId'; import { describeError } from '@/react-query/queryClient'; import { useQueryClient } from '@tanstack/react-query'; -import { Outlet, useNavigate } from '@tanstack/react-router'; +import { Outlet, useNavigate, useRouter } from '@tanstack/react-router'; import { Loader2Icon, SearchIcon, TriangleAlertIcon, UserSearchIcon, UserXIcon } from 'lucide-react'; -import { FormEvent, useEffect, useRef, useState } from 'react'; +import { FormEvent, useRef, useState } from 'react'; type Lookup = { state: 'idle' | 'searching' } | { state: 'notFound'; email: string } | { state: 'failed'; @@ -22,14 +22,12 @@ type Lookup = { state: 'idle' | 'searching' } | { state: 'notFound'; email: stri export function UsersAdminLayout() { const navigate = useNavigate(); + const router = useRouter(); const queryClient = useQueryClient(); const [input, setInput] = useState(''); const [lookup, setLookup] = useState({ state: 'idle' }); - // Bumped by each lookup and on unmount, so an email lookup that resolves after either is dropped. + // Bumped by each lookup, so an email lookup that resolves after a newer one is dropped. const latestLookup = useRef(0); - useEffect(() => () => { - latestLookup.current++; - }, []); const open = (userId: string) => { setLookup({ state: 'idle' }); @@ -40,6 +38,7 @@ export function UsersAdminLayout() { const lookUp = async (event: FormEvent) => { event.preventDefault(); const thisLookup = ++latestLookup.current; + const startedAt = router.latestLocation.href; const pasted = detectEntityId(input); if (pasted?.kind === 'user') { open(pasted.id); @@ -54,7 +53,8 @@ export function UsersAdminLayout() { if (!(error instanceof DuplicateAccountError)) { console.error(error); } result = { state: 'failed', message: describeError(error).message }; } - if (thisLookup !== latestLookup.current) { return; } + // Also dropped once any navigation has started: this page stays mounted while the next one loads. + if (thisLookup !== latestLookup.current || router.latestLocation.href !== startedAt) { return; } if (typeof result === 'string') { open(result); return; From 4a76d816488c18a74fd6b79b3e2fe5f5424f3bf2 Mon Sep 17 00:00:00 2001 From: David C Date: Mon, 26 Oct 2026 19:28:48 -0400 Subject: [PATCH 18/23] fix(admin): drop an email lookup on any navigation or unmount, and free 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 --- src/features/admin/users/index.test.tsx | 20 ++++++++++++++++++++ src/features/admin/users/index.tsx | 21 ++++++++++++++++----- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 9d77e36e5..9316ef3c8 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -376,6 +376,26 @@ describe('admin users', () => { expect(screen.getByText('Slow')).toBeTruthy(); }); + it('drops an email lookup across a round trip away and back, and frees the form', async () => { + let answer!: (value: unknown) => void; + const router = await renderAt(); + get.mockReturnValueOnce(new Promise((resolve) => (answer = resolve))); + await lookUp('pat@example.com'); + + await act(async () => { + await router.navigate({ to: '/admin/users/$userId', params: { userId: 'usr-2' } } as never); + }); + await act(async () => { + await router.navigate({ to: '/admin/users' } as never); + }); + await act(async () => { + answer({ data: [{ id: 'usr-1', status: 'ACTIVE' }] }); + }); + + expect(router.state.location.href).toBe('/admin/users'); + expect(screen.getByRole('button', { name: 'Look up' })).toHaveProperty('disabled', false); + }); + it('offers no deactivation for a deleted account', async () => { user.status = 'DELETED'; await renderAt('/admin/users/usr-1'); diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index 9b9a5bbbd..761ddd7de 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -13,7 +13,7 @@ import { describeError } from '@/react-query/queryClient'; import { useQueryClient } from '@tanstack/react-query'; import { Outlet, useNavigate, useRouter } from '@tanstack/react-router'; import { Loader2Icon, SearchIcon, TriangleAlertIcon, UserSearchIcon, UserXIcon } from 'lucide-react'; -import { FormEvent, useRef, useState } from 'react'; +import { FormEvent, useEffect, useRef, useState } from 'react'; type Lookup = { state: 'idle' | 'searching' } | { state: 'notFound'; email: string } | { state: 'failed'; @@ -26,8 +26,18 @@ export function UsersAdminLayout() { const queryClient = useQueryClient(); const [input, setInput] = useState(''); const [lookup, setLookup] = useState({ state: 'idle' }); - // Bumped by each lookup, so an email lookup that resolves after a newer one is dropped. + // Bumped by each lookup, each navigation (this page stays mounted while the next one loads) and unmount, so an + // email lookup acts only if nothing else happened while it was out. const latestLookup = useRef(0); + useEffect(() => { + const unsubscribe = router.subscribe('onBeforeNavigate', () => { + latestLookup.current++; + }); + return () => { + unsubscribe(); + latestLookup.current++; + }; + }, [router]); const open = (userId: string) => { setLookup({ state: 'idle' }); @@ -38,7 +48,6 @@ export function UsersAdminLayout() { const lookUp = async (event: FormEvent) => { event.preventDefault(); const thisLookup = ++latestLookup.current; - const startedAt = router.latestLocation.href; const pasted = detectEntityId(input); if (pasted?.kind === 'user') { open(pasted.id); @@ -53,8 +62,10 @@ export function UsersAdminLayout() { if (!(error instanceof DuplicateAccountError)) { console.error(error); } result = { state: 'failed', message: describeError(error).message }; } - // Also dropped once any navigation has started: this page stays mounted while the next one loads. - if (thisLookup !== latestLookup.current || router.latestLocation.href !== startedAt) { return; } + if (thisLookup !== latestLookup.current) { + setLookup({ state: 'idle' }); + return; + } if (typeof result === 'string') { open(result); return; From 654946c2cf5da5e1a91ee4585aea01b38da7fe9f Mon Sep 17 00:00:00 2001 From: David C Date: Mon, 26 Oct 2026 19:36:59 -0400 Subject: [PATCH 19/23] fix(admin): keep an email lookup through a router reload in place 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 --- src/features/admin/users/index.test.tsx | 17 +++++++++++++++++ src/features/admin/users/index.tsx | 5 +++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 9316ef3c8..68b40f3c8 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -396,6 +396,23 @@ describe('admin users', () => { expect(screen.getByRole('button', { name: 'Look up' })).toHaveProperty('disabled', false); }); + it('keeps an email lookup through a reload in place', async () => { + let answer!: (value: unknown) => void; + const router = await renderAt(); + get.mockReturnValueOnce(new Promise((resolve) => (answer = resolve))); + await lookUp('pat@example.com'); + + await act(async () => { + await router.invalidate(); + }); + await act(async () => { + answer({ data: [{ id: 'usr-1', status: 'ACTIVE' }] }); + }); + + expect(await screen.findByText('Pat Lee')).toBeTruthy(); + expect(router.state.location.href).toBe('/admin/users/usr-1'); + }); + it('offers no deactivation for a deleted account', async () => { user.status = 'DELETED'; await renderAt('/admin/users/usr-1'); diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index 761ddd7de..53c840e08 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -30,8 +30,9 @@ export function UsersAdminLayout() { // email lookup acts only if nothing else happened while it was out. const latestLookup = useRef(0); useEffect(() => { - const unsubscribe = router.subscribe('onBeforeNavigate', () => { - latestLookup.current++; + // hrefChanged: router.invalidate() reloads in place on every auth change, and that is not a navigation. + const unsubscribe = router.subscribe('onBeforeNavigate', (event) => { + if (event.hrefChanged) { latestLookup.current++; } }); return () => { unsubscribe(); From a9dc1d1b202f73155583c06c199f73b3e071fac0 Mon Sep 17 00:00:00 2001 From: David C Date: Wed, 28 Oct 2026 09:38:48 -0400 Subject: [PATCH 20/23] feat(admin): reactivate a deactivated account from the settings menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- e2e/tests/admin-users.anon.spec.ts | 28 ++++++++++++ src/features/admin/users/UserDetails.tsx | 17 +++++-- .../admin/users/components/Reactivation.tsx | 45 +++++++++++++++++++ .../users/components/UserActionsMenu.tsx | 39 +++++++++++++--- .../users/components/UserProfileCard.tsx | 8 +++- src/features/admin/users/index.test.tsx | 38 +++++++++++++++- src/features/admin/users/userAdminApi.ts | 12 +++++ 7 files changed, 173 insertions(+), 14 deletions(-) create mode 100644 src/features/admin/users/components/Reactivation.tsx diff --git a/e2e/tests/admin-users.anon.spec.ts b/e2e/tests/admin-users.anon.spec.ts index 9ce7deefc..9bc8a81b6 100644 --- a/e2e/tests/admin-users.anon.spec.ts +++ b/e2e/tests/admin-users.anon.spec.ts @@ -130,3 +130,31 @@ test('deactivating confirms the split, then shows what the server did', async ({ expect(patched).toEqual({ status: 'DEACTIVATED' }); await page.screenshot({ path: testInfo.outputPath('users-result.png') }); }); + +test('reactivating sets a deactivated account back to active', async ({ page }, testInfo) => { + await mockCentralManager(page); + let current = { ...pat, status: 'DEACTIVATED' }; + await page.route(`**/Admin/User/${pat.id}?details=true`, (route) => route.fulfill({ json: current })); + let patched: unknown; + await page.route(`**/Admin/User/${pat.id}`, async (route) => { + if (route.request().method() !== 'PATCH') { return route.fallback(); } + patched = route.request().postDataJSON(); + current = { ...current, status: 'ACTIVE' }; + await route.fulfill({ status: 204, body: '' }); + }); + await page.setViewportSize({ width: 1440, height: 1000 }); + await page.goto(`/#/admin/users/${pat.id}`); + + await page.getByRole('button', { name: 'Account settings' }).click(); + await expect(page.getByRole('menuitem', { name: /Reactivate…/ })).toBeVisible(); + await page.screenshot({ path: testInfo.outputPath('users-menu-deactivated.png'), animations: 'disabled' }); + await page.getByRole('menuitem', { name: /Reactivate…/ }).click(); + const dialog = page.getByRole('dialog'); + await expect(dialog.getByText(`Reactivate ${pat.email}?`)).toBeVisible(); + await page.screenshot({ path: testInfo.outputPath('users-reactivate.png'), animations: 'disabled' }); + + await dialog.getByRole('button', { name: 'Reactivate' }).click(); + await expect(dialog).toBeHidden(); + expect(patched).toEqual({ status: 'ACTIVE' }); + await expect(page.getByText('Active', { exact: true })).toBeVisible(); +}); diff --git a/src/features/admin/users/UserDetails.tsx b/src/features/admin/users/UserDetails.tsx index 077e0346b..d74312df5 100644 --- a/src/features/admin/users/UserDetails.tsx +++ b/src/features/admin/users/UserDetails.tsx @@ -2,6 +2,7 @@ import { Skeleton } from '@/components/ui/skeleton'; import { DeactivateDialog } from '@/features/admin/users/components/Deactivation'; import { EmptyState } from '@/features/admin/users/components/EmptyState'; import { OrganizationCard } from '@/features/admin/users/components/OrganizationCard'; +import { ReactivateDialog } from '@/features/admin/users/components/Reactivation'; import { UserProfileCard } from '@/features/admin/users/components/UserProfileCard'; import { AdminUser, userDetailsQueryOptions } from '@/features/admin/users/userAdminApi'; import { useQuery } from '@tanstack/react-query'; @@ -18,6 +19,7 @@ export function UserDetailsPage() { function UserDetailsView({ userId }: { userId: string }) { const { data: user, isPending, isError, error } = useQuery(userDetailsQueryOptions(userId)); const [deactivating, setDeactivating] = useState(null); + const [reactivating, setReactivating] = useState(null); return ( <> @@ -27,7 +29,11 @@ function UserDetailsView({ userId }: { userId: string }) { ? ( <> {isError && Could not refresh this account; showing the last one loaded.} - setDeactivating(user)} /> + setDeactivating(user)} + onReactivate={() => setReactivating(user)} + /> ) : isError @@ -39,14 +45,19 @@ function UserDetailsView({ userId }: { userId: string }) { )} {deactivating && setDeactivating(null)} />} + {reactivating && setReactivating(null)} />} ); } -function UserDetails({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { +function UserDetails({ user, onDeactivate, onReactivate }: { + user: AdminUser; + onDeactivate: () => void; + onReactivate: () => void; +}) { return (
      - +

      Organizations

      diff --git a/src/features/admin/users/components/Reactivation.tsx b/src/features/admin/users/components/Reactivation.tsx new file mode 100644 index 000000000..cc415f90d --- /dev/null +++ b/src/features/admin/users/components/Reactivation.tsx @@ -0,0 +1,45 @@ +import { Button } from '@/components/ui/button'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog'; +import { AdminUser, useReactivateUserMutation } from '@/features/admin/users/userAdminApi'; +import { Loader2Icon } from 'lucide-react'; +import { toast } from 'sonner'; + +export function ReactivateDialog({ user, onClose }: { user: AdminUser; onClose: () => void }) { + const mutation = useReactivateUserMutation(); + + const reactivate = () => + mutation.mutate(user.id, { + onSuccess: () => { + toast.success('Account reactivated.'); + onClose(); + }, + onError: onClose, + }); + + return ( + !open && !mutation.isPending && onClose()}> + + + {`Reactivate ${user.email}?`} + + They can sign in again. Organization memberships removed when they were deactivated are not restored. + + + + + + + + + ); +} diff --git a/src/features/admin/users/components/UserActionsMenu.tsx b/src/features/admin/users/components/UserActionsMenu.tsx index 3b867d087..65833a530 100644 --- a/src/features/admin/users/components/UserActionsMenu.tsx +++ b/src/features/admin/users/components/UserActionsMenu.tsx @@ -1,7 +1,13 @@ import { Button } from '@/components/ui/button'; -import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger } from '@/components/ui/dropdownMenu'; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/components/ui/dropdownMenu'; import { AdminUser } from '@/features/admin/users/userAdminApi'; -import { SettingsIcon, UserXIcon } from 'lucide-react'; +import { SettingsIcon, UserCheckIcon, UserXIcon } from 'lucide-react'; // A deactivated account may have kept its memberships on purpose, so removing them is named as its own action // rather than offered as deactivating again. @@ -26,7 +32,11 @@ function deactivation(user: AdminUser): { label: string; available: boolean; det }; } -export function UserActionsMenu({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { +export function UserActionsMenu({ user, onDeactivate, onReactivate }: { + user: AdminUser; + onDeactivate: () => void; + onReactivate: () => void; +}) { const { label, available, detail } = deactivation(user); return ( @@ -36,14 +46,29 @@ export function UserActionsMenu({ user, onDeactivate }: { user: AdminUser; onDea + {user.status === 'DEACTIVATED' && ( + <> + + + + + + + )} - - {label} - {detail} - + ); } + +function ItemText({ label, detail }: { label: string; detail: string }) { + return ( + + {label} + {detail} + + ); +} diff --git a/src/features/admin/users/components/UserProfileCard.tsx b/src/features/admin/users/components/UserProfileCard.tsx index 914085723..1aeb9fe16 100644 --- a/src/features/admin/users/components/UserProfileCard.tsx +++ b/src/features/admin/users/components/UserProfileCard.tsx @@ -18,7 +18,11 @@ const USER_STATUS: Record = { PENDING: { label: 'Pending', variant: 'warning' }, }; -export function UserProfileCard({ user, onDeactivate }: { user: AdminUser; onDeactivate: () => void }) { +export function UserProfileCard({ user, onDeactivate, onReactivate }: { + user: AdminUser; + onDeactivate: () => void; + onReactivate: () => void; +}) { const name = [user.firstname, user.lastname].filter(Boolean).join(' ') || user.email; const status = USER_STATUS[user.status ?? ''] ?? { label: user.status ?? 'Unknown', variant: 'secondary' }; const [copyEmail, copyId] = useCopyToClipboard(user.email, user.id); @@ -30,7 +34,7 @@ export function UserProfileCard({ user, onDeactivate }: { user: AdminUser; onDea
      - +
      { if (url.endsWith('?details=true')) { throw notFound(); } throw new Error(`unexpected GET ${url}`); }); - patch.mockImplementation(async () => { + patch.mockImplementation(async (_url: string, body: { status: string }) => { + if (body.status === 'ACTIVE') { + user = { ...user, status: 'ACTIVE' }; + return { data: '' }; + } user = { ...user, status: 'DEACTIVATED', organizations: [user.organizations[0]] }; return { data: { @@ -147,7 +151,7 @@ async function openSettings() { await act(async () => { fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false }); }); - return within(screen.getByRole('menu')).getByRole('menuitem'); + return within(screen.getByRole('menu')).getByRole('menuitem', { name: /Deactivate…|Remove from organizations…/ }); } async function openDeactivate() { @@ -413,6 +417,36 @@ describe('admin users', () => { expect(router.state.location.href).toBe('/admin/users/usr-1'); }); + it('reactivates a deactivated account, leaving its memberships as they are', async () => { + user.status = 'DEACTIVATED'; + await renderAt('/admin/users/usr-1'); + const trigger = await screen.findByRole('button', { name: 'Account settings' }); + await act(async () => { + fireEvent.pointerDown(trigger, { button: 0, ctrlKey: false }); + }); + await act(async () => { + fireEvent.click(within(screen.getByRole('menu')).getByRole('menuitem', { name: /Reactivate…/ })); + }); + + const dialog = screen.getByRole('dialog'); + expect(within(dialog).getByText('Reactivate pat@example.com?')).toBeTruthy(); + await act(async () => { + fireEvent.click(within(dialog).getByRole('button', { name: 'Reactivate' })); + }); + + expect(patch).toHaveBeenCalledWith('/Admin/User/usr-1', { status: 'ACTIVE' }); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(await screen.findByText('Active')).toBeTruthy(); + expect(screen.getByText('Shared Co')).toBeTruthy(); + }); + + it('offers reactivation only to a deactivated account', async () => { + await renderAt('/admin/users/usr-1'); + await openSettings(); + + expect(within(screen.getByRole('menu')).queryByRole('menuitem', { name: /Reactivate/ })).toBeNull(); + }); + it('offers no deactivation for a deleted account', async () => { user.status = 'DELETED'; await renderAt('/admin/users/usr-1'); diff --git a/src/features/admin/users/userAdminApi.ts b/src/features/admin/users/userAdminApi.ts index e9863b420..01c625c73 100644 --- a/src/features/admin/users/userAdminApi.ts +++ b/src/features/admin/users/userAdminApi.ts @@ -118,3 +118,15 @@ export function useDeactivateUserMutation() { }, }); } + +export function useReactivateUserMutation() { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: async (userId: string) => { + await apiClient.patch(`${BASE}${encodeURIComponent(userId)}` as '/Admin/User/{id}', { status: 'ACTIVE' }); + }, + onSettled: () => { + void queryClient.invalidateQueries({ queryKey: userDetailsQueryPrefix }); + }, + }); +} From 578bf3ef4440fc24fc1775d11da78eb81b2b9eeb Mon Sep 17 00:00:00 2001 From: David C Date: Wed, 28 Oct 2026 10:07:41 -0400 Subject: [PATCH 21/23] refactor(admin): say exactly what drops an email lookup Co-Authored-By: Claude Opus 5.5 --- src/features/admin/users/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/features/admin/users/index.tsx b/src/features/admin/users/index.tsx index 53c840e08..a144d0381 100644 --- a/src/features/admin/users/index.tsx +++ b/src/features/admin/users/index.tsx @@ -26,8 +26,8 @@ export function UsersAdminLayout() { const queryClient = useQueryClient(); const [input, setInput] = useState(''); const [lookup, setLookup] = useState({ state: 'idle' }); - // Bumped by each lookup, each navigation (this page stays mounted while the next one loads) and unmount, so an - // email lookup acts only if nothing else happened while it was out. + // Bumped by each lookup, each URL change (this page stays mounted while the next one loads) and unmount; an email + // lookup whose answer arrives after any of those is dropped. const latestLookup = useRef(0); useEffect(() => { // hrefChanged: router.invalidate() reloads in place on every auth change, and that is not a navigation. From 56c345dbbf1a105ff2b2fe24ba57a4ed8b3f4309 Mon Sep 17 00:00:00 2001 From: David C Date: Thu, 29 Oct 2026 12:51:22 -0400 Subject: [PATCH 22/23] fix(admin): check lastAdmin, gate Users on the reads it makes, title 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 --- .../__tests__/adminShellVisibility.test.ts | 12 +++++++-- src/features/admin/components/AdminShell.tsx | 20 +++++++++++--- .../admin/users/components/Deactivation.tsx | 8 +++++- src/features/admin/users/index.test.tsx | 27 +++++++++++++++++++ src/features/admin/users/userAdminApi.ts | 6 ++++- src/integrations/api/api.patch.d.ts | 1 + 6 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/features/admin/__tests__/adminShellVisibility.test.ts b/src/features/admin/__tests__/adminShellVisibility.test.ts index bc4ae740a..f1ae61faa 100644 --- a/src/features/admin/__tests__/adminShellVisibility.test.ts +++ b/src/features/admin/__tests__/adminShellVisibility.test.ts @@ -68,7 +68,15 @@ describe('admin section visibility', () => { }); it('shows Users only to holders of user:status, not to every role that can read users', () => { - expect(labels(user('fabric_admin', ['user:read', 'user:status']))).toEqual(['Users']); - expect(labels(user('fabric_readonly', ['user:read']))).toEqual([]); + expect(labels(user('fabric_admin', ['user:read', 'user:status', 'cluster:read']))).toEqual([ + 'Monitoring', + 'Users', + ]); + expect(labels(user('fabric_readonly', ['user:read', 'cluster:read']))).toEqual(['Monitoring']); + }); + + it('hides Users from a holder of user:status who cannot also read the users and clusters it shows', () => { + expect(labels(user('fabric_admin', ['user:status', 'cluster:read']))).toEqual(['Monitoring']); + expect(labels(user('fabric_admin', ['user:status', 'user:read']))).toEqual([]); }); }); diff --git a/src/features/admin/components/AdminShell.tsx b/src/features/admin/components/AdminShell.tsx index 6e66e9160..7ac604769 100644 --- a/src/features/admin/components/AdminShell.tsx +++ b/src/features/admin/components/AdminShell.tsx @@ -7,21 +7,32 @@ import { BellIcon, GlobeIcon, KeyRoundIcon, ServerIcon, SirenIcon, UsersIcon } f /** * Shell for the Admin section: a responsive sub-nav rail (so future admin * endpoints slot in as new items) plus the active page. Each item names the - * staff permission its page needs, plus any account carve-out; the rail shows + * staff permissions its page needs, plus any account carve-out; the rail shows * only the pages the account holds, and holding any of them grants entry. The * dashboard route guard already handles the unauthenticated redirect. */ // Order matters: /admin redirects to the first item's route (see adminIndexRoute); // anyone not permitted there is bounced to their first visible page below. const items: Array< - SubNavItem & { permission: StaffPermission; ssoAccountOnly?: boolean; needsPermissionsField?: boolean } + SubNavItem & { + permission: StaffPermission; + alsoNeeds?: StaffPermission[]; + ssoAccountOnly?: boolean; + needsPermissionsField?: boolean; + } > = [ { to: '/admin/notifications', label: 'Notifications', icon: BellIcon, permission: 'systemStatus:write' }, { to: '/admin/regions', label: 'Regions', icon: GlobeIcon, permission: 'region:read' }, { to: '/admin/hosts', label: 'Hosts', icon: ServerIcon, permission: 'host:read' }, { to: '/admin/monitoring', label: 'Monitoring', icon: SirenIcon, permission: 'cluster:read' }, - // Gated on deactivation's permission rather than user:read, which keeps the page fabric_admin-only. - { to: '/admin/users', label: 'Users', icon: UsersIcon, permission: 'user:status' }, + // Gated on deactivation's permission, which keeps the page fabric_admin-only; the details lookup needs both reads. + { + to: '/admin/users', + label: 'Users', + icon: UsersIcon, + permission: 'user:status', + alsoNeeds: ['user:read', 'cluster:read'], + }, // Only an API that sends apiToken:self has the token endpoints this page drives (DESIGN.md); // super_user cannot mint a staff credential. { @@ -39,6 +50,7 @@ export function visibleAdminItems(user: User | LocalUser | null) { const hasPermissionsField = user !== null && 'staffPermissions' in user && user.staffPermissions !== undefined; return items.filter((item) => hasStaffPermission(user, item.permission) + && (item.alsoNeeds ?? []).every((permission) => hasStaffPermission(user, permission)) && !(item.ssoAccountOnly && isSuperUser) && !(item.needsPermissionsField && !hasPermissionsField) ); diff --git a/src/features/admin/users/components/Deactivation.tsx b/src/features/admin/users/components/Deactivation.tsx index e5b470e6c..51b8357b2 100644 --- a/src/features/admin/users/components/Deactivation.tsx +++ b/src/features/admin/users/components/Deactivation.tsx @@ -50,7 +50,7 @@ export function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: {cleanup - ? result ? 'Memberships removed' : `Remove ${user.email} from organizations?` + ? result ? cleanupTitle(result) : `Remove ${user.email} from organizations?` : result ? 'User deactivated' : `Deactivate ${user.email}?`} @@ -157,6 +157,12 @@ export function DeactivateDialog({ user, onClose }: { user: AdminUser; onClose: ); } +// From what the server did, not what the preview offered: a concurrent change can leave nothing to remove. +function cleanupTitle({ removedFrom }: DeactivateUserResult) { + if (removedFrom === null) { return 'Removal finished'; } + return removedFrom.length ? 'Memberships removed' : 'No memberships removed'; +} + const TONES: Record<'leave' | 'stay', { icon: ReactNode; box: string }> = { leave: { icon: , box: 'bg-muted/40' }, stay: { icon: , box: 'border-yellow/40 bg-yellow/10' }, diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 016df5ff3..9e99906b4 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -481,6 +481,15 @@ describe('admin users', () => { expect(screen.queryByRole('button', { name: 'Account settings' })).toBeNull(); }); + it('refuses details that do not say where the account is the last admin', async () => { + const { lastAdmin: _omitted, ...unmarked } = user.organizations[0]; + get.mockResolvedValue({ data: { ...user, organizations: [unmarked, user.organizations[1]] } }); + await renderAt('/admin/users/usr-1'); + + expect(await screen.findByText(/cannot show user details/)).toBeTruthy(); + expect(screen.queryByRole('button', { name: 'Account settings' })).toBeNull(); + }); + it('confirms, deactivates, and shows what actually happened', async () => { await renderAt('/admin/users/usr-1'); await openDeactivate(); @@ -616,6 +625,24 @@ describe('admin users', () => { expect(await screen.findByText('Memberships removed')).toBeTruthy(); }); + it.each([ + ['removed nothing', { removedFrom: [], retainedIn: [] }, 'No memberships removed'], + ['did not report what it removed', {}, 'Removal finished'], + ])('titles a removal by what the server did when it %s', async (_case, outcome, title) => { + user.status = 'DEACTIVATED'; + patch.mockResolvedValueOnce({ data: { id: 'usr-1', status: 'DEACTIVATED', ...outcome } }); + await renderAt('/admin/users/usr-1'); + await openDeactivate(); + await typeConfirmation(); + await act(async () => { + fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Remove' })); + }); + + const done = await screen.findByRole('dialog'); + expect(await within(done).findByText(title)).toBeTruthy(); + expect(within(done).queryByText('Memberships removed')).toBeNull(); + }); + it('closes the confirm dialog when the PATCH fails', async () => { patch.mockRejectedValueOnce(new Error('no answer')); await renderAt('/admin/users/usr-1'); diff --git a/src/features/admin/users/userAdminApi.ts b/src/features/admin/users/userAdminApi.ts index 01c625c73..e91c9ab6e 100644 --- a/src/features/admin/users/userAdminApi.ts +++ b/src/features/admin/users/userAdminApi.ts @@ -80,7 +80,11 @@ async function getUserDetails(userId: string): Promise { const organizations = (data as Partial | undefined)?.organizations; if ( !Array.isArray(organizations) - || organizations.some((membership) => !Array.isArray(membership.clusters) || !Array.isArray(membership.roleNames)) + || organizations.some((membership) => + !Array.isArray(membership.clusters) + || !Array.isArray(membership.roleNames) + || typeof membership.lastAdmin !== 'boolean' + ) ) { throw new Error('This central manager cannot show user details yet.'); } diff --git a/src/integrations/api/api.patch.d.ts b/src/integrations/api/api.patch.d.ts index be986c2c5..95cb7fee2 100644 --- a/src/integrations/api/api.patch.d.ts +++ b/src/integrations/api/api.patch.d.ts @@ -57,6 +57,7 @@ export type StaffPermission = | 'region:read' | 'region:write' | 'monitoringPolicy:write' + | 'user:read' | 'user:status' | 'systemStatus:write' | 'apiToken:create' From 2adc23ceee544ea2851b99d7509080bf6c1da6b8 Mon Sep 17 00:00:00 2001 From: David C Date: Thu, 29 Oct 2026 15:27:48 -0400 Subject: [PATCH 23/23] fix(admin): refuse a null membership in user details like any other malformed one Co-Authored-By: Claude Opus 5.5 --- src/features/admin/users/index.test.tsx | 8 ++++++++ src/features/admin/users/userAdminApi.ts | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/features/admin/users/index.test.tsx b/src/features/admin/users/index.test.tsx index 9e99906b4..1b3c3bdf8 100644 --- a/src/features/admin/users/index.test.tsx +++ b/src/features/admin/users/index.test.tsx @@ -490,6 +490,14 @@ describe('admin users', () => { expect(screen.queryByRole('button', { name: 'Account settings' })).toBeNull(); }); + it('refuses details with an empty membership the same way, not with a raw error', async () => { + get.mockResolvedValue({ data: { ...user, organizations: [null, user.organizations[1]] } }); + await renderAt('/admin/users/usr-1'); + + expect(await screen.findByText(/cannot show user details/)).toBeTruthy(); + expect(screen.queryByText(/Cannot read properties/)).toBeNull(); + }); + it('confirms, deactivates, and shows what actually happened', async () => { await renderAt('/admin/users/usr-1'); await openDeactivate(); diff --git a/src/features/admin/users/userAdminApi.ts b/src/features/admin/users/userAdminApi.ts index e91c9ab6e..d8de22190 100644 --- a/src/features/admin/users/userAdminApi.ts +++ b/src/features/admin/users/userAdminApi.ts @@ -81,9 +81,9 @@ async function getUserDetails(userId: string): Promise { if ( !Array.isArray(organizations) || organizations.some((membership) => - !Array.isArray(membership.clusters) - || !Array.isArray(membership.roleNames) - || typeof membership.lastAdmin !== 'boolean' + !Array.isArray(membership?.clusters) + || !Array.isArray(membership?.roleNames) + || typeof membership?.lastAdmin !== 'boolean' ) ) { throw new Error('This central manager cannot show user details yet.');