mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-30 18:46:00 +00:00
fix(costs): freeze the FX rate so settled expenses don't reopen when rates drift (#1335)
Settle-up transfers are stored as fixed amounts, but a foreign-currency expense was re-converted with live rates on every settlement calc. When the rate drifted, the fixed transfer no longer cancelled the re-valued expense and a few-cent residual re-opened the settled position. Foreign-currency expenses now freeze the live rate at entry time into the existing budget_items.exchange_rate column, and the settlement converts with that frozen rate when working in the trip currency. Legacy rows (exchange_rate = 1) keep using live rates, so historical data is unchanged until re-edited; rate fetch failures fall back to live rates.
This commit is contained in:
@@ -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);
|
||||
}
|
||||
|
||||
@@ -41,11 +41,39 @@ export class BudgetService {
|
||||
return svc.calculateSettlement(tripId, { base: effectiveBase, rates, tripCurrency });
|
||||
}
|
||||
|
||||
create(tripId: string, data: Parameters<typeof svc.createBudgetItem>[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<void> {
|
||||
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<typeof svc.createBudgetItem>[1]) {
|
||||
await this.freezeForeignRate(tripId, data);
|
||||
return svc.createBudgetItem(tripId, data);
|
||||
}
|
||||
|
||||
update(id: string, tripId: string, data: Parameters<typeof svc.updateBudgetItem>[2]) {
|
||||
async update(id: string, tripId: string, data: Parameters<typeof svc.updateBudgetItem>[2]) {
|
||||
await this.freezeForeignRate(tripId, data, id);
|
||||
return svc.updateBudgetItem(id, tripId, data);
|
||||
}
|
||||
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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<unknown>): 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<BudgetService>);
|
||||
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<BudgetService>);
|
||||
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<BudgetService>);
|
||||
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<BudgetService>);
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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 ──────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user