From a3de87f3873f777c59174f65fdfe3e566cc07d60 Mon Sep 17 00:00:00 2001 From: jubnl Date: Mon, 13 Jul 2026 22:21:24 +0200 Subject: [PATCH] fix(budget): settle in the trip's real currency, not always EUR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The settlement route read `currency` off the row returned by canAccessTrip, whose SELECT never included the column. A cast hid the mistake, so trip.currency was always undefined and the settlement was told every trip is in EUR. Balances are netted in the trip currency and converted to the display currency once. With the trip mislabelled EUR, expenses in the trip's own currency still cancelled out, but an expense booked in a foreign currency was divided by its frozen rate into trip-currency units, then converted again as if those were euros — inflating balances by the EUR/trip rate (~27x for a RUB trip with a USD expense, #1543). MCP was unaffected: it reads the currency with its own SELECT. Select the currency in canAccessTrip so the read is real. The settlement maths was correct all along; it was simply being lied to about the base currency. Fixes #1543 --- server/src/db/database.ts | 3 +- server/src/nest/budget/budget.controller.ts | 2 +- server/tests/unit/db/trip-access.test.ts | 54 +++++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 server/tests/unit/db/trip-access.test.ts diff --git a/server/src/db/database.ts b/server/src/db/database.ts index 499e900d6..09d0cdb3e 100644 --- a/server/src/db/database.ts +++ b/server/src/db/database.ts @@ -129,11 +129,12 @@ function getPlaceWithTags(placeId: number | string): PlaceWithTags | null { interface TripAccess { id: number; user_id: number; + currency: string | null; } function canAccessTrip(tripId: number | string, userId: number): TripAccess | undefined { return db.prepare(` - SELECT t.id, t.user_id FROM trips t + SELECT t.id, t.user_id, t.currency FROM trips t LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) `).get(userId, tripId, userId) as TripAccess | undefined; diff --git a/server/src/nest/budget/budget.controller.ts b/server/src/nest/budget/budget.controller.ts index 4a1c3e631..1ea0be54b 100644 --- a/server/src/nest/budget/budget.controller.ts +++ b/server/src/nest/budget/budget.controller.ts @@ -64,7 +64,7 @@ export class BudgetController { @Query('base') base?: string, ) { const trip = this.requireTrip(tripId, user); - return this.budget.settlement(tripId, base, (trip as { currency?: string }).currency || 'EUR'); + return this.budget.settlement(tripId, base, trip.currency || 'EUR'); } @Get('settlements') diff --git a/server/tests/unit/db/trip-access.test.ts b/server/tests/unit/db/trip-access.test.ts new file mode 100644 index 000000000..a24cfc2e0 --- /dev/null +++ b/server/tests/unit/db/trip-access.test.ts @@ -0,0 +1,54 @@ +/** + * canAccessTrip is the shared trip-access check every domain reads its trip row + * from. It must return the trip's `currency`: the budget settlement takes its base + * currency straight off this row, and when the column was missing from the SELECT + * it silently fell back to 'EUR', inflating balances on every non-EUR trip that had + * a foreign-currency expense (#1543). + */ +import { describe, it, expect, beforeEach } from 'vitest'; +import { db, canAccessTrip } from '../../../src/db/database'; + +function seedUser(username: string): number { + return Number( + db.prepare("INSERT INTO users (username, email, password_hash, role) VALUES (?, ?, 'x', 'user')") + .run(username, `${username}@example.test`).lastInsertRowid, + ); +} + +describe('canAccessTrip', () => { + beforeEach(() => { + db.exec('DELETE FROM trip_members; DELETE FROM trips; DELETE FROM users;'); + }); + + it('returns the trip currency for the owner (#1543)', () => { + const owner = seedUser('owner'); + const tripId = Number( + db.prepare("INSERT INTO trips (user_id, title, currency) VALUES (?, 'Trip', 'RUB')") + .run(owner).lastInsertRowid, + ); + + expect(canAccessTrip(tripId, owner)).toMatchObject({ id: tripId, user_id: owner, currency: 'RUB' }); + }); + + it('returns the trip currency for a member too', () => { + const owner = seedUser('owner2'); + const member = seedUser('member2'); + const tripId = Number( + db.prepare("INSERT INTO trips (user_id, title, currency) VALUES (?, 'Trip', 'JPY')") + .run(owner).lastInsertRowid, + ); + db.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(tripId, member); + + expect(canAccessTrip(tripId, member)).toMatchObject({ currency: 'JPY' }); + }); + + it('returns undefined for a user with no access', () => { + const owner = seedUser('owner3'); + const stranger = seedUser('stranger3'); + const tripId = Number( + db.prepare("INSERT INTO trips (user_id, title) VALUES (?, 'Trip')").run(owner).lastInsertRowid, + ); + + expect(canAccessTrip(tripId, stranger)).toBeUndefined(); + }); +});