mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(trips): a trip created without a currency takes the display currency
The service wrote `data.currency || 'EUR'`, and only the trip dialog ever sends a currency, pre-filled from the display currency in Settings. MCP create_trip and the plugin RPC send none, so every trip an assistant created was a EUR trip whatever the person reads amounts in. The service now asks SettingsService for the owner's display currency, admin default merged in, and falls back to EUR only when that is left on "Trip currency". The REST route inherits the fallback; the web client always sends the field, so it sees no change.
This commit is contained in:
@@ -88,7 +88,7 @@ export class TripsMcp {
|
||||
description: z.string().max(2000).optional().describe('Trip description'),
|
||||
start_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional().describe('Start date (YYYY-MM-DD)'),
|
||||
end_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional().describe('End date (YYYY-MM-DD)'),
|
||||
currency: z.string().length(3).optional().describe('Currency code (e.g. EUR, USD)'),
|
||||
currency: z.string().length(3).optional().describe('Currency code (e.g. EUR, USD). Left out, the trip takes the display currency from the settings (see get_display_settings), or EUR when none is set.'),
|
||||
day_count: z.number().int().min(1).max(MAX_TRIP_DAYS).optional().describe(
|
||||
'How many days a trip without dates gets (default 7). Ignored when start_date and end_date are both set, because the range decides the count.'),
|
||||
reminder_days: z.number().int().min(0).max(30).optional().describe(
|
||||
|
||||
@@ -32,6 +32,7 @@ import { StorageModule } from '../storage/storage.module';
|
||||
import { StorageService } from '../storage/storage.service';
|
||||
import { buildStorageUploadOptions } from '../storage/storage-upload.factory';
|
||||
import { MAX_COVER_SIZE, TRIP_COVER_FILE_FILTER } from './trips.controller';
|
||||
import { SettingsModule } from '../settings/settings.module';
|
||||
|
||||
/** Trips aggregate root (C1 — Phase 3). Uses exact strangler prefixes so it does
|
||||
* not capture the nested sub-domain mounts (collab, files, ...). */
|
||||
@@ -48,7 +49,7 @@ import { MAX_COVER_SIZE, TRIP_COVER_FILE_FILTER } from './trips.controller';
|
||||
}),
|
||||
}),
|
||||
StorageModule,
|
||||
McpSharedModule, TodoModule, PackingModule, FilesModule, ReservationsModule, DaysModule, PermissionsModule, AuditModule, BudgetModule, CollabModule, VacayModule, PlacesModule, AuthModule, AppConfigModule, UnsplashModule, RealtimeModule, PluginGuardsModule, AddonsModule, TripMembershipModule, CalendarModule, AccommodationsModule, TripMembersModule, TripReadModelModule],
|
||||
McpSharedModule, TodoModule, PackingModule, FilesModule, ReservationsModule, DaysModule, PermissionsModule, AuditModule, BudgetModule, CollabModule, VacayModule, PlacesModule, AuthModule, AppConfigModule, UnsplashModule, RealtimeModule, PluginGuardsModule, AddonsModule, TripMembershipModule, CalendarModule, AccommodationsModule, TripMembersModule, TripReadModelModule, SettingsModule],
|
||||
controllers: [TripsController],
|
||||
providers: [TripsService, TripsMcp, TripPromptsMcp, TripsRpc],
|
||||
// Exported for FeedsModule (ICS feeds) and PluginsModule (RPC host injection).
|
||||
|
||||
@@ -11,6 +11,7 @@ import { ReservationsService } from '../reservations/reservations.service';
|
||||
import { VacayService } from '../vacay/vacay.service';
|
||||
import { UnsplashService } from '../unsplash/unsplash.service';
|
||||
import { StorageService } from '../storage/storage.service';
|
||||
import { SettingsService } from '../settings/settings.service';
|
||||
import { NotFoundError, ValidationError } from '../common/domain-errors';
|
||||
|
||||
export const MS_PER_DAY = 86400000;
|
||||
@@ -134,15 +135,16 @@ export interface GuestMember {
|
||||
/**
|
||||
* Trip aggregate root, DI-native. Membership, the calendar export and the two
|
||||
* read aggregates live in their own domains now; what is left is the write core
|
||||
* plus day generation, which is why the constructor is eight parameters instead
|
||||
* of fourteen. The SQL moved 1:1 from the legacy
|
||||
* plus day generation, which is why the constructor is far shorter than the
|
||||
* fourteen parameters it started with. The SQL moved 1:1 from the legacy
|
||||
* services/tripService.ts: identical statements, the `||` falsy-coercion
|
||||
* defaults, the post-write TRIP_SELECT re-selects and the mixed
|
||||
* named/positional parameter styles are all preserved byte-for-byte.
|
||||
* Post-migration quirk fixes on top of the 1:1 move: the multi-statement
|
||||
* deletes (remove, deleteGuest's re-split + user delete) run in
|
||||
* db.transaction(), and listMembers' owner row COALESCEs display_name like
|
||||
* the member rows. Auth (canAccessTrip), per-field permission checks and
|
||||
* db.transaction(), listMembers' owner row COALESCEs display_name like
|
||||
* the member rows, and create() defaults the currency to the owner's display
|
||||
* currency instead of the legacy EUR literal. Auth (canAccessTrip), per-field permission checks and
|
||||
* audit logging stay in the controller (1:1 with the legacy route);
|
||||
* trip:updated / trip:deleted broadcasts stay in the controller too — this
|
||||
* service emits none.
|
||||
@@ -159,12 +161,24 @@ export class TripsService {
|
||||
private readonly realtime: RealtimeService,
|
||||
private readonly unsplash: UnsplashService,
|
||||
private readonly storage: StorageService,
|
||||
private readonly settings: SettingsService,
|
||||
) {}
|
||||
|
||||
private get db() {
|
||||
return this.dbs.connection;
|
||||
}
|
||||
|
||||
/**
|
||||
* The currency a trip gets when the caller names none: the person's display
|
||||
* currency, admin default merged in, else EUR. The trip dialog pre-fills the
|
||||
* same value on the client, so a trip created through MCP or a plugin lands
|
||||
* in the currency the person reads amounts in instead of always in EUR.
|
||||
*/
|
||||
private defaultCurrencyFor(userId: number): string {
|
||||
const preferred = this.settings.getUserSettings(userId).default_currency;
|
||||
return typeof preferred === 'string' && preferred.trim() ? preferred.trim() : 'EUR';
|
||||
}
|
||||
|
||||
canAccessTrip(tripId: string | number, userId: number) {
|
||||
return this.dbs.canAccessTrip(tripId, userId) as { user_id: number } | null | undefined;
|
||||
}
|
||||
@@ -328,7 +342,7 @@ export class TripsService {
|
||||
const result = this.db.prepare(`
|
||||
INSERT INTO trips (user_id, title, description, start_date, end_date, currency, reminder_days)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)
|
||||
`).run(userId, data.title, data.description || null, data.start_date || null, data.end_date || null, data.currency || 'EUR', rd);
|
||||
`).run(userId, data.title, data.description || null, data.start_date || null, data.end_date || null, data.currency || this.defaultCurrencyFor(userId), rd);
|
||||
|
||||
const tripId = result.lastInsertRowid;
|
||||
this.generateDays(tripId, data.start_date || null, data.end_date || null, data.day_count);
|
||||
|
||||
@@ -101,6 +101,9 @@ const { db } = vi.hoisted(() => {
|
||||
// StorageRegistryService (behind StorageModule, now in this module chain) reads
|
||||
// this at onModuleInit.
|
||||
tmp.exec('CREATE TABLE app_settings (key TEXT PRIMARY KEY, value TEXT);');
|
||||
// A trip created without a currency takes the owner's display currency, read
|
||||
// off the per-user settings rows.
|
||||
tmp.exec('CREATE TABLE settings (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL, key TEXT NOT NULL, value TEXT, UNIQUE(user_id, key));');
|
||||
return { db: tmp };
|
||||
});
|
||||
|
||||
@@ -160,6 +163,7 @@ describe('Trips e2e (real auth guard + temp SQLite)', () => {
|
||||
db.prepare('DELETE FROM trip_members').run();
|
||||
db.prepare('DELETE FROM days').run();
|
||||
db.prepare('DELETE FROM audit_log').run();
|
||||
db.prepare('DELETE FROM settings').run();
|
||||
canAccessTrip.mockReturnValue({ user_id: 1 });
|
||||
checkPermission.mockReturnValue(true);
|
||||
});
|
||||
@@ -198,6 +202,16 @@ describe('Trips e2e (real auth guard + temp SQLite)', () => {
|
||||
expect(forbidden.status).toBe(403);
|
||||
});
|
||||
|
||||
it('201 create without a currency takes the display currency from the settings', async () => {
|
||||
db.prepare("INSERT INTO settings (user_id, key, value) VALUES (1, 'default_currency', ?)").run(JSON.stringify('USD'));
|
||||
const preferred = await request(server).post('/api/trips').set('Cookie', sessionCookie(1)).send({ title: 'Road trip' });
|
||||
expect(preferred.status).toBe(201);
|
||||
expect(preferred.body.trip).toMatchObject({ title: 'Road trip', currency: 'USD' });
|
||||
const explicit = await request(server).post('/api/trips').set('Cookie', sessionCookie(1)).send({ title: 'Tokyo', currency: 'JPY' });
|
||||
expect(explicit.status).toBe(201);
|
||||
expect(explicit.body.trip).toMatchObject({ title: 'Tokyo', currency: 'JPY' });
|
||||
});
|
||||
|
||||
it('201 create keeps every day of a trip longer than a year (#2403)', async () => {
|
||||
// 2025-01-26 .. 2026-01-28 is 368 days; the day list used to stop at 365.
|
||||
const res = await request(server).post('/api/trips').set('Cookie', sessionCookie(1))
|
||||
|
||||
@@ -186,6 +186,7 @@ export function createMcpTestRegistry(): McpRegistry {
|
||||
realtimeService,
|
||||
new UnsplashService(dbService, new RuntimeEnvService(), generalStorage),
|
||||
generalStorage,
|
||||
new SettingsService(dbService),
|
||||
);
|
||||
const readModelService = new TripReadModelService(
|
||||
dbService, membersService, daysService, accommodationsService, budgetService,
|
||||
|
||||
@@ -113,7 +113,7 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF
|
||||
const places = new PlacesService(dbs, permissions, realtime, new MapsService(dbs, photoCache), queryHelpers, unsplash, photoCache, journey, generalStorage, accommodations);
|
||||
// After accommodations: a hotel booking writes the stay's day stop through it.
|
||||
const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadRepository(dbs), accommodations);
|
||||
const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash, generalStorage);
|
||||
const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash, generalStorage, new SettingsService(dbs));
|
||||
const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget), permissions, realtime, notificationsStub());
|
||||
const guards = new PluginGuards(dbs, permissions, addons);
|
||||
|
||||
|
||||
@@ -204,6 +204,25 @@ describe('Tool: create_trip', () => {
|
||||
expect(testDb.prepare('SELECT COUNT(*) as c FROM trips').get()).toEqual({ c: 0 });
|
||||
});
|
||||
});
|
||||
|
||||
it('gives a trip without a currency the display currency from the settings, not EUR', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'default_currency', ?)").run(user.id, JSON.stringify('USD'));
|
||||
await withHarness(user.id, async (h) => {
|
||||
const fromSettings = parseToolResult(await h.client.callTool({ name: 'create_trip', arguments: { title: 'Road trip' } })) as any;
|
||||
expect(fromSettings.trip.currency).toBe('USD');
|
||||
const explicit = parseToolResult(await h.client.callTool({ name: 'create_trip', arguments: { title: 'Tokyo', currency: 'JPY' } })) as any;
|
||||
expect(explicit.trip.currency).toBe('JPY');
|
||||
});
|
||||
});
|
||||
|
||||
it('falls back to EUR when neither the user nor the admin set a display currency', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const data = parseToolResult(await h.client.callTool({ name: 'create_trip', arguments: { title: 'Plain' } })) as any;
|
||||
expect(data.trip.currency).toBe('EUR');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -88,6 +88,7 @@ import fs from 'fs';
|
||||
import path from 'path';
|
||||
import { notificationsStub } from '../../helpers/notifications';
|
||||
import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service';
|
||||
import { SettingsService } from '../../../src/nest/settings/settings.service';
|
||||
|
||||
// Real sibling services over the same in-memory DB — updateTrip's date-shift
|
||||
// resyncs and the summary/bundle aggregation run their actual SQL.
|
||||
@@ -128,6 +129,7 @@ const svc = new TripsService(
|
||||
new RealtimeService(),
|
||||
undefined as never, // unsplash — not exercised here
|
||||
coversFx.storage,
|
||||
new SettingsService(dbs()),
|
||||
);
|
||||
const membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc), new PermissionsService(dbs()), new RealtimeService(), notificationsStub());
|
||||
const readModelSvc = new TripReadModelService(
|
||||
@@ -745,6 +747,34 @@ describe('folded trip CRUD', () => {
|
||||
expect(getDays(tripId)).toHaveLength(3);
|
||||
});
|
||||
|
||||
it('TRIP-SVC-069: create without a currency takes the display currency, admin default included, else EUR', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const setUser = (value: string) =>
|
||||
testDb.prepare("INSERT INTO settings (user_id, key, value) VALUES (?, 'default_currency', ?) ON CONFLICT(user_id, key) DO UPDATE SET value = excluded.value")
|
||||
.run(user.id, JSON.stringify(value));
|
||||
const setAdmin = (value: string) =>
|
||||
testDb.prepare("INSERT INTO app_settings (key, value) VALUES ('default_user_setting_default_currency', ?) ON CONFLICT(key) DO UPDATE SET value = excluded.value")
|
||||
.run(JSON.stringify(value));
|
||||
const currencyOf = (data: Parameters<typeof svc.create>[1]) => (svc.create(user.id, data).trip as any).currency;
|
||||
|
||||
expect(currencyOf({ title: 'Nothing set' })).toBe('EUR');
|
||||
// "Trip currency" in the settings stores an empty string, which counts as unset.
|
||||
setUser('');
|
||||
expect(currencyOf({ title: 'Cleared, no admin default' })).toBe('EUR');
|
||||
setUser(' ');
|
||||
expect(currencyOf({ title: 'Blank, no admin default' })).toBe('EUR');
|
||||
setUser('');
|
||||
setAdmin('CHF');
|
||||
expect(currencyOf({ title: 'Admin default' })).toBe('CHF');
|
||||
setUser('USD');
|
||||
expect(currencyOf({ title: 'Own display currency' })).toBe('USD');
|
||||
setUser('');
|
||||
expect(currencyOf({ title: 'Back on the admin default' })).toBe('CHF');
|
||||
// An explicit currency always wins.
|
||||
setUser('USD');
|
||||
expect(currencyOf({ title: 'Explicit', currency: 'JPY' })).toBe('JPY');
|
||||
});
|
||||
|
||||
it('TRIP-SVC-064: create refuses a range past MAX_TRIP_DAYS and writes nothing', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const before = (testDb.prepare('SELECT COUNT(*) AS n FROM trips').get() as { n: number }).n;
|
||||
@@ -1193,6 +1223,7 @@ describe('quirk fixes', () => {
|
||||
new RealtimeService(),
|
||||
undefined as never,
|
||||
coversFx.storage,
|
||||
new SettingsService(dbs()),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user