diff --git a/.changeset/email-link-prepared-signal.md b/.changeset/email-link-prepared-signal.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/email-link-prepared-signal.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts b/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts index 1f178b0f450..3b9ed5e9a2a 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-contact.types.ts @@ -10,12 +10,13 @@ export type UserProfileEmail = UserProfileContact; export type UserProfilePhone = UserProfileContact; export type UserProfileEmailVerification = - | { method: 'code'; sent: Promise } + | { method: 'code' } | { method: 'link'; verified: Promise; cancel: () => void } | { method: 'sso'; connect: () => Promise }; export interface UserProfileEmailVerifier { - start: () => UserProfileEmailVerification; + prepare: () => Promise; + cancel: () => void; verifyCode: (code: string) => Promise; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.controller.test.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.controller.test.ts index f17f65de695..aa657c5e5f2 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.controller.test.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.controller.test.ts @@ -9,7 +9,7 @@ import { useUserProfileAddEmailController } from '../user-profile-add-email.cont const idleReverification = { status: 'idle', visible: false } satisfies ReverificationController; -const sentCode = (): UserProfileEmailVerification => ({ method: 'code', sent: Promise.resolve() }); +const sentCode = (): UserProfileEmailVerification => ({ method: 'code' }); function sentLink(verified: Promise, cancel = vi.fn()) { return (): UserProfileEmailVerification => ({ method: 'link', verified, cancel }); @@ -19,7 +19,11 @@ function verifier( start: () => UserProfileEmailVerification = sentCode, verifyCode: (code: string) => Promise = () => Promise.resolve(), ) { - return { start: vi.fn(start), verifyCode: vi.fn(verifyCode) } satisfies UserProfileEmailVerifier; + return { + prepare: vi.fn(async () => start()), + cancel: vi.fn(), + verifyCode: vi.fn(verifyCode), + } satisfies UserProfileEmailVerifier; } const created = () => Promise.resolve(verifier()); @@ -125,7 +129,7 @@ describe('useUserProfileAddEmailController', () => { }); expect(result.current.resendSeconds).toBe(30); act(() => result.current.onResend()); - expect(email.start).toHaveBeenCalledTimes(1); + expect(email.prepare).toHaveBeenCalledTimes(1); for (let second = 0; second < 30; second++) { await act(async () => vi.advanceTimersByTimeAsync(1000)); } @@ -138,7 +142,7 @@ describe('useUserProfileAddEmailController', () => { result.current.onOpenChange(false); await Promise.resolve(); }); - expect(email.start).toHaveBeenCalledTimes(2); + expect(email.prepare).toHaveBeenCalledTimes(2); expect(email.verifyCode).not.toHaveBeenCalled(); expect(result.current.open).toBe(true); expect(result.current.code).toBe(''); @@ -185,7 +189,7 @@ describe('useUserProfileAddEmailController', () => { expect(result.current.open).toBe(true); await waitFor(() => expect(result.current.step).toBe('verify')); expect(onCreate).toHaveBeenCalledExactlyOnceWith('new@example.com'); - await waitFor(() => expect(email.start).toHaveBeenCalledOnce()); + await waitFor(() => expect(email.prepare).toHaveBeenCalledOnce()); act(() => result.current.onSubmit('123456')); await waitFor(() => expect(result.current.open).toBe(false)); @@ -199,11 +203,11 @@ describe('useUserProfileAddEmailController', () => { useUserProfileAddEmailController({ reverification: idleReverification, onCreate }), ); - act(() => result.current.onVerifyEmail(other, email)); + await act(() => result.current.onVerifyEmail(other, email)); expect(result.current.open).toBe(true); expect(result.current.step).toBe('verify'); expect(result.current.emailAddress).toBe('other@example.com'); - await waitFor(() => expect(email.start).toHaveBeenCalledOnce()); + await waitFor(() => expect(email.prepare).toHaveBeenCalledOnce()); act(() => result.current.onSubmit('123456')); await waitFor(() => expect(result.current.open).toBe(false)); @@ -218,11 +222,11 @@ describe('useUserProfileAddEmailController', () => { ); expect(result.current.verifyingId).toBeUndefined(); - act(() => result.current.onVerifyEmail(other, email)); + await act(() => result.current.onVerifyEmail(other, email)); expect(result.current.verifyingId).toBe('email_2'); await waitFor(() => expect(result.current.step).toBe('verify')); - act(() => result.current.onVerifyEmail({ id: 'email_3', value: 'person@acme.co' }, email)); + await act(() => result.current.onVerifyEmail({ id: 'email_3', value: 'person@acme.co' }, email)); expect(result.current.verifyingId).toBe('email_2'); expect(result.current.emailAddress).toBe('other@example.com'); @@ -234,23 +238,62 @@ describe('useUserProfileAddEmailController', () => { expect(result.current.verifyingId).toBeUndefined(); }); - it('stays on the code step when sending fails, and lets the user resend at once', async () => { + it('creates a different email when the value changes after failed preparation', async () => { const email = verifier(); - email.start.mockImplementationOnce(() => ({ - method: 'code', - sent: Promise.reject(saveError('Too many requests')), - })); + email.prepare.mockRejectedValueOnce(saveError('Try again')); + const onCreate = vi.fn(async () => email); const { result } = renderHook(() => - useUserProfileAddEmailController({ reverification: idleReverification, onCreate: created }), + useUserProfileAddEmailController({ reverification: idleReverification, onCreate }), ); + act(() => result.current.onOpenChange(true)); + act(() => result.current.onEmailAddressChange('first@example.com')); + await act(() => result.current.onSubmit()); + await waitFor(() => expect(result.current.errorMessage).toBe('Try again')); + act(() => result.current.onEmailAddressChange('second@example.com')); + await act(() => result.current.onSubmit()); + await waitFor(() => expect(result.current.step).toBe('verify')); + expect(onCreate.mock.calls).toEqual([['first@example.com'], ['second@example.com']]); + }); - act(() => result.current.onVerifyEmail(other, email)); - await waitFor(() => expect(result.current.errorMessage).toBe('Too many requests')); + it('keeps the code input and zero cooldown after failed resend', async () => { + vi.useFakeTimers(); + const email = verifier(); + const { result } = renderHook(() => useUserProfileAddEmailController({ reverification: idleReverification })); + await act(() => result.current.onVerifyEmail(other, email)); + for (let second = 0; second < 30; second++) { + await act(async () => vi.advanceTimersByTimeAsync(1000)); + } + email.prepare.mockRejectedValueOnce(saveError('Send failed')); + await act(() => result.current.onResend()); expect(result.current.step).toBe('verify'); + expect(result.current.errorMessage).toBe('Send failed'); expect(result.current.resendSeconds).toBe(0); + }); - act(() => result.current.onResend()); - await waitFor(() => expect(email.start).toHaveBeenCalledTimes(2)); + it('preserves a failed resend error after the previous link expired', async () => { + vi.useFakeTimers(); + const expired = createDeferredPromise(); + const email = verifier(sentLink(expired.promise.then(() => undefined))); + const { result } = renderHook(() => useUserProfileAddEmailController({ reverification: idleReverification })); + await act(() => result.current.onVerifyEmail(other, email)); + await act(async () => { + expired.reject(saveError('Link expired')); + await Promise.resolve(); + }); + expect(result.current.errorMessage).toBe('Link expired'); + for (let second = 0; second < 60; second++) { + await act(async () => vi.advanceTimersByTimeAsync(1000)); + } + email.prepare.mockRejectedValueOnce(saveError('Too many requests')); + await act(() => result.current.onResend()); + expect(result.current.step).toBe('link'); + expect(result.current.errorMessage).toBe('Too many requests'); + expect(result.current.resendSeconds).toBe(0); + email.prepare.mockResolvedValueOnce(sentLink(new Promise(() => {}))()); + await act(() => result.current.onResend()); + expect(result.current.step).toBe('link'); + expect(result.current.resendSeconds).toBe(60); + expect(result.current.errorMessage).toBeUndefined(); }); it('shows the error for the field that failed', async () => { @@ -270,7 +313,7 @@ describe('useUserProfileAddEmailController', () => { const verified = createDeferredPromise(); const { result } = renderHook(() => useUserProfileAddEmailController({ reverification: idleReverification })); - act(() => result.current.onVerifyEmail(other, verifier(sentLink(verified.promise.then(() => undefined))))); + await act(() => result.current.onVerifyEmail(other, verifier(sentLink(verified.promise.then(() => undefined))))); expect(result.current.step).toBe('link'); expect(result.current.resendSeconds).toBe(60); @@ -288,12 +331,12 @@ describe('useUserProfileAddEmailController', () => { useUserProfileAddEmailController({ reverification: idleReverification }), ); - act(() => result.current.onVerifyEmail(other, email)); + await act(() => result.current.onVerifyEmail(other, email)); await waitFor(() => expect(result.current.step).toBe('link')); act(() => result.current.onOpenChange(false)); expect(cancel).toHaveBeenCalledOnce(); - act(() => result.current.onVerifyEmail(other, email)); + await act(() => result.current.onVerifyEmail(other, email)); await waitFor(() => expect(result.current.step).toBe('link')); unmount(); expect(cancel).toHaveBeenCalledTimes(2); @@ -303,7 +346,7 @@ describe('useUserProfileAddEmailController', () => { const email = verifier(sentLink(Promise.reject(saveError('Link expired')))); const { result } = renderHook(() => useUserProfileAddEmailController({ reverification: idleReverification })); - act(() => result.current.onVerifyEmail(other, email)); + await act(() => result.current.onVerifyEmail(other, email)); await waitFor(() => expect(result.current.errorMessage).toBe('Link expired')); expect(result.current.step).toBe('link'); expect(result.current.open).toBe(true); @@ -314,7 +357,7 @@ describe('useUserProfileAddEmailController', () => { const email = verifier(() => ({ method: 'sso', connect })); const { result } = renderHook(() => useUserProfileAddEmailController({ reverification: idleReverification })); - act(() => result.current.onVerifyEmail({ id: 'email_3', value: 'person@acme.co' }, email)); + await act(() => result.current.onVerifyEmail({ id: 'email_3', value: 'person@acme.co' }, email)); expect(result.current.step).toBe('sso'); expect(result.current.resendSeconds).toBe(0); diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.dialog.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.dialog.test.tsx index edf4f8d4a05..d4c39e6c991 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.dialog.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-add-email.dialog.test.tsx @@ -213,6 +213,17 @@ describe('UserProfileAddEmailDialog', () => { await user.click(screen.getByRole('button', { name: 'Didn’t receive a link? Resend' })); expect(props.onResend).toHaveBeenCalledOnce(); + rerender( + + + , + ); + expect(screen.getByRole('button', { name: 'Sending link…' })).toBeDisabled(); + await user.click(screen.getByRole('button', { name: 'Cancel' })); expect(props.onOpenChange).toHaveBeenCalledWith(false, expect.anything()); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-actions.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-actions.test.tsx index 610b4c8b0d4..15b138fe34b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-actions.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-actions.test.tsx @@ -13,7 +13,8 @@ import type { ReadyEmailSectionModel } from '../user-profile-email-section.types import { UserProfileEmailSectionView } from '../user-profile-email-section.view'; const codeVerifier = { - start: () => ({ method: 'code', sent: Promise.resolve() }) as const, + prepare: async () => ({ method: 'code' }) as const, + cancel: () => {}, verifyCode: () => Promise.resolve(), }; diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx index 5086ae72501..57d6fda68f5 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/__tests__/user-profile-email-section.feature.test.tsx @@ -82,22 +82,73 @@ describe('the user profile email section', () => { expect(emailRow()).not.toHaveTextContent('Unverified'); }); - it('holds the code field until the code has been sent', async () => { + it('keeps Continue pending until the code has been sent', async () => { const { actor } = await renderSection(signedInWithEmails([])); const prepare = holdRequests('post', '/v1/me/email_addresses/:id/prepare_verification'); + try { + await actor.click(screen.getByRole('button', { name: 'Add email' })); + await actor.type(screen.getByRole('textbox', { name: 'Email' }), 'new@example.com'); + await actor.click(screen.getByRole('button', { name: 'Continue' })); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + + expect(screen.getByRole('button', { name: 'Continue' })).toHaveAttribute('aria-busy', 'true'); + expect(screen.getByRole('textbox', { name: 'Email' })).toBeVisible(); + expect(screen.queryByRole('textbox', { name: 'Verification code' })).not.toBeInTheDocument(); + + prepare.release(); + await enterCode(actor, VERIFICATION_CODE); + + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + expect(emailsListed()).toEqual(['new@example.com']); + } finally { + prepare.release(); + } + }); - await actor.click(screen.getByRole('button', { name: 'Add email' })); - await actor.type(screen.getByRole('textbox', { name: 'Email' }), 'new@example.com'); - await actor.click(screen.getByRole('button', { name: 'Continue' })); - await waitFor(() => expect(prepare.requests).toHaveLength(1)); - - expect(await screen.findByRole('textbox', { name: 'Verification code' })).toHaveAttribute('aria-disabled', 'true'); - - prepare.release(); - await enterCode(actor, VERIFICATION_CODE); + it('keeps existing email Verify pending until code preparation succeeds', async () => { + const pending = fapiEmailAddress({ id: 'idn_pending', email_address: 'pending@example.com' }); + const { actor } = await renderSection(signedInWithEmails([PRIMARY, pending])); + const prepare = holdRequests('post', '/v1/me/email_addresses/:id/prepare_verification'); + try { + await manageEmail(actor, 'pending@example.com', 'Verify'); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + expect(screen.getByRole('button', { name: 'Add email' })).toBeDisabled(); + expect(within(emailRow()).getByText('pending@example.com')).toBeVisible(); + expect(screen.queryByRole('dialog')).toBeNull(); + prepare.release(); + await screen.findByRole('dialog'); + await waitFor(() => expect(screen.getByRole('textbox', { name: 'Verification code' })).toHaveFocus()); + } finally { + prepare.release(); + } + }); - await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); - expect(emailsListed()).toEqual(['new@example.com']); + it('retries failed code preparation from the entry form without another email', async () => { + const { actor, fapi } = await renderSection(signedInWithEmails([])); + const prepare = holdRequests('post', '/v1/me/email_addresses/:id/prepare_verification'); + try { + await actor.click(screen.getByRole('button', { name: 'Add email' })); + await actor.type(screen.getByRole('textbox', { name: 'Email' }), 'new@example.com'); + await actor.click(screen.getByRole('button', { name: 'Continue' })); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + prepare.fail('action_blocked'); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Continue' })).not.toHaveAttribute('aria-busy', 'true'), + ); + expect(screen.getByRole('textbox', { name: 'Email' })).toHaveValue('new@example.com'); + const retry = holdRequests('post', '/v1/me/email_addresses/:id/prepare_verification'); + try { + await actor.click(screen.getByRole('button', { name: 'Continue' })); + await waitFor(() => expect(retry.requests).toHaveLength(1)); + expect(fapi.client.sessions[0]?.user.email_addresses).toHaveLength(1); + retry.release(); + expect(await screen.findByRole('textbox', { name: 'Verification code' })).toBeVisible(); + } finally { + retry.release(); + } + } finally { + prepare.release(); + } }); it('verifies an address left unverified', async () => { @@ -325,7 +376,7 @@ describe('email reverification', () => { ); }); - it('waits for a verification factor before opening the dialog', async () => { + it('keeps the profile visible until primary-action verification is ready', async () => { const second = fapiEmailAddress({ id: 'idn_second', email_address: 'bob@example.com', verification: VERIFIED }); const { actor } = await renderSection({ ...signedInWithEmails([PRIMARY, second]), @@ -334,12 +385,61 @@ describe('email reverification', () => { demandReverificationOnce('/v1/me'); const start = holdRequests('post', '/v1/client/sessions/:id/verify'); - await manageEmail(actor, 'bob@example.com', 'Set as primary'); - await waitFor(() => expect(start.requests).toHaveLength(1)); - expect(screen.queryByRole('dialog')).toBeNull(); + try { + await manageEmail(actor, 'bob@example.com', 'Set as primary'); + await waitFor(() => expect(start.requests).toHaveLength(1)); + expect(within(emailRow()).getByText('bob@example.com')).toBeVisible(); + expect(screen.queryByRole('dialog')).toBeNull(); + start.release(); + await waitFor(() => expect(screen.getByLabelText('Password')).toHaveFocus()); + } finally { + start.release(); + } + }); - start.release(); - expect(await screen.findByText('Enter your current password to continue')).toBeVisible(); + it('shows the section error when the primary action fails after closing the menu', async () => { + const second = fapiEmailAddress({ id: 'idn_second', email_address: 'bob@example.com', verification: VERIFIED }); + const { actor } = await renderSection(signedInWithEmails([PRIMARY, second])); + const mutation = holdRequests('post', '/v1/me'); + const trigger = within(emailRow()).getByRole('button', { name: 'Manage bob@example.com' }); + + try { + await manageEmail(actor, 'bob@example.com', 'Set as primary'); + await waitFor(() => expect(mutation.requests).toHaveLength(1)); + mutation.fail('action_blocked', 'Unable to set primary address.'); + await waitFor(() => + expect(within(emailRow()).getByRole('alert')).toHaveTextContent("This action couldn't be completed."), + ); + await openMenu(actor, trigger); + expect(screen.getByRole('menuitem', { name: 'Set as primary' })).not.toHaveAttribute('aria-disabled'); + } finally { + mutation.release(); + } + }); + + it('replaces a failed verify error with a later primary action error', async () => { + const pending = fapiEmailAddress({ id: 'idn_pending', email_address: 'pending@example.com' }); + const second = fapiEmailAddress({ id: 'idn_second', email_address: 'bob@example.com', verification: VERIFIED }); + const { actor } = await renderSection(signedInWithEmails([PRIMARY, pending, second])); + const prepare = holdRequests('post', '/v1/me/email_addresses/:id/prepare_verification'); + const mutation = holdRequests('post', '/v1/me'); + + try { + await manageEmail(actor, 'pending@example.com', 'Verify'); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + prepare.fail('too_many_requests', 'Too many requests.'); + await waitFor(() => expect(within(emailRow()).getByRole('alert')).toHaveTextContent('Too many requests.')); + + await manageEmail(actor, 'bob@example.com', 'Set as primary'); + await waitFor(() => expect(mutation.requests).toHaveLength(1)); + mutation.fail('action_blocked', 'Unable to set primary address.'); + await waitFor(() => + expect(within(emailRow()).getByRole('alert')).toHaveTextContent("This action couldn't be completed."), + ); + } finally { + prepare.release(); + mutation.release(); + } }); it('focuses Close when the dialog opens on a verification that could not start', async () => { @@ -370,6 +470,53 @@ describe('email reverification', () => { await actor.click(screen.getByRole('button', { name: 'Continue' })); } + it('holds password confirmation until email code preparation finishes', async () => { + const { actor } = await renderSection(challengedSeed()); + demandReverificationOnce('/v1/me/email_addresses'); + const prepare = holdRequests('post', '/v1/me/email_addresses/:id/prepare_verification'); + + try { + await beginAdd(actor); + await actor.type(await screen.findByPlaceholderText('Enter your password'), 'hunter2{Enter}'); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + + expect(screen.getByLabelText('Password')).toBeVisible(); + expect(screen.getByRole('button', { name: 'Continue' })).toHaveAttribute('aria-busy', 'true'); + expect(screen.queryByRole('textbox', { name: 'Email' })).toBeNull(); + expect(screen.queryByRole('button', { name: 'Close' })).toBeNull(); + await actor.keyboard('{Escape}{Enter}'); + expect(screen.getByRole('dialog')).toBeVisible(); + expect(prepare.requests).toHaveLength(1); + + prepare.release(); + const input = await screen.findByRole('textbox', { name: 'Verification code' }); + await waitFor(() => expect(input).not.toHaveAttribute('aria-disabled')); + expect(input).toBeVisible(); + } finally { + prepare.release(); + } + }); + + it('returns to the email form after preparation fails', async () => { + const { actor } = await renderSection(challengedSeed()); + demandReverificationOnce('/v1/me/email_addresses'); + const prepare = holdRequests('post', '/v1/me/email_addresses/:id/prepare_verification'); + + try { + await beginAdd(actor); + await actor.type(await screen.findByPlaceholderText('Enter your password'), 'hunter2{Enter}'); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + expect(screen.getByLabelText('Password')).toBeVisible(); + prepare.fail('form_param_invalid', 'Could not send code.', 'email_address'); + + const input = await screen.findByRole('textbox', { name: 'Email' }); + await waitFor(() => expect(input).toHaveFocus()); + await waitFor(() => expect(input).toHaveAccessibleDescription('Could not send code.')); + } finally { + prepare.release(); + } + }); + it('confirms it is the user before an email address is added, then verifies its code', async () => { const { actor, fapi } = await renderSection(challengedSeed()); demandReverificationOnce('/v1/me/email_addresses'); diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts index 5b48368eb39..fdf483fdaf9 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts @@ -1,11 +1,11 @@ -import { useEffect, useMemo } from 'react'; +import { useEffect, useMemo, useRef } from 'react'; import type { DialogHandle } from '../../../components/dialog'; import { Dialog } from '../../../components/dialog'; import { useForm } from '../../../components/form'; import { useErrorText } from '../../../localization'; import { setup } from '../../../machine/setup'; -import type { ErrorInvokeEvent } from '../../../machine/types'; +import type { DoneInvokeEvent, ErrorInvokeEvent } from '../../../machine/types'; import { useMachine } from '../../../machine/use-machine'; import type { FormError } from '../../../utils/errors'; import { toFormError } from '../../../utils/errors'; @@ -29,7 +29,7 @@ export interface UserProfileAddEmailControllerOptions { export interface UserProfileAddEmailController extends UserProfileAddEmailDialogProps { handle: DialogHandle; verifyingId: string | undefined; - onVerifyEmail: (email: Pick, verifier: UserProfileEmailVerifier) => void; + onVerifyEmail: (email: Pick, verifier: UserProfileEmailVerifier) => Promise; } interface Context { @@ -43,7 +43,13 @@ interface Context { type Event = | { type: 'ADD' } - | { type: 'START'; id?: string; emailAddress: string; verifier: UserProfileEmailVerifier } + | { + type: 'READY'; + id?: string; + emailAddress: string; + verifier: UserProfileEmailVerifier; + verification: UserProfileEmailVerification; + } | { type: 'VERIFIED' } | { type: 'CANCEL' } | { type: 'CONNECT' } @@ -62,19 +68,24 @@ const LINK_RESEND_SECONDS = 60; const tick = { actions: assign(context => ({ resendSeconds: Math.max(0, context.resendSeconds - 1) })) }; const fail = assign((_, event) => ({ error: toFormError(event.error) })); const resend = { - target: 'starting', + target: 'resending', guard: (context: Context) => context.resendSeconds === 0, actions: assign(() => ({ error: undefined })), }; -const start = { - target: 'starting', - actions: assign>((_, event) => ({ +const ready = { + target: 'ready', + actions: assign>((_, event) => ({ verifyingId: event.id, emailAddress: event.emailAddress, verifier: event.verifier, - verification: undefined, + verification: event.verification, error: undefined, - resendSeconds: 0, + resendSeconds: + event.verification.method === 'link' + ? LINK_RESEND_SECONDS + : event.verification.method === 'code' + ? CODE_RESEND_SECONDS + : 0, })), }; @@ -102,36 +113,37 @@ const machine = createMachine({ resendSeconds: 0, })), }, - START: start, + READY: ready, }, }, email: { - on: { CANCEL: 'idle', START: start }, + on: { CANCEL: 'idle', READY: ready }, }, - starting: { - entry: assign(context => ({ - verification: context.verifier ? context.verifier.start() : undefined, - error: undefined, - })), - always: ({ context }) => { - const method = context.verification?.method; - if (method === 'code') { - return { target: 'sending' }; - } - if (method === 'sso') { - return { target: 'connecting' }; - } - return { target: 'waiting', context: { resendSeconds: method === 'link' ? LINK_RESEND_SECONDS : 0 } }; - }, + ready: { + always: ({ context }) => ({ + target: + context.verification?.method === 'code' + ? 'verify' + : context.verification?.method === 'sso' + ? 'connecting' + : 'waiting', + }), }, - sending: { - invoke: fromPromise( - context => (context.verification?.method === 'code' ? context.verification.sent : missingDependency()), - { - onDone: { target: 'verify', actions: assign(() => ({ resendSeconds: CODE_RESEND_SECONDS })) }, - onError: { target: 'verify', actions: fail }, + resending: { + on: { CANCEL: { target: 'idle', guard: context => context.verification?.method === 'link' } }, + invoke: fromPromise(context => (context.verifier ? context.verifier.prepare() : missingDependency()), { + onDone: { + target: 'ready', + actions: assign>((_, event) => ({ + verification: event.output, + resendSeconds: event.output.method === 'link' ? LINK_RESEND_SECONDS : CODE_RESEND_SECONDS, + })), }, - ), + onError: [ + { target: 'linkFailed', guard: context => context.verification?.method === 'link', actions: fail }, + { target: 'verify', actions: fail }, + ], + }), }, waiting: { on: { CANCEL: 'idle', TICK: tick, RESEND: resend }, @@ -139,10 +151,13 @@ const machine = createMachine({ context => (context.verification?.method === 'link' ? context.verification.verified : missingDependency()), { onDone: 'idle', - onError: { actions: fail }, + onError: { target: 'linkFailed', actions: fail }, }, ), }, + linkFailed: { + on: { CANCEL: 'idle', TICK: tick, RESEND: resend }, + }, connecting: { on: { CANCEL: 'idle', CONNECT: 'redirecting' }, }, @@ -172,13 +187,33 @@ export function useUserProfileAddEmailController({ const errorText = useErrorText(); const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send, actor] = useMachine(machine); + const activeVerifier = useRef(undefined); + const created = useRef<{ value: string; verifier: UserProfileEmailVerifier }>(undefined); + const generation = useRef(0); + useEffect(() => { + return () => { + activeVerifier.current?.cancel(); + }; + }, []); const { resendSeconds, error, verification } = snapshot.context; const emailForm = useForm({ initialValues: { emailAddress: initialEmailAddress }, canSubmit: values => values.emailAddress !== '' && values.emailAddress !== username, onSubmit: async ({ emailAddress }) => { if (onCreate) { - send({ type: 'START', emailAddress, verifier: await onCreate(emailAddress) }); + if (created.current?.value !== emailAddress) { + const verifier = await onCreate(emailAddress); + if (actor.getSnapshot().status !== 'active') { + return; + } + created.current = { value: emailAddress, verifier }; + } + const { verifier } = created.current; + activeVerifier.current = verifier; + const verification = await verifier.prepare(); + if (actor.getSnapshot().status === 'active') { + send({ type: 'READY', emailAddress, verifier, verification }); + } } }, }); @@ -214,7 +249,7 @@ export function useUserProfileAddEmailController({ return { handle, resendSeconds, - isResending: state === 'sending', + isResending: state === 'resending', open, step, reverification, @@ -231,11 +266,17 @@ export function useUserProfileAddEmailController({ } reverification.onCancel(); } + if (actor.getSnapshot().value === 'resending' && verification?.method !== 'link') { + return; + } + generation.current++; if (open) { + created.current = undefined; emailForm.reset(); codeForm.reset(); send({ type: 'ADD' }); } else { + activeVerifier.current?.cancel(); send({ type: 'CANCEL' }); } }, @@ -260,9 +301,23 @@ export function useUserProfileAddEmailController({ }, onConnect: () => send({ type: 'CONNECT' }), verifyingId: snapshot.context.verifyingId, - onVerifyEmail: (email, verifier) => { - codeForm.reset(); - send({ type: 'START', id: email.id, emailAddress: email.value, verifier }); + onVerifyEmail: async (email, verifier) => { + if (actor.getSnapshot().value !== 'idle') { + return; + } + const started = generation.current; + activeVerifier.current = verifier; + const verification = await verifier.prepare(); + if ( + actor.getSnapshot().status === 'active' && + generation.current === started && + actor.getSnapshot().value === 'idle' + ) { + codeForm.reset(); + send({ type: 'READY', id: email.id, emailAddress: email.value, verifier, verification }); + } else { + verifier.cancel(); + } }, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.dialog.tsx b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.dialog.tsx index a54f6f89993..3dd1234367b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.dialog.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.dialog.tsx @@ -1,7 +1,7 @@ import { useMergeRefs } from '@floating-ui/react'; import * as stylex from '@stylexjs/stylex'; import type { FormEvent, Ref } from 'react'; -import { useId, useRef } from 'react'; +import { useId, useRef, useState } from 'react'; import { Button, SubmitButton } from '../../../components/button'; import { Card } from '../../../components/card'; @@ -52,18 +52,24 @@ export interface UserProfileAddEmailDialogProps { export function UserProfileAddEmailDialog(props: UserProfileAddEmailDialogProps) { const emailRef = useRef(null); const codeRef = useRef(null); + const [lastShownStep, setLastShownStep] = useState(props.step); + const step = + props.open && props.isPending && props.step === 'email' && lastShownStep === 'reverify' ? 'reverify' : props.step; + if (step !== lastShownStep) { + setLastShownStep(step); + } return ( {current => ( @@ -97,6 +103,7 @@ export function UserProfileAddEmailDialog(props: UserProfileAddEmailDialogProps) @@ -296,6 +303,7 @@ function VerifyEmailStep(props: VerifyEmailStepProps) { } interface VerifyLinkStepProps { + isResending?: boolean; emailAddress: string; onResend: () => void; errorMessage?: string; @@ -322,6 +330,7 @@ function VerifyLinkStep(props: VerifyLinkStepProps) { ) : null} diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.messages.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.messages.ts index ac0bcf3bd32..b4171e14b71 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.messages.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.messages.ts @@ -23,6 +23,7 @@ export const userProfileAddEmailMessages = { description: 'Open the link we sent to {emailAddress}', cancel: 'Cancel', resend: 'Didn’t receive a link? Resend', + resending: 'Sending link…', resendCountdown: 'Didn’t receive a link? Resend ({seconds})', }, sso: { diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts index 822bdeca9b9..1358c1a2a8a 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.controller.ts @@ -24,6 +24,7 @@ export function useUserProfileEmailSectionController({ reverification: createReverification, }); const setPrimary = usePendingAction(); + const verify = usePendingAction(); const rowRef = useRef(null); const removalFocus = useListRemovalFocus({ ids: emails.map(email => email.id), @@ -39,7 +40,8 @@ export function useUserProfileEmailSectionController({ emails, canAdd: Boolean(onCreateEmail), verification, - error: setPrimary.error, + error: verify.error ?? setPrimary.error, + pendingVerifyId: verify.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, remove: onRemoveEmail ? remove : undefined, @@ -50,9 +52,13 @@ export function useUserProfileEmailSectionController({ onVerify: id => { const email = emails.find(email => email.id === id); if (email) { - verification.onVerifyEmail(email, getEmailVerifier(id)); + setPrimary.reset(); + void verify.run(id, () => verification.onVerifyEmail(email, getEmailVerifier(id))); } }, - onSetPrimary: id => void setPrimary.run(id, () => onSetPrimaryEmail(id)), + onSetPrimary: id => { + verify.reset(); + void setPrimary.run(id, () => onSetPrimaryEmail(id)); + }, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.model.ts index d72a5ef20b6..c50b9bd4ad2 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.model.ts @@ -28,12 +28,11 @@ function verifyRedirectUrl(userProfileUrl: string): string { return buildURL({ base: userProfileUrl, hashPath: '/verify' }, { stringify: true }); } -function startEmailVerification( - email: EmailAddressResource, - linkRedirectUrl: string | undefined, +async function prepareEmailVerification( + currentEmail: () => EmailAddressResource, router: MosaicRouter, -): UserProfileEmailVerification { - if (email.matchesSsoConnection) { +): Promise { + if (currentEmail().matchesSsoConnection) { return { method: 'sso', /* @@ -48,7 +47,7 @@ function startEmailVerification( */ connect: () => save(async () => { - const prepared = await email.prepareVerification({ + const prepared = await currentEmail().prepareVerification({ strategy: 'enterprise_sso', redirectUrl: window.location.href, }); @@ -60,25 +59,28 @@ function startEmailVerification( }), }; } - if (linkRedirectUrl === undefined) { - return { method: 'code', sent: save(() => email.prepareVerification({ strategy: 'email_code' })) }; - } - const { startEmailLinkFlow, cancelEmailLinkFlow } = email.createEmailLinkFlow(); - return { - method: 'link', - verified: save(() => startEmailLinkFlow({ redirectUrl: linkRedirectUrl })), - cancel: cancelEmailLinkFlow, - }; + await save(() => currentEmail().prepareVerification({ strategy: 'email_code' })); + currentEmail(); + return { method: 'code' }; } function toEmailVerifier( - email: EmailAddressResource, + currentEmail: () => EmailAddressResource, linkRedirectUrl: string | undefined, router: MosaicRouter, ): UserProfileEmailVerifier { + const linkFlow = linkRedirectUrl === undefined ? undefined : currentEmail().createEmailLinkFlow(); return { - start: () => startEmailVerification(email, linkRedirectUrl, router), - verifyCode: code => save(() => email.attemptVerification({ code }), ADD_EMAIL_FIELDS), + prepare: async () => { + if (linkFlow && linkRedirectUrl && !currentEmail().matchesSsoConnection) { + linkFlow.cancelEmailLinkFlow(); + const verified = save(() => linkFlow.startEmailLinkFlow({ redirectUrl: linkRedirectUrl })); + return { method: 'link' as const, verified, cancel: linkFlow.cancelEmailLinkFlow }; + } + return prepareEmailVerification(currentEmail, router); + }, + cancel: () => linkFlow?.cancelEmailLinkFlow(), + verifyCode: code => save(() => currentEmail().attemptVerification({ code }), ADD_EMAIL_FIELDS), }; } @@ -113,7 +115,8 @@ export function useUserProfileEmailSectionModel(): UserProfileEmailSectionModel const verifiesByLink = Boolean(attributes.email_address?.verifications.includes('email_link')); const linkRedirectUrl = verifiesByLink ? verifyRedirectUrl(environment.displayConfig.userProfileUrl) : undefined; - const verifierFor = (email: EmailAddressResource) => toEmailVerifier(email, linkRedirectUrl, router); + const verifierFor = (email: EmailAddressResource) => + toEmailVerifier(() => byId(currentUser().emailAddresses, email.id, 'email address'), linkRedirectUrl, router); return { status: 'ready', diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts index 1b4985b0644..eafa33577aa 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.types.ts @@ -18,6 +18,7 @@ export interface UserProfileEmailSectionViewProps { verificationFinalFocus: DialogFocusTarget; setPrimaryReverification: ReverificationController; setPrimaryFinalFocus: DialogFocusTarget; + pendingVerifyId?: string; onVerify: (id: string) => void; onSetPrimary: (id: string) => void; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx index 11c03774c45..e25f7de94c9 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-email-section.view.tsx @@ -22,6 +22,7 @@ export function UserProfileEmailSectionView({ verificationFinalFocus, setPrimaryReverification, setPrimaryFinalFocus, + pendingVerifyId, onVerify, onSetPrimary, }: UserProfileEmailSectionViewProps) { @@ -32,6 +33,7 @@ export function UserProfileEmailSectionView({ render={ + ) : null} diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.controller.test.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.controller.test.ts index 3340d462079..672e1ac134f 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.controller.test.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.controller.test.ts @@ -43,12 +43,12 @@ describe('useUserProfileAddPhoneController', () => { const verifier = { sendCode, verifyCode: () => Promise.resolve() }; expect(result.current.verifyingId).toBeUndefined(); - act(() => result.current.onVerifyPhone({ id: 'phone_2', value: '+18015550101' }, verifier)); + await act(() => result.current.onVerifyPhone({ id: 'phone_2', value: '+18015550101' }, verifier)); expect(result.current.verifyingId).toBe('phone_2'); await act(async () => vi.advanceTimersByTimeAsync(0)); expect(result.current.step).toBe('verify'); - act(() => result.current.onVerifyPhone({ id: 'phone_3', value: '+18015550102' }, verifier)); + await act(() => result.current.onVerifyPhone({ id: 'phone_3', value: '+18015550102' }, verifier)); expect(result.current.verifyingId).toBe('phone_2'); expect(result.current.phoneNumber).toBe('+18015550101'); diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.dialog.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.dialog.test.tsx index 9cc88879a47..9a28ea458c6 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.dialog.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-add-phone.dialog.test.tsx @@ -55,8 +55,7 @@ describe('UserProfileAddPhoneDialog', () => { const user = userEvent.setup(); const onSubmit = vi.fn(); render(); - await waitFor(() => expect(screen.getByRole('button', { name: 'Close', exact: true })).toHaveFocus()); - await user.click(screen.getByRole('textbox', { name: 'Verification code' })); + await waitFor(() => expect(screen.getByRole('textbox', { name: 'Verification code' })).toHaveFocus()); if (method === 'typing') { await user.keyboard('12345'); diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx index 1fd442dabb0..1d3189a20e2 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/__tests__/user-profile-phone-section.feature.test.tsx @@ -262,6 +262,50 @@ describe('phone reverification', () => { }); const savedPhones = (fapi: ReturnType) => fapi.client.sessions[0]?.user.phone_numbers; + it('holds password confirmation until phone code preparation finishes', async () => { + const { actor } = await renderSection(challenged()); + demandReverificationOnce('/v1/me/phone_numbers'); + const prepare = holdRequests('post', '/v1/me/phone_numbers/:id/prepare_verification'); + + try { + await addPhone(actor, '8015550100'); + await actor.type(await screen.findByPlaceholderText('Enter your password'), 'hunter2{Enter}'); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + + expect(screen.getByLabelText('Password')).toBeVisible(); + expect(screen.getByRole('button', { name: 'Continue' })).toHaveAttribute('aria-busy', 'true'); + expect(screen.queryByRole('button', { name: 'Close' })).toBeNull(); + await actor.keyboard('{Escape}{Enter}'); + expect(prepare.requests).toHaveLength(1); + + prepare.release(); + const input = await screen.findByRole('textbox', { name: 'Verification code' }); + await waitFor(() => expect(input).not.toHaveAttribute('aria-disabled')); + } finally { + prepare.release(); + } + }); + + it('returns to the phone form after preparation fails', async () => { + const { actor } = await renderSection(challenged()); + demandReverificationOnce('/v1/me/phone_numbers'); + const prepare = holdRequests('post', '/v1/me/phone_numbers/:id/prepare_verification'); + + try { + await addPhone(actor, '8015550100'); + await actor.type(await screen.findByPlaceholderText('Enter your password'), 'hunter2{Enter}'); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + expect(screen.getByLabelText('Password')).toBeVisible(); + prepare.fail('form_param_invalid', 'Could not send code.', 'phone_number'); + + const input = await screen.findByRole('textbox', { name: 'Phone' }); + await waitFor(() => expect(input).toHaveFocus()); + await waitFor(() => expect(input).toHaveAccessibleDescription('Could not send code.')); + } finally { + prepare.release(); + } + }); + it('confirms it is the user before a phone number is added, then verifies its code', async () => { const { actor, fapi } = await renderSection(challenged()); demandReverificationOnce('/v1/me/phone_numbers'); @@ -498,6 +542,8 @@ describe('phone reverification', () => { await manage(actor, WORK_LABEL, 'Set as primary'); await waitFor(() => expect(start.requests).toHaveLength(1)); + expect(screen.queryByRole('menu')).toBeNull(); + expect(within(row()).getByText(WORK_LABEL)).toBeVisible(); expect(screen.queryByRole('dialog')).toBeNull(); start.release(); @@ -523,3 +569,67 @@ describe('Removing a number', () => { expect(screen.queryByRole('alertdialog')).toBeNull(); }); }); + +it('keeps Send code pending until phone verification is ready', async () => { + const { actor } = await renderSection(); + const prepare = holdRequests('post', '/v1/me/phone_numbers/:id/prepare_verification'); + try { + await addPhone(actor, '8015550100'); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + expect(screen.getByRole('button', { name: 'Send code' })).toHaveAttribute('aria-busy', 'true'); + expect(screen.getByRole('textbox', { name: 'Phone' })).toBeVisible(); + expect(screen.queryByRole('textbox', { name: 'Verification code' })).not.toBeInTheDocument(); + prepare.release(); + await enterCode(actor, VERIFICATION_CODE); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + } finally { + prepare.release(); + } +}); + +it('keeps the profile visible until phone verification is ready and focuses the code field', async () => { + const pending = fapiPhoneNumber({ + id: 'idn_pending', + phone_number: '+18015550199', + verification: fapiVerification('phone_code', { status: 'unverified' }), + }); + const { actor } = await renderSection(signedIn([HOME, pending])); + const prepare = holdRequests('post', '/v1/me/phone_numbers/:id/prepare_verification'); + try { + await manage(actor, WORK_LABEL, 'Verify phone number'); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + await waitFor(() => expect(screen.queryByRole('menu')).toBeNull()); + expect(within(row()).getByText(WORK_LABEL)).toBeVisible(); + expect(screen.getByRole('button', { name: 'Add phone number' })).toBeDisabled(); + expect(screen.queryByRole('dialog')).toBeNull(); + prepare.release(); + await waitFor(() => expect(screen.getByRole('textbox', { name: 'Verification code' })).toHaveFocus()); + } finally { + prepare.release(); + } +}); + +it('retries phone preparation from the entry form without creating another number', async () => { + const { actor, fapi } = await renderSection(); + const prepare = holdRequests('post', '/v1/me/phone_numbers/:id/prepare_verification'); + try { + await addPhone(actor, '8015550100'); + await waitFor(() => expect(prepare.requests).toHaveLength(1)); + prepare.fail('action_blocked'); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Send code' })).not.toHaveAttribute('aria-busy', 'true'), + ); + const retry = holdRequests('post', '/v1/me/phone_numbers/:id/prepare_verification'); + try { + await actor.click(screen.getByRole('button', { name: 'Send code' })); + await waitFor(() => expect(retry.requests).toHaveLength(1)); + expect(fapi.client.sessions[0]?.user.phone_numbers).toHaveLength(1); + retry.release(); + expect(await screen.findByRole('textbox', { name: 'Verification code' })).toBeVisible(); + } finally { + retry.release(); + } + } finally { + prepare.release(); + } +}); diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.controller.ts index 7e6bf1d0a0b..ce5001d642d 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.controller.ts @@ -1,4 +1,4 @@ -import { useEffect, useMemo } from 'react'; +import { useEffect, useMemo, useRef } from 'react'; import type { DialogHandle } from '../../../components/dialog'; import { Dialog } from '../../../components/dialog'; @@ -23,7 +23,7 @@ export interface UserProfileAddPhoneControllerOptions { export interface UserProfileAddPhoneController extends UserProfileAddPhoneDialogProps { handle: DialogHandle; verifyingId: string | undefined; - onVerifyPhone: (phone: Pick, verifier: UserProfilePhoneVerifier) => void; + onVerifyPhone: (phone: Pick, verifier: UserProfilePhoneVerifier) => Promise; } interface Context { @@ -36,7 +36,7 @@ interface Context { type Event = | { type: 'ADD' } - | { type: 'START'; id?: string; phoneNumber: string; verifier: UserProfilePhoneVerifier } + | { type: 'READY'; id?: string; phoneNumber: string; verifier: UserProfilePhoneVerifier } | { type: 'VERIFIED' } | { type: 'CANCEL' } | { type: 'RESEND' } @@ -52,14 +52,14 @@ const CODE_RESEND_SECONDS = 30; const tick = { actions: assign(context => ({ resendSeconds: Math.max(0, context.resendSeconds - 1) })) }; const fail = assign((_, event) => ({ error: toFormError(event.error) })); -const start = { - target: 'sending', - actions: assign>((_, event) => ({ +const ready = { + target: 'verify', + actions: assign>((_, event) => ({ verifyingId: event.id, phoneNumber: event.phoneNumber, verifier: event.verifier, error: undefined, - resendSeconds: 0, + resendSeconds: CODE_RESEND_SECONDS, })), }; @@ -80,11 +80,11 @@ const machine = createMachine({ target: 'phone', actions: assign(() => ({ verifyingId: undefined, verifier: undefined, error: undefined, resendSeconds: 0 })), }, - START: start, + READY: ready, }, }, phone: { - on: { CANCEL: 'idle', START: start }, + on: { CANCEL: 'idle', READY: ready }, }, sending: { invoke: fromPromise(context => (context.verifier ? context.verifier.sendCode() : missingDependency()), { @@ -114,12 +114,25 @@ export function useUserProfileAddPhoneController({ const errorText = useErrorText(); const handle = useMemo(() => Dialog.createHandle(), []); const [snapshot, send, actor] = useMachine(machine); + const created = useRef<{ value: string; verifier: UserProfilePhoneVerifier }>(undefined); + const generation = useRef(0); const { resendSeconds, error } = snapshot.context; const phoneForm = useForm({ initialValues: { phoneNumber: '' }, onSubmit: async ({ phoneNumber }) => { if (onCreate) { - send({ type: 'START', phoneNumber, verifier: await onCreate(phoneNumber) }); + if (created.current?.value !== phoneNumber) { + const verifier = await onCreate(phoneNumber); + if (actor.getSnapshot().status !== 'active') { + return; + } + created.current = { value: phoneNumber, verifier }; + } + const { verifier } = created.current; + await verifier.sendCode(); + if (actor.getSnapshot().status === 'active') { + send({ type: 'READY', phoneNumber, verifier }); + } } }, }); @@ -167,7 +180,9 @@ export function useUserProfileAddPhoneController({ } reverification.onCancel(); } + generation.current++; if (open) { + created.current = undefined; phoneForm.reset(); codeForm.reset(); send({ type: 'ADD' }); @@ -195,9 +210,20 @@ export function useUserProfileAddPhoneController({ } }, verifyingId: snapshot.context.verifyingId, - onVerifyPhone: (phone, verifier) => { - codeForm.reset(); - send({ type: 'START', id: phone.id, phoneNumber: phone.value, verifier }); + onVerifyPhone: async (phone, verifier) => { + if (actor.getSnapshot().value !== 'idle') { + return; + } + const started = generation.current; + await verifier.sendCode(); + if ( + actor.getSnapshot().status === 'active' && + generation.current === started && + actor.getSnapshot().value === 'idle' + ) { + codeForm.reset(); + send({ type: 'READY', id: phone.id, phoneNumber: phone.value, verifier }); + } }, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.dialog.tsx b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.dialog.tsx index dac98862d02..80d338be7f7 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.dialog.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-add-phone.dialog.tsx @@ -1,4 +1,4 @@ -import { useRef } from 'react'; +import { useRef, useState } from 'react'; import { Card } from '../../../components/card'; import type { DialogFocusTarget, DialogHandle } from '../../../components/dialog'; @@ -31,18 +31,25 @@ export interface UserProfileAddPhoneDialogProps { export function UserProfileAddPhoneDialog(props: UserProfileAddPhoneDialogProps) { const phoneRef = useRef(null); + const codeRef = useRef(null); + const [lastShownStep, setLastShownStep] = useState(props.step); + const step = + props.open && props.isPending && props.step === 'phone' && lastShownStep === 'reverify' ? 'reverify' : props.step; + if (step !== lastShownStep) { + setLastShownStep(step); + } return ( {current => ( @@ -76,6 +83,7 @@ export function UserProfileAddPhoneDialog(props: UserProfileAddPhoneDialogProps) (null); const removalFocus = useListRemovalFocus({ ids: phones.map(phone => phone.id), @@ -40,7 +41,8 @@ export function useUserProfilePhoneSectionController({ defaultPhoneCountry, canAdd: Boolean(onCreatePhone), verification, - error: setPrimary.error, + error: verify.error ?? setPrimary.error, + pendingVerifyId: verify.pendingKey, rowRef, registerTrigger: removalFocus.registerTrigger, remove: onRemovePhone ? remove : undefined, @@ -51,9 +53,13 @@ export function useUserProfilePhoneSectionController({ onVerify: id => { const phone = phones.find(phone => phone.id === id); if (phone) { - verification.onVerifyPhone(phone, getPhoneVerifier(id)); + setPrimary.reset(); + void verify.run(id, () => verification.onVerifyPhone(phone, getPhoneVerifier(id))); } }, - onSetPrimary: id => void setPrimary.run(id, () => onSetPrimaryPhone(id)), + onSetPrimary: id => { + verify.reset(); + void setPrimary.run(id, () => onSetPrimaryPhone(id)); + }, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.model.ts index aec4af43bfb..98f39011b09 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.model.ts @@ -12,10 +12,14 @@ import type { UserProfilePhoneSectionModel } from './user-profile-phone-section. const ADD_PHONE_FIELDS: readonly UserProfileAddPhoneField[] = ['phoneNumber', 'code']; -function toPhoneVerifier(phone: PhoneNumberResource): UserProfilePhoneVerifier { +function toPhoneVerifier(currentPhone: () => PhoneNumberResource): UserProfilePhoneVerifier { return { - sendCode: () => save(() => phone.prepareVerification()), - verifyCode: code => save(() => phone.attemptVerification({ code }), ADD_PHONE_FIELDS), + sendCode: () => + save(async () => { + await currentPhone().prepareVerification(); + currentPhone(); + }), + verifyCode: code => save(() => currentPhone().attemptVerification({ code }), ADD_PHONE_FIELDS), }; } @@ -56,10 +60,12 @@ export function useUserProfilePhoneSectionModel(): UserProfilePhoneSectionModel ? async phoneNumber => { const request = createPhone(currentUser, phoneNumber); await save(() => request, ADD_PHONE_FIELDS); - return toPhoneVerifier(await request); + currentUser(); + const phone = await request; + return toPhoneVerifier(() => byId(currentUser().phoneNumbers, phone.id, 'phone number')); } : undefined, - getPhoneVerifier: id => toPhoneVerifier(byId(user.phoneNumbers, id, 'phone number')), + getPhoneVerifier: id => toPhoneVerifier(() => byId(currentUser().phoneNumbers, id, 'phone number')), onSetPrimaryPhone: id => saveAsUser(current => setPrimaryPhone(current, id)), onRemovePhone: access.canRemove ? id => saveAsUser(current => removePhone(current, id)) : undefined, removeReverification, diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts index b9a659e3f4c..e769ed57c8f 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.types.ts @@ -20,6 +20,7 @@ export interface UserProfilePhoneSectionViewProps { verificationFinalFocus: DialogFocusTarget; setPrimaryReverification: ReverificationController; setPrimaryFinalFocus: DialogFocusTarget; + pendingVerifyId?: string; onVerify: (id: string) => void; onSetPrimary: (id: string) => void; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx index cd13d1f4408..26d6269e575 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-phone-section/user-profile-phone-section.view.tsx @@ -23,6 +23,7 @@ export function UserProfilePhoneSectionView({ verificationFinalFocus, setPrimaryReverification, setPrimaryFinalFocus, + pendingVerifyId, onVerify, onSetPrimary, }: UserProfilePhoneSectionViewProps) { @@ -33,6 +34,7 @@ export function UserProfilePhoneSectionView({ render={