From 33e606959c68fd3aa9c12bc9a2f0ff1961cb68e5 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 12 Oct 2026 14:28:06 +0200 Subject: [PATCH] fix(budget): settle-up hands out whole won and yen, so balances add up and settle clears them The ledger nets every currency in hundredths, and the settlement handed those hundredths out as they were. For a currency shown without decimals the panels then rounded each balance and flow on its own: 10,000 KRW three ways read +6,667 / -3,333 / -3,333, two flows of 3,333 sat under "You're owed 6,667", and Settle booked 3,333.34 won. The balances, flows and final budget lines are now allocated by largest remainder in whole steps of the display currency (whole won or yen, unchanged for two and three decimal ones), so the shown set adds up to zero and the flow amount Settle sends is the whole amount on the row. The ledger, stored rows and manual settlements are untouched. REST and the MCP settlement summary share the same method. Refs #2604 --- server/src/nest/budget/budget.service.ts | 33 ++-- server/tests/e2e/budget.e2e.test.ts | 48 ++++++ .../unit/mcp/tools-budget-advanced.test.ts | 43 +++++ .../unit/nest/budget.service.calc.test.ts | 149 ++++++++++++++++++ 4 files changed, 261 insertions(+), 12 deletions(-) diff --git a/server/src/nest/budget/budget.service.ts b/server/src/nest/budget/budget.service.ts index 587a3438d..8c0ecd550 100644 --- a/server/src/nest/budget/budget.service.ts +++ b/server/src/nest/budget/budget.service.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import type { BudgetFallbackFx, BudgetParticipantFinal, BudgetUnconverted, TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; -import { splitEqualShares, sumMinor, toMinor } from '@trek/shared'; +import { currencyDecimals, splitEqualShares, sumMinor, toMinor } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; import { avatarUrl } from '../common/avatarUrl'; @@ -107,17 +107,23 @@ function sumMoney(amounts: number[]): number { * is what the set has to sum to. By default that is the rounded conversion of its * own sum; the rows behind a figure pass the figure's already allocated cents * instead, so a list nested under a line lands exactly on that line. + * + * `unit` is the smallest step the currency is shown in, in cents (100 for won or + * yen, else 1). The ledger keeps hundredths, but a set handed out in hundredths of + * a won and rounded one by one on screen stops adding up (#2604). In the trip's own + * currency the remainders are integers, so booking what is offered clears it. */ -function allocateDisplayCents(cents: number[], factor: number, total = Math.round(cents.reduce((a, c) => a + c, 0) * factor)): number[] { - if (factor === 1) return [...cents]; - const exact = cents.map(c => c * factor); +function allocateDisplayCents(cents: number[], factor: number, total?: number, unit = 1): number[] { + if (factor === 1 && unit === 1) return [...cents]; + const exact = cents.map(c => (c * factor) / unit); const out = exact.map(v => Math.floor(v)); - const drift = total - out.reduce((a, v) => a + v, 0); + const target = total === undefined ? Math.round((cents.reduce((a, c) => a + c, 0) * factor) / unit) : total / unit; + const drift = target - out.reduce((a, v) => a + v, 0); const byFraction = exact - .map((v, i) => ({ i, frac: v - Math.floor(v) })) + .map((v, i) => ({ i, frac: factor === 1 ? cents[i] - out[i] * unit : v - Math.floor(v) })) .sort((a, b) => b.frac - a.frac || a.i - b.i); for (let k = 0; k < drift && k < byFraction.length; k++) out[byFraction[k].i] += 1; - return out; + return unit === 1 ? out : out.map(v => v * unit); } /** @@ -1195,6 +1201,9 @@ export class BudgetService { const quoted = requested === tripCurrency ? 1 : (quoteRatio(rates, requested, tripCurrency) ?? baseRate); const currency = quoted === null ? tripCurrency : requested; const displayFactor = quoted ?? 1; + // Every figure goes out in whole steps of that currency: whole won or yen (#2604). + const displayUnit = 10 ** Math.max(0, 2 - currencyDecimals(currency)); + const toDisplay = (cents: number[], total?: number) => allocateDisplayCents(cents, displayFactor, total, displayUnit); // A recorded settle-up amount is entered in whatever display currency the payer // was viewing. New rows capture that currency and the rate frozen at settle time // (#1445), so a settled position stays balanced when live rates drift — mirroring @@ -1346,15 +1355,15 @@ export class BudgetService { // derived from the same integers, so what the balances say is owed is exactly // what "Settle up" offers to move, down to the last cent (#1382). const ledger = Object.values(balances); - const displayCents = allocateDisplayCents(ledger.map(b => b.cents), displayFactor); + const displayCents = toDisplay(ledger.map(b => b.cents)); // Each component of the final budget is re-denominated as its own set, for the // same reason the balances are: rounding one person at a time lets the column // drift away from the figure it converted from. The final itself is then // subtracted in display cents rather than converted separately, so the three // lines the breakdown shows always add up to the total beside them, whatever // currency the viewer picked. - const frontedDisplayCents = allocateDisplayCents(ledger.map(b => frontedCents[b.user_id] || 0), displayFactor); - const reimbursedDisplayCents = allocateDisplayCents(ledger.map(b => reimbursedCents[b.user_id] || 0), displayFactor); + const frontedDisplayCents = toDisplay(ledger.map(b => frontedCents[b.user_id] || 0)); + const reimbursedDisplayCents = toDisplay(ledger.map(b => reimbursedCents[b.user_id] || 0)); // Calculate optimized payment flows (greedy algorithm) const people = ledger @@ -1396,8 +1405,8 @@ export class BudgetService { // figure as the target, so each list adds up to the line it sits under. const fronted = frontedRows[b.user_id] || []; const moved = movedRows[b.user_id] || []; - const frontedDisplay = allocateDisplayCents(fronted.map(r => r.cents), displayFactor, frontedDisplayCents[i]); - const movedDisplay = allocateDisplayCents(moved.map(r => r.cents), displayFactor, reimbursedDisplayCents[i]); + const frontedDisplay = toDisplay(fronted.map(r => r.cents), frontedDisplayCents[i]); + const movedDisplay = toDisplay(moved.map(r => r.cents), reimbursedDisplayCents[i]); return { user_id: b.user_id, username: b.username, avatar_url: b.avatar_url, expenses: frontedDisplayCents[i] / 100, diff --git a/server/tests/e2e/budget.e2e.test.ts b/server/tests/e2e/budget.e2e.test.ts index 53afda57b..be25a72a0 100644 --- a/server/tests/e2e/budget.e2e.test.ts +++ b/server/tests/e2e/budget.e2e.test.ts @@ -279,6 +279,54 @@ describe('Budget e2e (real auth guard + temp SQLite, real budget SQL)', () => { } }); + it('a won trip offers whole won, and settling what it offers squares everyone (#2604)', async () => { + // ₩10,000 three ways nets to 6,666.67 / -3,333.33 / -3,333.34 in the ledger. The + // settlement answered those hundredths, the panel rounded each to 6,667 / -3,333 / + // -3,333, and Settle booked 3,333.34 won. + await makeUser(orm, { + id: 5, + username: 'e2e-third', + email: 'third@example.test', + role: 'user', + password_version: 0, + }); + const krwTrip = (await makeTrip(orm, 1, { title: 'FIX KRW', currency: 'KRW' })).id; + await addTripMember(orm, krwTrip, 2); + await addTripMember(orm, krwTrip, 5); + + const dinner = await request(server) + .post(`/api/trips/${krwTrip}/budget`) + .set('Cookie', sessionCookie(1)) + .send({ name: 'Dinner', currency: 'KRW', payers: [{ user_id: 1, amount: 10000 }], member_ids: [1, 2, 5] }); + expect(dinner.status).toBe(201); + + const read = () => + request(server).get(`/api/trips/${krwTrip}/budget/settlement?base=KRW`).set('Cookie', sessionCookie(1)); + const before = await read(); + expect(before.status).toBe(200); + const balances = before.body.balances as { user_id: number; balance: number }[]; + const flows = before.body.flows as { from: { user_id: number }; to: { user_id: number }; amount: number }[]; + // Which of the three carries the odd hundredth depends on the item id; what is + // shown is whole won either way, adds up to zero, and the flows move exactly it. + for (const b of balances) expect(Number.isInteger(b.balance)).toBe(true); + expect(balances.reduce((a, b) => a + b.balance, 0)).toBe(0); + expect(flows.every((f) => Number.isInteger(f.amount))).toBe(true); + expect(flows.reduce((a, f) => a + f.amount, 0)).toBe(balances.find((b) => b.user_id === 1)?.balance); + + const paid = await Promise.all( + flows.map((f) => + request(server) + .post(`/api/trips/${krwTrip}/budget/settlements`) + .set('Cookie', sessionCookie(1)) + .send({ from_user_id: f.from.user_id, to_user_id: f.to.user_id, amount: f.amount, currency: 'KRW' }), + ), + ); + expect(paid.map((r) => r.status)).toEqual(flows.map(() => 201)); + const after = await read(); + expect((after.body.balances as { balance: number }[]).map((b) => b.balance)).toEqual([0, 0, 0]); + expect(after.body.flows).toEqual([]); + }); + it('GET /summary/per-person reads a dollar bill on a euro trip at its booked rate (#2525)', async () => { const hotel = await request(server) .post(`/api/trips/${tripId}/budget`) diff --git a/server/tests/unit/mcp/tools-budget-advanced.test.ts b/server/tests/unit/mcp/tools-budget-advanced.test.ts index 1de43a0de..b1f82bad0 100644 --- a/server/tests/unit/mcp/tools-budget-advanced.test.ts +++ b/server/tests/unit/mcp/tools-budget-advanced.test.ts @@ -444,6 +444,49 @@ describe('Settlement tools', () => { } }); + it('get_settlement_summary hands out whole won on a won trip, like REST (#2604)', async () => { + // ₩10,001 for two is 5,000.50 each. A whole won is the smallest amount that can + // change hands, so that is what both surfaces offer, and an agent booking the + // flow it was given squares the trip. + const { user, other, trip } = tripWithTwo(); + await updateRows(orm, Trips, { id: trip.id }, { currency: 'KRW' }); + const item = createBudgetItem(testDb, trip.id, { total_price: 10001 }); + await updateRows(orm, BudgetItems, { id: item.id }, { currency: 'KRW' }); + await addBudgetItemMember(orm, item.id, user.id, { paid: 0 }); + await addBudgetItemMember(orm, item.id, other.id, { paid: 0 }); + await addBudgetItemPayer(orm, item.id, user.id, 10001); + + const controller = new BudgetController( + new BudgetService( + new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), + new ExchangeRatesService(), + new RealtimeService(), + await createTestUnitOfWork(testDb), + ...(await budgetRepoArgs(testDb)), + ), + ); + const access = { id: trip.id, user_id: user.id, currency: 'KRW' } as TripAccess; + const rest = await controller.settlement({ id: user.id } as User, access, String(trip.id), {}); + + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'get_settlement_summary', arguments: { tripId: trip.id } }); + const data = parseToolResult(result) as { summary: typeof rest }; + expect(data.summary).toEqual(rest); + expect(data.summary.balances.map((b) => b.balance)).toEqual([5001, -5001]); + expect(data.summary.flows.map((f) => f.amount)).toEqual([5001]); + + await h.client.callTool({ + name: 'create_settlement', + arguments: { tripId: trip.id, from_user_id: other.id, to_user_id: user.id, amount: 5001, currency: 'KRW' }, + }); + const after = parseToolResult( + await h.client.callTool({ name: 'get_settlement_summary', arguments: { tripId: trip.id } }), + ) as { summary: typeof rest }; + expect(after.summary.balances.map((b) => b.balance)).toEqual([0, 0]); + expect(after.summary.flows).toEqual([]); + }); + }); + it('get_settlement_summary returns balances and flows', async () => { try { const { user, other, trip } = tripWithTwo(); diff --git a/server/tests/unit/nest/budget.service.calc.test.ts b/server/tests/unit/nest/budget.service.calc.test.ts index 020df6c43..ba30a783a 100644 --- a/server/tests/unit/nest/budget.service.calc.test.ts +++ b/server/tests/unit/nest/budget.service.calc.test.ts @@ -1285,6 +1285,155 @@ describe('calculateSettlement: unconverted rows', () => { }); }); +// ── Currencies shown without decimals (#2604) ──────────────────────────────── + +describe('calculateSettlement: currencies shown in whole units (#2604)', () => { + const KRW = { base: 'KRW', tripCurrency: 'KRW', rates: null }; + type Result = Awaited>; + + /** + * What the panels print for a won is a whole won, so every figure has to be + * one already and the printed set has to add up the way the ledger does: the + * balances to zero, each person's flows to their balance, each final-budget + * line to the rows listed under it. + */ + const expectWholeAndSquare = (result: Result) => { + const figures = [ + ...result.balances.map((b) => b.balance), + ...result.flows.map((f) => f.amount), + ...result.finalBudgets.flatMap((f) => [f.expenses, f.reimbursed, f.pending, f.final]), + ...result.finalBudgets.flatMap((f) => [ + ...f.sources.fronted.map((r) => r.cents / 100), + ...f.sources.moved.map((r) => r.cents / 100), + ...f.sources.outstanding.map((r) => r.cents / 100), + ]), + ]; + for (const v of figures) expect(Number.isInteger(v)).toBe(true); + expect(result.balances.reduce((a, b) => a + b.balance, 0)).toBe(0); + for (const b of result.balances) { + const moved = + result.flows.filter((f) => f.to.user_id === b.user_id).reduce((a, f) => a + f.amount, 0) - + result.flows.filter((f) => f.from.user_id === b.user_id).reduce((a, f) => a + f.amount, 0); + expect(moved).toBe(b.balance); + } + for (const f of result.finalBudgets) { + expect(f.sources.fronted.reduce((a, r) => a + r.cents, 0)).toBe(Math.round(f.expenses * 100)); + expect(f.sources.moved.reduce((a, r) => a + r.cents, 0)).toBe(Math.round(f.reimbursed * 100)); + expect(f.sources.outstanding.reduce((a, r) => a + r.cents, 0)).toBe(Math.round(f.pending * 100)); + expect(f.final).toBe(f.expenses - f.reimbursed - f.pending); + } + }; + + /** Books every flow the settlement offers, as the Settle button does, and reads the trip again. */ + const settleEverything = async ( + items: BudgetItem[], + members: ReturnType[], + payers: ReturnType[], + opts: Parameters[1], + currency: string, + ) => { + const first = await budget.calculateSettlement(1, opts); + const booked = first.flows.map((f, i) => + makeSettlementRow(i + 1, f.from.user_id, f.to.user_id, f.amount, currency), + ); + await setupDb(items, members, payers, booked); + return budget.calculateSettlement(1, opts); + }; + + it('₩10,000 three ways: balances in whole won that add up, and settle-up moves exactly them', async () => { + // The report: the ledger holds 6,666.67 / -3,333.33 / -3,333.34 and the panel + // rounded each one on its own to +6,667 / -3,333 / -3,333, one won out, with + // two flows of 3,333 under "You're owed 6,667". + const items = [{ ...makeItem(1, 10000), currency: 'KRW' } as BudgetItem]; + const members = [makeMember(1, 1, 'alice'), makeMember(1, 2, 'bob'), makeMember(1, 3, 'carol')]; + const payers = [makePayer(1, 1, 10000, 'alice')]; + await setupDb(items, members, payers); + + const result = await budget.calculateSettlement(1, KRW); + expectWholeAndSquare(result); + expect(result.balances.find((b) => b.user_id === 1)?.balance).toBe(6667); + expect(result.flows.map((f) => f.amount).sort()).toEqual([3333, 3334]); + + const after = await settleEverything(items, members, payers, KRW, 'KRW'); + expect(after.balances.map((b) => b.balance)).toEqual([0, 0, 0]); + expect(after.flows).toEqual([]); + expectWholeAndSquare(after); + }); + + it.each([ + [60001, 4], + [100001, 3], + ])('₩%i split %i ways squares up in whole won', async (total, people) => { + const names = ['alice', 'bob', 'carol', 'dave'].slice(0, people); + const items = [{ ...makeItem(1, total), currency: 'KRW' } as BudgetItem]; + const members = names.map((n, i) => makeMember(1, i + 1, n)); + const payers = [makePayer(1, 1, total, 'alice')]; + await setupDb(items, members, payers); + + expectWholeAndSquare(await budget.calculateSettlement(1, KRW)); + + const after = await settleEverything(items, members, payers, KRW, 'KRW'); + expect(after.balances.every((b) => b.balance === 0)).toBe(true); + expect(after.flows).toEqual([]); + }); + + it('a yen bill on a won trip, split four ways, squares up in whole won', async () => { + // ¥4,000 at a frozen 0.1086 yen per won is 36,832.41 won: neither the total nor + // the shares land on a whole won, which summed to -1 on screen. + const items = [{ ...makeItem(1, 4000), currency: 'JPY', exchange_rate: 0.1086 } as BudgetItem]; + const members = ['alice', 'bob', 'carol', 'dave'].map((n, i) => makeMember(1, i + 1, n)); + const payers = [makePayer(1, 1, 4000, 'alice')]; + await setupDb(items, members, payers); + + expectWholeAndSquare(await budget.calculateSettlement(1, KRW)); + const after = await settleEverything(items, members, payers, KRW, 'KRW'); + expect(after.balances.every((b) => b.balance === 0)).toBe(true); + expect(after.flows).toEqual([]); + }); + + it('a transfer stored with a fraction of a won still reads in whole won', async () => { + // What the Settle button used to send: the unrounded 3,333.34. The ledger keeps + // it to the hundredth; what is shown stays whole and adds up. + await setupDb( + [{ ...makeItem(1, 10000), currency: 'KRW' } as BudgetItem], + [makeMember(1, 1, 'alice'), makeMember(1, 2, 'bob'), makeMember(1, 3, 'carol')], + [makePayer(1, 1, 10000, 'alice')], + [makeSettlementRow(1, 3, 1, 3333.34, 'KRW')], + ); + const result = await budget.calculateSettlement(1, KRW); + expectWholeAndSquare(result); + expect(result.flows.map((f) => f.amount)).toEqual([3333]); + }); + + it('a euro trip read in won is shown in whole won and still adds up', async () => { + const items = [makeItem(1, 100.01)]; + const members = [makeMember(1, 1, 'alice'), makeMember(1, 2, 'bob'), makeMember(1, 3, 'carol')]; + const payers = [makePayer(1, 1, 100.01, 'alice')]; + await setupDb(items, members, payers, [makeSettlementRow(1, 2, 1, 12.34, 'EUR')]); + const results = await Promise.all( + [1473.21, 1500, 1612.77].map((krwPerEur) => + budget.calculateSettlement(1, { base: 'KRW', tripCurrency: 'EUR', rates: { EUR: 1, KRW: krwPerEur } }), + ), + ); + for (const result of results) { + expect(result.currency).toBe('KRW'); + expectWholeAndSquare(result); + } + }); + + it('a two-decimal display currency keeps its cents', async () => { + // The euro case the rest of this file pins, read once more beside the won ones: + // 10.00 three ways stays 3.34 / 3.33 / 3.33, not whole euros. + await setupDb( + [makeItem(1, 10)], + [makeMember(1, 1, 'alice'), makeMember(1, 2, 'bob'), makeMember(1, 3, 'carol')], + [makePayer(1, 1, 10, 'alice')], + ); + const result = await budget.calculateSettlement(1); + expect(result.flows.map((f) => f.amount).sort()).toEqual([3.33, 3.34]); + }); +}); + // ── freezeForeignRate (write-path FX freeze, #1445) ─────────────────────────── // The trip seeded by beforeEach/seedBaseline is EUR — every case here relies on // that baseline instead of reconfiguring a `FROM trips` stub.