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 (