diff --git a/server/scripts/tx-writes-baseline.json b/server/scripts/tx-writes-baseline.json index a1db60507..05dcbee14 100644 --- a/server/scripts/tx-writes-baseline.json +++ b/server/scripts/tx-writes-baseline.json @@ -9,7 +9,7 @@ "src/nest/auth/passkey.service.ts#PasskeyService.passkeyRegisterOptions": 2, "src/nest/auth/passkey.service.ts#PasskeyService.passkeyRegisterVerify": 2, "src/nest/auth/user-cleanup.service.ts#UserCleanupService.erasePluginUserData": 2, - "src/nest/booking-import/booking-import.service.ts#BookingImportService.confirm": 6, + "src/nest/booking-import/booking-import.service.ts#BookingImportService.confirm": 4, "src/nest/doc-sync/doc-sync-config.service.ts#DocSyncConfigService.upsertConnection": 3, "src/nest/doc-sync/doc-sync.controller.ts#DocSyncController.createLink": 4, "src/nest/doc-sync/doc-sync.controller.ts#DocSyncController.deleteConnection": 2, diff --git a/server/src/nest/booking-import/booking-import.service.ts b/server/src/nest/booking-import/booking-import.service.ts index 8da985b8b..089c671bf 100644 --- a/server/src/nest/booking-import/booking-import.service.ts +++ b/server/src/nest/booking-import/booking-import.service.ts @@ -235,10 +235,40 @@ export class BookingImportService { return { items: allItems, warnings: allWarnings, files: fileReports }; } + /** + * The linked cost an extracted price becomes (Costs addon), so the booking shows + * up as an expense and not just a price in metadata, or undefined when there is + * none to write. The live FX rate of a foreign-currency price is frozen here, so + * a settled position isn't re-opened when live rates drift (#1445). A rate that + * cannot be frozen leaves the booking without its cost, as before. + */ + private async linkedCost( + tripId: string, + item: BookingImportPreviewItem, + ): Promise<{ total_price: number; category: string; currency?: string | null; exchange_rate?: number } | undefined> { + if (!(await this.addons.isAddonEnabled(ADDON_IDS.BUDGET))) return undefined; + const meta = item.metadata && typeof item.metadata === 'object' ? (item.metadata as Record) : null; + const price = meta && meta.price != null ? Number(meta.price) : Number.NaN; + if (!Number.isFinite(price) || price <= 0) return undefined; + const entry: { total_price: number; category: string; currency?: string | null; exchange_rate?: number } = { + total_price: price, + category: typeToCostCategory(item.type), + currency: meta && typeof meta.priceCurrency === 'string' ? meta.priceCurrency : null, + }; + try { + await this.budget.freezeForeignRate(tripId, entry); + } catch (err) { + console.error(`[booking-import] Failed to create cost for "${item.title}":`, err instanceof Error ? err.message : err); + return undefined; + } + return entry; + } + /** * Persist a confirmed list of parsed items. - * Creates place rows for hotel/restaurant/event venues, then calls createReservation. - * Broadcasts reservation:created (and accommodation:created if applicable) per item. + * Creates place rows for hotel/restaurant/event venues, then writes each booking with + * its linked cost (createWithCost). Broadcasts reservation:created, accommodation:created + * if applicable and the cost per item, after each write commits. */ async confirm( tripId: string, @@ -325,47 +355,21 @@ export class BookingImportService { }; } - const { reservation, accommodationCreated } = await this.reservations.create(tripId, { + // The booking and its linked cost are one write, through the same service + // method REST, MCP and the plugin RPC take. The cost's rate is frozen first, + // outside that write, since freezing it can fetch rates over the network. + const cost = await this.linkedCost(tripId, item); + const { reservation, accommodationCreated, costEvents } = await this.reservations.createWithCost(tripId, { ...reservationData, place_id: placeId, create_accommodation: createAccommodation, - } as any); + } as any, cost); this.realtime.broadcast(tripId, 'reservation:created', { reservation }, socketId); if (accommodationCreated) { this.realtime.broadcast(tripId, 'accommodation:created', {}, socketId); } - - // Turn an extracted price into a real linked cost (Costs addon), so the - // booking shows up as an expense — not just a price in metadata. - if ((await this.addons.isAddonEnabled(ADDON_IDS.BUDGET))) { - const meta = - reservationData.metadata && typeof reservationData.metadata === 'object' - ? (reservationData.metadata as Record) - : null; - const price = meta && meta.price != null ? Number(meta.price) : Number.NaN; - if (Number.isFinite(price) && price > 0) { - try { - const budgetData = { - category: typeToCostCategory(item.type), - name: item.title, - total_price: price, - currency: meta && typeof meta.priceCurrency === 'string' ? meta.priceCurrency : null, - reservation_id: reservation.id, - }; - // Freeze the live FX rate for a foreign-currency booking price so a - // settled position isn't re-opened when live rates drift (#1445). - await this.budget.freezeForeignRate(tripId, budgetData); - const budgetItem = await this.budget.createBudgetItem(tripId, budgetData); - this.realtime.broadcast(tripId, 'budget:created', { item: budgetItem }, socketId); - } catch (err) { - console.error( - `[booking-import] Failed to create cost for "${item.title}":`, - err instanceof Error ? err.message : err, - ); - } - } - } + this.reservations.announceCost(tripId, costEvents, socketId); created.push(reservation); } catch (err) { diff --git a/server/tests/unit/nest/booking-import/booking-import.service.test.ts b/server/tests/unit/nest/booking-import/booking-import.service.test.ts index 714d3380b..2b3128ea0 100644 --- a/server/tests/unit/nest/booking-import/booking-import.service.test.ts +++ b/server/tests/unit/nest/booking-import/booking-import.service.test.ts @@ -208,7 +208,7 @@ describe('BookingImportService.preview endpoint geocoding (#1969)', () => { // used to reach the place row exactly as the mail or the review form had it. describe('BookingImportService.confirm venue website (#2483)', () => { it('BOOKING-IMPORT-2483-01: a bare host is saved with https, a script link or nothing not at all', async () => { - const reservations = { create: vi.fn(() => ({ reservation: { id: 1 }, accommodationCreated: false })) }; + const reservations = { createWithCost: vi.fn(() => ({ reservation: { id: 1 }, accommodationCreated: false, costEvents: [] })), announceCost: vi.fn() }; const places = { create: vi.fn((_tripId: string, _input: { website?: string }) => ({ id: 7 })) }; // No dates on a restaurant, so the day repositories are never reached. const svc = new BookingImportService( @@ -233,3 +233,67 @@ describe('BookingImportService.confirm venue website (#2483)', () => { ]); }); }); + +// The booking and the cost its extracted price becomes go in as one write, through +// ReservationsService.createWithCost, with the FX rate frozen before it. +describe('BookingImportService.confirm writes the booking with its cost', () => { + function makeConfirm(opts: { budget?: boolean; freeze?: (tripId: string, entry: { currency?: string | null; exchange_rate?: number }) => Promise } = {}) { + const order: string[] = []; + const costEvents = [{ event: 'budget:created', payload: { item: { id: 9 } } }]; + const reservations = { + createWithCost: vi.fn(async () => { order.push('write'); return { reservation: { id: 1 }, accommodationCreated: false, costEvents }; }), + announceCost: vi.fn(() => { order.push('announceCost'); }), + }; + const budget = { + freezeForeignRate: vi.fn(async (tripId: string, entry: { currency?: string | null; exchange_rate?: number }) => { + order.push('freeze'); + await (opts.freeze ?? (async (_t: string, e: { exchange_rate?: number }) => { e.exchange_rate = 0.5; }))(tripId, entry); + }), + }; + const realtime = { broadcast: vi.fn((_tripId: string, event: string) => { order.push(event); }) }; + const svc = new BookingImportService( + {} as never, {} as never, undefined as never, undefined as never, reservations as never, permissionsStub as never, + budget as never, { isAddonEnabled: async () => opts.budget ?? true } as never, realtime as never, + { geocodeQuery: vi.fn() } as never, { create: vi.fn() } as never, + ); + return { svc, reservations, budget, order, costEvents }; + } + const priced = { type: 'train', title: 'ICE 123', metadata: { price: '49.9', priceCurrency: 'CHF' }, source: { fileName: 'ticket.pdf', index: 0 } }; + + it('BOOKING-IMPORT-TX-001: freezes the rate first, writes booking and cost in one call, announces the cost after the booking', async () => { + const { svc, reservations, order, costEvents } = makeConfirm(); + + const res = await svc.confirm('5', [priced as never], 'sock'); + + expect(res.created).toEqual([{ id: 1 }]); + expect(reservations.createWithCost).toHaveBeenCalledWith('5', expect.objectContaining({ title: 'ICE 123', type: 'train' }), { + total_price: 49.9, category: 'transport', currency: 'CHF', exchange_rate: 0.5, + }); + expect(reservations.announceCost).toHaveBeenCalledWith('5', costEvents, 'sock'); + expect(order).toEqual(['freeze', 'write', 'reservation:created', 'announceCost']); + }); + + it('BOOKING-IMPORT-TX-002: no cost without the Costs addon or without a price', async () => { + const off = makeConfirm({ budget: false }); + await off.svc.confirm('5', [priced as never], undefined); + expect(off.budget.freezeForeignRate).not.toHaveBeenCalled(); + expect(off.reservations.createWithCost).toHaveBeenCalledWith('5', expect.anything(), undefined); + + const free = makeConfirm(); + await free.svc.confirm('5', [{ ...priced, metadata: { price: '0' } } as never], undefined); + expect(free.budget.freezeForeignRate).not.toHaveBeenCalled(); + expect(free.reservations.createWithCost).toHaveBeenCalledWith('5', expect.anything(), undefined); + }); + + it('BOOKING-IMPORT-TX-003: a rate that cannot be frozen keeps the booking, without its cost', async () => { + const quiet = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + const { svc, reservations } = makeConfirm({ freeze: async () => { throw new Error('rates down'); } }); + const res = await svc.confirm('5', [priced as never], undefined); + expect(res.created).toEqual([{ id: 1 }]); + expect(reservations.createWithCost).toHaveBeenCalledWith('5', expect.anything(), undefined); + } finally { + quiet.mockRestore(); + } + }); +});