From 07117949ef6d98f6294d354cdbb06b01c6965c32 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 17:29:22 +0200 Subject: [PATCH] fix(mobile): editing a bus, car, taxi or ferry on the phone keeps its price (#2233) Editing a transport without metadata of its own (bus, car, taxi, ferry without a transit itinerary) in the phone sheet wiped the expense price shown on the booking card. The phone built the payload with emptyMetadataAsNull and sent metadata: null, which the reservations service reads as "clear the column", so keepMirroredPrice had nothing to carry price and priceCurrency into. The desktop and both booking forms send an empty object, which keeps the price. The option only existed to keep the phone's null, so it is gone and both views now send the rebuilt metadata object as is. Red test: FE-MOB-TRFRM-060 in client/tests/unit/mobile/trip/MTransportFormSheet.test.tsx failed with "AssertionError: expected null to deeply equal {}". RESV-SVC-043 in server/tests/unit/nest/reservations.service.test.ts shows the server side: {} keeps the stored price, null drops it. Changed assertions (they asserted the null): - MTransportFormSheet.test.tsx FE-MOB-TRFRM-006: payload metadata null is now {}. - MTransportFormSheet.test.tsx FE-MOB-TRFRM-011: metadata toBeNull is now toEqual({}). - MTransportFormSheet.test.tsx FE-MOB-TRFRM-027: metadata toBeNull is now toEqual({}). - transportEndpoints.test.ts FE-PLANNER-TRANSPORTEP-014: phone metadata toBeNull is now toEqual({}) (title updated to match), and the emptyMetadataAsNull field is dropped from DESKTOP_SAVE and PHONE_SAVE. - useTransportForm.test.ts FE-PLANNER-TRANSPORTFORM-007: metadata: null is now metadata: {}. --- .../src/components/Planner/TransportModal.tsx | 2 +- .../Planner/transportEndpoints.test.ts | 6 ++---- .../components/Planner/transportEndpoints.ts | 6 +++--- .../Planner/useTransportForm.test.ts | 3 +-- .../trip/sheets/MTransportFormSheet.tsx | 2 +- .../mobile/trip/MTransportFormSheet.test.tsx | 21 ++++++++++++++++--- .../unit/nest/reservations.service.test.ts | 17 +++++++++++++++ 7 files changed, 43 insertions(+), 14 deletions(-) diff --git a/client/src/components/Planner/TransportModal.tsx b/client/src/components/Planner/TransportModal.tsx index 5d5f399b7..a5d25e0a0 100644 --- a/client/src/components/Planner/TransportModal.tsx +++ b/client/src/components/Planner/TransportModal.tsx @@ -137,7 +137,7 @@ export function TransportModal({ isOpen, onClose, onSave, reservation, days, sel setIsSaving(true) try { const saved = await onSave(payload(days, { - reservation, prefill, budgetEnabled: isBudgetEnabled, anchorOnStations: false, emptyMetadataAsNull: false, url: form.url, + reservation, prefill, budgetEnabled: isBudgetEnabled, anchorOnStations: false, url: form.url, })) // Persist the traveler assignment once we have the reservation id (create → save // result, edit → existing reservation), and only when it actually changed (#1517). diff --git a/client/src/components/Planner/transportEndpoints.test.ts b/client/src/components/Planner/transportEndpoints.test.ts index 7a95828a7..b5e6186f6 100644 --- a/client/src/components/Planner/transportEndpoints.test.ts +++ b/client/src/components/Planner/transportEndpoints.test.ts @@ -82,7 +82,6 @@ const DESKTOP_SAVE: TransportPayloadOptions = { prefill: null, budgetEnabled: false, anchorOnStations: false, - emptyMetadataAsNull: false, url: '', }; const PHONE_SAVE: TransportPayloadOptions = { @@ -90,7 +89,6 @@ const PHONE_SAVE: TransportPayloadOptions = { prefill: null, budgetEnabled: false, anchorOnStations: true, - emptyMetadataAsNull: true, }; describe('transport types and endpoints', () => { @@ -392,10 +390,10 @@ describe('buildTransportPayload', () => { }); }); - it('FE-PLANNER-TRANSPORTEP-014: the phone sends no url and an empty metadata as null; a url is sent when given', () => { + it('FE-PLANNER-TRANSPORTEP-014: the phone sends no url and an empty metadata as an empty object; a url is sent when given', () => { const phone = buildTransportPayload(draft(), days, PHONE_SAVE); expect(phone).not.toHaveProperty('url'); - expect(phone.metadata).toBeNull(); + expect(phone.metadata).toEqual({}); expect(buildTransportPayload(draft(), days, { ...DESKTOP_SAVE, url: 'https://x.test' }).url).toBe('https://x.test'); }); diff --git a/client/src/components/Planner/transportEndpoints.ts b/client/src/components/Planner/transportEndpoints.ts index ddb829ccf..dc3465c82 100644 --- a/client/src/components/Planner/transportEndpoints.ts +++ b/client/src/components/Planner/transportEndpoints.ts @@ -497,8 +497,6 @@ export interface TransportPayloadOptions { * first and last rows. */ anchorOnStations: boolean; - /** The phone sends a booking without metadata as null; the desktop sends an empty object (#2233). */ - emptyMetadataAsNull: boolean; /** The desktop's link field; the phone has none and sends no url. */ url?: string; } @@ -731,7 +729,9 @@ export function buildTransportPayload( confirmation_number: form.confirmation_number || null, notes: form.notes || null, ...(opts.url !== undefined ? { url: opts.url || null } : {}), - metadata: opts.emptyMetadataAsNull && Object.keys(metadata).length === 0 ? null : metadata, + // Always an object, never null: the server reads null as "clear the column" and + // would drop the expense price mirrored into metadata (#2233). + metadata, endpoints, needs_review: false, }; diff --git a/client/src/components/Planner/useTransportForm.test.ts b/client/src/components/Planner/useTransportForm.test.ts index 8e808c9ff..005c22063 100644 --- a/client/src/components/Planner/useTransportForm.test.ts +++ b/client/src/components/Planner/useTransportForm.test.ts @@ -150,9 +150,8 @@ describe('useTransportForm', () => { prefill: null, budgetEnabled: false, anchorOnStations: true, - emptyMetadataAsNull: true, }); - expect(payload).toMatchObject({ title: 'Coach', type: 'bus', day_id: 1, metadata: null }); + expect(payload).toMatchObject({ title: 'Coach', type: 'bus', day_id: 1, metadata: {} }); expect(payload.endpoints).toEqual([ expect.objectContaining({ role: 'to', name: 'B', sequence: 1, local_date: '2026-05-01' }), ]); diff --git a/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx b/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx index 6f9b398e8..6ee2295a7 100644 --- a/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx @@ -128,7 +128,7 @@ export default function MTransportFormSheet({ planner, onOpenExpense }: MTranspo setIsSaving(true) try { const saved = await saveTransport(buildPayload(days, { - reservation: res, prefill, budgetEnabled: isBudgetEnabled, anchorOnStations: true, emptyMetadataAsNull: true, + reservation: res, prefill, budgetEnabled: isBudgetEnabled, anchorOnStations: true, })) // Persist the traveler assignment once we have the reservation id (from the // save result on create, or the edited reservation) — only when it changed. diff --git a/client/tests/unit/mobile/trip/MTransportFormSheet.test.tsx b/client/tests/unit/mobile/trip/MTransportFormSheet.test.tsx index 942d1fe67..8a442ba84 100644 --- a/client/tests/unit/mobile/trip/MTransportFormSheet.test.tsx +++ b/client/tests/unit/mobile/trip/MTransportFormSheet.test.tsx @@ -248,7 +248,7 @@ describe('MTransportFormSheet', () => { location: null, confirmation_number: 'BUS-4711', notes: 'Seat 4 reserved', - metadata: null, + metadata: {}, endpoints: [ { role: 'from', sequence: 0, name: 'Osaka Station', code: null, lat: 34.7, lng: 135.5, timezone: null, local_date: '2026-05-01', local_time: '08:15' }, { role: 'to', sequence: 1, name: 'Kyoto Station', code: null, lat: 34.98, lng: 135.75, timezone: null, local_date: '2026-05-02', local_time: '11:00' }, @@ -395,7 +395,7 @@ describe('MTransportFormSheet', () => { await submit() const payload = handleSaveTransport.mock.calls[0][0] - expect(payload.metadata).toBeNull() + expect(payload.metadata).toEqual({}) expect(payload.endpoints).toEqual([]) expect(payload.day_id).toBeNull() }) @@ -791,7 +791,7 @@ describe('MTransportFormSheet', () => { await submit('common.update') const payload = handleSaveTransport.mock.calls[0][0] - expect(payload.metadata).toBeNull() + expect(payload.metadata).toEqual({}) expect(payload.endpoints).toEqual([ expect.objectContaining({ role: 'from', name: 'Osaka Station' }), expect.objectContaining({ role: 'to', sequence: 1, name: 'Nara Station' }), @@ -1142,4 +1142,19 @@ describe('MTransportFormSheet', () => { expect(handleSaveTransport).toHaveBeenCalled() expect(addFile).not.toHaveBeenCalled() }) + + it('FE-MOB-TRFRM-060: editing a bus with a stored price sends an empty metadata object, not null, so the price survives (#2233)', async () => { + const handleSaveTransport = makeSave() + const editingTransport = { + id: 62, trip_id: 1, type: 'bus', title: 'Airport shuttle', status: 'confirmed', day_id: 12, + metadata: { price: '45', priceCurrency: 'EUR' }, endpoints: [], + } as unknown as Reservation + renderSheet(makePlanner({ editingTransport, handleSaveTransport })) + typeTitle('Airport shuttle (late)') + + await submit('common.update') + // The server reads null as "clear the column" and only carries the expense + // price over into an object, so null here would wipe price / priceCurrency. + expect(handleSaveTransport.mock.calls[0][0].metadata).toEqual({}) + }) }) diff --git a/server/tests/unit/nest/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index 31bb98b13..4e29d8a6f 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/server/tests/unit/nest/reservations.service.test.ts @@ -263,6 +263,23 @@ describe('ReservationsService (DI-native, real SQL)', () => { expect(reservation.metadata).toBeNull(); }); + it('RESV-SVC-043: a bus edit with an empty metadata object keeps the price, the same edit with null drops it (#2233)', async () => { + // A bus, car, taxi or ferry has no metadata of its own, so the booking form + // rebuilds it as {}. The phone used to send null instead, which this + // service rightly reads as "clear the column", and the price went with it. + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id, { title: 'Shuttle', type: 'bus' }); + await updateRows(orm, Reservations, { id: res.id }, { metadata: JSON.stringify({ price: '45', priceCurrency: 'EUR' }) }); + + let current = (await svc.getReservation(String(res.id), String(trip.id)))!; + let { reservation } = await svc.update(String(res.id), String(trip.id), { metadata: {} }, current); + expect(JSON.parse(reservation.metadata as string)).toEqual({ price: '45', priceCurrency: 'EUR' }); + + current = (await svc.getReservation(String(res.id), String(trip.id)))!; + ({ reservation } = await 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', async () => { const { trip } = ownerTrip(); const res = createReservation(testDb, trip.id, { title: 'Bus' });