mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-22 15:06:32 +00:00
fix(booking-import): write an imported booking with its cost in one transaction
Confirming an import created the booking, then froze the FX rate and created the linked cost in a second commit, so a failed cost write left the booking without its expense. REST, MCP and the plugin RPC already go through ReservationsService.createWithCost. The import now builds the cost entry and freezes its rate first, outside the write since that can fetch rates, then calls createWithCost and announces the cost after the commit. A rate that cannot be frozen keeps the booking without its cost, as before. The place a venue becomes still commits on its own, with endpoint geocoding between it and the booking, so confirm stays in the lint:tx baseline at 4.
This commit is contained in:
Maurice
@@ -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,
|
||||
|
||||
@@ -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<string, unknown>) : 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<string, unknown>)
|
||||
: 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) {
|
||||
|
||||
@@ -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<void> } = {}) {
|
||||
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();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user