feat: setup admin management flow with backend endpoints#25
Conversation
📝 WalkthroughWalkthroughAdmin management now uses server-backed API actions, parameterized query hooks, extracted dialogs, and normalized status/last_login fields. Login now returns a typed success/failure result, and the login form handles that result plus session-start errors. ChangesAdmin management API migration
Login result handling
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant AdminManagementHooks
participant AdminManagementActions
participant authApi
participant AdminManagementUI
AdminManagementHooks->>AdminManagementActions: invoke admin action or list query
AdminManagementActions->>authApi: GET/POST/PATCH /admin/admins
authApi-->>AdminManagementActions: response or error
AdminManagementActions-->>AdminManagementHooks: AdminManagementResult / AdminAccountsPage
AdminManagementHooks-->>AdminManagementUI: data, toast updates, invalidation
sequenceDiagram
participant AdminLoginForm
participant loginAction
participant publicApi
participant signIn
AdminLoginForm->>loginAction: login(body)
loginAction->>publicApi: post credentials
publicApi-->>loginAction: response or error
loginAction-->>AdminLoginForm: LoginResult
AdminLoginForm->>signIn: signIn("credentials") if success
signIn-->>AdminLoginForm: signInResult
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/auth.ts (1)
23-52: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep the full login flow inside the
LoginResultboundary.Line 50 and Line 52 can still throw/reject outside the
try/catch, which reintroduces thrown server-action errors instead of returning{ success: false }. Wrap cookie persistence and response unwrapping too; unwrap before persisting cookies so invalid envelopes do not leave cookies behind.Proposed fix
export async function login(body: LoginInput): Promise<LoginResult> { - let res: Awaited< - ReturnType<typeof publicApi.post<ApiEnvelope<LoginResponseData>>> - >; - try { - res = await publicApi.post<ApiEnvelope<LoginResponseData>>( + const res = await publicApi.post<ApiEnvelope<LoginResponseData>>( "/admin/auth/login", body, ); - } catch (err) { - return { success: false, error: toApiError(err).message }; - } - // Forward API auth cookies to the browser so subsequent server-action API - // calls can read and proxy them via getServerCookieHeader(). - const cookies = setCookieHeadersFrom(res.headers) - .map(parseSetCookieHeader) - .filter((c): c is NonNullable<typeof c> => c != null); + const data = unwrapData(res); - if (cookies.length === 0) { - return { - success: false, - error: "Authentication failed: no session cookies returned by the API.", - }; - } + // Forward API auth cookies to the browser so subsequent server-action API + // calls can read and proxy them via getServerCookieHeader(). + const cookies = setCookieHeadersFrom(res.headers) + .map(parseSetCookieHeader) + .filter((c): c is NonNullable<typeof c> => c != null); - await persistServerCookies(cookies); + if (cookies.length === 0) { + return { + success: false, + error: "Authentication failed: no session cookies returned by the API.", + }; + } - return { success: true, data: unwrapData(res) }; + await persistServerCookies(cookies); + + return { success: true, data }; + } catch (err) { + return { success: false, error: toApiError(err).message }; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/actions/auth.ts` around lines 23 - 52, The `login` flow is only partially protected by the `try/catch`, so `persistServerCookies` and `unwrapData` can still throw and escape the `LoginResult` contract. Move the cookie persistence and response unwrapping into the same guarded flow in `login`, and ensure `unwrapData(res)` happens before `persistServerCookies(cookies)` so invalid API envelopes do not leave cookies persisted. Keep any failure in `login` returning `{ success: false, error: ... }` instead of rejecting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/actions/admin-management.ts`:
- Around line 34-102: The six mutation functions in admin-management.ts all
repeat the same try/catch, ok, and fail(toApiError(err).message) pattern, so
extract a shared helper to centralize this flow. Add a reusable wrapper around
authApi calls that accepts the request promise and success message, then update
inviteAdmin, resetAdminPassword, changeAdminEmail, changeAdminRole,
deactivateAdminAccount, and reactivateAdminAccount to use it while preserving
their current messages and request payloads.
In `@src/components/admin-management/admin-action-dialogs.tsx`:
- Around line 112-117: The new email field in the admin action dialog currently
allows empty submissions because the Input only sets type="email" without
enforcing required validation. Update the Input used for newEmail in
admin-action-dialogs.tsx to include the required attribute so browser validation
blocks empty submits before the form reaches the server.
- Around line 61-63: The Reset Password action in admin-action-dialogs should
not let AlertDialogAction auto-close the dialog before the mutation completes.
Control the dialog’s open state around the relevant AlertDialog and
AlertDialogAction usage, then close it only from the password reset mutation’s
onSuccess path (or replace the action with a plain button) so failures keep the
dialog open for retries.
In `@src/hooks/api/use-admin-management.ts`:
- Around line 34-127: The five hooks in use-admin-management.ts repeat the same
mutation flow, so extract a shared factory around useMutation that centralizes
the result.ok handling, success toast, optional onSuccess callback, and
invalidation of adminManagementKeys.accounts(). Keep the per-hook differences
limited to the mutationFn and the onError message, and reuse the factory for
useResetAdminPassword, useChangeAdminEmail, useChangeAdminRole,
useDeactivateAdmin, and useReactivateAdmin. While refactoring, make sure the
shared result type (such as AdminAccountResult) matches the actual server action
return shape with ok and message.
---
Outside diff comments:
In `@src/actions/auth.ts`:
- Around line 23-52: The `login` flow is only partially protected by the
`try/catch`, so `persistServerCookies` and `unwrapData` can still throw and
escape the `LoginResult` contract. Move the cookie persistence and response
unwrapping into the same guarded flow in `login`, and ensure `unwrapData(res)`
happens before `persistServerCookies(cookies)` so invalid API envelopes do not
leave cookies persisted. Keep any failure in `login` returning `{ success:
false, error: ... }` instead of rejecting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e907136b-23c8-4943-9e21-1fcee5c29b17
📒 Files selected for processing (11)
src/actions/admin-management.tssrc/actions/auth.tssrc/components/admin-management/admin-accounts-table.tsxsrc/components/admin-management/admin-action-dialogs.tsxsrc/components/admin-management/admin-toggle-activation-modal.tsxsrc/components/admin-management/columns.tsxsrc/components/auth/admin-login-form.tsxsrc/hooks/api/keys.tssrc/hooks/api/use-admin-management.tssrc/mocks/admin-management.tssrc/types/api/admin-management.ts
- Extract shared callAdminApi/createAdminMutation helpers to remove duplicated try/catch and toast/invalidation boilerplate. - Keep the reset password dialog open until the mutation resolves instead of letting AlertDialogAction auto-close it. - Require the new email field to block empty submissions client-side.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/admin-management/admin-action-dialogs.tsx (1)
74-138: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStale local state when
adminchanges or dialog reopens.
useState(admin?.email ?? "")only initializes on first mount. Since the dialog stays mounted whenopen={false}, switching to a different admin or reopening the dialog retains the previously typed email. This can cause the wrong email to be submitted for a different admin.The same issue applies to
ChangeRoleDialogat line 141 whereuseState(admin?.role ?? "admin")won't update whenadminchanges.🛡️ Proposed fix: sync local state when dialog opens
export function ChangeEmailDialog({ admin, open, onClose }: DialogProps) { const [newEmail, setNewEmail] = React.useState(admin?.email ?? ""); const mutation = useChangeAdminEmail(onClose); + + React.useEffect(() => { + if (open) { + setNewEmail(admin?.email ?? ""); + } + }, [open, admin]); const serverError = mutation.data?.ok === false ? mutation.data.message : "";Apply the same pattern to
ChangeRoleDialog:export function ChangeRoleDialog({ admin, open, onClose }: DialogProps) { const [selectedRole, setSelectedRole] = React.useState<AdminRole>( admin?.role ?? "admin", ); const mutation = useChangeAdminRole(onClose); + + React.useEffect(() => { + if (open) { + setSelectedRole(admin?.role ?? "admin"); + } + }, [open, admin]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/admin-management/admin-action-dialogs.tsx` around lines 74 - 138, The dialog state is initialized once from `admin` and then becomes stale when a different admin is selected or the dialog is reopened. Update `ChangeEmailDialog` and `ChangeRoleDialog` to sync their local state with the current `admin` (for example when `open` changes to true or `admin` changes) so `newEmail` and the role value always reflect the active admin before submit. Use the existing `ChangeEmailDialog` and `ChangeRoleDialog` components and their `useState`/dialog open handling to reset the form state instead of relying on the initial `useState(admin?.email ?? "")` and `useState(admin?.role ?? "admin")` values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/admin-management/admin-action-dialogs.tsx`:
- Around line 74-138: The dialog state is initialized once from `admin` and then
becomes stale when a different admin is selected or the dialog is reopened.
Update `ChangeEmailDialog` and `ChangeRoleDialog` to sync their local state with
the current `admin` (for example when `open` changes to true or `admin` changes)
so `newEmail` and the role value always reflect the active admin before submit.
Use the existing `ChangeEmailDialog` and `ChangeRoleDialog` components and their
`useState`/dialog open handling to reset the form state instead of relying on
the initial `useState(admin?.email ?? "")` and `useState(admin?.role ??
"admin")` values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a8489991-73a0-4e64-961b-95a56b032e32
📒 Files selected for processing (3)
src/actions/admin-management.tssrc/components/admin-management/admin-action-dialogs.tsxsrc/hooks/api/use-admin-management.ts
Description
This PR wires up the six admin management REST endpoints (replacing mock data) and fixes a production login error caused by Next.js 15 error sanitization in server actions..
Changes Proposed
What were you told to do?
GET /admin/admins,POST /admin/admins/invite,POST /admin/admins/{id}/reset-password,PATCH /admin/admins/{id}/email,PATCH /admin/admins/{id}/role,PATCH /admin/admins/{id}/deactivate, andPATCH /admin/admins/{id}/reactivatefollowing the existing actions/hooks/types pattern.What did you do?
Admin management API integration:
src/types/api/admin-management.ts— alignedAdminAccountStatusto API values ("active","pending_setup","deactivated"), renamedlastLogin→last_login, addedAdminAccountsQueryParams,AdminAccountsPage, andGetAdminAccountsResponsetypes. Addedconfirm_downgrade?: booleantoChangeAdminRoleInputand aADMIN_STATUS_LABELSmap for display formatting.src/actions/admin-management.tswith realauthApicalls. All mutation actions catch errors viatoApiErrorand return{ ok: false, message }— they never throw, keeping the existing result-based error handling in the table component intact.getAdminAccountshandles the double-wrapped API envelope (data.data.items).src/hooks/api/keys.tsto addadminManagementKeys.list(params)andsrc/hooks/api/use-admin-management.tsto acceptAdminAccountsQueryParamswithkeepPreviousData.columns.tsx(status variant map,last_logincell,isDeactivatedcheck) andadmin-accounts-table.tsx(data?.items ?? [], passesconfirm_downgradeon super admin downgrade).Production login fix:
src/actions/auth.ts: changedlogin()from throwing to returningLoginResult = { success: true; data } | { success: false; error: string }. All error paths (API failure, missing cookies) now return, never throw.src/components/auth/admin-login-form.tsx: updated to checkresult.success.signIn()is now wrapped in its own try/catch since it can throw on NextAuth endpoint failure.Types of changes
Check List
Summary by CodeRabbit