diff --git a/.changeset/funky-suns-press.md b/.changeset/funky-suns-press.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/funky-suns-press.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/eslint-suppressions.json b/packages/mosaic/eslint-suppressions.json index 245446609fe..4ea77788f19 100644 --- a/packages/mosaic/eslint-suppressions.json +++ b/packages/mosaic/eslint-suppressions.json @@ -29,11 +29,6 @@ "count": 1 } }, - "src/features/reverification/reverification.controller.ts": { - "sonarjs/cognitive-complexity": { - "count": 1 - } - }, "src/features/reverification/use-reverification-with-state.ts": { "@typescript-eslint/consistent-type-assertions": { "count": 1 diff --git a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx index ce53ff08276..9c8b094194e 100644 --- a/packages/mosaic/src/blocks/confirm/confirm-shell.tsx +++ b/packages/mosaic/src/blocks/confirm/confirm-shell.tsx @@ -88,7 +88,7 @@ function ConfirmCard({ {confirmStep} controller.onOpenChange(false)} /> diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index 05b87485cfd..6924f103b5b 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -139,7 +139,6 @@ export function useConfirmController({ if (snapshot.value !== 'confirming') { return; } - reverification?.reset(); send({ type: 'CONFIRM', run: async () => { diff --git a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx index dfd860f6fc9..16dbd7199c1 100644 --- a/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx +++ b/packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx @@ -297,12 +297,10 @@ describe('Confirmation shared by rows', () => { function ReverifiedHost({ action, cleanupMs, - settleMs, beforeRunMs, }: { action: () => Promise; cleanupMs?: number; - settleMs?: number; beforeRunMs?: number; }) { const [run, reverification] = useReverificationFlow(action); @@ -312,11 +310,7 @@ function ReverifiedHost({ await new Promise(resolve => setTimeout(resolve, beforeRunMs)); } try { - const result = await run(); - if (settleMs !== undefined) { - await new Promise(resolve => setTimeout(resolve, settleMs)); - } - return result; + return await run(); } catch (error) { if (cleanupMs !== undefined) { await new Promise(resolve => setTimeout(resolve, cleanupMs)); @@ -435,9 +429,13 @@ describe('Confirmation with reverification', () => { } for (const settleMs of [undefined, 400]) { - it(`does not go back to the confirm step after a successful verification (host settles after ${settleMs ?? 0}ms)`, async () => { + it(`does not go back to the confirm step after a successful verification (action settles after ${settleMs ?? 0}ms)`, async () => { serveFapi(passwordVerification); - const { user } = await renderReverified({ action: interruptedOnce(), settleMs }); + const action = vi + .fn() + .mockResolvedValueOnce(reverificationError()) + .mockImplementation(() => new Promise(resolve => setTimeout(() => resolve({ done: true }), settleMs ?? 0))); + const { user } = await renderReverified({ action }); await confirmIntoVerification(user); const field = await screen.findByLabelText('Password'); diff --git a/packages/mosaic/src/blocks/destructive/__tests__/destructive.feature.test.tsx b/packages/mosaic/src/blocks/destructive/__tests__/destructive.feature.test.tsx index 085e115085a..dcace47583b 100644 --- a/packages/mosaic/src/blocks/destructive/__tests__/destructive.feature.test.tsx +++ b/packages/mosaic/src/blocks/destructive/__tests__/destructive.feature.test.tsx @@ -7,7 +7,7 @@ import { describe, expect, it, vi } from 'vitest'; import { deferred } from '../../../__tests__/async'; import { clerkApiError } from '../../../__tests__/clerk-errors'; import { openDialog } from '../../../__tests__/feature/dialog'; -import { serveFapi } from '../../../__tests__/feature/fake-fapi'; +import { holdRequests, serveFapi } from '../../../__tests__/feature/fake-fapi'; import { fapiClient, fapiSession, fapiUser } from '../../../__tests__/feature/fapi'; import { renderWithClerk } from '../../../__tests__/feature/render'; import { Button } from '../../../components/button'; @@ -207,9 +207,17 @@ describe('Destructive shared by rows', () => { }); }); -function ReverifiedHost({ action }: { action: () => Promise }) { +function ReverifiedHost({ action, before }: { action: () => Promise; before?: () => Promise }) { const [run, reverification] = useReverificationFlow(action); - const controller = useConfirmController({ onConfirm: run, reverification }); + const controller = useConfirmController({ + onConfirm: before + ? async () => { + await before(); + await run(); + } + : run, + reverification, + }); return ( { expect(action).toHaveBeenCalledTimes(2); }); }); + +function recordFrames() { + const frames: string[] = []; + const frame = () => { + const dialog = document.querySelector('[role="alertdialog"]'); + if (!dialog) { + return 'gone'; + } + const step = dialog.querySelector('.cl-flow-root')?.getAttribute('data-value') ?? 'none'; + return `${step}/${dialog.hasAttribute('data-open') ? 'open' : 'closing'}`; + }; + const push = () => { + const current = frame(); + if (frames.at(-1) !== current) { + frames.push(current); + } + }; + const observer = new MutationObserver(push); + observer.observe(document.body, { + subtree: true, + childList: true, + attributes: true, + attributeFilter: ['data-value', 'data-open', 'data-closed'], + }); + push(); + return { frames, stop: () => observer.disconnect() }; +} + +const VERIFY = '/v1/client/sessions/sess_1/verify'; + +describe('Destructive with reverification frames', () => { + async function renderReverified(action: () => Promise, before?: () => Promise) { + serveFapi({ + client: fapiClient([session]), + verification: { secrets: { password: 'hunter2' }, firstFactors: [{ strategy: 'password' as const }] }, + }); + await renderWithClerk( + , + ); + const user = userEvent.setup(); + const open = () => + openDialog(user, screen.getByRole('button', { name: 'Delete my account' }), { + role: 'alertdialog', + name: TITLE, + }); + const closed = () => waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull()); + return { user, open, closed }; + } + + it('closes from the verify step when the retried action succeeds', async () => { + const action = vi.fn().mockResolvedValueOnce(reverificationError()).mockResolvedValue({ done: true }); + const { user, open, closed } = await renderReverified(action); + await open(); + await user.type(field(), 'Delete{Enter}'); + const password = await screen.findByLabelText('Password'); + await waitFor(() => expect(screen.queryByText(TITLE)).toBeNull()); + + const recorder = recordFrames(); + await user.type(password, 'hunter2{Enter}'); + await closed(); + recorder.stop(); + + expect(action).toHaveBeenCalledTimes(2); + expect(recorder.frames[0]).toBe('verify/open'); + expect(recorder.frames).not.toContain('confirm/open'); + }); + + it('closes from the verify step when the challenge is cancelled', async () => { + const action = vi.fn().mockResolvedValue(reverificationError()); + const { user, open, closed } = await renderReverified(action); + await open(); + await user.type(field(), 'Delete{Enter}'); + await screen.findByLabelText('Password'); + await waitFor(() => expect(screen.queryByText(TITLE)).toBeNull()); + + const recorder = recordFrames(); + await user.click(screen.getByRole('button', { name: 'Close' })); + await closed(); + recorder.stop(); + + expect(recorder.frames[0]).toBe('verify/open'); + expect(recorder.frames).not.toContain('confirm/open'); + }); + + it('shows the confirm step until the next challenge is ready when confirming again after a cancel', async () => { + const action = vi.fn().mockResolvedValue(reverificationError()); + const { user, open, closed } = await renderReverified(action); + await open(); + await user.type(field(), 'Delete{Enter}'); + await screen.findByLabelText('Password'); + await user.click(screen.getByRole('button', { name: 'Close' })); + await closed(); + + await open(); + const verify = holdRequests('post', VERIFY); + const recorder = recordFrames(); + await user.type(field(), 'Delete{Enter}'); + await waitFor(() => expect(verify.requests).toHaveLength(1)); + expect(recorder.frames).toEqual(['confirm/open']); + verify.release(); + + await screen.findByLabelText('Password'); + recorder.stop(); + expect(recorder.frames).toEqual(['confirm/open', 'verify/open']); + expect(action).toHaveBeenCalledTimes(2); + }); + + it('shows the confirm step while an action does other work before the run, after a cancel', async () => { + const action = vi.fn().mockResolvedValue(reverificationError()); + const gates = [deferred(), deferred()]; + const { user, open, closed } = await renderReverified(action, () => gates[action.mock.calls.length].promise); + await open(); + await user.type(field(), 'Delete{Enter}'); + gates[0].resolve(); + await screen.findByLabelText('Password'); + await user.click(screen.getByRole('button', { name: 'Close' })); + await closed(); + + await open(); + const recorder = recordFrames(); + await user.type(field(), 'Delete{Enter}'); + await waitFor(() => expect(screen.getByRole('alertdialog').querySelector('[aria-busy="true"]')).not.toBeNull()); + expect(recorder.frames).toEqual(['confirm/open']); + gates[1].resolve(); + + await screen.findByLabelText('Password'); + recorder.stop(); + expect(recorder.frames).toEqual(['confirm/open', 'verify/open']); + }); +}); diff --git a/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx b/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx index e3427b184e4..4db396625e3 100644 --- a/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx +++ b/packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx @@ -142,7 +142,7 @@ function Host({ action }: { action: () => Promise }) { {outcome} {running ? ( - + {reverification.onCancel ? ( - {outcome} {String(reverification.visible)} - + {reverification.onCancel ? (