diff --git a/server/src/nest/budget/budget.controller.ts b/server/src/nest/budget/budget.controller.ts index 9d6bc8a3..278cccb8 100644 --- a/server/src/nest/budget/budget.controller.ts +++ b/server/src/nest/budget/budget.controller.ts @@ -136,7 +136,7 @@ export class BudgetController { } @Post() - create( + async create( @CurrentUser() user: User, @Param('tripId') tripId: string, @Body() body: { name?: string; category?: string; total_price?: number; persons?: number | null; days?: number | null; note?: string | null; expense_date?: string | null; reservation_id?: number }, @@ -147,7 +147,7 @@ export class BudgetController { if (!body.name) { throw new HttpException({ error: 'Name is required' }, 400); } - const item = this.budget.create(tripId, body as { name: string }); + const item = await this.budget.create(tripId, body as { name: string }); this.budget.broadcast(tripId, 'budget:created', { item }, socketId); return { item }; } @@ -181,7 +181,7 @@ export class BudgetController { } @Put(':id') - update( + async update( @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @@ -190,7 +190,7 @@ export class BudgetController { ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - const updated = this.budget.update(id, tripId, body); + const updated = await this.budget.update(id, tripId, body); if (!updated) { throw new HttpException({ error: 'Budget item not found' }, 404); } diff --git a/server/src/nest/budget/budget.service.ts b/server/src/nest/budget/budget.service.ts index 70bd5c18..d4a37b17 100644 --- a/server/src/nest/budget/budget.service.ts +++ b/server/src/nest/budget/budget.service.ts @@ -41,11 +41,39 @@ export class BudgetService { return svc.calculateSettlement(tripId, { base: effectiveBase, rates, tripCurrency }); } - create(tripId: string, data: Parameters[1]) { + // Freeze the live FX rate at entry time into budget_items.exchange_rate so a settled + // position isn't re-opened when live rates drift later (#1335). Only for a foreign + // currency with no explicit rate; degrades to live rates if the fetch fails. On update + // it (re)freezes only when the currency changes, so an unrelated edit never moves money. + private async freezeForeignRate( + tripId: string, + data: { currency?: string | null; exchange_rate?: number }, + existingItemId?: string, + ): Promise { + if (data.exchange_rate != null) return; // an explicit rate from the caller wins + const cur = (data.currency || '').toUpperCase(); + if (!cur) return; // currency not being set in this request + if (existingItemId != null) { + const existing = db.prepare('SELECT currency FROM budget_items WHERE id = ?') + .get(existingItemId) as { currency?: string } | undefined; + if (existing && (existing.currency || '').toUpperCase() === cur) return; // currency unchanged + } + const trip = db.prepare('SELECT currency FROM trips WHERE id = ?') + .get(tripId) as { currency?: string } | undefined; + const tripCur = (trip?.currency || 'EUR').toUpperCase(); + if (cur === tripCur) return; // same as the trip currency → no conversion to freeze + const rates = await getRates(tripCur); + const r = rates?.[cur]; + if (r && r > 0) data.exchange_rate = r; + } + + async create(tripId: string, data: Parameters[1]) { + await this.freezeForeignRate(tripId, data); return svc.createBudgetItem(tripId, data); } - update(id: string, tripId: string, data: Parameters[2]) { + async update(id: string, tripId: string, data: Parameters[2]) { + await this.freezeForeignRate(tripId, data, id); return svc.updateBudgetItem(id, tripId, data); } diff --git a/server/src/services/budgetService.ts b/server/src/services/budgetService.ts index e707c1c6..5383042e 100644 --- a/server/src/services/budgetService.ts +++ b/server/src/services/budgetService.ts @@ -349,9 +349,17 @@ export function calculateSettlement( const rates = opts.rates ?? null; // Amount in some currency → base. Pre-rework rows store currency = NULL, which // means "the trip's own currency". rates[X] = units of X per 1 base. - const toBase = (amount: number, itemCurrency: string | null | undefined): number => { + const toBase = (amount: number, itemCurrency: string | null | undefined, itemRate?: number | null): number => { const cur = (itemCurrency || tripCurrency).toUpperCase(); - if (cur === base || !rates) return amount; + if (cur === base) return amount; + // Prefer the FX rate frozen at entry time (#1335): a settled expense keeps the rate it + // was booked at, so a later live-rate drift doesn't re-open it with a few-cent residual. + // The stored rate is units of item-currency per 1 trip-currency, so it only applies when + // converting to the trip's own currency; otherwise (and for legacy rows) use live rates. + if (base === tripCurrency && itemRate != null && itemRate > 0 && itemRate !== 1) { + return amount / itemRate; + } + if (!rates) return amount; const r = rates[cur]; return r && r > 0 ? amount / r : amount; }; @@ -384,11 +392,11 @@ export function calculateSettlement( const payers = allPayers.filter(p => p.budget_item_id === item.id); if (members.length === 0) continue; // planning-only entry → doesn't affect balances - const paidBase = payers.reduce((a, p) => a + toBase(p.amount > 0 ? p.amount : 0, item.currency), 0); + const paidBase = payers.reduce((a, p) => a + toBase(p.amount > 0 ? p.amount : 0, item.currency, item.exchange_rate), 0); const sharePerMember = paidBase / members.length; // Payers are credited what they actually paid (converted to base)… - for (const p of payers) ensure(p.user_id, p).balance += toBase(p.amount > 0 ? p.amount : 0, item.currency); + for (const p of payers) ensure(p.user_id, p).balance += toBase(p.amount > 0 ? p.amount : 0, item.currency, item.exchange_rate); // …and every split participant owes an equal share of the base total. for (const m of members) ensure(m.user_id, m).balance -= sharePerMember; } diff --git a/server/tests/unit/nest/budget.controller.test.ts b/server/tests/unit/nest/budget.controller.test.ts index 7fa6d8b7..6e2dd0e1 100644 --- a/server/tests/unit/nest/budget.controller.test.ts +++ b/server/tests/unit/nest/budget.controller.test.ts @@ -28,6 +28,17 @@ function thrown(fn: () => unknown): { status: number; body: unknown } { throw new Error('expected the handler to throw'); } +async function thrownAsync(fn: () => Promise): Promise<{ status: number; body: unknown }> { + try { + await fn(); + } catch (err) { + expect(err).toBeInstanceOf(HttpException); + const e = err as HttpException; + return { status: e.getStatus(), body: e.getResponse() }; + } + throw new Error('expected the handler to throw'); +} + describe('BudgetController (parity with the legacy /api/trips/:tripId/budget route)', () => { it('404 when the trip is not accessible', () => { const svc = makeService({ verifyTripAccess: vi.fn().mockReturnValue(undefined) }); @@ -145,51 +156,51 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou }); describe('POST /', () => { - it('403 without budget_edit', () => { + it('403 without budget_edit', async () => { const svc = makeService({ canEdit: vi.fn().mockReturnValue(false) }); - expect(thrown(() => new BudgetController(svc).create(user, '5', { name: 'Hotel' }))).toEqual({ + expect(await thrownAsync(() => new BudgetController(svc).create(user, '5', { name: 'Hotel' }))).toEqual({ status: 403, body: { error: 'No permission' }, }); }); - it('400 when name missing', () => { - expect(thrown(() => new BudgetController(makeService()).create(user, '5', {}))).toEqual({ + it('400 when name missing', async () => { + expect(await thrownAsync(() => new BudgetController(makeService()).create(user, '5', {}))).toEqual({ status: 400, body: { error: 'Name is required' }, }); }); - it('creates and broadcasts', () => { + it('creates and broadcasts', async () => { const create = vi.fn().mockReturnValue({ id: 9, name: 'Hotel' }); const broadcast = vi.fn(); const svc = makeService({ create, broadcast } as Partial); - expect(new BudgetController(svc).create(user, '5', { name: 'Hotel', total_price: 200 }, 'sock')).toEqual({ item: { id: 9, name: 'Hotel' } }); + expect(await new BudgetController(svc).create(user, '5', { name: 'Hotel', total_price: 200 }, 'sock')).toEqual({ item: { id: 9, name: 'Hotel' } }); expect(broadcast).toHaveBeenCalledWith('5', 'budget:created', { item: { id: 9, name: 'Hotel' } }, 'sock'); }); }); describe('PUT /:id', () => { - it('404 when item missing', () => { + it('404 when item missing', async () => { const svc = makeService({ update: vi.fn().mockReturnValue(null) } as Partial); - expect(thrown(() => new BudgetController(svc).update(user, '5', '9', { name: 'X' }))).toEqual({ + expect(await thrownAsync(() => new BudgetController(svc).update(user, '5', '9', { name: 'X' }))).toEqual({ status: 404, body: { error: 'Budget item not found' }, }); }); - it('syncs the reservation price when a linked item changes total_price', () => { + it('syncs the reservation price when a linked item changes total_price', async () => { const update = vi.fn().mockReturnValue({ id: 9, reservation_id: 42, total_price: 250 }); const syncReservationPrice = vi.fn(); const broadcast = vi.fn(); const svc = makeService({ update, syncReservationPrice, broadcast } as Partial); - new BudgetController(svc).update(user, '5', '9', { total_price: 250 }, 'sock'); + await new BudgetController(svc).update(user, '5', '9', { total_price: 250 }, 'sock'); expect(syncReservationPrice).toHaveBeenCalledWith('5', 42, 250, 'sock'); expect(broadcast).toHaveBeenCalledWith('5', 'budget:updated', { item: { id: 9, reservation_id: 42, total_price: 250 } }, 'sock'); }); - it('does not sync when the item has no linked reservation', () => { + it('does not sync when the item has no linked reservation', async () => { const update = vi.fn().mockReturnValue({ id: 9, reservation_id: null, total_price: 250 }); const syncReservationPrice = vi.fn(); const svc = makeService({ update, syncReservationPrice } as Partial); - new BudgetController(svc).update(user, '5', '9', { total_price: 250 }); + await new BudgetController(svc).update(user, '5', '9', { total_price: 250 }); expect(syncReservationPrice).not.toHaveBeenCalled(); }); }); diff --git a/server/tests/unit/nest/budget.service.test.ts b/server/tests/unit/nest/budget.service.test.ts index cf931f6b..bb0ffce5 100644 --- a/server/tests/unit/nest/budget.service.test.ts +++ b/server/tests/unit/nest/budget.service.test.ts @@ -103,10 +103,10 @@ describe('BudgetService', () => { }); }); - it('create / update / remove / members / paid / payers delegate', () => { - svc().create('5', { name: 'Hotel' } as never); + it('create / update / remove / members / paid / payers delegate', async () => { + await svc().create('5', { name: 'Hotel' } as never); expect(budget.createBudgetItem).toHaveBeenCalledWith('5', { name: 'Hotel' }); - svc().update('9', '5', { name: 'X' }); + await svc().update('9', '5', { name: 'X' }); expect(budget.updateBudgetItem).toHaveBeenCalledWith('9', '5', { name: 'X' }); svc().remove('9', '5'); expect(budget.deleteBudgetItem).toHaveBeenCalledWith('9', '5'); diff --git a/server/tests/unit/services/budgetService.test.ts b/server/tests/unit/services/budgetService.test.ts index fd7fe9ca..12e2b7fa 100644 --- a/server/tests/unit/services/budgetService.test.ts +++ b/server/tests/unit/services/budgetService.test.ts @@ -209,6 +209,33 @@ describe('calculateSettlement', () => { expect.objectContaining({ amount: 30, from: expect.objectContaining({ user_id: 1 }), to: expect.objectContaining({ user_id: 2 }) }), ]); }); + + it('#1335 converts a foreign expense with the frozen exchange_rate, not live rates', () => { + // $110 booked at a frozen rate of 1.1 (USD per 1 EUR) = 100 EUR. Live rates have since + // drifted to 1.2, but the converted amount must stay on the frozen rate so an already + // settled position isn't re-opened with a residual. + setupDb( + [{ ...makeItem(1, 110), currency: 'USD', exchange_rate: 1.1 } as BudgetItem], + [makeMember(1, 1, 'alice'), makeMember(1, 2, 'bob')], + [makePayer(1, 1, 110, 'alice')], + ); + const result = calculateSettlement(1, { base: 'EUR', tripCurrency: 'EUR', rates: { EUR: 1, USD: 1.2 } }); + const bob = result.balances.find(b => b.user_id === 2)!; + // 110 / 1.1 = 100 EUR; Bob owes half = 50 (frozen). With the live 1.2 it would be ~45.83. + expect(bob.balance).toBeCloseTo(-50, 2); + }); + + it('#1335 a legacy row (exchange_rate = 1) still converts with live rates', () => { + setupDb( + [{ ...makeItem(1, 120), currency: 'USD', exchange_rate: 1 } as BudgetItem], + [makeMember(1, 1, 'alice'), makeMember(1, 2, 'bob')], + [makePayer(1, 1, 120, 'alice')], + ); + const result = calculateSettlement(1, { base: 'EUR', tripCurrency: 'EUR', rates: { EUR: 1, USD: 1.2 } }); + const bob = result.balances.find(b => b.user_id === 2)!; + // 120 / 1.2 (live) = 100 EUR; Bob owes 50 — unchanged behaviour for pre-#1335 rows. + expect(bob.balance).toBeCloseTo(-50, 2); + }); }); // ── updateSettlement ──────────────────────────────────────────────────────────