From 2b20b389b7cec30f3c54807e09b8de398c609f26 Mon Sep 17 00:00:00 2001 From: lobo Date: Mon, 5 Oct 2026 15:37:16 +0200 Subject: [PATCH] refactor(hooks): move import polling and settings load into hooks useImportPolling and useSettingsLoad take the effects out of ImportView and SettingsPage, line for line. useSettingsLoad no longer sets state after unmount. Tests for both hooks. Issue #6. --- src/components/import/ImportView.jsx | 74 +----------------- src/hooks/useImportPolling.js | 77 ++++++++++++++++++ src/hooks/useImportPolling.test.jsx | 112 +++++++++++++++++++++++++++ src/hooks/useSettingsLoad.js | 39 ++++++++++ src/hooks/useSettingsLoad.test.jsx | 83 ++++++++++++++++++++ src/pages/SettingsPage.jsx | 26 +------ 6 files changed, 318 insertions(+), 93 deletions(-) create mode 100644 src/hooks/useImportPolling.js create mode 100644 src/hooks/useImportPolling.test.jsx create mode 100644 src/hooks/useSettingsLoad.js create mode 100644 src/hooks/useSettingsLoad.test.jsx diff --git a/src/components/import/ImportView.jsx b/src/components/import/ImportView.jsx index 6bfe4b0..312359e 100644 --- a/src/components/import/ImportView.jsx +++ b/src/components/import/ImportView.jsx @@ -1,4 +1,4 @@ -import {useEffect, useRef, useState} from 'react'; +import {useState} from 'react'; import {useTranslation} from 'react-i18next'; import {toast} from 'sonner'; import {CheckCircle2, Circle, Loader2, RotateCcw, XCircle} from 'lucide-react'; @@ -8,82 +8,16 @@ import {Card} from '@/components/ui/card'; import {cn} from '@/lib/utils'; import ImportReview from './ImportReview.jsx'; import {useCurrentUserContext} from '../../context/CurrentUserContext.jsx'; -import {getImport, retryImport, RUNNING, STEPS, stepOf} from '../../utils/imports.js'; +import useImportPolling from '../../hooks/useImportPolling.js'; +import {retryImport, RUNNING, STEPS, stepOf} from '../../utils/imports.js'; -const POLL_MS = 2000; -const MAX_BACKOFF_MS = 30_000; -const MAX_FAILURES = 10; - -// Polls while a worker has the import. export default function ImportView({id, onDone, onChanged}) { const {t} = useTranslation(); - const [data, setData] = useState(null); - const [failed, setFailed] = useState(false); - // Grows after a save or a retry: the import is back with a worker, so polling starts again. - const [version, setVersion] = useState(0); - const reload = () => setVersion((current) => current + 1); - const lastStatus = useRef(null); + const {data, failed, reload} = useImportPolling(id, onChanged); // undefined while loading: the review preselects with it, so it waits. const {user} = useCurrentUserContext(); const favorite = user === undefined ? undefined : user?.settings?.favoritePlatform ?? null; - useEffect(() => { - let timer; - let cancelled = false; - let failures = 0; - let paused = false; - // A hidden tab waits for the visibilitychange instead of polling. - const poll = () => { - if (document.hidden) paused = true; - else load(); - }; - const onVisible = () => { - if (!document.hidden && paused) { - paused = false; - load(); - } - }; - // A 5xx while Core restarts or a network error is asked again, each time twice as late. - const retry = () => { - if (++failures >= MAX_FAILURES) setFailed(true); - else timer = setTimeout(poll, Math.min(POLL_MS * 2 ** failures, MAX_BACKOFF_MS)); - }; - const load = async () => { - try { - const response = await getImport(id); - if (cancelled) return; - if (response.status === 401) return; - // A 4xx ends the polling. - if (response.status >= 400 && response.status < 500) { - setFailed(true); - return; - } - if (!response.ok) { - retry(); - return; - } - const next = await response.json(); - if (cancelled) return; - failures = 0; - // A new status also shows in the history. - if (lastStatus.current && lastStatus.current !== next.status) onChanged(); - lastStatus.current = next.status; - setData(next); - if (RUNNING.includes(next.status)) timer = setTimeout(poll, POLL_MS); - } catch (err) { - console.error('Import load error:', err); - if (!cancelled) retry(); - } - }; - document.addEventListener('visibilitychange', onVisible); - load(); - return () => { - cancelled = true; - clearTimeout(timer); - document.removeEventListener('visibilitychange', onVisible); - }; - }, [id, version, onChanged]); - if (failed) return

{t('IMPORT.LOAD_FAILED')}

