From c22d9ecf5801a75c84da927788a5bce2092c56d9 Mon Sep 17 00:00:00 2001 From: ConnorYoh <40631091+ConnorYoh@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:22:05 +0000 Subject: [PATCH] feat(editor): move the admin directory onto TanStack Query (#7726) # Description of Changes Step 5 of the TanStack Query rollout, covering the admin People, Teams and Team details screens. Follows #7264, #7283, #7285. ## The problem Two separate ones, in the same three files. **Reads.** Each section fetched and held its own copy of the same resources: People read the roster and the team list, Teams read the team list plus the roster again when its add-member modal opened, Team details read all three. Cost scaled with how many screens you visited rather than with how much data exists. **Writes.** Thirteen handlers each did the same five things by hand: set a processing flag, call the service, toast the outcome, dig a message out of an axios error, and reload their own slice. Refreshing was a convention, not a mechanism, and one handler had already forgotten it. ## The fix Three shared query keys (`adminUsers`, `teams`, `teamDetails`), and one `useAdminMutation` helper that every write is declared against: ```ts const createTeam = useAdminMutation({ write: (name: string) => teamService.createTeam(name), invalidates: ["teams"], success: t("workspace.teams.createTeam.success"), errorFallback: t("workspace.teams.createTeam.error"), onDone: () => { setNewTeamName(""); setCreateModalOpened(false); }, }); ``` Each write names the slices it disturbs, which is the part that only works when reads and writes are designed together: `createTeam` invalidates the team list, while a membership move invalidates the list, both teams' detail rows and the roster, because it genuinely changes all three. Invalidation refetches only mounted queries, so this costs nothing extra. The blanket "invalidate everything" helper survives in exactly one role: child components (invite, password change, seat update) that write through their own services, where the affected scopes are not visible from the call site. ## Why it is better, measured Request counts come from one harness driving `teams -> team details -> back -> people`, run against the branch point and against this branch. The assertion is committed, so it cannot silently regress. | | Before | After | |---|---|---| | Requests | 7 | **3** | | `getTeams` | 4 | **1** | | `getUsers` | 2 | **1** | | `getTeamDetails` | 1 | 1 | | Committed renders | 17 | **15** | Three is one per distinct resource, the floor for that sequence. The four `getTeams` were the Teams table, Team details fetching the same list for its "move to team" dropdown, the explicit refresh on the back button, and People. Renders barely move, which is expected: this changes where data lives, not how often React draws. It is reported because a caching change can quietly cost renders, and this one does not. On the code itself, across the three sections: | | | |---|---| | Net lines | **-216** | | `useState`/`useEffect` removed | **11**, none added | | Duplicated `isAxiosError` blocks | 13 to **1** | | `setProcessing` calls | 19 to **0** | `isAxiosError` is no longer imported by any of the three files. ## Bug fixed `disableMfaByAdmin` showed a success toast and never refreshed. The menu item renders only when `user.mfaEnabled` is true, so an admin disabled MFA, was told it worked, and watched the option stay on screen until a manual reload. It is covered by a test that fails if the invalidation is removed. ## Behaviour worth checking in review - A write no longer blocks its handler before closing the modal. The dialog closes when the write succeeds and the table updates when the refetch lands, rather than the button spinning through both. - Modal submit buttons now track their own mutation rather than one shared flag. Team details still derives a single busy flag, now from its five mutations rather than a `useState`, so its row actions disable together as before. - The per-handler `console.error` is kept, once, in the shared error path. ## Testing Five tests, each verified by breaking the implementation and confirming that one test, and only that one, fails: | Mutation | Caught by | |---|---| | Drop the shared stale window (`staleTime: 0`) | request-count test | | Make invalidation a no-op | write-visibility test | | Ignore the login-enabled gate | login-disabled test | | Stop invalidating after the MFA write | MFA-refresh test | | Fall back to the generic error message | server-message test | The write tests drive the real flows through their modals and menus rather than calling hooks directly. `task frontend:check` passes typecheck, lint and oxfmt, and 2383 of 2385 editor tests. The two failures, `workbenchSession.test.ts` and `notificationActions.test.tsx`, are untouched here and fail identically with this branch's changes reverted. ## Scope The three services keep their current shape; nothing outside these three sections and the new hook module changes. Child modals that write through their own services still refresh via the blanket helper, and converting those is separate work. --- frontend/editor/src/core/query/keys.ts | 4 + .../config/configSections/PeopleSection.tsx | 477 +++++++----------- .../configSections/TeamDetailsSection.tsx | 393 ++++++--------- .../config/configSections/TeamsSection.tsx | 216 +++----- .../config/configSections/adminReads.test.tsx | 295 +++++++++++ .../proprietary/hooks/useAdminDirectory.ts | 151 ++++++ 6 files changed, 885 insertions(+), 651 deletions(-) create mode 100644 frontend/editor/src/proprietary/components/shared/config/configSections/adminReads.test.tsx create mode 100644 frontend/editor/src/proprietary/hooks/useAdminDirectory.ts diff --git a/frontend/editor/src/core/query/keys.ts b/frontend/editor/src/core/query/keys.ts index 5354b56b63..d95f674011 100644 --- a/frontend/editor/src/core/query/keys.ts +++ b/frontend/editor/src/core/query/keys.ts @@ -1,5 +1,7 @@ /** Editor query keys: ["editor", , ...params]. */ export const qk = { + /** The admin directory payload: a different endpoint and shape to qk.users(). */ + adminUsers: () => ["editor", "adminUsers"] as const, appConfig: () => ["editor", "appConfig"] as const, endpointsAvailability: () => ["editor", "endpointsAvailability"] as const, endpointEnabled: (endpoint: string) => @@ -9,5 +11,7 @@ export const qk = { /** Keyed on the asking identity: two users must never share one answer. */ portalAccess: (userId: string | null) => ["editor", "portalAccess", userId] as const, + teamDetails: (teamId: number) => ["editor", "teamDetails", teamId] as const, + teams: () => ["editor", "teams"] as const, users: () => ["editor", "users"] as const, } as const; diff --git a/frontend/editor/src/proprietary/components/shared/config/configSections/PeopleSection.tsx b/frontend/editor/src/proprietary/components/shared/config/configSections/PeopleSection.tsx index d955cb341b..6b14d2b421 100644 --- a/frontend/editor/src/proprietary/components/shared/config/configSections/PeopleSection.tsx +++ b/frontend/editor/src/proprietary/components/shared/config/configSections/PeopleSection.tsx @@ -1,5 +1,4 @@ -import { useState, useEffect } from "react"; -import { isAxiosError } from "axios"; +import { useMemo, useState } from "react"; import { Trans, useTranslation } from "react-i18next"; import { Stack, @@ -20,14 +19,12 @@ import { import { Button } from "@app/ui/Button"; import { ActionIcon } from "@app/ui/ActionIcon"; import LocalIcon from "@app/components/shared/LocalIcon"; -import { alert } from "@app/components/toast"; import { userManagementService, User, } from "@app/services/userManagementService"; -import { teamService, Team } from "@app/services/teamService"; +import { type Team } from "@app/services/teamService"; import { Z_INDEX_OVER_CONFIG_MODAL } from "@app/styles/zIndex"; -import { useAppConfig } from "@app/contexts/AppConfigContext"; import InviteMembersModal from "@app/components/shared/InviteMembersModal"; import { useLoginRequired } from "@app/hooks/useLoginRequired"; import LoginRequiredBanner from "@app/components/shared/config/LoginRequiredBanner"; @@ -36,17 +33,109 @@ import UpdateSeatsButton from "@app/components/shared/UpdateSeatsButton"; import { useLicense } from "@app/contexts/LicenseContext"; import ChangeUserPasswordModal from "@app/components/shared/ChangeUserPasswordModal"; import { useAuth } from "@app/auth/UseSession"; +import { + useAdminUsers, + useTeams, + useAdminMutation, + useInvalidateAdminDirectory, +} from "@app/hooks/useAdminDirectory"; + +const EXAMPLE_USERS: User[] = [ + { + id: 1, + username: "admin", + email: "admin@example.com", + enabled: true, + roleName: "ROLE_ADMIN", + rolesAsString: "ROLE_ADMIN", + authenticationType: "password", + isActive: true, + lastRequest: Date.now(), + team: { id: 1, name: "Engineering" }, + }, + { + id: 2, + username: "john.doe", + email: "john.doe@example.com", + enabled: true, + roleName: "ROLE_USER", + rolesAsString: "ROLE_USER", + authenticationType: "password", + isActive: false, + lastRequest: Date.now() - 86400000, + team: { id: 1, name: "Engineering" }, + }, + { + id: 3, + username: "jane.smith", + email: "jane.smith@example.com", + enabled: true, + roleName: "ROLE_USER", + rolesAsString: "ROLE_USER", + authenticationType: "oauth", + isActive: true, + lastRequest: Date.now(), + team: { id: 2, name: "Marketing" }, + }, + { + id: 4, + username: "bob.wilson", + email: "bob.wilson@example.com", + enabled: false, + roleName: "ROLE_USER", + rolesAsString: "ROLE_USER", + authenticationType: "password", + isActive: false, + lastRequest: Date.now() - 604800000, + team: undefined, + }, +]; + +const EXAMPLE_TEAMS: Team[] = [ + { id: 1, name: "Engineering", userCount: 3 }, + { id: 2, name: "Marketing", userCount: 2 }, +]; + +const EXAMPLE_LICENSE = { + maxAllowedUsers: 10, + availableSlots: 6, + grandfatheredUserCount: 0, + licenseMaxUsers: 5, + premiumEnabled: true, + totalUsers: 4, +}; export default function PeopleSection() { const { t } = useTranslation(); - const { config } = useAppConfig(); const { loginEnabled } = useLoginRequired(); const { user: currentUser } = useAuth(); const navigate = useNavigate(); const { licenseInfo: globalLicenseInfo } = useLicense(); - const [users, setUsers] = useState([]); - const [teams, setTeams] = useState([]); - const [loading, setLoading] = useState(true); + const admin = useAdminUsers(loginEnabled); + const { data: fetchedTeams } = useTeams(loginEnabled); + const refreshDirectory = useInvalidateAdminDirectory(); + + // Session and MFA state arrive alongside the roster, keyed by username. + const fetchedUsers = useMemo(() => { + if (!admin.data) return []; + return admin.data.users.map((user) => ({ + ...user, + isActive: admin.data.userSessions[user.username] || false, + lastRequest: admin.data.userLastRequest[user.username] || undefined, + mfaEnabled: + ( + admin.data.userSettings?.[user.username] as + | Record + | undefined + )?.mfaEnabled === "true", + })); + }, [admin.data]); + + // Login off means the endpoints are not callable, so the table shows a + // worked example instead of an empty state. + const users = loginEnabled ? fetchedUsers : EXAMPLE_USERS; + const teams = loginEnabled ? (fetchedTeams ?? []) : EXAMPLE_TEAMS; + const loading = loginEnabled && admin.isPending; const [searchQuery, setSearchQuery] = useState(""); const [inviteModalOpened, setInviteModalOpened] = useState(false); const [editUserModalOpened, setEditUserModalOpened] = useState(false); @@ -54,19 +143,20 @@ export default function PeopleSection() { useState(false); const [passwordUser, setPasswordUser] = useState(null); const [selectedUser, setSelectedUser] = useState(null); - const [processing, setProcessing] = useState(false); - const [mailEnabled, setMailEnabled] = useState(false); - const [lockedUsers, setLockedUsers] = useState([]); - - // License information - const [licenseInfo, setLicenseInfo] = useState<{ - maxAllowedUsers: number; - availableSlots: number; - grandfatheredUserCount: number; - licenseMaxUsers: number; - premiumEnabled: boolean; - totalUsers: number; - } | null>(null); + const mailEnabled = loginEnabled ? (admin.data?.mailEnabled ?? false) : false; + const lockedUsers = loginEnabled ? (admin.data?.lockedUsers ?? []) : []; + const licenseInfo = loginEnabled + ? admin.data + ? { + maxAllowedUsers: admin.data.maxAllowedUsers, + availableSlots: admin.data.availableSlots, + grandfatheredUserCount: admin.data.grandfatheredUserCount, + licenseMaxUsers: admin.data.licenseMaxUsers, + premiumEnabled: admin.data.premiumEnabled, + totalUsers: admin.data.totalUsers, + } + : null + : EXAMPLE_LICENSE; const hasNoSlots = licenseInfo ? licenseInfo.availableSlots === 0 : false; const handleAddMembersClick = () => { if (!loginEnabled) { @@ -115,253 +205,103 @@ export default function PeopleSection() { teamId: undefined as number | undefined, }); - useEffect(() => { - fetchData(); - }, []); + const updateUserRole = useAdminMutation({ + write: (payload: { username: string; role: string; teamId?: number }) => + userManagementService.updateUserRole(payload), + // A role edit can also move the user, which changes both teams' counts. + invalidates: ["users", "teams"], + success: t("workspace.people.editMember.success"), + errorFallback: t("workspace.people.editMember.error"), + onDone: () => closeEditModal(), + }); - useEffect(() => { - if (config) { - console.log( - "[PeopleSection] Email invites enabled:", - config.enableEmailInvites, - ); - } - }, [config]); + const toggleEnabled = useAdminMutation({ + write: (user: User) => + userManagementService.toggleUserEnabled(user.username, !user.enabled), + invalidates: ["users"], + success: t("workspace.people.toggleEnabled.success"), + errorFallback: t("workspace.people.toggleEnabled.error"), + }); - const fetchData = async () => { - try { - setLoading(true); + const deleteUser = useAdminMutation({ + write: (username: string) => userManagementService.deleteUser(username), + invalidates: ["users", "teams"], + success: t( + "workspace.people.deleteUserSuccess", + "User deleted successfully", + ), + errorFallback: t( + "workspace.people.deleteUserError", + "Failed to delete user", + ), + }); - if (loginEnabled) { - const [adminData, teamsData] = await Promise.all([ - userManagementService.getUsers(), - teamService.getTeams(), - ]); + const unlockUser = useAdminMutation({ + write: (username: string) => userManagementService.unlockUser(username), + invalidates: ["users"], + success: t( + "workspace.people.unlockUserSuccess", + "User account unlocked successfully", + ), + errorFallback: t( + "workspace.people.unlockUserError", + "Failed to unlock user account", + ), + }); - // Enrich users with session data - const enrichedUsers = adminData.users.map((user) => ({ - ...user, - isActive: adminData.userSessions[user.username] || false, - lastRequest: adminData.userLastRequest[user.username] || undefined, - mfaEnabled: - ( - adminData.userSettings?.[user.username] as - | Record - | undefined - )?.mfaEnabled === "true", - })); + const disableMfa = useAdminMutation({ + write: (username: string) => + userManagementService.disableMfaByAdmin(username), + invalidates: ["users"], + success: t( + "workspace.people.mfa.adminDisableSuccess", + "MFA disabled successfully for user", + ), + errorFallback: t( + "workspace.people.mfa.adminDisableError", + "Failed to disable MFA for user", + ), + }); - setUsers(enrichedUsers); - setTeams(teamsData); - - // Store license information - setLicenseInfo({ - maxAllowedUsers: adminData.maxAllowedUsers, - availableSlots: adminData.availableSlots, - grandfatheredUserCount: adminData.grandfatheredUserCount, - licenseMaxUsers: adminData.licenseMaxUsers, - premiumEnabled: adminData.premiumEnabled, - totalUsers: adminData.totalUsers, - }); - setMailEnabled(adminData.mailEnabled); - setLockedUsers(adminData.lockedUsers || []); - } else { - // Provide example data when login is disabled - const exampleUsers: User[] = [ - { - id: 1, - username: "admin", - email: "admin@example.com", - enabled: true, - roleName: "ROLE_ADMIN", - rolesAsString: "ROLE_ADMIN", - authenticationType: "password", - isActive: true, - lastRequest: Date.now(), - team: { id: 1, name: "Engineering" }, - }, - { - id: 2, - username: "john.doe", - email: "john.doe@example.com", - enabled: true, - roleName: "ROLE_USER", - rolesAsString: "ROLE_USER", - authenticationType: "password", - isActive: false, - lastRequest: Date.now() - 86400000, - team: { id: 1, name: "Engineering" }, - }, - { - id: 3, - username: "jane.smith", - email: "jane.smith@example.com", - enabled: true, - roleName: "ROLE_USER", - rolesAsString: "ROLE_USER", - authenticationType: "oauth", - isActive: true, - lastRequest: Date.now(), - team: { id: 2, name: "Marketing" }, - }, - { - id: 4, - username: "bob.wilson", - email: "bob.wilson@example.com", - enabled: false, - roleName: "ROLE_USER", - rolesAsString: "ROLE_USER", - authenticationType: "password", - isActive: false, - lastRequest: Date.now() - 604800000, - team: undefined, - }, - ]; - - const exampleTeams: Team[] = [ - { id: 1, name: "Engineering", userCount: 3 }, - { id: 2, name: "Marketing", userCount: 2 }, - ]; - - setUsers(exampleUsers); - setTeams(exampleTeams); - setMailEnabled(false); - setLockedUsers([]); - - // Example license information - setLicenseInfo({ - maxAllowedUsers: 10, - availableSlots: 6, - grandfatheredUserCount: 0, - licenseMaxUsers: 5, - premiumEnabled: true, - totalUsers: 4, - }); - } - } catch (error) { - console.error("[PeopleSection] Failed to fetch people data:", error); - alert({ alertType: "error", title: "Failed to load people data" }); - } finally { - setLoading(false); - } - }; - - const handleUpdateUserRole = async () => { + const handleUpdateUserRole = () => { if (!selectedUser) return; - - try { - setProcessing(true); - await userManagementService.updateUserRole({ - username: selectedUser.username, - role: editForm.role, - teamId: editForm.teamId, - }); - alert({ - alertType: "success", - title: t("workspace.people.editMember.success"), - }); - closeEditModal(); - fetchData(); - } catch (error: unknown) { - console.error("[PeopleSection] Failed to update user:", error); - const errorMessage = isAxiosError(error) - ? error.response?.data?.message || - error.response?.data?.error || - error.message - : (error instanceof Error ? error.message : undefined) || - t("workspace.people.editMember.error"); - alert({ alertType: "error", title: errorMessage }); - } finally { - setProcessing(false); - } + updateUserRole.mutate({ + username: selectedUser.username, + role: editForm.role, + teamId: editForm.teamId, + }); }; - const handleToggleEnabled = async (user: User) => { - try { - await userManagementService.toggleUserEnabled( - user.username, - !user.enabled, - ); - alert({ - alertType: "success", - title: t("workspace.people.toggleEnabled.success"), - }); - fetchData(); - } catch (error: unknown) { - console.error("[PeopleSection] Failed to toggle user status:", error); - const errorMessage = isAxiosError(error) - ? error.response?.data?.message || - error.response?.data?.error || - error.message - : (error instanceof Error ? error.message : undefined) || - t("workspace.people.toggleEnabled.error"); - alert({ alertType: "error", title: errorMessage }); - } + const handleToggleEnabled = (user: User) => { + toggleEnabled.mutate(user); }; - const handleDeleteUser = async (user: User) => { + const handleDeleteUser = (user: User) => { const confirmMessage = t( "workspace.people.confirmDelete", "Are you sure you want to delete this user? This action cannot be undone.", ); - if (!window.confirm(`${confirmMessage}\n\nUser: ${user.username}`)) { - return; - } + if ( + !window.confirm(`${confirmMessage} - try { - await userManagementService.deleteUser(user.username); - alert({ - alertType: "success", - title: t( - "workspace.people.deleteUserSuccess", - "User deleted successfully", - ), - }); - fetchData(); - } catch (error: unknown) { - console.error("[PeopleSection] Failed to delete user:", error); - const errorMessage = isAxiosError(error) - ? error.response?.data?.message || - error.response?.data?.error || - error.message - : (error instanceof Error ? error.message : undefined) || - t("workspace.people.deleteUserError", "Failed to delete user"); - alert({ alertType: "error", title: errorMessage }); - } +User: ${user.username}`) + ) + return; + deleteUser.mutate(user.username); }; - const handleUnlockUser = async (user: User) => { + const handleUnlockUser = (user: User) => { const confirmMessage = t( "workspace.people.confirmUnlock", "Are you sure you want to unlock this user account?", ); - if (!window.confirm(`${confirmMessage}\n\nUser: ${user.username}`)) { - return; - } + if ( + !window.confirm(`${confirmMessage} - try { - await userManagementService.unlockUser(user.username); - alert({ - alertType: "success", - title: t( - "workspace.people.unlockUserSuccess", - "User account unlocked successfully", - ), - }); - fetchData(); - } catch (error: unknown) { - console.error("[PeopleSection] Failed to unlock user:", error); - const errorMessage = isAxiosError(error) - ? error.response?.data?.message || - error.response?.data?.error || - error.message - : (error instanceof Error ? error.message : undefined) || - t( - "workspace.people.unlockUserError", - "Failed to unlock user account", - ); - alert({ alertType: "error", title: errorMessage }); - } +User: ${user.username}`) + ) + return; + unlockUser.mutate(user.username); }; const openEditModal = (user: User) => { @@ -549,7 +489,7 @@ export default function PeopleSection() { • - + )} @@ -891,40 +831,7 @@ export default function PeopleSection() { height="1rem" /> } - onClick={async () => { - try { - await userManagementService.disableMfaByAdmin( - user.username, - ); - alert({ - alertType: "success", - title: t( - "workspace.people.mfa.adminDisableSuccess", - "MFA disabled successfully for user", - ), - }); - } catch (error: unknown) { - console.error( - "[PeopleSection] Failed to disable MFA for user:", - error, - ); - const errorMessage = isAxiosError(error) - ? error.response?.data?.message || - error.response?.data?.error || - error.message - : (error instanceof Error - ? error.message - : undefined) || - t( - "workspace.people.mfa.adminDisableError", - "Failed to disable MFA for user", - ); - alert({ - alertType: "error", - title: errorMessage, - }); - } - }} + onClick={() => disableMfa.mutate(user.username)} disabled={!loginEnabled} > {t( @@ -968,14 +875,14 @@ export default function PeopleSection() { setInviteModalOpened(false)} - onSuccess={fetchData} + onSuccess={refreshDirectory} /> @@ -1075,7 +982,7 @@ export default function PeopleSection() { />