diff --git a/src/components/providers/AlertProvider.tsx b/src/components/providers/AlertProvider.tsx index 40543fc9f..8b27be069 100644 --- a/src/components/providers/AlertProvider.tsx +++ b/src/components/providers/AlertProvider.tsx @@ -42,7 +42,7 @@ export function AlertProvider({ children }: { children: React.ReactNode }) { }; useEffect(() => { if (!isOpen) setAlert(undefined); - }, [open]); + }, [isOpen]); return ( ({ + useRouter: () => ({ refresh }), +})); + +vi.mock('@/lib/hooks/use-toast', () => ({ + toast: (...args: unknown[]) => toast(...args), +})); + +vi.mock('@/lib/api/settings/admins', () => ({ + postNewAdminUser: (...args: unknown[]) => postNewAdminUser(...args), +})); + +function OpenSheetHarness({ + onOpenChange, +}: { + onOpenChange?: (open: boolean) => void; +}) { + const [open, setOpen] = useState(true); + return ( + + { + setOpen(next); + onOpenChange?.(next); + }} + > + + + + ); +} + +function renderSheet() { + return render(); +} + +describe('AddAdminSheet', () => { + beforeAll(() => { + if (typeof globalThis.ResizeObserver === 'undefined') { + globalThis.ResizeObserver = class ResizeObserver { + observe() {} + unobserve() {} + disconnect() {} + }; + } + Element.prototype.scrollIntoView = () => {}; + Element.prototype.hasPointerCapture = () => false; + Element.prototype.setPointerCapture = () => {}; + Element.prototype.releasePointerCapture = () => {}; + }); + + afterEach(() => { + cleanup(); + refresh.mockReset(); + toast.mockReset(); + postNewAdminUser.mockReset(); + }); + + it('keeps username and password inputs controlled with empty defaults', () => { + renderSheet(); + expect(screen.getByPlaceholderText('Enter a username')).toHaveAttribute( + 'value', + '' + ); + const [password, confirm] = screen.getAllByPlaceholderText('very secret'); + expect(password).toHaveAttribute('value', ''); + expect(confirm).toHaveAttribute('value', ''); + }); + + it('closes after a successful create without tripping the unsaved-changes alert', async () => { + postNewAdminUser.mockResolvedValue(undefined); + const onOpenChange = vi.fn(); + + render(); + + fireEvent.change(screen.getByPlaceholderText('Enter a username'), { + target: { value: 'uiadmin1' }, + }); + const [password, confirm] = screen.getAllByPlaceholderText('very secret'); + fireEvent.change(password, { target: { value: 'secretpass' } }); + fireEvent.change(confirm, { target: { value: 'secretpass' } }); + fireEvent.click(screen.getByRole('button', { name: 'Create' })); + + await waitFor(() => { + expect(postNewAdminUser).toHaveBeenCalledWith('uiadmin1', 'secretpass'); + }); + await waitFor(() => { + expect(onOpenChange).toHaveBeenCalledWith(false); + }); + expect( + screen.queryByText(/unsaved changes will be lost/i) + ).not.toBeInTheDocument(); + expect(refresh).toHaveBeenCalled(); + }); +}); diff --git a/src/components/settings/admin-users/AddAdminSheet.tsx b/src/components/settings/admin-users/AddAdminSheet.tsx index fe7a61b1c..501e661be 100644 --- a/src/components/settings/admin-users/AddAdminSheet.tsx +++ b/src/components/settings/admin-users/AddAdminSheet.tsx @@ -1,3 +1,5 @@ +'use client'; + import { Sheet, SheetContent, @@ -29,11 +31,17 @@ import { LucideX } from 'lucide-react'; import { ErrorPre } from '@/components/ui/error-pre'; import { useRouter } from 'next/navigation'; +const emptyValues = { + username: '', + password: '', + confirmPassword: '', +}; + const FormSchema = z .object({ - username: z.string(), - password: z.string(), - confirmPassword: z.string(), + username: z.string().min(1, 'Username is required'), + password: z.string().min(1, 'Password is required'), + confirmPassword: z.string().min(1, 'Confirm your password'), }) .refine( schema => { @@ -66,29 +74,32 @@ export const AddAdminSheet = ({ ); const isControlled = controlledOpen !== undefined; const open = isControlled ? controlledOpen : uncontrolledOpen; - const setOpen = (next: boolean) => { + const setOpenState = (next: boolean) => { if (!isControlled) setUncontrolledOpen(next); controlledOnOpenChange?.(next); }; const { addAlert } = useAlerts(); const form = useForm>({ resolver: rhfZodResolver(FormSchema), + defaultValues: emptyValues, }); const router = useRouter(); const { formState, reset, control, handleSubmit } = form; + const { isDirty, isSubmitted } = formState; - useEffect(() => { - if (defaultOpen !== undefined && !isControlled) - setUncontrolledOpen(defaultOpen); - }, [defaultOpen, isControlled]); + const discardAndClose = () => { + reset(emptyValues); + setOpenState(false); + onClose?.(); + }; - useEffect(() => { - if (!open && formState.isSubmitted) { - onClose?.(); - return reset(); + const requestOpenChange = (next: boolean) => { + if (next) { + setOpenState(true); + return; } - if (!open && formState.isDirty) { + if (isDirty && !isSubmitted) { addAlert({ title: 'Add Admin', description: @@ -96,22 +107,24 @@ export const AddAdminSheet = ({ cancelText: 'Cancel', actionText: 'Close', onDecision: cancel => { - if (!cancel) { - onClose?.(); - return reset(); - } - setOpen(true); + if (!cancel) discardAndClose(); }, }); - } else if (!open) { - onClose?.(); + return; } - }, [open, setOpen]); + discardAndClose(); + }; + + useEffect(() => { + if (defaultOpen !== undefined && !isControlled) + setUncontrolledOpen(defaultOpen); + }, [defaultOpen, isControlled]); function onSubmit(data: z.infer) { postNewAdminUser(data.username, data.password) - .then(res => { - setOpen(false); + .then(() => { + reset(emptyValues); + setOpenState(false); toast({ title: 'New Admin', description: ( @@ -139,7 +152,7 @@ export const AddAdminSheet = ({ } return ( - + {children}
diff --git a/src/lib/api/communications/templates.ts b/src/lib/api/communications/templates.ts index c212e71ac..e6b97604f 100644 --- a/src/lib/api/communications/templates.ts +++ b/src/lib/api/communications/templates.ts @@ -8,14 +8,14 @@ import { import { MigrationResponse } from '@/lib/models/communications/template-row'; import { formatCommunicationsApiError, - isNextNavigationError, + rethrowNextControlFlowError, } from '@/lib/logic/api-error'; async function withCommunicationsError(fn: () => Promise): Promise { try { return await fn(); } catch (err) { - if (isNextNavigationError(err)) throw err; + rethrowNextControlFlowError(err); throw new Error(formatCommunicationsApiError(err)); } } diff --git a/src/lib/api/settings/admins/index.ts b/src/lib/api/settings/admins/index.ts index a781bd8b9..f38ae3c7b 100644 --- a/src/lib/api/settings/admins/index.ts +++ b/src/lib/api/settings/admins/index.ts @@ -1,40 +1,51 @@ 'use server'; import { Admin } from '@/lib/models/User'; import { getApiClient } from '@/lib/api'; +import { withAdminApiError } from '@/lib/logic/api-error'; export const getAdminById = async (id: string) => { - const res = await (await getApiClient()).get(`/admins/${id}`); - return res.data; + return withAdminApiError(async () => { + const res = await (await getApiClient()).get(`/admins/${id}`); + return res.data; + }); }; export const getAdmins = async ( skip: number, limit: number ): Promise<{ admins: Admin[]; count: number }> => { - const res = await ( - await getApiClient() - ).get(`/admins`, { - params: { - skip, - limit, - }, + return withAdminApiError(async () => { + const res = await ( + await getApiClient() + ).get(`/admins`, { + params: { + skip, + limit, + }, + }); + return res.data; }); - return res.data; }; export const postNewAdminUser = async (username: string, password: string) => { - await (await getApiClient()).post(`/admins`, { username, password }); + await withAdminApiError(async () => { + await (await getApiClient()).post(`/admins`, { username, password }); + }); }; export const changeAdminsPasswordById = async ( adminId: string, newPassword: string ) => { - await ( - await getApiClient() - ).put(`/admins/${adminId}/change-password`, { - newPassword, + await withAdminApiError(async () => { + await ( + await getApiClient() + ).put(`/admins/${adminId}/change-password`, { + newPassword, + }); }); }; export const deleteAdmin = async (id: string) => { - await (await getApiClient()).delete(`/admins/${id}`); + await withAdminApiError(async () => { + await (await getApiClient()).delete(`/admins/${id}`); + }); }; diff --git a/src/lib/logic/api-error.test.ts b/src/lib/logic/api-error.test.ts new file mode 100644 index 000000000..354763c3e --- /dev/null +++ b/src/lib/logic/api-error.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it } from 'vitest'; +import { + formatAdminApiError, + isNextNavigationError, + withAdminApiError, +} from '@/lib/logic/api-error'; + +describe('formatAdminApiError', () => { + it('prefers the API response message from axios-like errors', () => { + expect( + formatAdminApiError({ + message: 'Request failed with status code 500', + response: { + status: 500, + data: { + message: + 'Admin validation failed: username: Path `username` is required.', + }, + }, + }) + ).toBe('Admin validation failed: username: Path `username` is required.'); + }); +}); + +describe('withAdminApiError', () => { + it('rethrows Next.js navigation errors unchanged', async () => { + const redirect = Object.assign(new Error('NEXT_REDIRECT'), { + digest: 'NEXT_REDIRECT;replace;/login;307;', + }); + await expect( + withAdminApiError(async () => { + throw redirect; + }) + ).rejects.toBe(redirect); + expect(isNextNavigationError(redirect)).toBe(true); + }); + + it('rethrows static-generation bailouts so next build can mark the route dynamic', async () => { + const dynamic = Object.assign( + new Error( + "Dynamic server usage: Route /settings/user-settings couldn't be rendered statically because it used `cookies`." + ), + { digest: 'DYNAMIC_SERVER_USAGE' } + ); + await expect( + withAdminApiError(async () => { + throw dynamic; + }) + ).rejects.toBe(dynamic); + }); + + it('rethrows a static-generation bailout nested as an error cause', async () => { + const dynamic = Object.assign(new Error('Dynamic server usage: cookies'), { + digest: 'DYNAMIC_SERVER_USAGE', + }); + const wrapped = new Error('wrapper', { cause: dynamic }); + await expect( + withAdminApiError(async () => { + throw wrapped; + }) + ).rejects.toBe(dynamic); + }); + + it('converts axios-like failures into a plain Error so Server Actions can serialize them', async () => { + const axiosLike = { + message: 'Request failed with status code 500', + response: { + status: 500, + data: { message: 'Username already exists' }, + }, + }; + await expect( + withAdminApiError(async () => { + throw axiosLike; + }) + ).rejects.toEqual( + expect.objectContaining({ message: 'Username already exists' }) + ); + await expect( + withAdminApiError(async () => { + throw axiosLike; + }) + ).rejects.toBeInstanceOf(Error); + }); +}); diff --git a/src/lib/logic/api-error.ts b/src/lib/logic/api-error.ts index c95fc1508..7ecb97809 100644 --- a/src/lib/logic/api-error.ts +++ b/src/lib/logic/api-error.ts @@ -19,15 +19,58 @@ export function isAxiosNotFoundError(err: unknown): boolean { return getAxiosResponseStatus(err) === 404; } +function errorDigest(err: unknown): string | undefined { + if (!err || typeof err !== 'object' || !('digest' in err)) return undefined; + const digest = (err as { digest?: unknown }).digest; + return typeof digest === 'string' ? digest : undefined; +} + export function isNextNavigationError(err: unknown): boolean { - if (!err || typeof err !== 'object' || !('digest' in err)) return false; - const digest = (err as { digest?: string }).digest; - if (typeof digest !== 'string') return false; + const digest = errorDigest(err); + if (!digest) return false; + return ( + digest.startsWith('NEXT_REDIRECT') || + digest.startsWith('NEXT_NOT_FOUND') || + digest.startsWith('NEXT_HTTP_ERROR_FALLBACK') + ); +} + +const DYNAMIC_RENDERING_DIGESTS = new Set([ + 'DYNAMIC_SERVER_USAGE', + 'BAILOUT_TO_CLIENT_SIDE_RENDERING', + 'HANGING_PROMISE_REJECTION', + 'NEXT_PRERENDER_INTERRUPTED', +]); + +function isDynamicRenderingError(err: unknown): boolean { + const digest = errorDigest(err); + if (digest && DYNAMIC_RENDERING_DIGESTS.has(digest)) return true; + if (!err || typeof err !== 'object' || !('message' in err)) return false; + const message = (err as { message?: unknown }).message; + if (typeof message !== 'string') return false; return ( - digest.startsWith('NEXT_REDIRECT') || digest.startsWith('NEXT_NOT_FOUND') + message.includes( + 'needs to bail out of prerendering at this point because it used' + ) && + message.includes( + 'Learn more: https://nextjs.org/docs/messages/ppr-caught-error' + ) ); } +/** + * Next.js interrupts rendering with special errors (`redirect`, `notFound`, + * `cookies()` during static generation). Callers that translate failures into + * plain `Error`s must rethrow these first, or `next build` treats the route + * as a failed prerender. + */ +export function rethrowNextControlFlowError(err: unknown): void { + if (isNextNavigationError(err) || isDynamicRenderingError(err)) throw err; + if (err instanceof Error && err.cause !== undefined) { + rethrowNextControlFlowError(err.cause); + } +} + export function formatAdminApiError(err: unknown): string { if (isAxiosLikeError(err)) { const data = err.response?.data; @@ -41,6 +84,15 @@ export function formatAdminApiError(err: unknown): string { return err instanceof Error ? err.message : 'Request failed'; } +export async function withAdminApiError(fn: () => Promise): Promise { + try { + return await fn(); + } catch (err) { + rethrowNextControlFlowError(err); + throw new Error(formatAdminApiError(err)); + } +} + export function formatCommunicationsApiError(err: unknown): string { if (isAxiosLikeError(err)) { if (err.response?.status === 404) {