; if (!data || favorite === undefined) return ; return ( diff --git a/src/hooks/useImportPolling.js b/src/hooks/useImportPolling.js new file mode 100644 index 0000000..311d189 --- /dev/null +++ b/src/hooks/useImportPolling.js @@ -0,0 +1,77 @@ +import {useEffect, useRef, useState} from 'react'; +import {getImport, RUNNING} from '../utils/imports.js'; + +const POLL_MS = 2000; +const MAX_BACKOFF_MS = 30_000; +const MAX_FAILURES = 10; + +/** + * Polls one import while a worker has it. `onChanged` runs when the status changes. + */ +export default function useImportPolling(id, onChanged) { + const [data, setData] = useState(null); + const [failed, setFailed] = useState(false); + // Grows after a save or a retry: the import is back with a worker, so polling starts again. + const [version, setVersion] = useState(0); + const reload = () => setVersion((current) => current + 1); + const lastStatus = useRef(null); + + useEffect(() => { + let timer; + let cancelled = false; + let failures = 0; + let paused = false; + // A hidden tab waits for the visibilitychange instead of polling. + const poll = () => { + if (document.hidden) paused = true; + else load(); + }; + const onVisible = () => { + if (!document.hidden && paused) { + paused = false; + load(); + } + }; + // A 5xx while Core restarts or a network error is asked again, each time twice as late. + const retry = () => { + if (++failures >= MAX_FAILURES) setFailed(true); + else timer = setTimeout(poll, Math.min(POLL_MS * 2 ** failures, MAX_BACKOFF_MS)); + }; + const load = async () => { + try { + const response = await getImport(id); + if (cancelled) return; + if (response.status === 401) return; + // A 4xx ends the polling. + if (response.status >= 400 && response.status < 500) { + setFailed(true); + return; + } + if (!response.ok) { + retry(); + return; + } + const next = await response.json(); + if (cancelled) return; + failures = 0; + // A new status also shows in the history. + if (lastStatus.current && lastStatus.current !== next.status) onChanged(); + lastStatus.current = next.status; + setData(next); + if (RUNNING.includes(next.status)) timer = setTimeout(poll, POLL_MS); + } catch (err) { + console.error('Import load error:', err); + if (!cancelled) retry(); + } + }; + document.addEventListener('visibilitychange', onVisible); + load(); + return () => { + cancelled = true; + clearTimeout(timer); + document.removeEventListener('visibilitychange', onVisible); + }; + }, [id, version, onChanged]); + + return {data, failed, reload}; +} diff --git a/src/hooks/useImportPolling.test.jsx b/src/hooks/useImportPolling.test.jsx new file mode 100644 index 0000000..cbe2949 --- /dev/null +++ b/src/hooks/useImportPolling.test.jsx @@ -0,0 +1,112 @@ +// @vitest-environment jsdom +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {act, cleanup, renderHook} from '@testing-library/react'; +import useImportPolling from './useImportPolling.js'; +import {fetchRoutes, json} from '../test/fetchRoutes.js'; + +const ROUTE = 'GET /api/library/imports/7'; +// A stable callback, as ImportPage passes: a new one per render would restart the polling. +const noop = () => {}; +const tick = (ms) => act(() => vi.advanceTimersByTimeAsync(ms)); + +beforeEach(() => { + vi.useFakeTimers(); + vi.spyOn(console, 'error').mockImplementation(() => {}); + localStorage.setItem('tbp_session', '1'); +}); + +afterEach(() => { + cleanup(); + vi.useRealTimers(); + vi.restoreAllMocks(); + localStorage.clear(); +}); + +describe('useImportPolling', () => { + it('polls every 2000 ms while the status is running', async () => { + const fetch = fetchRoutes({[ROUTE]: json(200, {status: 'MATCHING', matched: 1})}); + const {result} = renderHook(() => useImportPolling(7, noop)); + await tick(0); + expect(fetch).toHaveBeenCalledTimes(1); + expect(result.current.data.matched).toBe(1); + + await tick(2000); + expect(fetch).toHaveBeenCalledTimes(2); + await tick(2000); + expect(fetch).toHaveBeenCalledTimes(3); + }); + + it('stops after DONE and reports the change once', async () => { + let answer = json(200, {status: 'MATCHING'}); + const fetch = fetchRoutes({[ROUTE]: () => answer}); + const onChanged = vi.fn(); + const {result} = renderHook(() => useImportPolling(7, onChanged)); + await tick(0); + answer = json(200, {status: 'DONE'}); + await tick(2000); + const calls = fetch.mock.calls.length; + + await tick(30_000); + + expect(fetch.mock.calls.length).toBe(calls); + expect(result.current.data.status).toBe('DONE'); + expect(onChanged).toHaveBeenCalledTimes(1); + }); + + it('backs off on 503 and gives up after 10 failures', async () => { + const fetch = fetchRoutes({[ROUTE]: json(503, {})}); + const {result} = renderHook(() => useImportPolling(7, noop)); + await tick(0); + expect(fetch).toHaveBeenCalledTimes(1); + + await tick(3999); + expect(fetch).toHaveBeenCalledTimes(1); + await tick(1); + expect(fetch).toHaveBeenCalledTimes(2); + await tick(7999); + expect(fetch).toHaveBeenCalledTimes(2); + await tick(1); + expect(fetch).toHaveBeenCalledTimes(3); + + await tick(300_000); + expect(fetch).toHaveBeenCalledTimes(10); + expect(result.current.failed).toBe(true); + }); + + it('does not call again after unmount with a timer pending', async () => { + const fetch = fetchRoutes({[ROUTE]: json(200, {status: 'MATCHING'})}); + const {unmount} = renderHook(() => useImportPolling(7, noop)); + await tick(0); + + unmount(); + await tick(10_000); + + expect(fetch).toHaveBeenCalledTimes(1); + }); + + it('does not call again after unmount with a request pending', async () => { + let release; + const fetch = fetchRoutes({[ROUTE]: () => new Promise((resolve) => { release = resolve; })}); + const {unmount} = renderHook(() => useImportPolling(7, noop)); + await tick(0); + + unmount(); + release(json(200, {status: 'MATCHING'})); + await tick(10_000); + + expect(fetch).toHaveBeenCalledTimes(1); + }); + + it('reload() restarts the loop: one poll per 2000 ms, not two', async () => { + const fetch = fetchRoutes({[ROUTE]: json(200, {status: 'MATCHING'})}); + const {result} = renderHook(() => useImportPolling(7, noop)); + await tick(0); + + act(() => result.current.reload()); + await tick(0); + const calls = fetch.mock.calls.length; + await tick(2000); + + expect(fetch.mock.calls.length).toBe(calls + 1); + }); +}); diff --git a/src/hooks/useSettingsLoad.js b/src/hooks/useSettingsLoad.js new file mode 100644 index 0000000..dccba86 --- /dev/null +++ b/src/hooks/useSettingsLoad.js @@ -0,0 +1,39 @@ +import {useEffect, useState} from 'react'; +import {useCurrentUserContext} from '../context/CurrentUserContext.jsx'; +import {refreshSession} from '../utils/api.js'; +import {markSession} from '../utils/auth.js'; + +/** + * Loads the profile for the settings page. After a Steam link (`steamOutcome === 'linked'`) the session is refreshed + * first. `loadError` is a translation key. + */ +export default function useSettingsLoad(steamOutcome) { + const {reload: reloadUser} = useCurrentUserContext(); + const [isLoading, setIsLoading] = useState(true); + const [loadError, setLoadError] = useState(null); + // From this load: the context user can still be empty when the form mounts. + const [displayName, setDisplayName] = useState(''); + + useEffect(() => { + let cancelled = false; + const load = async () => { + if (steamOutcome === 'linked') { + // Refresh reads the new Steam ID from the database, so Core gets it in the token at once. + await refreshSession().catch(() => false); + markSession(); + } + const {ok, status, data} = await reloadUser(); + if (cancelled) return; + if (ok) setDisplayName(data.displayName ?? ''); + else if (status !== 401) setLoadError(status === 0 ? 'INFO.ERROR.NETWORK_FAILURE' : 'INFO.ERROR.LOAD_PROFILE'); + setIsLoading(false); + }; + + load(); + return () => { + cancelled = true; + }; + }, [reloadUser, steamOutcome]); + + return {isLoading, loadError, displayName}; +} diff --git a/src/hooks/useSettingsLoad.test.jsx b/src/hooks/useSettingsLoad.test.jsx new file mode 100644 index 0000000..a4bb1dc --- /dev/null +++ b/src/hooks/useSettingsLoad.test.jsx @@ -0,0 +1,83 @@ +// @vitest-environment jsdom +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {cleanup, renderHook, waitFor} from '@testing-library/react'; +import useSettingsLoad from './useSettingsLoad.js'; +import {CurrentUserProvider} from '../context/CurrentUserContext.jsx'; +import {fetchRoutes, json} from '../test/fetchRoutes.js'; + +const renderLoad = (steamOutcome = null) => + renderHook(() => useSettingsLoad(steamOutcome), {wrapper: CurrentUserProvider}); + +beforeEach(() => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + localStorage.setItem('tbp_session', '1'); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + localStorage.clear(); +}); + +describe('useSettingsLoad', () => { + it('returns the display name when /me answers', async () => { + fetchRoutes({'GET /api/users/me': json(200, {displayName: 'Lobo', settings: {}})}); + const {result} = renderLoad(); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.displayName).toBe('Lobo'); + expect(result.current.loadError).toBeNull(); + }); + + it('reports a server error', async () => { + fetchRoutes({'GET /api/users/me': json(500, {})}); + const {result} = renderLoad(); + + await waitFor(() => expect(result.current.loadError).toBe('INFO.ERROR.LOAD_PROFILE')); + expect(result.current.isLoading).toBe(false); + }); + + it('reports a network failure', async () => { + globalThis.fetch = vi.fn().mockRejectedValue(new TypeError('network')); + const {result} = renderLoad(); + + await waitFor(() => expect(result.current.loadError).toBe('INFO.ERROR.NETWORK_FAILURE')); + }); + + it('shows no error on 401', async () => { + fetchRoutes({'GET /api/users/me': json(401, {}), 'POST /api/auth/refresh': json(401, {})}); + const {result} = renderLoad(); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.loadError).toBeNull(); + }); + + it('refreshes the session before /me after a Steam link', async () => { + const fetch = fetchRoutes({ + 'POST /api/auth/refresh': json(204, null), + 'GET /api/users/me': json(200, {displayName: 'Lobo', settings: {}}), + }); + const {result} = renderLoad('linked'); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + const paths = fetch.mock.calls.map(([url]) => new URL(url, 'http://x').pathname); + expect(paths.findIndex((p) => p.endsWith('/auth/refresh'))).toBeLessThan( + paths.findIndex((p) => p.endsWith('/users/me'))); + }); + it('marksSessionAfterSteamRefreshEvenWhenUnmounted', async () => { + let answerRefresh; + fetchRoutes({ + 'POST /api/auth/refresh': () => new Promise((resolve) => { + answerRefresh = resolve; + }), + 'GET /api/users/me': json(200, {displayName: 'Lobo', settings: {}}), + }); + const {unmount} = renderLoad('linked'); + await waitFor(() => expect(answerRefresh).toBeDefined()); + + unmount(); + answerRefresh(json(204, null)); + + await waitFor(() => expect(localStorage.getItem('tbp_session')).not.toBe('1')); + }); +}); diff --git a/src/pages/SettingsPage.jsx b/src/pages/SettingsPage.jsx index 85e2b60..e74683e 100644 --- a/src/pages/SettingsPage.jsx +++ b/src/pages/SettingsPage.jsx @@ -13,8 +13,7 @@ import PreferencesCard from '../components/PreferencesCard.jsx'; import ProfileCard from '../components/ProfileCard.jsx'; import SteamAccountCard from '../components/SteamAccountCard.jsx'; import {useCurrentUserContext} from '../context/CurrentUserContext.jsx'; -import {refreshSession} from '../utils/api.js'; -import {markSession} from '../utils/auth.js'; +import useSettingsLoad from '../hooks/useSettingsLoad.js'; // The `steam` query parameter Auth adds when "Sign in through Steam" sends the user back. const STEAM_LINK_TOASTS = { @@ -26,14 +25,11 @@ const STEAM_LINK_TOASTS = { export default function SettingsPage() { const {t} = useTranslation(); - const {user: profile, updateUser: updateProfile, reload: reloadUser} = useCurrentUserContext(); - const [isLoading, setIsLoading] = useState(true); - const [loadError, setLoadError] = useState(null); - // From this page's own load: the context user can still be empty when the form mounts. - const [displayName, setDisplayName] = useState(''); + const {user: profile, updateUser: updateProfile} = useCurrentUserContext(); const [searchParams, setSearchParams] = useSearchParams(); // Read once: removing the parameter below must not load the profile again. const [steamOutcome] = useState(() => searchParams.get('steam')); + const {isLoading, loadError, displayName} = useSettingsLoad(steamOutcome); // A language change gives a new `t` and runs the effect again; the toast shows once. const steamToastShown = useRef(false); @@ -48,22 +44,6 @@ export default function SettingsPage() { }, {replace: true}); }, [steamOutcome, setSearchParams, t]); - useEffect(() => { - const load = async () => { - if (steamOutcome === 'linked') { - // Refresh reads the new Steam ID from the database, so Core gets it in the token at once. - await refreshSession().catch(() => false); - markSession(); - } - const {ok, status, data} = await reloadUser(); - if (ok) setDisplayName(data.displayName ?? ''); - else if (status !== 401) setLoadError(status === 0 ? 'INFO.ERROR.NETWORK_FAILURE' : 'INFO.ERROR.LOAD_PROFILE'); - setIsLoading(false); - }; - - load(); - }, [reloadUser, steamOutcome]); - if (isLoading) { return (