From 33db4ea7cb827af4521d43faa7ca05a015516d39 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 17:29:44 +0200 Subject: [PATCH] fix(dashboard): the desktop currency widget always shows the rate for the currency picked last Switching the from currency of the desktop dashboard currency widget quickly could show the rate of the previous currency: when the older rate request answered after the newer one, its answer overwrote the newer rates. Cause: useCurrencyConverter only aborted a request still in flight in its phone mode (abortStale true); the desktop dashboard passed false and fetched without any stale guard. Fix: the hook now always aborts the previous request when a new one starts and when the widget goes away, and ignores answers of an aborted request. The abortStale option only existed to keep the desktop without the guard, so it is removed and both dashboards call useCurrencyConverter(). Red test: FE-COMP-FXHOOK-008 (useCurrencyConverter.test.ts) failed on the unfixed code with "AssertionError: expected 1.1 to be 1.3". Changed assertions: - useCurrencyConverter.test.ts FE-COMP-FXHOOK-002: was "the desktop mode fetches without a signal, the phone mode with one" expecting the desktop call to be [url] only; now expects [url, { signal: AbortSignal }]. - FE-COMP-FXHOOK-003 no longer names the phone mode; its assertions are unchanged. All tests call the hook without the removed option. --- .../Dashboard/useCurrencyConverter.test.ts | 51 +++++++++++++------ .../Dashboard/useCurrencyConverter.ts | 30 +++++------ .../mobile/screens/dashboard/MDashWidgets.tsx | 2 +- client/src/pages/DashboardPage.tsx | 2 +- 4 files changed, 50 insertions(+), 35 deletions(-) diff --git a/client/src/components/Dashboard/useCurrencyConverter.test.ts b/client/src/components/Dashboard/useCurrencyConverter.test.ts index d55521078..dff4017a1 100644 --- a/client/src/components/Dashboard/useCurrencyConverter.test.ts +++ b/client/src/components/Dashboard/useCurrencyConverter.test.ts @@ -1,4 +1,4 @@ -// FE-COMP-FXHOOK-001 to -007: the currency converter both dashboards share. +// FE-COMP-FXHOOK-001 to -008: the currency converter both dashboards share. import { act, renderHook, waitFor } from '@testing-library/react'; import { useSettingsStore } from '../../store/settingsStore'; @@ -39,7 +39,7 @@ afterEach(() => { describe('useCurrencyConverter', () => { it('FE-COMP-FXHOOK-001: defaults to EUR to USD and converts the amount with the fetched rate', async () => { - const { result } = renderHook(() => useCurrencyConverter({ abortStale: false })); + const { result } = renderHook(() => useCurrencyConverter()); expect(result.current.from).toBe('EUR'); expect(result.current.to).toBe('USD'); expect(result.current.currencies).toBe(CURRENCIES); @@ -50,19 +50,18 @@ describe('useCurrencyConverter', () => { expect(result.current.converted).toBeCloseTo(2.75); }); - it('FE-COMP-FXHOOK-002: the desktop mode fetches without a signal, the phone mode with one', async () => { - renderHook(() => useCurrencyConverter({ abortStale: false })); + it('FE-COMP-FXHOOK-002: both dashboards fetch the rates with an abort signal', async () => { + renderHook(() => useCurrencyConverter()); await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); - expect(fetchMock.mock.calls[0]).toEqual(['https://api.frankfurter.dev/v2/rates?base=EUR']); - - renderHook(() => useCurrencyConverter({ abortStale: true })); - await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); - expect(fetchMock.mock.calls[1][1]).toEqual({ signal: expect.any(AbortSignal) }); + expect(fetchMock.mock.calls[0]).toEqual([ + 'https://api.frankfurter.dev/v2/rates?base=EUR', + { signal: expect.any(AbortSignal) }, + ]); }); - it('FE-COMP-FXHOOK-003: the phone mode aborts the request on unmount and on a refetch', async () => { + it('FE-COMP-FXHOOK-003: it aborts the request on unmount and on a refetch', async () => { fetchMock.mockImplementation(() => new Promise(() => {})); - const { result, unmount } = renderHook(() => useCurrencyConverter({ abortStale: true })); + const { result, unmount } = renderHook(() => useCurrencyConverter()); const first = (fetchMock.mock.calls[0][1] as { signal: AbortSignal }).signal; act(() => result.current.fetchRates()); expect(first.aborted).toBe(true); @@ -73,20 +72,20 @@ describe('useCurrencyConverter', () => { it('FE-COMP-FXHOOK-004: a failed or malformed answer leaves no rate', async () => { fetchMock.mockImplementationOnce(() => Promise.reject(new Error('offline'))); - const failed = renderHook(() => useCurrencyConverter({ abortStale: false })); + const failed = renderHook(() => useCurrencyConverter()); await waitFor(() => expect(fetchMock).toHaveBeenCalled()); expect(failed.result.current.rate).toBeNull(); expect(failed.result.current.converted).toBeNull(); fetchMock.mockImplementationOnce(() => respond({ error: 'nope' })); - const malformed = renderHook(() => useCurrencyConverter({ abortStale: true })); + const malformed = renderHook(() => useCurrencyConverter()); await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); expect(malformed.result.current.rate).toBeNull(); }); it('FE-COMP-FXHOOK-005: picking and swapping write the pair to the settings', () => { seedSettings({ dashboard_fx_from: 'GBP', dashboard_fx_to: 'JPY' }); - const { result } = renderHook(() => useCurrencyConverter({ abortStale: false })); + const { result } = renderHook(() => useCurrencyConverter()); act(() => result.current.swap()); expect(updateSetting).toHaveBeenCalledWith('dashboard_fx_from', 'JPY'); expect(updateSetting).toHaveBeenCalledWith('dashboard_fx_to', 'GBP'); @@ -98,7 +97,7 @@ describe('useCurrencyConverter', () => { localStorage.setItem('trek_fx_from', 'CAD'); localStorage.setItem('trek_fx_to', 'AUD'); seedSettings({}, true); - renderHook(() => useCurrencyConverter({ abortStale: false })); + renderHook(() => useCurrencyConverter()); expect(updateSetting).toHaveBeenCalledWith('dashboard_fx_from', 'CAD'); expect(updateSetting).toHaveBeenCalledWith('dashboard_fx_to', 'AUD'); await waitFor(() => expect(localStorage.getItem('trek_fx_from')).toBeNull()); @@ -109,9 +108,29 @@ describe('useCurrencyConverter', () => { localStorage.setItem('trek_fx_from', 'CAD'); updateSetting.mockRejectedValue(new Error('down')); seedSettings({}, true); - renderHook(() => useCurrencyConverter({ abortStale: false })); + renderHook(() => useCurrencyConverter()); await waitFor(() => expect(updateSetting).toHaveBeenCalled()); await Promise.resolve(); expect(localStorage.getItem('trek_fx_from')).toBe('CAD'); }); + + it('FE-COMP-FXHOOK-008: a late answer for the previous from currency never overwrites the newer one', async () => { + let resolveEur: (body: unknown) => void = () => {}; + fetchMock.mockImplementation((url: string) => + url.endsWith('base=EUR') + ? new Promise((resolve) => { + resolveEur = (body) => resolve({ json: () => Promise.resolve(body) } as Response); + }) + : respond([{ quote: 'USD', rate: 1.3 }]) + ); + const { result } = renderHook(() => useCurrencyConverter()); + act(() => seedSettings({ dashboard_fx_from: 'GBP' })); + await waitFor(() => expect(result.current.rate).toBe(1.3)); + await act(async () => { + resolveEur([{ quote: 'USD', rate: 1.1 }]); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(result.current.rate).toBe(1.3); + }); }); diff --git a/client/src/components/Dashboard/useCurrencyConverter.ts b/client/src/components/Dashboard/useCurrencyConverter.ts index 623b96584..c99c9b40f 100644 --- a/client/src/components/Dashboard/useCurrencyConverter.ts +++ b/client/src/components/Dashboard/useCurrencyConverter.ts @@ -7,10 +7,11 @@ import { CURRENCIES } from '../Budget/BudgetPanel.constants'; * The dashboard's currency converter: the pair lives in the user's settings, the * rates come from Frankfurter for the `from` currency. * - * `abortStale` (the phone widget) aborts a request still in flight when a new one - * starts or the widget goes away, so a late answer never lands on a gone tree. + * A request still in flight is aborted when a new one starts or the widget goes + * away, so a late answer for an older `from` never overwrites a newer one and never + * lands on a gone tree. */ -export function useCurrencyConverter({ abortStale }: { abortStale: boolean }) { +export function useCurrencyConverter() { const isLoaded = useSettingsStore((s) => s.isLoaded); const updateSetting = useSettingsStore((s) => s.updateSetting); const from = useSettingsStore((s) => s.settings.dashboard_fx_from) || 'EUR'; @@ -26,18 +27,14 @@ export function useCurrencyConverter({ abortStale }: { abortStale: boolean }) { const inFlight = useRef(null); const fetchRates = useCallback(() => { - let signal: AbortSignal | undefined; - if (abortStale) { - inFlight.current?.abort(); - const controller = new AbortController(); - inFlight.current = controller; - signal = controller.signal; - } - const url = `https://api.frankfurter.dev/v2/rates?base=${from}`; - (signal ? fetch(url, { signal }) : fetch(url)) + inFlight.current?.abort(); + const controller = new AbortController(); + inFlight.current = controller; + const { signal } = controller; + fetch(`https://api.frankfurter.dev/v2/rates?base=${from}`, { signal }) .then((r) => r.json()) .then((d: Array<{ quote: string; rate: number }>) => { - if (signal?.aborted) return; + if (signal.aborted) return; if (!Array.isArray(d)) { setRates(null); return; @@ -49,16 +46,15 @@ export function useCurrencyConverter({ abortStale }: { abortStale: boolean }) { }) .catch(() => { // An abort is not a failure: it means nobody is waiting for the answer. - if (!signal?.aborted) setRates(null); + if (!signal.aborted) setRates(null); }); - }, [from, abortStale]); + }, [from]); useEffect(() => { fetchRates(); - if (!abortStale) return; const pending = inFlight; return () => pending.current?.abort(); - }, [fetchRates, abortStale]); + }, [fetchRates]); // One-time migration of the pre-3.1.3 localStorage values into the user's settings, // so a (docker) upgrade no longer resets the widget (#1311). diff --git a/client/src/mobile/screens/dashboard/MDashWidgets.tsx b/client/src/mobile/screens/dashboard/MDashWidgets.tsx index cef1c7cd2..5cf89cac5 100644 --- a/client/src/mobile/screens/dashboard/MDashWidgets.tsx +++ b/client/src/mobile/screens/dashboard/MDashWidgets.tsx @@ -101,7 +101,7 @@ function MCurrencyWidget(): React.ReactElement { const { t } = useTranslation() // The request outlives the widget: this is a third-party endpoint on a mobile // screen a user can leave immediately, so a pending one is aborted on the way out. - const { from, to, setFrom, setTo, amount, setAmount, currencies, rate, converted, swap, fetchRates } = useCurrencyConverter({ abortStale: true }) + const { from, to, setFrom, setTo, amount, setAmount, currencies, rate, converted, swap, fetchRates } = useCurrencyConverter() return ( ({ value: c, label: c })) return (