k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(dashboard): the desktop currency widget always shows the rate for the currency picked last

Browse Source 
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.
This commit is contained in:
Maurice
2026-10-09 17:29:44 +02:00
parent 1ad0ef6d78
commit 33db4ea7cb
4 changed files with 50 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/src/components/Dashboard/useCurrencyConverter.test.ts
+35 -16
View File
@@ -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);
});
});
client/src/components/Dashboard/useCurrencyConverter.ts
+13 -17
View File
@@ -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<AbortController | null>(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).
client/src/mobile/screens/dashboard/MDashWidgets.tsx
+1 -1
View File
@@ -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 (
<WidgetPanel
client/src/pages/DashboardPage.tsx
+1 -1
View File
@@ -662,7 +662,7 @@ function TripCard({ trip, locale, badges, matchedPlaces, onOpen, onEdit, onCopy,
// ── Currency tool (self-contained, mirrors the design's fx widget) ───────────
function CurrencyTool(): React.ReactElement {
const { t } = useTranslation()
const { from, to, setFrom, setTo, amount, setAmount, currencies, rate, converted, swap, fetchRates: fetchRate } = useCurrencyConverter({ abortStale: false })
const { from, to, setFrom, setTo, amount, setAmount, currencies, rate, converted, swap, fetchRates: fetchRate } = useCurrencyConverter()
const ccyOptions = currencies.map(c => ({ value: c, label: c }))
return (