From 8c9a31e5ef868eab25cfeaa7e021b5b3fa8356c0 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 17:48:16 +0200 Subject: [PATCH] fix(planner): switching an edited flight or train to car no longer brings back stops from an earlier booking The transport form stays mounted between openings. Editing a flight or a train after a car kept the car's stops (desktop and phone) and, on the desktop, the from and to picks of that earlier booking as well. Switching the type to car in the same dialog showed those stale stops and places, and saving wrote them onto the booking. The desktop also kept stale flight and train rows when opening a car, bus or other booking. seedTransportRoute only reset the rows a type does not show when the phone asked for it (resetHiddenRoutes), and never reset the car stops for a flight or a train. It now returns every row for every type, so a seed always replaces the whole route. The option is removed; both views get the same clean route. Red test: FE-PLANNER-TRANSPORTFORM-008 in client/src/components/Planner/useTransportForm.test.ts failed with "expected [ { location: { ... }, time: '' } ] to deeply equal []". Changed assertions (they asserted the stale rows were kept): - useTransportForm.test.ts FE-PLANNER-TRANSPORTFORM-005: desktop fromPick toBe(picked) is now toEqual({}) (title updated), and the resetHiddenRoutes field is dropped from seedOptions and the phone call. - transportEndpoints.test.ts FE-PLANNER-TRANSPORTEP-005: desktop route keys toEqual(['waypoints']) is now empty fromPick, toPick, carStops and trainWaypoints. - transportEndpoints.test.ts FE-PLANNER-TRANSPORTEP-010: desktop car waypoints toBeUndefined() is now two empty rows (plus empty train rows), and the bus route now also carries the two empty row sets. - transportEndpoints.test.ts: resetHiddenRoutes dropped from DESKTOP and PHONE. --- .../src/components/Planner/TransportModal.tsx | 1 - .../Planner/transportEndpoints.test.ts | 22 ++++++++---- .../components/Planner/transportEndpoints.ts | 32 ++++++++++------- .../Planner/useTransportForm.test.ts | 35 ++++++++++++++++--- .../trip/sheets/MTransportFormSheet.tsx | 1 - 5 files changed, 66 insertions(+), 25 deletions(-) diff --git a/client/src/components/Planner/TransportModal.tsx b/client/src/components/Planner/TransportModal.tsx index a5d25e0a0..22d81ce98 100644 --- a/client/src/components/Planner/TransportModal.tsx +++ b/client/src/components/Planner/TransportModal.tsx @@ -119,7 +119,6 @@ export function TransportModal({ isOpen, onClose, onSave, reservation, days, sel fallbackType: 'transport_other', dayId: selectedDayId ?? '', stopDaysFromEndpoints: false, - resetHiddenRoutes: false, extraFields: (s, meta) => ({ url: s.url || '', meta_airline: meta.airline || '', diff --git a/client/src/components/Planner/transportEndpoints.test.ts b/client/src/components/Planner/transportEndpoints.test.ts index b5e6186f6..c623f447a 100644 --- a/client/src/components/Planner/transportEndpoints.test.ts +++ b/client/src/components/Planner/transportEndpoints.test.ts @@ -62,8 +62,8 @@ const airport = (iata: string, city = 'City'): Airport => ({ tz: 'Europe/Berlin', }); -const DESKTOP: RouteSeedOptions = { isEdit: true, stopDaysFromEndpoints: false, resetHiddenRoutes: false }; -const PHONE: RouteSeedOptions = { isEdit: true, stopDaysFromEndpoints: true, resetHiddenRoutes: true }; +const DESKTOP: RouteSeedOptions = { isEdit: true, stopDaysFromEndpoints: false }; +const PHONE: RouteSeedOptions = { isEdit: true, stopDaysFromEndpoints: true }; function draft(overrides: Partial = {}): TransportDraft { return { @@ -197,7 +197,10 @@ describe('seedTransportRoute', () => { it('FE-PLANNER-TRANSPORTEP-005: a flight seeds its airports; only the phone reads a stopover day from its endpoint', () => { const desktop = seedTransportRoute(flight, meta, 'flight', days, DESKTOP); - expect(Object.keys(desktop)).toEqual(['waypoints']); + expect(desktop.fromPick).toEqual({}); + expect(desktop.toPick).toEqual({}); + expect(desktop.carStops).toEqual([]); + expect(desktop.trainWaypoints).toEqual([emptyStationWaypoint(), emptyStationWaypoint()]); const [first, stop, last] = desktop.waypoints!; expect(first).toMatchObject({ depDayId: 1, depTime: '08:00', arrTime: '', airline: 'LH', seat: '3C' }); expect(first.airport?.iata).toBe('MUC'); @@ -300,7 +303,7 @@ describe('seedTransportRoute', () => { expect(arr).toMatchObject({ location: null, arrDayId: 2 }); }); - it('FE-PLANNER-TRANSPORTEP-010: a car reads its stops in order; another type keeps none; the phone resets the rest', () => { + it('FE-PLANNER-TRANSPORTEP-010: a car reads its stops in order; another type keeps none; the rest is reset', () => { const car = buildReservation({ type: 'car', endpoints: [ep('from', 0), ep('stop', 2, { local_time: '14:00' }), ep('stop', 1), ep('to', 3)], @@ -312,14 +315,21 @@ describe('seedTransportRoute', () => { { location: { name: 'stop-1', lat: 11, lng: 21, address: null }, time: '' }, { location: { name: 'stop-2', lat: 12, lng: 22, address: null }, time: '14:00' }, ]); - expect(desktop.waypoints).toBeUndefined(); + expect(desktop.waypoints).toEqual([emptyWaypoint(), emptyWaypoint()]); + expect(desktop.trainWaypoints).toEqual([emptyStationWaypoint(), emptyStationWaypoint()]); const phone = seedTransportRoute(car, {}, 'car', days, PHONE); expect(phone.waypoints).toEqual([emptyWaypoint(), emptyWaypoint()]); expect(phone.trainWaypoints).toEqual([emptyStationWaypoint(), emptyStationWaypoint()]); const bus = seedTransportRoute({ ...car, type: 'bus', endpoints: [] }, {}, 'bus', days, DESKTOP); - expect(bus).toEqual({ fromPick: { location: undefined }, toPick: { location: undefined }, carStops: [] }); + expect(bus).toEqual({ + fromPick: { location: undefined }, + toPick: { location: undefined }, + waypoints: [emptyWaypoint(), emptyWaypoint()], + trainWaypoints: [emptyStationWaypoint(), emptyStationWaypoint()], + carStops: [], + }); }); it('FE-PLANNER-TRANSPORTEP-011: a new booking starts every row on the given day', () => { diff --git a/client/src/components/Planner/transportEndpoints.ts b/client/src/components/Planner/transportEndpoints.ts index dc3465c82..50d22700b 100644 --- a/client/src/components/Planner/transportEndpoints.ts +++ b/client/src/components/Planner/transportEndpoints.ts @@ -291,8 +291,6 @@ export interface RouteSeedOptions { * local_date. The desktop leaves it empty. */ stopDaysFromEndpoints: boolean; - /** The phone also clears the rows the booking's type does not show. */ - resetHiddenRoutes: boolean; } /** The empty route of a new booking, every row starting on the given day. */ @@ -429,24 +427,34 @@ export function seedTransportRoute( type: TransportType, days: Day[], opts: RouteSeedOptions -): TransportRoute { +): Required { + // Every row is set, including those the type does not show: the form stays + // mounted between openings, and a row kept from the previous booking would come + // back (and be saved) as soon as the type is switched to one that shows it. if (type === 'flight') { - const waypoints = flightWaypoints(src, meta, days, opts); - return opts.resetHiddenRoutes ? { waypoints, fromPick: {}, toPick: {} } : { waypoints }; + return { + waypoints: flightWaypoints(src, meta, days, opts), + trainWaypoints: [emptyStationWaypoint(), emptyStationWaypoint()], + fromPick: {}, + toPick: {}, + carStops: [], + }; } if (usesStationRoute(type)) { - return { trainWaypoints: stationWaypoints(src, meta, days, opts), fromPick: {}, toPick: {} }; + return { + waypoints: [emptyWaypoint(), emptyWaypoint()], + trainWaypoints: stationWaypoints(src, meta, days, opts), + fromPick: {}, + toPick: {}, + carStops: [], + }; } const eps = src.endpoints || []; return { fromPick: { location: locationFromEndpoint(eps.find((e) => e.role === 'from')) || undefined }, toPick: { location: locationFromEndpoint(eps.find((e) => e.role === 'to')) || undefined }, - ...(opts.resetHiddenRoutes - ? { - waypoints: [emptyWaypoint(), emptyWaypoint()], - trainWaypoints: [emptyStationWaypoint(), emptyStationWaypoint()], - } - : {}), + waypoints: [emptyWaypoint(), emptyWaypoint()], + trainWaypoints: [emptyStationWaypoint(), emptyStationWaypoint()], // Stops persist for every type; only a car offers an editor for them, so only a // car reads them back into one. The others keep passing theirs through untouched. carStops: diff --git a/client/src/components/Planner/useTransportForm.test.ts b/client/src/components/Planner/useTransportForm.test.ts index 005c22063..70a376d06 100644 --- a/client/src/components/Planner/useTransportForm.test.ts +++ b/client/src/components/Planner/useTransportForm.test.ts @@ -1,4 +1,4 @@ -// FE-PLANNER-TRANSPORTFORM-001 to -007: the transport form state behind the +// FE-PLANNER-TRANSPORTFORM-001 to -008: the transport form state behind the // desktop TransportModal and the phone's transport sheet. import { act, renderHook } from '@testing-library/react'; import { describe, expect, it } from 'vitest'; @@ -30,7 +30,6 @@ const seedOptions = { fallbackType: 'transport_other' as const, dayId: '' as string | number, stopDaysFromEndpoints: false, - resetHiddenRoutes: false, }; describe('useTransportForm', () => { @@ -105,19 +104,19 @@ describe('useTransportForm', () => { expect(result.current.form).not.toHaveProperty('url'); }); - it('FE-PLANNER-TRANSPORTFORM-005: the desktop keeps the rows a flight does not show, the phone clears them', () => { + it('FE-PLANNER-TRANSPORTFORM-005: seeding a flight clears the rows a flight does not show, desktop and phone alike', () => { const flight = buildReservation({ type: 'flight', endpoints: [ep('from', 0, 'MUC'), ep('to', 1, 'BER')] }); const picked = { location: { name: 'kept', lat: 0, lng: 0 } }; const desktop = renderHook(() => useTransportForm(EMPTY_TRANSPORT_FIELDS)).result; act(() => desktop.current.setFromPick(picked)); act(() => desktop.current.seed(flight, days, seedOptions)); - expect(desktop.current.fromPick).toBe(picked); + expect(desktop.current.fromPick).toEqual({}); expect(desktop.current.waypoints.map((w) => w.airport?.iata)).toEqual(['MUC', 'BER']); const phone = renderHook(() => useTransportForm(EMPTY_TRANSPORT_FIELDS)).result; act(() => phone.current.setFromPick(picked)); - act(() => phone.current.seed(flight, days, { ...seedOptions, resetHiddenRoutes: true })); + act(() => phone.current.seed(flight, days, { ...seedOptions, stopDaysFromEndpoints: true })); expect(phone.current.fromPick).toEqual({}); }); @@ -156,4 +155,30 @@ describe('useTransportForm', () => { expect.objectContaining({ role: 'to', name: 'B', sequence: 1, local_date: '2026-05-01' }), ]); }); + + it('FE-PLANNER-TRANSPORTFORM-008: editing a flight or train after a car drops the car route, so switching back to car saves no stale stops', () => { + const car = buildReservation({ type: 'car', endpoints: [ep('from', 0), ep('stop', 1), ep('to', 2)] }); + const flight = buildReservation({ type: 'flight', endpoints: [ep('from', 0, 'MUC'), ep('to', 1, 'BER')] }); + const train = buildReservation({ type: 'train', endpoints: [ep('from', 0), ep('to', 1)] }); + + for (const next of [flight, train]) { + const { result } = renderHook(() => useTransportForm(EMPTY_TRANSPORT_FIELDS)); + act(() => result.current.seed(car, days, seedOptions)); + expect(result.current.carStops).toHaveLength(1); + + act(() => result.current.seed(next, days, seedOptions)); + expect(result.current.carStops).toEqual([]); + expect(result.current.fromPick).toEqual({}); + expect(result.current.toPick).toEqual({}); + + act(() => result.current.set('type', 'car')); + const payload = result.current.payload(days, { + reservation: next, + prefill: null, + budgetEnabled: false, + anchorOnStations: false, + }); + expect(payload.endpoints).toEqual([]); + } + }); }); diff --git a/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx b/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx index 6ee2295a7..ff697ca27 100644 --- a/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MTransportFormSheet.tsx @@ -92,7 +92,6 @@ export default function MTransportFormSheet({ planner, onOpenExpense }: MTranspo fallbackType: 'flight', dayId: transportModalDayId ?? '', stopDaysFromEndpoints: true, - resetHiddenRoutes: true, }) // eslint-disable-next-line react-hooks/exhaustive-deps }, [showTransportModal])