diff --git a/src/components/features/export/GoogleDriveAuthModal.tsx b/src/components/features/export/GoogleDriveAuthModal.tsx index acf122247..5bfe3c09d 100644 --- a/src/components/features/export/GoogleDriveAuthModal.tsx +++ b/src/components/features/export/GoogleDriveAuthModal.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useEffect } from 'react'; import { useTranslation } from 'react-i18next'; import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from '@/components/ui/dialog'; import { Button } from '@/components/ui/button'; @@ -6,6 +6,7 @@ import { useAuth } from '@/lib/supabase/AuthProvider'; import GoogleDocsIcon from '@/components/icons/GoogleDocsIcon'; import { ExternalLink, Loader2 } from 'lucide-react'; import { useToast } from '@/hooks/use-toast'; +import { useExternalRedirectPending } from '@/hooks/use-external-redirect-pending'; interface GoogleDriveAuthModalProps { open: boolean; @@ -16,14 +17,17 @@ export function GoogleDriveAuthModal({ open, onOpenChange }: GoogleDriveAuthModa const { t } = useTranslation(); const { signInWithGoogle } = useAuth(); const { toast } = useToast(); - const [isConnecting, setIsConnecting] = useState(false); + const { pending: isConnecting, run: runRedirect, reset: resetRedirect } = useExternalRedirectPending(); + + // Callers keep this dialog mounted while closed; reopening starts fresh. + useEffect(() => { + if (!open) resetRedirect(); + }, [open, resetRedirect]); const handleConnect = async () => { - setIsConnecting(true); try { - await signInWithGoogle({ driveAccess: true }); + await runRedirect(() => signInWithGoogle({ driveAccess: true })); } catch (error) { - setIsConnecting(false); toast({ title: t('googleDriveAuth.failedTitle', 'Could not open Google'), description: error instanceof Error ? error.message : t('googleDriveAuth.failedDesc', 'Please try again.'), diff --git a/src/components/features/subscription/PricingModal.tsx b/src/components/features/subscription/PricingModal.tsx index a427c98f8..c732dd18f 100644 --- a/src/components/features/subscription/PricingModal.tsx +++ b/src/components/features/subscription/PricingModal.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useEffect, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription } from '@/components/ui/dialog'; import { Button } from '@/components/ui/button'; @@ -8,6 +8,7 @@ import { Check, Sparkles, Zap, Crown } from 'lucide-react'; import { redirectToCheckout, redeemPromoCode } from '@/lib/stripe'; import { useToast } from '@/hooks/use-toast'; import { useSubscription } from '@/hooks/use-subscription'; +import { useExternalRedirectPending } from '@/hooks/use-external-redirect-pending'; interface PricingModalProps { open: boolean; @@ -19,10 +20,16 @@ export function PricingModal({ open, onOpenChange, onPromoRedeemed }: PricingMod const { t } = useTranslation(); const [promoCode, setPromoCode] = useState(''); const [promoLoading, setPromoLoading] = useState(false); - const [checkoutLoading, setCheckoutLoading] = useState(false); + const { pending: checkoutLoading, run: runCheckout, reset: resetCheckout } = useExternalRedirectPending(); const [showPromo, setShowPromo] = useState(false); const { toast } = useToast(); + // The modal stays mounted while closed, so reopening it must not show a + // button still waiting on a checkout the user already left. + useEffect(() => { + if (!open) resetCheckout(); + }, [open, resetCheckout]); + const FREE_FEATURES = [ t('pricingModal.freeFeature1'), t('pricingModal.freeFeature2'), @@ -39,16 +46,14 @@ export function PricingModal({ open, onOpenChange, onPromoRedeemed }: PricingMod ]; const handleUpgrade = async () => { - setCheckoutLoading(true); try { - await redirectToCheckout(promoCode || undefined); + await runCheckout(() => redirectToCheckout(promoCode || undefined)); } catch (err) { toast({ title: t('pricingModal.checkoutError'), description: err instanceof Error ? err.message : t('pricingModal.checkoutErrorDesc'), variant: 'destructive', }); - setCheckoutLoading(false); } }; diff --git a/src/hooks/use-external-redirect-pending.test.tsx b/src/hooks/use-external-redirect-pending.test.tsx new file mode 100644 index 000000000..6d323397f --- /dev/null +++ b/src/hooks/use-external-redirect-pending.test.tsx @@ -0,0 +1,94 @@ +import { act, renderHook } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { useExternalRedirectPending } from './use-external-redirect-pending'; +import { restoreFromBackForwardCache, restoreVisibility, setVisibility } from '@/test/helpers/page-lifecycle'; + +function deferred() { + let resolve!: () => void; + const promise = new Promise((r) => { resolve = r; }); + return { promise, resolve }; +} + +afterEach(() => { + restoreVisibility(); + vi.useRealTimers(); +}); + +describe('useExternalRedirectPending', () => { + it('clears pending when the page comes back from the back/forward cache', async () => { + const { result } = renderHook(() => useExternalRedirectPending()); + await act(() => result.current.run(async () => undefined)); + expect(result.current.pending).toBe(true); + + restoreFromBackForwardCache(); + expect(result.current.pending).toBe(false); + }); + + it('starts one navigation per click while pending', async () => { + const navigate = vi.fn(async () => undefined); + const { result } = renderHook(() => useExternalRedirectPending()); + await act(async () => { + await Promise.all([result.current.run(navigate), result.current.run(navigate)]); + }); + expect(navigate).toHaveBeenCalledTimes(1); + }); + + it('keeps pending through tab switches and the timeout until the request resolves', async () => { + vi.useFakeTimers(); + const request = deferred(); + const { result } = renderHook(() => useExternalRedirectPending(1000)); + let running!: Promise; + act(() => { running = result.current.run(() => request.promise); }); + + setVisibility('hidden'); + setVisibility('visible'); + act(() => { vi.advanceTimersByTime(5000); }); + expect(result.current.pending).toBe(true); + + await act(async () => { + request.resolve(); + await running; + }); + setVisibility('hidden'); + setVisibility('visible'); + expect(result.current.pending).toBe(false); + }); + + it('gives up waiting after the safety timeout', async () => { + vi.useFakeTimers(); + const { result } = renderHook(() => useExternalRedirectPending(1000)); + await act(() => result.current.run(async () => undefined)); + act(() => { vi.advanceTimersByTime(999); }); + expect(result.current.pending).toBe(true); + act(() => { vi.advanceTimersByTime(1); }); + expect(result.current.pending).toBe(false); + }); + + it('clears pending and rethrows when the navigation fails', async () => { + const { result } = renderHook(() => useExternalRedirectPending()); + await act(async () => { + await expect(result.current.run(async () => { throw new Error('no url'); })).rejects.toThrow('no url'); + }); + expect(result.current.pending).toBe(false); + }); + + it('keeps blocking a second request when reset while the first is still running', async () => { + const request = deferred(); + const navigate = vi.fn(() => request.promise); + const { result } = renderHook(() => useExternalRedirectPending()); + let running!: Promise; + act(() => { running = result.current.run(navigate); }); + + act(() => result.current.reset()); + expect(result.current.pending).toBe(true); + await act(() => result.current.run(navigate)); + expect(navigate).toHaveBeenCalledTimes(1); + + await act(async () => { + request.resolve(); + await running; + }); + act(() => result.current.reset()); + expect(result.current.pending).toBe(false); + }); +}); diff --git a/src/hooks/use-external-redirect-pending.ts b/src/hooks/use-external-redirect-pending.ts new file mode 100644 index 000000000..e85fbc55b --- /dev/null +++ b/src/hooks/use-external-redirect-pending.ts @@ -0,0 +1,70 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; + +const SAFETY_TIMEOUT_MS = 8000; + +/** + * Pending state for a button that leaves the site (Google sign-in, Stripe + * Checkout or portal). `run` sets pending, awaits the navigation helper, and + * clears pending if it throws. When the user comes back with Back, the browser + * may restore this page from the back/forward cache with React state frozen, + * so pending is also cleared on a persisted `pageshow`, when the page becomes + * visible again after it hid, and a few seconds after navigation started. + * + * Nothing clears pending while the helper is still running, `reset` included: + * the helper ends in a navigation that can't be cancelled, so releasing the + * button early would let a second click start a second request. + */ +export function useExternalRedirectPending(timeoutMs = SAFETY_TIMEOUT_MS) { + const [pending, setPending] = useState(false); + const pendingRef = useRef(false); + const requestingRef = useRef(false); + const hiddenRef = useRef(false); + const timerRef = useRef>(); + + const clear = useCallback(() => { + clearTimeout(timerRef.current); + pendingRef.current = false; + hiddenRef.current = false; + setPending(false); + }, []); + + const reset = useCallback(() => { + if (!requestingRef.current) clear(); + }, [clear]); + + const run = useCallback(async (navigate: () => Promise) => { + if (pendingRef.current) return; + pendingRef.current = true; + requestingRef.current = true; + setPending(true); + try { + await navigate(); + } catch (error) { + requestingRef.current = false; + clear(); + throw error; + } + requestingRef.current = false; + timerRef.current = setTimeout(clear, timeoutMs); + }, [clear, timeoutMs]); + + useEffect(() => { + const onPageShow = (event: PageTransitionEvent) => { + if (event.persisted && pendingRef.current) reset(); + }; + const onVisibilityChange = () => { + if (!pendingRef.current || requestingRef.current) return; + if (document.visibilityState === 'hidden') hiddenRef.current = true; + else if (hiddenRef.current) clear(); + }; + window.addEventListener('pageshow', onPageShow); + document.addEventListener('visibilitychange', onVisibilityChange); + return () => { + window.removeEventListener('pageshow', onPageShow); + document.removeEventListener('visibilitychange', onVisibilityChange); + clearTimeout(timerRef.current); + }; + }, [clear, reset]); + + return { pending, run, reset }; +} diff --git a/src/pages/LoginPage.tsx b/src/pages/LoginPage.tsx index f07fcf9f7..6d0aebd3c 100644 --- a/src/pages/LoginPage.tsx +++ b/src/pages/LoginPage.tsx @@ -9,6 +9,7 @@ import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; import { ArrowLeft } from 'lucide-react'; import { useAuth } from '@/lib/supabase/AuthProvider'; import { useToast } from '@/hooks/use-toast'; +import { useExternalRedirectPending } from '@/hooks/use-external-redirect-pending'; import { consumeWorkspaceInvite } from '@/lib/workspace/invite-handoff'; import { localizeAuthError } from '@/lib/auth-errors'; import { clearSessionExpiredNotice, sessionExpiredNotice } from '@/lib/supabase/session-expiry'; @@ -31,7 +32,7 @@ function isAuthMode(value: string): value is AuthMode { export default function LoginPage() { const { user, signInWithGoogle, signInWithEmailOtp, verifyEmailOtp, signInWithDemoCode } = useAuth(); - const [isRedirecting, setIsRedirecting] = useState(false); + const { pending: isRedirecting, run: runRedirect } = useExternalRedirectPending(); const [authMode, setAuthMode] = useState('email'); const [authStatus, setAuthStatus] = useState('idle'); const [email, setEmail] = useState(''); @@ -70,11 +71,9 @@ export default function LoginPage() { }, [authStatus, authMode, emailSent]); const handleGoogleSignIn = async () => { - setIsRedirecting(true); try { - await signInWithGoogle(); + await runRedirect(() => signInWithGoogle()); } catch (error) { - setIsRedirecting(false); toast({ title: t('login.failed', 'Could not start sign in'), description: localizeAuthError(error, t, 'login.tryAgain', 'Please try again.'), diff --git a/src/pages/SettingsPage.tsx b/src/pages/SettingsPage.tsx index 5ddf2a2df..4f6042d66 100644 --- a/src/pages/SettingsPage.tsx +++ b/src/pages/SettingsPage.tsx @@ -25,6 +25,7 @@ import { supabase } from '@/lib/supabase/client'; import { getActiveEnv } from '@/lib/supabase/env'; import { useToast } from '@/hooks/use-toast'; import { useSubscription } from '@/hooks/use-subscription'; +import { useExternalRedirectPending } from '@/hooks/use-external-redirect-pending'; import { redirectToPortal } from '@/lib/stripe'; import { deleteAccount, downloadAccountData } from '@/lib/account'; import { PricingModal } from '@/components/features/subscription/PricingModal'; @@ -80,7 +81,7 @@ export default function SettingsPage() { const signInMethod = signInMethodKey(user); const subscription = useSubscription(); const [pricingOpen, setPricingOpen] = useState(false); - const [portalLoading, setPortalLoading] = useState(false); + const { pending: portalLoading, run: runPortalRedirect } = useExternalRedirectPending(); const [deleteDialogOpen, setDeleteDialogOpen] = useState(false); const [deleteEmail, setDeleteEmail] = useState(''); const [exporting, setExporting] = useState(false); @@ -226,9 +227,8 @@ export default function SettingsPage() { }; const handleManageSubscription = async () => { - setPortalLoading(true); try { - await redirectToPortal(); + await runPortalRedirect(redirectToPortal); } catch (err) { const message = err instanceof Error ? err.message : 'Failed to open billing portal'; if (message.toLowerCase().includes('no active subscription')) { @@ -244,7 +244,6 @@ export default function SettingsPage() { variant: 'destructive', }); } - setPortalLoading(false); } }; diff --git a/src/test/auth/login-google-redirect.test.tsx b/src/test/auth/login-google-redirect.test.tsx new file mode 100644 index 000000000..49a1e3f86 --- /dev/null +++ b/src/test/auth/login-google-redirect.test.tsx @@ -0,0 +1,41 @@ +import { describe, expect, it, vi } from 'vitest'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import LoginPage from '@/pages/LoginPage'; +import { restoreFromBackForwardCache } from '@/test/helpers/page-lifecycle'; + +const auth = vi.hoisted(() => ({ + // Resolves once location.assign has been called, like the real one. + signInWithGoogle: vi.fn(async () => undefined), +})); + +vi.mock('@/lib/supabase/AuthProvider', () => ({ + useAuth: () => ({ + user: null as null, + signInWithGoogle: auth.signInWithGoogle, + signInWithEmailOtp: vi.fn(), + verifyEmailOtp: vi.fn(), + signInWithDemoCode: vi.fn(), + }), +})); + +// AUTH-15: Back from Google's account chooser restored /login from the +// back/forward cache with the button stuck on "Redirecting...". +describe('Continue with Google', () => { + it('works again after Back from Google restores the page', async () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Continue with Google' })); + const redirecting = await screen.findByRole('button', { name: 'Redirecting...' }); + expect(redirecting).toBeDisabled(); + fireEvent.click(redirecting); + expect(auth.signInWithGoogle).toHaveBeenCalledTimes(1); + + restoreFromBackForwardCache(); + const button = screen.getByRole('button', { name: 'Continue with Google' }); + expect(button).toBeEnabled(); + + fireEvent.click(button); + await waitFor(() => expect(auth.signInWithGoogle).toHaveBeenCalledTimes(2)); + }); +}); diff --git a/src/test/billing/pricing-modal-redirect.test.tsx b/src/test/billing/pricing-modal-redirect.test.tsx new file mode 100644 index 000000000..6e03f1eec --- /dev/null +++ b/src/test/billing/pricing-modal-redirect.test.tsx @@ -0,0 +1,64 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { PricingModal } from '@/components/features/subscription/PricingModal'; +import { restoreFromBackForwardCache } from '@/test/helpers/page-lifecycle'; + +const stripe = vi.hoisted(() => ({ + redirectToCheckout: vi.fn(async (_promoCode?: string) => undefined), + redeemPromoCode: vi.fn(), +})); + +vi.mock('@/lib/stripe', () => stripe); +vi.mock('@/hooks/use-subscription', () => ({ useSubscription: () => ({ tier: 'trial' }) })); +vi.mock('@/hooks/use-toast', () => ({ useToast: () => ({ toast: vi.fn() }) })); + +function renderModal(open = true) { + const props = { onOpenChange: vi.fn() }; + const view = render(); + return { ...view, reopen: (next: boolean) => view.rerender() }; +} + +async function startCheckout() { + fireEvent.click(screen.getByRole('button', { name: 'Upgrade Now' })); + const redirecting = await screen.findByRole('button', { name: 'Redirecting...' }); + expect(redirecting).toBeDisabled(); +} + +// AUTH-16: Back from Stripe Checkout left Upgrade Now disabled, and closing +// and reopening the dialog did not clear it. +describe('PricingModal Upgrade Now', () => { + beforeEach(() => stripe.redirectToCheckout.mockClear()); + + it('works again after Back from Checkout restores the page', async () => { + renderModal(); + await startCheckout(); + fireEvent.click(screen.getByRole('button', { name: 'Redirecting...' })); + expect(stripe.redirectToCheckout).toHaveBeenCalledTimes(1); + + restoreFromBackForwardCache(); + fireEvent.click(screen.getByRole('button', { name: 'Upgrade Now' })); + await waitFor(() => expect(stripe.redirectToCheckout).toHaveBeenCalledTimes(2)); + }); + + it('shows a fresh button after the dialog is closed and reopened', async () => { + const { reopen } = renderModal(); + await startCheckout(); + + reopen(false); + reopen(true); + expect(await screen.findByRole('button', { name: 'Upgrade Now' })).toBeEnabled(); + }); + + it('sends one checkout request when reopened while the first is still loading', async () => { + let finish!: () => void; + stripe.redirectToCheckout.mockImplementationOnce(() => new Promise((resolve) => { finish = () => resolve(undefined); })); + const { reopen } = renderModal(); + await startCheckout(); + + reopen(false); + reopen(true); + fireEvent.click(await screen.findByRole('button', { name: 'Redirecting...' })); + expect(stripe.redirectToCheckout).toHaveBeenCalledTimes(1); + finish(); + }); +}); diff --git a/src/test/helpers/page-lifecycle.ts b/src/test/helpers/page-lifecycle.ts new file mode 100644 index 000000000..4909c05a6 --- /dev/null +++ b/src/test/helpers/page-lifecycle.ts @@ -0,0 +1,22 @@ +import { act } from '@testing-library/react'; + +/** What a browser fires when it restores a page from the back/forward cache. */ +export function restoreFromBackForwardCache() { + const event = new Event('pageshow'); + Object.defineProperty(event, 'persisted', { value: true }); + act(() => { + window.dispatchEvent(event); + }); +} + +export function setVisibility(state: DocumentVisibilityState) { + Object.defineProperty(document, 'visibilityState', { configurable: true, get: () => state }); + act(() => { + document.dispatchEvent(new Event('visibilitychange')); + }); +} + +/** Drops the override from setVisibility so jsdom's own getter applies again. */ +export function restoreVisibility() { + delete (document as { visibilityState?: unknown }).visibilityState; +}