diff --git a/src/features/auth/SignUp.test.tsx b/src/features/auth/SignUp.test.tsx new file mode 100644 index 000000000..263262ba9 --- /dev/null +++ b/src/features/auth/SignUp.test.tsx @@ -0,0 +1,137 @@ +/** + * @vitest-environment jsdom + */ +import { MutationCache, QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { AxiosError } from 'axios'; +import { PropsWithChildren } from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { post } = vi.hoisted(() => ({ post: vi.fn() })); +vi.mock('@/config/apiClient', () => ({ apiClient: { post } })); + +const { navigate } = vi.hoisted(() => ({ navigate: vi.fn() })); +vi.mock('@tanstack/react-router', () => ({ + useNavigate: () => navigate, + useSearch: () => ({}), + Link: ({ children, ...rest }: PropsWithChildren<{ className?: string }>) => {children}, +})); + +vi.mock('sonner', () => ({ + toast: { error: vi.fn(), dismiss: vi.fn() }, +})); + +vi.mock('@/integrations/reo/reo', () => ({ reoClient: { identify: vi.fn() } })); + +import { toast } from 'sonner'; +// The app's own mutation-error routing, not a copy of it: whether the form's failure also +// reaches the global toast is part of what's under test, so restating that rule here would let +// these tests pass even if `skipGlobalErrorToast` stopped being honored. +import { mutationErrorHandler } from '@/react-query/queryClient'; +import { SignUp } from './SignUp'; + +function axiosError(status: number, data?: unknown): AxiosError { + return { isAxiosError: true, response: { status, data } } as AxiosError; +} + +// Fresh per test, so nothing leaks between them. +let queryClient: QueryClient; + +function renderSignUp() { + return render( + + + , + ); +} + +function fillValidForm() { + fireEvent.change(screen.getByLabelText('First Name'), { target: { value: 'Ada' } }); + fireEvent.change(screen.getByLabelText('Last Name'), { target: { value: 'Lovelace' } }); + fireEvent.change(screen.getByLabelText('Email'), { target: { value: 'taken@example.com' } }); + fireEvent.change(screen.getByLabelText('Password'), { target: { value: 'correct horse battery' } }); + fireEvent.change(screen.getByLabelText('Confirm Password'), { target: { value: 'correct horse battery' } }); + // `termsCheckbox` is rendered twice (above the OAuth buttons and inside the form); either + // one drives the same field. + fireEvent.click(screen.getAllByRole('checkbox')[0]); +} + +function submit() { + fireEvent.click(screen.getByRole('button', { name: 'Sign Up For Free' })); +} + +beforeEach(() => { + queryClient = new QueryClient({ + mutationCache: new MutationCache({ onError: mutationErrorHandler }), + defaultOptions: { mutations: { retry: false } }, + }); +}); + +afterEach(() => vi.clearAllMocks()); + +describe('SignUp', () => { + it("reports the server's reason in the form rather than a toast", async () => { + post.mockRejectedValue(axiosError(500, { code: 'InternalError', title: 'Signup is unavailable' })); + + renderSignUp(); + fillValidForm(); + submit(); + + await waitFor(() => expect(screen.getByRole('alert').textContent).toContain('Signup is unavailable')); + expect(toast.error).not.toHaveBeenCalled(); + expect(navigate).not.toHaveBeenCalled(); + }); + + // Whatever central-manager rejects with has to reach the user — the form maps no status + // codes of its own, so this must hold for a shape it has never seen. + it.each([ + [400, { error: 'Password is too short' }, 'Password is too short'], + [409, 'User already exists', 'User already exists'], + // A legacy "Code: sentence" body: the toast splits the first clause into its heading, and + // the inline line has no heading — it must still read as a whole sentence. + [409, 'Conflict: user already exists', 'Conflict: user already exists'], + [503, undefined, 'We had some trouble!'], + ])('surfaces a %i rejection inline', async (status, data, expected) => { + post.mockRejectedValue(axiosError(status, data)); + + renderSignUp(); + fillValidForm(); + submit(); + + await waitFor(() => expect(screen.getByRole('alert').textContent).toContain(expected)); + }); + + it('clears the previous failure when the form is resubmitted', async () => { + post.mockRejectedValueOnce(axiosError(503)); + + renderSignUp(); + fillValidForm(); + submit(); + await waitFor(() => expect(screen.getByRole('alert')).toBeTruthy()); + + post.mockResolvedValueOnce({ data: { id: 'usr-1', email: 'taken@example.com' } }); + submit(); + + await waitFor(() => expect(navigate).toHaveBeenCalled()); + expect(screen.queryByRole('alert')).toBeNull(); + }); + + it('disables the submit button while the sign-up request is in flight', async () => { + let settle: (() => void) | undefined; + post.mockReturnValue( + new Promise((_resolve, reject) => { + settle = () => reject(axiosError(503)); + }), + ); + + renderSignUp(); + fillValidForm(); + submit(); + + const button = screen.getByRole('button', { name: 'Sign Up For Free' }); + await waitFor(() => expect(button.hasAttribute('disabled')).toBe(true)); + + settle!(); + await waitFor(() => expect(button.hasAttribute('disabled')).toBe(false)); + }); +}); diff --git a/src/features/auth/SignUp.tsx b/src/features/auth/SignUp.tsx index b6ea52e2d..fc25b0323 100644 --- a/src/features/auth/SignUp.tsx +++ b/src/features/auth/SignUp.tsx @@ -12,6 +12,7 @@ import { personNameRegex } from '@/lib/string/regex/personNameRegex'; import { clearUtmParamsFromUrl } from '@/lib/urls/clearUtmParams'; import { zodRequireEmail } from '@/lib/zod/email'; import { zodRequirePassword } from '@/lib/zod/password'; +import { describeError } from '@/react-query/queryClient'; import { zodResolver } from '@hookform/resolvers/zod'; import { Link, useNavigate, useSearch } from '@tanstack/react-router'; import { MouseEvent, useCallback, useEffect, useState } from 'react'; @@ -73,17 +74,21 @@ export function SignUp() { const email = methods.watch('email'); const acceptTerms = methods.watch('acceptTerms'); - const { setFocus, control, handleSubmit } = methods; + const { setFocus, setError, clearErrors, control, handleSubmit, formState } = methods; + const submitError = formState.errors.root?.message; useEffect(() => { setFocus('firstname'); }, [setFocus]); - const { mutate: submitSignUpData } = useSignUpMutation(); + const { mutate: submitSignUpData, isPending } = useSignUpMutation(); const submitForm = useCallback(async (formData: z.infer) => { // eslint-disable-next-line @typescript-eslint/no-unused-vars const { confirmPassword, acceptTerms, ...userData } = formData; + // Drop the previous attempt's failure explicitly — `handleSubmit` reruns the resolver, + // which only rewrites field errors, so a stale `root` would outlive the retry. + clearErrors('root'); submitSignUpData(userData, { onSuccess: () => { const company = parseCompanyFromEmail(userData.email); @@ -97,8 +102,19 @@ export function SignUp() { clearUtmParamsFromUrl(); void navigate({ to: '/verifying?email=' + encodeURIComponent(userData.email) }); }, + // The sign-up mutation opts out of the global error toast (meta.skipGlobalErrorToast) + // and renders the failure in the form instead. RUM showed people resubmitting the + // same details two and three times before giving up (#1612): a toast that fades, + // away from the inputs, doesn't read as "this attempt failed". Deliberately status- + // agnostic — it reports whatever the server said rather than mapping specific codes. + onError: (error) => { + console.error(error); + // `message`, not `description`: the latter is the toast's body, with the first clause + // of a "Conflict: …" style message moved out into the heading this has no room for. + setError('root', { type: 'server', message: describeError(error).message }); + }, }); - }, [navigate, submitSignUpData]); + }, [clearErrors, navigate, setError, submitSignUpData]); const onOAuthClick = useCallback((e: MouseEvent) => { if (!acceptTerms) { @@ -279,7 +295,13 @@ export function SignUp() { /> {termsCheckbox} - diff --git a/src/features/auth/hooks/useSignUp.ts b/src/features/auth/hooks/useSignUp.ts index cd0160b02..f12dd238e 100644 --- a/src/features/auth/hooks/useSignUp.ts +++ b/src/features/auth/hooks/useSignUp.ts @@ -22,5 +22,9 @@ export async function onSignUpSubmit(signUpCredentials: SignUpCredentials) { export function useSignUpMutation() { return useMutation({ mutationFn: (loginData) => onSignUpSubmit(loginData), + // The sign-up form renders the failure inline, beside the inputs, instead of in a toast + // that fades away from them, so suppress the default global error toast for this + // mutation (see `SignUp`'s `onError`). + meta: { skipGlobalErrorToast: true }, }); } diff --git a/src/react-query/queryClient.ts b/src/react-query/queryClient.ts index 13419e349..cdb90b19d 100644 --- a/src/react-query/queryClient.ts +++ b/src/react-query/queryClient.ts @@ -3,11 +3,19 @@ import { MutationCache, QueryCache, QueryClient } from '@tanstack/react-query'; import { AxiosError } from 'axios'; import { toast } from 'sonner'; -export function errorHandler(rawErr: unknown) { +/** + * Turn an error of unknown shape into display text, in both the shapes the UI needs. + * + * `title` + `description` are the toast's heading and body. `message` is the same text as one + * sentence, for somewhere that has no heading — a form rendering the failure inline beside its + * inputs. They differ only for a legacy `"Conflict: user already exists"` body, where the split + * below moves the first clause into `title`: a single line built from `description` alone would + * lose it. Everything comes from this one extractor so the inline text and the toast can't drift. + */ +export function describeError(rawErr: unknown): { title: string; description: string; message: string } { let errorTitle = 'Error'; let errorMsg = 'We had some trouble!'; let splitTitleFromMsg = true; - console.error(rawErr); const axiosWrappedErr = rawErr as AxiosError< string | { error?: unknown; message?: unknown; code?: unknown; title?: unknown; detail?: unknown } >; @@ -37,6 +45,9 @@ export function errorHandler(rawErr: unknown) { } else { errorMsg = errorText(otherErr?.message) ?? errorMsg; } + // Captured before the split below, which is a toast-only presentation choice: it moves the + // first clause of the text into the heading, so only the pre-split value is a whole sentence. + const message = errorMsg; // The JSON fallback from errorText produces messages full of colons that are not // "Title: detail" shaped — don't split those. if (splitTitleFromMsg && errorMsg.includes(':') && !errorMsg.startsWith('{') && !errorMsg.startsWith('[')) { @@ -44,6 +55,13 @@ export function errorHandler(rawErr: unknown) { errorTitle = split.shift()!; errorMsg = split.join(':'); } + return { title: errorTitle, description: errorMsg, message }; +} + +export function errorHandler(rawErr: unknown) { + console.error(rawErr); + const { title: errorTitle, description: errorMsg } = describeError(rawErr); + const axiosWrappedErr = rawErr as AxiosError; // Axios surfaces request timeouts as ECONNABORTED / ETIMEDOUT. Multiple // queries can timeout in parallel and stack up identical toasts; collapse // them onto a single id so the user sees one instead of a wall. @@ -63,19 +81,32 @@ export function errorHandler(rawErr: unknown) { }); } +/** + * Every mutation error routes through the shared toast by default. A mutation can opt out + * — to render its own inline UI or redirect instead — with `meta: { skipGlobalErrorToast: true }` + * (e.g. the cloud login flow redirects an unverified user to the email-verification page, and + * sign-up renders the failure in the form, above the submit button, instead of a toast that + * fades away from the inputs). + * + * Exported so a test can build a throwaway `QueryClient` that routes errors the way the app + * does without restating the opt-out rule — a copy of it in a test would keep passing after + * this changed. + */ +export const mutationErrorHandler: NonNullable = ( + error, + _variables, + _onMutateResult, + mutation, +) => { + if (mutation.meta?.skipGlobalErrorToast) { + return; + } + errorHandler(error); +}; + export const queryClient = new QueryClient({ queryCache: new QueryCache({ onError: errorHandler, }), - mutationCache: new MutationCache({ - // Every mutation error routes through the shared toast by default. A mutation can opt out - // — to render its own inline UI or redirect instead — with `meta: { skipGlobalErrorToast: true }` - // (e.g. the cloud login flow redirects an unverified user to the email-verification page). - onError: (error, _variables, _context, mutation) => { - if (mutation.meta?.skipGlobalErrorToast) { - return; - } - errorHandler(error); - }, - }), + mutationCache: new MutationCache({ onError: mutationErrorHandler }), });