From ce58de29a5cd2903c447c1f0afde017d62c97579 Mon Sep 17 00:00:00 2001 From: Xheldon Date: Sat, 5 Sep 2026 00:35:04 +0800 Subject: [PATCH] fix(reservations): keep the linked expense price when a booking is edited A booking's linked expense price lives in reservations.metadata.price (written by BudgetService.syncReservationPrice). The booking form rebuilds metadata from its fields on every save and only carries transit / airtrail_ids over, so PUT /api/trips/:tripId/reservations/:id overwrote the stored metadata and the price vanished from the card until the expense was re-saved. ReservationsService.update() now carries price / priceCurrency from the stored metadata into an incoming metadata object that does not set them, as long as a budget item is still linked to the booking. A payload that sets price itself, a booking without a linked expense, and an explicit metadata: null behave exactly as before. Closes #2233 --- .../nest/reservations/reservations.service.ts | 39 +++++++++++++++++- .../unit/nest/reservations.service.test.ts | 41 +++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 107c436b0..62dd02a28 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -720,6 +720,15 @@ export class ReservationsService { } } + // metadata.price / priceCurrency are owned by the linked expense + // (BudgetService.syncReservationPrice writes them), not by the booking form: + // the client rebuilds metadata from the form on every save and only carries + // transit / airtrail_ids over, so a plain edit used to wipe the price off the + // card until the expense was re-saved. Keep them while a linked item exists; + // a payload that sets them itself, or a booking without a linked expense, + // is left untouched. + const nextMetadata = this.keepLinkedExpensePrice(id, tripId, metadata, current.metadata); + const resolvedType = (type ?? current.type) || 'other'; const nextReservationTime = resolvedType === 'hotel' ? null @@ -790,7 +799,7 @@ export class ReservationsService { status || null, type || null, resolvedAccId, - metadata !== undefined ? (metadata ? JSON.stringify(metadata) : null) : current.metadata, + nextMetadata !== undefined ? (nextMetadata ? JSON.stringify(nextMetadata) : null) : current.metadata, needs_review === undefined ? null : (needs_review ? 1 : 0), id ); @@ -800,7 +809,7 @@ export class ReservationsService { } // Sync check-in/out to accommodation if linked - const resolvedMeta = metadata !== undefined ? metadata : (current.metadata ? JSON.parse(current.metadata as string) : null); + const resolvedMeta = nextMetadata !== undefined ? nextMetadata : (current.metadata ? JSON.parse(current.metadata as string) : null); if (resolvedAccId && resolvedMeta) { const meta = (typeof resolvedMeta === 'string' ? JSON.parse(resolvedMeta) : resolvedMeta) as AccommodationTimesMeta; if (meta.check_in_time || meta.check_in_end_time || meta.check_out_time) { @@ -824,6 +833,32 @@ export class ReservationsService { return { reservation, accommodationChanged }; } + /** + * Carry `price` / `priceCurrency` from the stored metadata into an incoming + * metadata object that doesn't set them, as long as a budget item is still + * linked to the booking. Returns the incoming value untouched otherwise — + * `undefined` (field absent) and `null` (explicit clear) pass straight through. + */ + private keepLinkedExpensePrice(id: string | number, tripId: string | number, incoming: unknown, stored: string | null | undefined): unknown { + if (!incoming || typeof incoming !== 'object' || Array.isArray(incoming) || !stored) return incoming; + const next = incoming as Record; + if (next.price !== undefined) return incoming; + let prev: Record; + try { + const parsed: unknown = JSON.parse(stored); + if (!parsed || typeof parsed !== 'object') return incoming; + prev = parsed as Record; + } catch { + return incoming; + } + if (prev.price === undefined) return incoming; + const linked = this.db.get('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?', tripId, id); + if (!linked) return incoming; + const kept: Record = { ...next, price: prev.price }; + if (prev.priceCurrency !== undefined && next.priceCurrency === undefined) kept.priceCurrency = prev.priceCurrency; + return kept; + } + /** The accommodation + budget-item + reservation deletes are one logical * cascade — all-or-nothing. */ remove(id: string | number, tripId: string | number): { deleted: { id: number; title: string; type: string; accommodation_id: number | null } | undefined; accommodationDeleted: boolean; deletedBudgetItemId: number | null } { diff --git a/server/tests/unit/nest/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index 981ac0cb6..fcd3dd19c 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/server/tests/unit/nest/reservations.service.test.ts @@ -200,6 +200,47 @@ describe('ReservationsService (DI-native, real SQL)', () => { expect(reservation.accommodation_id).toBeNull(); }); + it('RESV-SVC-033: a metadata payload without `price` keeps the linked expense price (and currency) from the stored metadata', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id, { title: 'Flight', type: 'flight' }); + const item = createBudgetItem(testDb, trip.id, { name: 'Flight', total_price: 2040 }); + testDb.prepare('UPDATE budget_items SET reservation_id = ? WHERE id = ?').run(res.id, item.id); + testDb.prepare('UPDATE reservations SET metadata = ? WHERE id = ?') + .run(JSON.stringify({ airline: 'CZ', seat: '72K', price: '2040', priceCurrency: 'CNY' }), res.id); + const current = svc.getReservation(String(res.id), String(trip.id))!; + // What the booking form sends back: rebuilt from its fields, price not among them. + const { reservation } = svc.update(String(res.id), String(trip.id), { metadata: { airline: 'CZ', seat: '12A' } }, current); + expect(JSON.parse(reservation.metadata as string)).toEqual({ airline: 'CZ', seat: '12A', price: '2040', priceCurrency: 'CNY' }); + }); + + it('RESV-SVC-034: a payload that sets `price` itself, or a booking with no linked expense, is stored as sent', () => { + const { trip } = ownerTrip(); + const linkedRes = createReservation(testDb, trip.id, { title: 'Linked', type: 'flight' }); + const item = createBudgetItem(testDb, trip.id, { name: 'Linked', total_price: 100 }); + testDb.prepare('UPDATE budget_items SET reservation_id = ? WHERE id = ?').run(linkedRes.id, item.id); + testDb.prepare('UPDATE reservations SET metadata = ? WHERE id = ?').run(JSON.stringify({ price: '100' }), linkedRes.id); + let current = svc.getReservation(String(linkedRes.id), String(trip.id))!; + let { reservation } = svc.update(String(linkedRes.id), String(trip.id), { metadata: { price: '150' } }, current); + expect(JSON.parse(reservation.metadata as string)).toEqual({ price: '150' }); + + const orphanRes = createReservation(testDb, trip.id, { title: 'Orphan', type: 'flight' }); + testDb.prepare('UPDATE reservations SET metadata = ? WHERE id = ?').run(JSON.stringify({ price: '999' }), orphanRes.id); + current = svc.getReservation(String(orphanRes.id), String(trip.id))!; + ({ reservation } = svc.update(String(orphanRes.id), String(trip.id), { metadata: { seat: '1A' } }, current)); + expect(JSON.parse(reservation.metadata as string)).toEqual({ seat: '1A' }); + }); + + it('RESV-SVC-035: metadata null still clears the stored metadata, linked expense or not', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id, { title: 'Flight', type: 'flight' }); + const item = createBudgetItem(testDb, trip.id, { name: 'Flight', total_price: 50 }); + testDb.prepare('UPDATE budget_items SET reservation_id = ? WHERE id = ?').run(res.id, item.id); + testDb.prepare('UPDATE reservations SET metadata = ? WHERE id = ?').run(JSON.stringify({ price: '50' }), res.id); + const current = svc.getReservation(String(res.id), String(trip.id))!; + const { reservation } = svc.update(String(res.id), String(trip.id), { metadata: null }, current); + expect(reservation.metadata).toBeNull(); + }); + it('RESV-SVC-010: endpoints [] wipes stored endpoints; an absent field leaves them alone', () => { const { trip } = ownerTrip(); const res = createReservation(testDb, trip.id, { title: 'Bus' });