From 7f87dd512ded1e10220789313f500daf1073cb0c Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 17 Sep 2026 22:04:01 +0200 Subject: [PATCH] fix(costs): read a foreign-currency expense at the rate it was booked at The server freezes the FX rate when an expense or a settle-up transfer is entered and settles the balances with it, but the client ignored the column and converted every amount with today's rate. A bill paid in dollars was therefore worth something different each time it was opened, which a tester noticed after settling up: the figure moved under him, and the row no longer matched what had actually changed hands. convertBooked applies the server's own rule, in one place for both shells and the transfer rows: an amount already in the trip currency is left to the live step, a frozen rate converts to the trip currency before the live step, and a rate of 1 or none at all is a row written before the freeze existed and keeps converting live. --- .../src/components/Budget/CostsPanel.test.tsx | 20 ++++++++ client/src/components/Budget/CostsPanel.tsx | 30 ++++++++--- client/src/hooks/useExchangeRates.test.ts | 50 ++++++++++++++++++- client/src/hooks/useExchangeRates.ts | 27 ++++++++++ .../mobile/screens/trip/tabs/MCostsTab.tsx | 4 +- .../mobile/screens/trip/tabs/costsModel.ts | 19 +++++-- 6 files changed, 135 insertions(+), 15 deletions(-) diff --git a/client/src/components/Budget/CostsPanel.test.tsx b/client/src/components/Budget/CostsPanel.test.tsx index 577982115..f7d5b5ab5 100644 --- a/client/src/components/Budget/CostsPanel.test.tsx +++ b/client/src/components/Budget/CostsPanel.test.tsx @@ -1185,6 +1185,26 @@ describe('CostsPanel — expense rows', () => { expect(screen.getByText(/\$20\.00 → 10,00 €/)).toBeInTheDocument() }) + it('FE-W5COSTS-078: a booked rate outlives the live one, for the expense and for the transfer', async () => { + // What a tester hit after settling up: the euro moved, and so did a bill that had + // already been paid. A cost is money that changed hands at a rate that was true that + // day, so the frozen rate wins over whatever the market says this morning. + localStorage.setItem('trek_fx_EUR', JSON.stringify({ rates: { EUR: 1, USD: 2 }, ts: Date.now() })) + mount([expense({ + id: 131, name: 'Diner', category: 'food', total_price: 120, currency: 'USD', exchange_rate: 1.2, + expense_date: '2025-06-15', + payers: [{ user_id: 1, amount: 120 }], + members: [{ user_id: 1, username: 'alice' }, { user_id: 2, username: 'bob' }], + })], { settlements: [{ id: 9, from_user_id: 2, to_user_id: 1, amount: 24, currency: 'USD', exchange_rate: 1.2, created_at: '2025-06-15 09:00:00' }] }) + + await screen.findByText('Diner') + // 120 USD at the booked 1.2 per euro is 100 euro. At today's 2 it would read 60,00 euro. + expect(screen.getByText(/\$120\.00 → 100,00 €/)).toBeInTheDocument() + expect(screen.queryByText(/60,00 €/)).toBeNull() + // The settled transfer is read back the same way: 24 USD booked at 1.2 is 20 euro. + expect(screen.getByText(/\$24\.00 → 20,00 €/)).toBeInTheDocument() + }) + it('FE-W5COSTS-022: deleting an expense removes it, and a failure is reported', async () => { const addToast = vi.fn() window.__addToast = addToast as unknown as typeof window.__addToast diff --git a/client/src/components/Budget/CostsPanel.tsx b/client/src/components/Budget/CostsPanel.tsx index c27e5aba8..95e2d5850 100644 --- a/client/src/components/Budget/CostsPanel.tsx +++ b/client/src/components/Budget/CostsPanel.tsx @@ -9,7 +9,7 @@ import { useToast } from '../shared/Toast' import { useTranslation } from '../../i18n' import { budgetApi } from '../../api/client' import { saveWithReceipts } from './receiptUploads' -import { useExchangeRates } from '../../hooks/useExchangeRates' +import { convertBooked, useExchangeRates } from '../../hooks/useExchangeRates' import { useIsMobile } from '../../hooks/useIsMobile' import { formatMoney, currencyDecimals, currencyLocale, localizeAmountInput, amountToInputString } from '../../utils/formatters' import { downloadBlob, openFile } from '../../utils/fileDownload' @@ -41,6 +41,9 @@ interface Settlement { // The currency the transfer was entered in. Legacy rows predate it (null) and are // read as the display currency, which is what the server assumes for them too. currency?: string | null + // The rate frozen when the transfer was settled, in units of `currency` per 1 trip + // currency (#1445). Absent, or exactly 1, on rows written before the freeze existed. + exchange_rate?: number created_at?: string // The day the transfer actually happened; editable, unlike created_at (when it // was recorded). Null/absent on rows predating this field — settlementDate() @@ -135,8 +138,19 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps }, [searchParams]) // ── derived expense maths (everything converted to the base currency) ──── - const baseTotal = (e: BudgetItem) => convert(e.total_price || 0, curOf(e)) - const myPaidOf = (e: BudgetItem) => convert(paidByUser(e, me), curOf(e)) + // Booked, not live: an expense entered in a foreign currency keeps the rate it was + // entered at, which is the same rule the server settles by (#1335). + const booked = useCallback( + (amount: number, e: BudgetItem) => convertBooked(amount, e.currency, e.exchange_rate, tripCurrency, convert), + [convert, tripCurrency], + ) + const baseTotal = (e: BudgetItem) => booked(e.total_price || 0, e) + // A transfer freezes its own rate at settle time, in its own table (#1445). + const settled = useCallback( + (s: Settlement) => convertBooked(s.amount, s.currency, s.exchange_rate, tripCurrency, convert), + [convert, tripCurrency], + ) + const myPaidOf = (e: BudgetItem) => booked(paidByUser(e, me), e) // "Unfinished": a recorded total nobody has paid yet — counts toward the trip // total but stays out of settlements until who-paid is filled in. A negative // total (a refund, #2176) is just as unfinished until its recipient is named. @@ -148,11 +162,11 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps const myMember = (e.members || []).find(m => m.user_id === me) if (!myMember) return 0 if (myMember.amount !== null && myMember.amount !== undefined) { - return convert(myMember.amount, curOf(e)) + return booked(myMember.amount, e) } const shares = splitEqualShares(e.total_price || 0, e.members || [], e.id) const myShare = shares[me] || 0 - return convert(myShare, curOf(e)) + return booked(myShare, e) } const totals = useMemo(() => { @@ -835,7 +849,7 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps {payers.map(p => ( - {fmt(convert(p.amount, cur))} + {fmt(booked(p.amount, e))} ))} @@ -887,7 +901,7 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps
{t('costs.payment')} - {cur !== base && · {fmt(s.amount, cur)} → {fmt(convert(s.amount, cur))}} + {cur !== base && · {fmt(s.amount, cur)} → {fmt(settled(s))}}
@@ -898,7 +912,7 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps on the same axis as every expense above it. */} {!isMobile && }
- {fmt(convert(s.amount, cur))} + {fmt(settled(s))}
{canEdit && ( diff --git a/client/src/hooks/useExchangeRates.test.ts b/client/src/hooks/useExchangeRates.test.ts index de6d3904c..45eaac209 100644 --- a/client/src/hooks/useExchangeRates.test.ts +++ b/client/src/hooks/useExchangeRates.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, beforeEach } from 'vitest' import { http, HttpResponse } from 'msw' import { server } from '../../tests/helpers/msw/server' -import { fetchExchangeRates, clearExchangeRateCache } from './useExchangeRates' +import { convertBooked, fetchExchangeRates, clearExchangeRateCache } from './useExchangeRates' const FX_URL = 'https://api.frankfurter.dev/v2/rates' @@ -45,3 +45,51 @@ describe('fetchExchangeRates (#1561)', () => { expect(await fetchExchangeRates('NOK')).toEqual({ NOK: 1, USD: 0.1 }) }) }) + +// A cost entered in a foreign currency is money that was actually paid, at a rate that +// was true that day. Reading it back at today's rate quietly rewrites history, which is +// what a tester reported after settling up: the figure moved under him. +describe('convertBooked', () => { + // Display currency is EUR, so rates are units per 1 EUR. The trip is booked in EUR too + // unless a test says otherwise. + const live = (amount: number, from: string | null | undefined): number => { + const rates: Record = { EUR: 1, USD: 2, SEK: 10 } + const r = rates[(from || 'EUR').toUpperCase()] + return r && r > 0 ? amount / r : amount + } + + it('leaves an amount already in the trip currency to the live step alone', () => { + // Nothing was ever frozen here, so this is just trip currency to display currency. + expect(convertBooked(100, null, 1, 'EUR', live)).toBe(100) + expect(convertBooked(100, 'EUR', 1, 'EUR', live)).toBe(100) + }) + + it('reads a foreign amount at the rate it was booked at, not at today s', () => { + // 120 USD booked when a euro bought 1.2 dollars is 100 EUR, and stays 100 EUR even + // though the live table above says a euro now buys 2. + expect(convertBooked(120, 'USD', 1.2, 'EUR', live)).toBe(100) + }) + + it('falls back to live rates for a row that never froze one', () => { + // Rate 1 is the column default, not a booked rate: rows predating the freeze carry it. + expect(convertBooked(120, 'USD', 1, 'EUR', live)).toBe(60) + expect(convertBooked(120, 'USD', undefined, 'EUR', live)).toBe(60) + expect(convertBooked(120, 'USD', null, 'EUR', live)).toBe(60) + // A rate that cannot divide is no rate at all. + expect(convertBooked(120, 'USD', 0, 'EUR', live)).toBe(60) + expect(convertBooked(120, 'USD', -2, 'EUR', live)).toBe(60) + }) + + it('goes through the trip currency when the reader displays a third one', () => { + // 200 SEK booked at 10 SEK per euro is 20 EUR of trip money, which the live step then + // takes to the display currency. The frozen rate is against the trip, never the display. + expect(convertBooked(200, 'SEK', 10, 'EUR', live)).toBe(20) + // Trip in USD, display in EUR: 200 SEK at 5 SEK per dollar is 40 USD, then 20 EUR. + expect(convertBooked(200, 'SEK', 5, 'USD', live)).toBe(20) + }) + + it('treats currency case as noise', () => { + expect(convertBooked(120, 'usd', 1.2, 'eur', live)).toBe(100) + expect(convertBooked(100, 'eur', 1, 'EUR', live)).toBe(100) + }) +}) diff --git a/client/src/hooks/useExchangeRates.ts b/client/src/hooks/useExchangeRates.ts index 46a40ed93..2652312fa 100644 --- a/client/src/hooks/useExchangeRates.ts +++ b/client/src/hooks/useExchangeRates.ts @@ -52,6 +52,33 @@ export async function fetchExchangeRates(base: string): Promise number, +): number { + const trip = (tripCurrency || 'EUR').toUpperCase() + // A NULL currency means the trip's own, and then there was never anything to freeze. + const cur = (rowCurrency || trip).toUpperCase() + if (cur === trip) return convertLive(amount, trip) + // A rate of exactly 1 is the column default, not a booked rate: rows written before the + // freeze existed carry it, and those still convert live, as they always did. + if (frozenRate != null && frozenRate > 0 && frozenRate !== 1) return convertLive(amount / frozenRate, trip) + return convertLive(amount, cur) +} + /** Test-only: the module-level cache outlives a vitest file's individual tests. */ export function clearExchangeRateCache(): void { mem.clear() diff --git a/client/src/mobile/screens/trip/tabs/MCostsTab.tsx b/client/src/mobile/screens/trip/tabs/MCostsTab.tsx index dd28c4d11..3476410bd 100644 --- a/client/src/mobile/screens/trip/tabs/MCostsTab.tsx +++ b/client/src/mobile/screens/trip/tabs/MCostsTab.tsx @@ -1,3 +1,4 @@ +import { convertBooked } from '../../../../hooks/useExchangeRates' import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from 'react' import { AlertCircle, ArrowDown, ArrowLeftRight, ArrowRight, ArrowUp, Check, ChevronDown, ChevronUp, @@ -754,7 +755,8 @@ function PaymentRow({ settlement, ctx, base, locale, t, personName, canEdit, onE onUndo: () => void }) { const cur = (settlement.currency || base).toUpperCase() - const amount = ctx.convert(settlement.amount, cur) + // At the rate it was settled at, not today's (#1445), matching the desktop ledger. + const amount = convertBooked(settlement.amount, settlement.currency, settlement.exchange_rate, ctx.tripCurrency, ctx.convert) return (
diff --git a/client/src/mobile/screens/trip/tabs/costsModel.ts b/client/src/mobile/screens/trip/tabs/costsModel.ts index 6ccbbfaf8..aa8c67962 100644 --- a/client/src/mobile/screens/trip/tabs/costsModel.ts +++ b/client/src/mobile/screens/trip/tabs/costsModel.ts @@ -1,6 +1,7 @@ import type { BudgetParticipantFinal, CostCategory } from '@trek/shared' import { paidByUser, readUserNote, settlementDate, splitEqualShares } from '../../../../components/Budget/CostsPanel.helpers' import { catMeta, COST_CATEGORY_LIST } from '../../../../components/Budget/costsCategories' +import { convertBooked } from '../../../../hooks/useExchangeRates' import { currencyDecimals } from '../../../../utils/formatters' import type { BudgetItem } from '../../../../types' @@ -26,14 +27,19 @@ export function currencyOf(e: BudgetItem, ctx: CostsCtx): string { return (e.currency || ctx.tripCurrency).toUpperCase() } -/** Expense total converted to the display/base currency. */ +/** An amount of this expense in the display currency, at the rate the expense was booked at. */ +export function booked(amount: number, e: BudgetItem, ctx: CostsCtx): number { + return convertBooked(amount, e.currency, e.exchange_rate, ctx.tripCurrency, ctx.convert) +} + +/** Expense total converted to the display/base currency, at its booked rate. */ export function baseTotal(e: BudgetItem, ctx: CostsCtx): number { - return ctx.convert(e.total_price || 0, currencyOf(e, ctx)) + return booked(e.total_price || 0, e, ctx) } /** How much `ctx.me` personally fronted for this expense, in the base currency. */ export function myPaidOf(e: BudgetItem, ctx: CostsCtx): number { - return ctx.convert(paidByUser(e, ctx.me), currencyOf(e, ctx)) + return booked(paidByUser(e, ctx.me), e, ctx) } /** A given member's share of this expense (explicit custom amount, else equal split), base currency. */ @@ -41,10 +47,10 @@ export function memberShareOf(e: BudgetItem, userId: number, ctx: CostsCtx): num const member = (e.members || []).find(m => m.user_id === userId) if (!member) return 0 if (member.amount !== null && member.amount !== undefined) { - return ctx.convert(member.amount, currencyOf(e, ctx)) + return booked(member.amount, e, ctx) } const shares = splitEqualShares(e.total_price || 0, e.members || [], e.id) - return ctx.convert(shares[userId] || 0, currencyOf(e, ctx)) + return booked(shares[userId] || 0, e, ctx) } /** `ctx.me`'s own share — the common case of {@link memberShareOf}. */ @@ -84,6 +90,9 @@ export interface CostsSettlement { amount: number // Legacy rows predate this column (null) and are read as the display currency. currency?: string | null + // The rate frozen when the transfer was settled, units of `currency` per 1 trip + // currency (#1445). Absent, or exactly 1, on rows written before the freeze existed. + exchange_rate?: number created_at?: string // The day the transfer actually happened; editable, unlike created_at (when it // was recorded). Null/absent on rows predating this field.