From ea6843a96430e504474719f4d87bf4de7b791264 Mon Sep 17 00:00:00 2001 From: omarima-10 Date: Sat, 26 Sep 2026 02:21:26 +0100 Subject: [PATCH] feat(frontend): optimistic rollback, keyboard nav, tests, and accessible focus trap - TwoFactorAuthSetup: preserve QR/manual key and entered code on a network failure during verification instead of discarding setup progress, add a retry action, and distinguish network failures from invalid-code rejections (closes #1519) - TwoFactorAuthSetup: submit on Enter, clear on Escape, and announce step transitions via a live region for keyboard/screen-reader users (closes #1520) - TwoFactorAuthSetup: add unit and snapshot test coverage across all steps, including the new rollback/retry and keyboard behavior; fix a pre-existing deadlock where waitFor was awaited while fake timers were active (closes #1521) - AnalyticsCards: make each card an interactive detail dialog reusing the shared Modal component; fix Modal's focus trap, which was silently inert because its ref was never attached to the dialog element, and add the missing role="dialog"/aria-modal/aria-labelledby (closes #1522) --- frontend/messages/en.json | 5 +- frontend/messages/es.json | 5 +- frontend/messages/pt.json | 5 +- .../src/components/AnalyticsCards.test.tsx | 182 +++++ frontend/src/components/AnalyticsCards.tsx | 100 ++- .../components/TwoFactorAuthSetup.test.tsx | 420 ++++++---- .../src/components/TwoFactorAuthSetup.tsx | 107 ++- .../TwoFactorAuthSetup.test.tsx.snap | 729 ++++++++++++++++++ frontend/src/components/ui/Modal.tsx | 10 +- 9 files changed, 1364 insertions(+), 199 deletions(-) create mode 100644 frontend/src/components/AnalyticsCards.test.tsx create mode 100644 frontend/src/components/__snapshots__/TwoFactorAuthSetup.test.tsx.snap diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 06b3e987..cc0496b4 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -16,12 +16,15 @@ "codeInputPlaceholder": "000000", "verifyButton": "Verify & Enable", "verifying": "Verifying…", + "retryButton": "Retry", + "stepAnnouncement": "Step {current} of {total}", "successTitle": "Two-Factor Authentication Enabled", "successDescription": "Your account is now protected. You will be prompted for a code on each login.", "error": { "setupFailed": "Failed to start 2FA setup. Please try again.", "invalidCode": "Invalid code. Please try again.", - "codeLength": "Please enter the 6-digit code from your authenticator app." + "codeLength": "Please enter the 6-digit code from your authenticator app.", + "networkFailure": "Couldn't reach the server. Check your connection and retry — your code and QR setup are still here." } }, "localeSwitcher": { diff --git a/frontend/messages/es.json b/frontend/messages/es.json index ccb2e34c..1e424a06 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -16,12 +16,15 @@ "codeInputPlaceholder": "000000", "verifyButton": "Verificar y activar", "verifying": "Verificando…", + "retryButton": "Reintentar", + "stepAnnouncement": "Paso {current} de {total}", "successTitle": "Autenticacion en dos pasos activada", "successDescription": "Tu cuenta esta ahora protegida. Se te pedira un codigo en cada inicio de sesion.", "error": { "setupFailed": "No se pudo iniciar la configuracion de 2FA. Intentalo de nuevo.", "invalidCode": "Codigo invalido. Intentalo de nuevo.", - "codeLength": "Ingresa el codigo de 6 digitos de tu aplicacion de autenticacion." + "codeLength": "Ingresa el codigo de 6 digitos de tu aplicacion de autenticacion.", + "networkFailure": "No se pudo conectar con el servidor. Revisa tu conexion y reintenta — tu codigo y la configuracion QR siguen aqui." } }, "localeSwitcher": { diff --git a/frontend/messages/pt.json b/frontend/messages/pt.json index bebc4db0..d3396e96 100644 --- a/frontend/messages/pt.json +++ b/frontend/messages/pt.json @@ -16,12 +16,15 @@ "codeInputPlaceholder": "000000", "verifyButton": "Verificar e ativar", "verifying": "Verificando…", + "retryButton": "Tentar novamente", + "stepAnnouncement": "Etapa {current} de {total}", "successTitle": "Autenticacao em duas etapas ativada", "successDescription": "Sua conta esta agora protegida. Voce sera solicitado um codigo em cada login.", "error": { "setupFailed": "Falha ao iniciar a configuracao de 2FA. Tente novamente.", "invalidCode": "Codigo invalido. Tente novamente.", - "codeLength": "Insira o codigo de 6 digitos do seu aplicativo de autenticacao." + "codeLength": "Insira o codigo de 6 digitos do seu aplicativo de autenticacao.", + "networkFailure": "Nao foi possivel conectar ao servidor. Verifique sua conexao e tente novamente — seu codigo e a configuracao QR continuam aqui." } }, "localeSwitcher": { diff --git a/frontend/src/components/AnalyticsCards.test.tsx b/frontend/src/components/AnalyticsCards.test.tsx new file mode 100644 index 00000000..2c028cf0 --- /dev/null +++ b/frontend/src/components/AnalyticsCards.test.tsx @@ -0,0 +1,182 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import React from "react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import "@testing-library/jest-dom/vitest"; +import AnalyticsCards from "./AnalyticsCards"; + +vi.mock("next-intl", () => ({ + useLocale: () => "en", +})); + +vi.mock("@/lib/merchant-store", () => ({ + useMerchantApiKey: () => "mock-api-key", + useMerchantHydrated: () => true, + useHydrateMerchantStore: vi.fn(), +})); + +vi.mock("@/lib/display-preferences", async () => { + const actual = await vi.importActual( + "@/lib/display-preferences" + ); + return { + ...actual, + useDisplayPreferences: () => ({ hideCents: false, setHideCents: vi.fn() }), + }; +}); + +const METRICS_RESPONSE = { total_volume: 1500 }; +const PAYMENTS_RESPONSE = { + payments: [ + { id: "1", status: "confirmed" }, + { id: "2", status: "confirmed" }, + { id: "3", status: "pending" }, + { id: "4", status: "failed" }, + ], +}; + +function mockFetchSuccess() { + (globalThis.fetch as any) = vi + .fn() + .mockResolvedValueOnce({ ok: true, json: async () => METRICS_RESPONSE }) + .mockResolvedValueOnce({ ok: true, json: async () => PAYMENTS_RESPONSE }); +} + +describe("AnalyticsCards", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("renders a loading skeleton before data resolves", () => { + (globalThis.fetch as any) = vi.fn().mockReturnValue(new Promise(() => {})); + render(); + expect(document.querySelectorAll(".animate-pulse")).toHaveLength(3); + }); + + it("renders the three metric cards after data resolves", async () => { + mockFetchSuccess(); + render(); + + await waitFor(() => { + expect(screen.getByText("Total Volume (7D)")).toBeInTheDocument(); + expect(screen.getByText("Success Rate")).toBeInTheDocument(); + expect(screen.getByText("Active intents")).toBeInTheDocument(); + }); + }); + + it("computes success rate from confirmed vs. resolved payments", async () => { + mockFetchSuccess(); + render(); + + // 2 confirmed / (2 confirmed + 1 failed) = 66.7% + await waitFor(() => { + expect(screen.getByText("66.7%")).toBeInTheDocument(); + }); + }); + + it("counts pending payments as active intents", async () => { + mockFetchSuccess(); + render(); + + await waitFor(() => { + const activeIntentsCard = screen.getByText("Active intents").closest("button")!; + expect(activeIntentsCard).toHaveTextContent("1"); + }); + }); + + // ── Focus-trapped detail dialog (#1522) ───────────────────────────────────── + + describe("card detail dialog", () => { + it("each card is a button that announces it opens a dialog", async () => { + mockFetchSuccess(); + render(); + + await waitFor(() => { + const cards = screen.getAllByRole("button"); + expect(cards).toHaveLength(3); + cards.forEach((card) => expect(card).toHaveAttribute("aria-haspopup", "dialog")); + }); + }); + + it("opens an accessible dialog with the card's detail when clicked", async () => { + mockFetchSuccess(); + render(); + + await waitFor(() => screen.getByText("Total Volume (7D)")); + fireEvent.click(screen.getByText("Total Volume (7D)").closest("button")!); + + const dialog = screen.getByRole("dialog"); + expect(dialog).toHaveAttribute("aria-modal", "true"); + expect(dialog).toHaveTextContent(/total payment volume processed/i); + }); + + it("dialog is labelled by the card's title", async () => { + mockFetchSuccess(); + render(); + + await waitFor(() => screen.getByText("Success Rate")); + fireEvent.click(screen.getByText("Success Rate").closest("button")!); + + const dialog = screen.getByRole("dialog"); + const labelledBy = dialog.getAttribute("aria-labelledby"); + expect(labelledBy).toBeTruthy(); + expect(document.getElementById(labelledBy!)).toHaveTextContent("Success Rate"); + }); + + it("closes the dialog when the close button is clicked", async () => { + mockFetchSuccess(); + render(); + + await waitFor(() => screen.getByText("Active intents")); + fireEvent.click(screen.getByText("Active intents").closest("button")!); + expect(screen.getByRole("dialog")).toBeInTheDocument(); + + fireEvent.click(screen.getByTestId("modal-close")); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + it("closes the dialog on Escape", async () => { + mockFetchSuccess(); + render(); + + await waitFor(() => screen.getByText("Total Volume (7D)")); + fireEvent.click(screen.getByText("Total Volume (7D)").closest("button")!); + expect(screen.getByRole("dialog")).toBeInTheDocument(); + + fireEvent.keyDown(document, { key: "Escape" }); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + it("traps Tab focus within the open dialog", async () => { + mockFetchSuccess(); + render(); + + await waitFor(() => screen.getByText("Total Volume (7D)")); + fireEvent.click(screen.getByText("Total Volume (7D)").closest("button")!); + + const dialog = screen.getByRole("dialog"); + const closeButton = screen.getByTestId("modal-close"); + + // Only the close button is focusable inside this dialog's body (plain text detail). + closeButton.focus(); + expect(document.activeElement).toBe(closeButton); + + fireEvent.keyDown(document, { key: "Tab" }); + expect(document.activeElement).toBe(closeButton); + void dialog; + }); + + it("restores focus to the triggering card when closed", async () => { + mockFetchSuccess(); + render(); + + await waitFor(() => screen.getByText("Total Volume (7D)")); + const trigger = screen.getByText("Total Volume (7D)").closest("button")!; + trigger.focus(); + fireEvent.click(trigger); + + fireEvent.keyDown(document, { key: "Escape" }); + expect(document.activeElement).toBe(trigger); + }); + }); +}); diff --git a/frontend/src/components/AnalyticsCards.tsx b/frontend/src/components/AnalyticsCards.tsx index 05aaf13e..2dccb0c3 100644 --- a/frontend/src/components/AnalyticsCards.tsx +++ b/frontend/src/components/AnalyticsCards.tsx @@ -11,6 +11,7 @@ import { useDisplayPreferences, formatAmount, } from "@/lib/display-preferences"; +import { Modal } from "./ui/Modal"; interface MetricsResponse { total_volume: number; @@ -25,12 +26,20 @@ interface PaymentsResponse { payments: Payment[]; } +interface CardDetail { + id: string; + label: string; + value: string; + description: string; +} + export default function AnalyticsCards() { const [totalVolume, setTotalVolume] = useState(0); const [successRate, setSuccessRate] = useState(0); const [activeIntents, setActiveIntents] = useState(0); const [loading, setLoading] = useState(true); - + const [openCardId, setOpenCardId] = useState(null); + const apiKey = useMerchantApiKey(); const hydrated = useMerchantHydrated(); const locale = useLocale(); @@ -92,43 +101,62 @@ export default function AnalyticsCards() { ); } + const cards: CardDetail[] = [ + { + id: "total-volume", + label: "Total Volume (7D)", + value: formatAmount(totalVolume, locale, hideCents), + description: "Total payment volume processed across all confirmed transactions in the last 7 days.", + }, + { + id: "success-rate", + label: "Success Rate", + value: `${successRate.toFixed(1)}%`, + description: "Share of resolved payments (confirmed vs. confirmed + failed/refunded) in the last 7 days. Pending payments aren't counted until they resolve.", + }, + { + id: "active-intents", + label: "Active intents", + value: String(activeIntents), + description: "Payments currently awaiting confirmation. These are not yet counted in the success rate above.", + }, + ]; + + const openCard = cards.find((c) => c.id === openCardId) ?? null; + return (
- {/* Total Volume */} -
-
-

- {formatAmount(totalVolume, locale, hideCents)} -

-

- Total Volume (7D) -

-
-
- - {/* Success Rate */} -
-
-

- {successRate.toFixed(1)}% -

-

- Success Rate -

-
-
- - {/* Active Intents */} -
-
-

- {activeIntents} -

-

- Active intents -

-
-
+ {cards.map((card) => ( + + ))} + + setOpenCardId(null)} + title={openCard?.label ?? ""} + > + {openCard && ( +
+

{openCard.value}

+

{openCard.description}

+
+ )} +
); } diff --git a/frontend/src/components/TwoFactorAuthSetup.test.tsx b/frontend/src/components/TwoFactorAuthSetup.test.tsx index 1241c1c7..49884297 100644 --- a/frontend/src/components/TwoFactorAuthSetup.test.tsx +++ b/frontend/src/components/TwoFactorAuthSetup.test.tsx @@ -1,6 +1,5 @@ import { render, screen, fireEvent, waitFor, act } from "@testing-library/react"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import userEvent from "@testing-library/user-event"; import "@testing-library/jest-dom/vitest"; import { TwoFactorAuthSetup } from "./TwoFactorAuthSetup"; @@ -30,11 +29,14 @@ vi.mock("next-intl", () => ({ "codeInputPlaceholder": "000000", "verifyButton": "Verify & Enable", "verifying": "Verifying…", + "retryButton": "Retry", + "stepAnnouncement": "Step {current} of {total}", "successTitle": "Two-Factor Authentication Enabled", "successDescription": "Your account is now protected. You will be prompted for a code on each login.", "error.setupFailed": "Failed to start 2FA setup. Please try again.", "error.invalidCode": "Invalid code. Please try again.", "error.codeLength": "Please enter the 6-digit code from your authenticator app.", + "error.networkFailure": "Couldn't reach the server. Check your connection and retry — your code and QR setup are still here.", }; return (key: string, params?: Record) => { @@ -70,6 +72,34 @@ function makeVerifyCode(shouldFail = false, delay = 0) { ); } +/** + * Drives the component from idle to the scan step using fake timers. + * + * IMPORTANT: `waitFor` must never be awaited while fake timers are active — + * it polls via `setTimeout`, which is frozen once `vi.useFakeTimers()` runs, + * so it hangs until Vitest's own test timeout. Every state transition here + * is instead flushed synchronously via `act(() => vi.runAllTimers())`, then + * asserted on directly (matching the working pattern already used elsewhere + * in this repo, e.g. KycSubmissionForm.test.tsx). + */ +async function renderAndEnable( + generateSecret: ReturnType, + verifyCode: ReturnType, + onComplete?: () => void +) { + render( + + ); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); + vi.runAllTimers(); + }); +} + // ── Tests ──────────────────────────────────────────────────────────────────── describe("TwoFactorAuthSetup", () => { @@ -92,11 +122,11 @@ describe("TwoFactorAuthSetup", () => { // ── Loading state: enabling ───────────────────────────────────────────── - it("shows loading spinner and disables button while generating secret", async () => { + it("shows loading spinner and disables button while generating secret", () => { const generateSecret = makeGenerateSecret(500); render(); - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); + fireEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); const btn = screen.getByRole("button", { name: /setting up/i }); expect(btn).toBeDisabled(); @@ -104,76 +134,53 @@ describe("TwoFactorAuthSetup", () => { expect(screen.getByTestId("spinner")).toBeInTheDocument(); }); - it("section has aria-busy=true while enabling", async () => { + it("section has aria-busy=true while enabling", () => { const generateSecret = makeGenerateSecret(500); render(); - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); + fireEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); expect(screen.getByRole("region", { name: /two-factor authentication setup/i })) .toHaveAttribute("aria-busy", "true"); }); - it("shows QR skeleton while generating secret", async () => { - const generateSecret = makeGenerateSecret(500); - render(); - - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - - // After moving to scan step (without QR yet resolved), we should not see the QR image - // The skeleton appears when scanVisible=true and qrDataUrl is still null - // This is visible during the enabling → scan transition + it("shows the QR skeleton only until the QR image is available, never alongside it", async () => { + vi.useFakeTimers(); + // setQrDataUrl and setStep("scan") land in the same state-update batch + // (handleEnable's .then()), so the skeleton and the real QR image are + // never both/neither present from an external observer's perspective — + // this asserts that invariant holds once the scan step is reached. + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode()); + + expect(screen.queryByLabelText(/generating qr code/i)).not.toBeInTheDocument(); + expect(screen.getByAltText(/totp qr code/i)).toBeInTheDocument(); + vi.useRealTimers(); }); // ── Scan step ─────────────────────────────────────────────────────────── it("shows QR code and manual key after secret is generated", async () => { vi.useFakeTimers(); - const generateSecret = makeGenerateSecret(0); - render(); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - vi.runAllTimers(); - }); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode()); - await waitFor(() => { - expect(screen.getByAltText(/totp qr code/i)).toBeInTheDocument(); - expect(screen.getByText("JBSWY3DPEHPK3PXP")).toBeInTheDocument(); - }); + expect(screen.getByAltText(/totp qr code/i)).toBeInTheDocument(); + expect(screen.getByText("JBSWY3DPEHPK3PXP")).toBeInTheDocument(); vi.useRealTimers(); }); it("renders the code input field in scan step", async () => { vi.useFakeTimers(); - const generateSecret = makeGenerateSecret(0); - render(); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - vi.runAllTimers(); - }); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode()); - await waitFor(() => { - expect(screen.getByLabelText(/enter 6-digit code/i)).toBeInTheDocument(); - }); + expect(screen.getByLabelText(/enter 6-digit code/i)).toBeInTheDocument(); vi.useRealTimers(); }); it("verify button is disabled when fewer than 6 digits are entered", async () => { vi.useFakeTimers(); - const generateSecret = makeGenerateSecret(0); - render(); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - vi.runAllTimers(); - }); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode()); - await waitFor(() => screen.getByLabelText(/enter 6-digit code/i)); - - const input = screen.getByLabelText(/enter 6-digit code/i); - fireEvent.change(input, { target: { value: "123" } }); + fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "123" } }); expect(screen.getByRole("button", { name: /verify & enable/i })).toBeDisabled(); vi.useRealTimers(); @@ -181,15 +188,7 @@ describe("TwoFactorAuthSetup", () => { it("verify button is enabled with a 6-digit code", async () => { vi.useFakeTimers(); - const generateSecret = makeGenerateSecret(0); - render(); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - vi.runAllTimers(); - }); - - await waitFor(() => screen.getByLabelText(/enter 6-digit code/i)); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode()); fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "123456" } }); @@ -201,21 +200,11 @@ describe("TwoFactorAuthSetup", () => { it("shows verifying spinner and disables input while verifying", async () => { vi.useFakeTimers(); - const generateSecret = makeGenerateSecret(0); - const verifyCode = makeVerifyCode(false, 500); - render(); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - vi.runAllTimers(); - }); - - await waitFor(() => screen.getByLabelText(/enter 6-digit code/i)); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode(false, 500)); fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "123456" } }); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /verify & enable/i })); + act(() => { + fireEvent.click(screen.getByRole("button", { name: /verify & enable/i })); }); const verifyBtn = screen.getByRole("button", { name: /verifying/i }); @@ -231,7 +220,7 @@ describe("TwoFactorAuthSetup", () => { const generateSecret = vi.fn().mockRejectedValue(new Error("Network error")); render(); - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); + fireEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); await waitFor(() => { expect(screen.getByRole("button", { name: /enable 2fa/i })).toBeInTheDocument(); @@ -240,27 +229,15 @@ describe("TwoFactorAuthSetup", () => { it("shows error message when verification fails", async () => { vi.useFakeTimers(); - const generateSecret = makeGenerateSecret(0); - const verifyCode = makeVerifyCode(true, 0); - render(); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - vi.runAllTimers(); - }); - - await waitFor(() => screen.getByLabelText(/enter 6-digit code/i)); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode(true, 0)); fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "000000" } }); - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /verify & enable/i })); + fireEvent.click(screen.getByRole("button", { name: /verify & enable/i })); vi.runAllTimers(); }); - await waitFor(() => { - expect(screen.getByRole("alert")).toHaveTextContent("Invalid code"); - }); + expect(screen.getByRole("alert")).toHaveTextContent("Invalid code"); vi.useRealTimers(); }); @@ -268,7 +245,7 @@ describe("TwoFactorAuthSetup", () => { const generateSecret = vi.fn().mockRejectedValue(new Error("Setup failed")); render(); - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); + fireEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); await waitFor(() => { expect(screen.queryByRole("alert")).not.toBeInTheDocument(); @@ -280,61 +257,31 @@ describe("TwoFactorAuthSetup", () => { it("shows success state and calls onComplete after verification", async () => { vi.useFakeTimers(); const onComplete = vi.fn(); - const generateSecret = makeGenerateSecret(0); - const verifyCode = makeVerifyCode(false, 0); - render( - - ); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - vi.runAllTimers(); - }); - - await waitFor(() => screen.getByLabelText(/enter 6-digit code/i)); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode(false, 0), onComplete); fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "123456" } }); - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /verify & enable/i })); + fireEvent.click(screen.getByRole("button", { name: /verify & enable/i })); vi.runAllTimers(); }); - await waitFor(() => { - expect(screen.getByText(/two-factor authentication enabled/i)).toBeInTheDocument(); - expect(onComplete).toHaveBeenCalledTimes(1); - }); + expect(screen.getByText(/two-factor authentication enabled/i)).toBeInTheDocument(); + expect(onComplete).toHaveBeenCalledTimes(1); vi.useRealTimers(); }); it("success status region has aria-live=polite", async () => { vi.useFakeTimers(); - const generateSecret = makeGenerateSecret(0); - const verifyCode = makeVerifyCode(false, 0); - render(); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - vi.runAllTimers(); - }); - - await waitFor(() => screen.getByLabelText(/enter 6-digit code/i)); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode(false, 0)); fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "123456" } }); - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /verify & enable/i })); + fireEvent.click(screen.getByRole("button", { name: /verify & enable/i })); vi.runAllTimers(); }); - await waitFor(() => { - const status = screen.getByRole("status"); - expect(status).toHaveAttribute("aria-live", "polite"); - }); + const status = screen.getAllByRole("status").find((el) => el.textContent?.match(/enabled/i)); + expect(status).toHaveAttribute("aria-live", "polite"); vi.useRealTimers(); }); @@ -342,23 +289,17 @@ describe("TwoFactorAuthSetup", () => { it("first step dot is active on idle", () => { render(); - const step1 = screen.getByRole("listitem", { hidden: false }); - expect(step1).toBeInTheDocument(); + const steps = screen.getAllByRole("listitem"); + expect(steps).toHaveLength(3); + const firstDot = steps[0].querySelector('[aria-current="step"]'); + expect(firstDot).toBeInTheDocument(); }); // ── Accessibility ───────────────────────────────────────────────────────── it("code input strips non-numeric characters", async () => { vi.useFakeTimers(); - const generateSecret = makeGenerateSecret(0); - render(); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - vi.runAllTimers(); - }); - - await waitFor(() => screen.getByLabelText(/enter 6-digit code/i)); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode()); fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "12ab56" } }); expect(screen.getByLabelText(/enter 6-digit code/i)).toHaveValue("1256"); @@ -367,18 +308,211 @@ describe("TwoFactorAuthSetup", () => { it("code input is capped at 6 digits", async () => { vi.useFakeTimers(); - const generateSecret = makeGenerateSecret(0); - render(); - - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); - vi.runAllTimers(); - }); - - await waitFor(() => screen.getByLabelText(/enter 6-digit code/i)); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode()); fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "1234567890" } }); expect(screen.getByLabelText(/enter 6-digit code/i)).toHaveValue("123456"); vi.useRealTimers(); }); + + // ── Optimistic rollback on network failure (#1519) ───────────────────────── + + describe("network failure rollback", () => { + it("preserves the entered code and QR state when verification fails with a network error", async () => { + vi.useFakeTimers(); + const verifyCode = vi.fn().mockRejectedValue(new TypeError("Failed to fetch")); + await renderAndEnable(makeGenerateSecret(0), verifyCode); + + fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "123456" } }); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /verify & enable/i })); + vi.runAllTimers(); + }); + + // Code is preserved — the user shouldn't have to retype it after a network blip. + expect(screen.getByLabelText(/enter 6-digit code/i)).toHaveValue("123456"); + // QR/manual key stay visible — no needless re-scan. + expect(screen.getByText("JBSWY3DPEHPK3PXP")).toBeInTheDocument(); + vi.useRealTimers(); + }); + + it("shows a retry action after a network failure", async () => { + vi.useFakeTimers(); + const verifyCode = vi.fn().mockRejectedValue(new TypeError("Failed to fetch")); + await renderAndEnable(makeGenerateSecret(0), verifyCode); + + fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "123456" } }); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /verify & enable/i })); + vi.runAllTimers(); + }); + + expect(screen.getByRole("button", { name: /retry/i })).toBeInTheDocument(); + vi.useRealTimers(); + }); + + it("clears the code (no rollback) when verification fails with an invalid-code error, not a network error", async () => { + vi.useFakeTimers(); + const verifyCode = vi.fn().mockRejectedValue(new Error("Invalid code")); + await renderAndEnable(makeGenerateSecret(0), verifyCode); + + fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "000000" } }); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /verify & enable/i })); + vi.runAllTimers(); + }); + + expect(screen.getByLabelText(/enter 6-digit code/i)).toHaveValue(""); + expect(screen.queryByRole("button", { name: /retry/i })).not.toBeInTheDocument(); + vi.useRealTimers(); + }); + + it("retry button re-attempts verification with the preserved code", async () => { + vi.useFakeTimers(); + const verifyCode = vi + .fn() + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValueOnce(undefined); + await renderAndEnable(makeGenerateSecret(0), verifyCode); + + fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "123456" } }); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /verify & enable/i })); + vi.runAllTimers(); + }); + expect(screen.getByRole("button", { name: /retry/i })).toBeInTheDocument(); + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /retry/i })); + vi.runAllTimers(); + }); + + expect(verifyCode).toHaveBeenCalledTimes(2); + expect(verifyCode).toHaveBeenNthCalledWith(2, "123456"); + expect(screen.getByText(/two-factor authentication enabled/i)).toBeInTheDocument(); + vi.useRealTimers(); + }); + }); + + // ── Keyboard navigation (#1520) ───────────────────────────────────────────── + + describe("keyboard navigation", () => { + it("submits verification when Enter is pressed with a complete code", async () => { + vi.useFakeTimers(); + const verifyCode = makeVerifyCode(false, 0); + await renderAndEnable(makeGenerateSecret(0), verifyCode); + + const input = screen.getByLabelText(/enter 6-digit code/i); + fireEvent.change(input, { target: { value: "123456" } }); + + await act(async () => { + fireEvent.keyDown(input, { key: "Enter" }); + vi.runAllTimers(); + }); + + expect(verifyCode).toHaveBeenCalledWith("123456"); + vi.useRealTimers(); + }); + + it("does not submit on Enter when the code is incomplete", async () => { + vi.useFakeTimers(); + const verifyCode = makeVerifyCode(false, 0); + await renderAndEnable(makeGenerateSecret(0), verifyCode); + + const input = screen.getByLabelText(/enter 6-digit code/i); + fireEvent.change(input, { target: { value: "123" } }); + fireEvent.keyDown(input, { key: "Enter" }); + + expect(verifyCode).not.toHaveBeenCalled(); + vi.useRealTimers(); + }); + + it("clears the code when Escape is pressed", async () => { + vi.useFakeTimers(); + await renderAndEnable(makeGenerateSecret(0), makeVerifyCode()); + + const input = screen.getByLabelText(/enter 6-digit code/i); + fireEvent.change(input, { target: { value: "123456" } }); + fireEvent.keyDown(input, { key: "Escape" }); + + expect(input).toHaveValue(""); + vi.useRealTimers(); + }); + + it("announces step progress via a live region for keyboard/screen-reader users", () => { + render(); + expect(screen.getByText(/step 1 of 3/i)).toBeInTheDocument(); + }); + }); + + // ── Snapshot tests (#1521) ─────────────────────────────────────────────────── + // One per reachable step, so a future markup change surfaces as an intentional + // snapshot update rather than being caught only indirectly by behavioral tests. + + describe("snapshots", () => { + it("matches snapshot in idle state", () => { + const { container } = render( + + ); + expect(container).toMatchSnapshot(); + }); + + it("matches snapshot in enabling state", () => { + const { container } = render( + + ); + fireEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); + expect(container).toMatchSnapshot(); + }); + + it("matches snapshot in scan state", async () => { + vi.useFakeTimers(); + const { container } = render( + + ); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); + vi.runAllTimers(); + }); + expect(container).toMatchSnapshot(); + vi.useRealTimers(); + }); + + it("matches snapshot with a code entered and a network-failure retry action showing", async () => { + vi.useFakeTimers(); + const verifyCode = vi.fn().mockRejectedValue(new TypeError("Failed to fetch")); + const { container } = render( + + ); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); + vi.runAllTimers(); + }); + fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "123456" } }); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /verify & enable/i })); + vi.runAllTimers(); + }); + expect(container).toMatchSnapshot(); + vi.useRealTimers(); + }); + + it("matches snapshot in success state", async () => { + vi.useFakeTimers(); + const { container } = render( + + ); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /enable 2fa/i })); + vi.runAllTimers(); + }); + fireEvent.change(screen.getByLabelText(/enter 6-digit code/i), { target: { value: "123456" } }); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /verify & enable/i })); + vi.runAllTimers(); + }); + expect(container).toMatchSnapshot(); + vi.useRealTimers(); + }); + }); }); diff --git a/frontend/src/components/TwoFactorAuthSetup.tsx b/frontend/src/components/TwoFactorAuthSetup.tsx index 8c4289a0..cae8f029 100644 --- a/frontend/src/components/TwoFactorAuthSetup.tsx +++ b/frontend/src/components/TwoFactorAuthSetup.tsx @@ -17,6 +17,25 @@ interface TwoFactorAuthSetupProps { onVerifyCode?: (code: string) => Promise; } +// ── Network-failure detection ─────────────────────────────────────────────── + +/** + * Distinguishes a network/connectivity failure (fetch couldn't reach the + * server at all) from a server-side rejection (e.g. wrong code). Only the + * former should roll back optimistically without discarding in-flight setup + * state (QR/manual key, entered code) — a rejected code is a normal retry, + * not a connectivity problem, and clearing the QR would force a needless + * re-scan. + */ +function isNetworkFailure(error: unknown): boolean { + if (typeof navigator !== "undefined" && navigator.onLine === false) return true; + if (error instanceof TypeError) return true; // fetch's own connectivity failure signature + if (error instanceof Error) { + return /network|fetch|offline|connection/i.test(error.message); + } + return false; +} + // ── Skeleton helpers ───────────────────────────────────────────────────────── function QrSkeleton({ t }: { t: ReturnType }) { @@ -77,6 +96,7 @@ export function TwoFactorAuthSetup({ const [qrDataUrl, setQrDataUrl] = useState(null); const [manualKey, setManualKey] = useState(null); const [error, setError] = useState(null); + const [isNetworkError, setIsNetworkError] = useState(false); const isEnabling = step === "enabling"; const isVerifying = step === "verifying"; @@ -87,6 +107,7 @@ export function TwoFactorAuthSetup({ const handleEnable = async () => { setStep("enabling"); setError(null); + setIsNetworkError(false); try { const generate = onGenerateSecret ?? defaultGenerateSecret; const result = await generate(); @@ -95,6 +116,9 @@ export function TwoFactorAuthSetup({ setStep("scan"); } catch (err) { setError(err instanceof Error ? err.message : t("error.setupFailed")); + setIsNetworkError(isNetworkFailure(err)); + // Nothing to roll back to yet at this step — no QR/manual key has been + // committed, so returning to idle is already the correct rollback. setStep("idle"); } }; @@ -104,18 +128,39 @@ export function TwoFactorAuthSetup({ const handleVerify = async () => { if (code.trim().length !== 6) { setError(t("error.codeLength")); + setIsNetworkError(false); return; } setStep("verifying"); setError(null); + setIsNetworkError(false); try { const verify = onVerifyCode ?? defaultVerifyCode; await verify(code.trim()); setStep("success"); onComplete?.(); } catch (err) { - setError(err instanceof Error ? err.message : t("error.invalidCode")); + const networkFailure = isNetworkFailure(err); + setError(networkFailure ? t("error.networkFailure") : err instanceof Error ? err.message : t("error.invalidCode")); + setIsNetworkError(networkFailure); + // Optimistic rollback: return to the scan step without discarding the + // QR/manual key already shown, or the code the user typed. A network + // failure means the request never reached the server — clearing state + // here would force a needless re-scan for a problem that has nothing + // to do with the code's validity. Only a confirmed-invalid code should + // prompt the user to re-enter it (handled by the input's own clear-on-edit). setStep("scan"); + if (networkFailure) { + // Preserve the entered code so retrying doesn't require retyping it. + } else { + setCode(""); + } + } + }; + + const handleRetryVerify = () => { + if (isNetworkError) { + void handleVerify(); } }; @@ -144,6 +189,13 @@ export function TwoFactorAuthSetup({ + {/* Announces step transitions to screen reader / keyboard-only users, + who otherwise have no cue the flow advanced since the step dots + themselves aren't focusable (there is no valid "jump back" action — + each step is driven by an async call, not freely navigable). */} +

+ {t("stepAnnouncement", { current: isDone ? "3" : scanVisible ? "2" : "1", total: "3" })} +

{/* ── Idle / Enabling ──────────────────────────────────────────── */} {(step === "idle" || step === "enabling") && ( @@ -229,6 +281,18 @@ export function TwoFactorAuthSetup({ onChange={(e) => { setCode(e.target.value.replace(/\D/g, "").slice(0, 6)); setError(null); + setIsNetworkError(false); + }} + onKeyDown={(e) => { + if (e.key === "Enter" && !isVerifying && code.length === 6) { + e.preventDefault(); + void handleVerify(); + } else if (e.key === "Escape") { + e.preventDefault(); + setCode(""); + setError(null); + setIsNetworkError(false); + } }} disabled={isVerifying} aria-busy={isVerifying} @@ -244,22 +308,33 @@ export function TwoFactorAuthSetup({ )} - + {isNetworkError && !isVerifying && ( + )} - + )} diff --git a/frontend/src/components/__snapshots__/TwoFactorAuthSetup.test.tsx.snap b/frontend/src/components/__snapshots__/TwoFactorAuthSetup.test.tsx.snap new file mode 100644 index 00000000..83dedfe4 --- /dev/null +++ b/frontend/src/components/__snapshots__/TwoFactorAuthSetup.test.tsx.snap @@ -0,0 +1,729 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`TwoFactorAuthSetup > snapshots > matches snapshot in enabling state 1`] = ` +
+
+
+
+
+
+ + 1 + +
+
+
+
+
+`; + +exports[`TwoFactorAuthSetup > snapshots > matches snapshot in idle state 1`] = ` +
+
+
+
+
+
+ + 1 + +
+
+
+
+
+`; + +exports[`TwoFactorAuthSetup > snapshots > matches snapshot in scan state 1`] = ` +
+
+
+
+
+
+ +
+
+
+
+
+`; + +exports[`TwoFactorAuthSetup > snapshots > matches snapshot in success state 1`] = ` +
+
+
+
+
+
+ +
+
+
+
+
+`; + +exports[`TwoFactorAuthSetup > snapshots > matches snapshot with a code entered and a network-failure retry action showing 1`] = ` +
+
+
+
+
+
+ +
+
+
+
+
+`; diff --git a/frontend/src/components/ui/Modal.tsx b/frontend/src/components/ui/Modal.tsx index 529cbf98..9c6d668a 100644 --- a/frontend/src/components/ui/Modal.tsx +++ b/frontend/src/components/ui/Modal.tsx @@ -84,7 +84,15 @@ export const Modal: React.FC = ({ isOpen, onClose, title, children } {/* Modals are generally center or sliding sheet in this app. Let's make a center modal for the VRT component itself. */} {/* However, the PaymentDetail was a right sliding sheet. We'll build a standard modal dialog. */} -
+

{title}