From 299beb65b132d30426cbc057ec0223fad92cb718 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 29 Oct 2026 20:51:24 +0100 Subject: [PATCH] fix(roadtrip): hand a hire car without a return day back on its pick-up day A car booking without an end day is a one-day booking everywhere else in the plan, but the road trip read it as a car never handed back: only its pick-up desk stood on the road. Two one-way car bookings on one day (Bologna to Villach, then Villach to Klagenfurt) therefore drove from the first pick-up to the second and on to whatever came before, counting the road between the two cities twice and never reaching Klagenfurt. This applies to the planner, the phone and calculate_roadtrip alike, since all three seat the desks through the shared carrierSeam. Such a car now has its return desk on the pick-up day. A return without a time goes back before the next car of that day is collected rather than closing the day behind it, so each one-way car drives its own road once. Bookings with a return day and cars without a located return desk are unchanged. Closes #2894 --- shared/src/roadtrip/carriers.spec.ts | 62 +++++++++++++++++++++++++--- shared/src/roadtrip/carriers.ts | 36 ++++++++++++---- wiki/Road-Trip.md | 2 +- 3 files changed, 86 insertions(+), 14 deletions(-) diff --git a/shared/src/roadtrip/carriers.spec.ts b/shared/src/roadtrip/carriers.spec.ts index b94ce66b0..9fd2c7230 100644 --- a/shared/src/roadtrip/carriers.spec.ts +++ b/shared/src/roadtrip/carriers.spec.ts @@ -198,9 +198,7 @@ describe('a hire car', () => { expect(sameDesk.arrival).toMatchObject({ name: 'Europcar MUC' }); }); - it('ROADTRIP-CARRIERS-017: a hire car without a return day or a return desk stands on the road at its pick-up only, nothing is guessed', () => { - const noDay = carrierSeam(rental({ end_day_id: null }))!; - expect(noDay.arrival).toBeNull(); + it('ROADTRIP-CARRIERS-017: a hire car without a return desk stands on the road at its pick-up only, nothing is guessed', () => { const noDesk = carrierSeam( rental({ endpoints: [{ role: 'from', sequence: 0, name: 'Sixt Hauptbahnhof', code: null, lat: 53.55, lng: 10.0 }], @@ -208,11 +206,63 @@ describe('a hire car', () => { )!; expect(noDesk.arrival).toBeNull(); expect(carrierSeam(rental({ endpoints: [] }))).toBeNull(); - const seated = seatCarrierStops(1, [stop({ ownerIndex: 0, time: '10:00' })], [0], [noDay]); + const seated = seatCarrierStops(1, [stop({ ownerIndex: 0, time: '10:00' })], [0], [noDesk]); expect(seated.map((s) => s.carrier?.role ?? s.name)).toEqual(['pickup', 'Stop 0']); - expect(seatCarrierStops(3, [stop({ ownerIndex: 0 })], [0], [noDay]).map((s) => s.carrier?.role ?? s.name)).toEqual([ - 'Stop 0', + expect(seatCarrierStops(3, [stop({ ownerIndex: 0 })], [0], [noDesk]).map((s) => s.carrier?.role ?? s.name)).toEqual( + ['Stop 0'], + ); + }); + + it('ROADTRIP-CARRIERS-036: a hire car without a return day is handed back on its pick-up day, as the day plan reads it (#2894)', () => { + const oneWay = carrierSeam(rental({ end_day_id: null, reservation_end_time: null }))!; + expect(oneWay.arrival).toMatchObject({ dayId: 1, clock: null, name: 'Sixt Airport' }); + const seated = seatCarrierStops(1, [stop({ ownerIndex: 0, time: '10:00' })], [0], [oneWay]); + expect(seated.map((s) => s.carrier?.role ?? s.name)).toEqual(['pickup', 'Stop 0', 'return']); + }); + + it('ROADTRIP-CARRIERS-037: two one-way cars on one day: the first goes back before the second is collected, and the road runs once through each', () => { + const desk = (role: 'from' | 'to', name: string, lat: number, lng: number) => ({ + role, + sequence: role === 'from' ? 0 : 1, + name, + code: null, + lat, + lng, + }); + const bologna = desk('from', 'Bologna', 44.49, 11.34); + const villach = { lat: 46.61, lng: 13.85 }; + const outward = carrierSeam( + rental({ + id: 1, + title: 'Bologna → Villach', + end_day_id: null, + reservation_time: '2026-11-02T08:00', + reservation_end_time: null, + endpoints: [bologna, desk('to', 'Villach', villach.lat, villach.lng)], + }), + )!; + const onward = carrierSeam( + rental({ + id: 2, + title: 'Villach → Klagenfurt', + end_day_id: null, + reservation_time: '2026-11-02T17:30', + reservation_end_time: null, + endpoints: [desk('from', 'Villach', villach.lat, villach.lng), desk('to', 'Klagenfurt', 46.62, 14.31)], + }), + )!; + const event = stop({ ownerIndex: 0, name: 'CUAS', lat: villach.lat, lng: villach.lng, time: '13:00' }); + + const seated = seatCarrierStops(1, [event], [0], [onward, outward]); + + expect(seated.map((s) => `${s.carrier?.role ?? 'stop'} ${s.name}`)).toEqual([ + 'pickup Bologna', + 'stop CUAS', + 'return Villach', + 'pickup Villach', + 'return Klagenfurt', ]); + expect(seated.map((s) => s.carrier?.reservationId ?? null)).toEqual([1, null, 1, 2, 2]); }); it('ROADTRIP-CARRIERS-018: the pick-up opens its day when nothing timed comes first, the return closes its day, and the road runs through both', () => { diff --git a/shared/src/roadtrip/carriers.ts b/shared/src/roadtrip/carriers.ts index e6ad8640a..d18e3b52f 100644 --- a/shared/src/roadtrip/carriers.ts +++ b/shared/src/roadtrip/carriers.ts @@ -111,8 +111,8 @@ export interface CarrierSeam { departure: CarrierEnd; /** * A ride always has both ends. A hire car has a return only when the booking says - * when and where: without a return day or a located return desk, nothing is guessed - * and only the pick-up stands on the road. + * where: without a located return desk, nothing is guessed and only the pick-up stands + * on the road. */ arrival: CarrierEnd | null; } @@ -220,9 +220,12 @@ export function carrierSeam(booking: CarrierBooking): CarrierSeam | null { const last = legs[legs.length - 1]; const depDayId = departureDayOf(booking, legs); if (depDayId == null) return null; - // A ride lands on the day it left unless the booking says otherwise. A hire car is - // handed back on the day the booking names, and on no day when it names none. - const arrDayId = last?.arr_day_id ?? booking.end_day_id ?? (kind === 'ride' ? depDayId : null); + // A booking without an end day ends on the day it starts, as the day plan reads it + // (`getSpanPhase`): a ride lands there and a hire car is handed back there. Read as + // a car never handed back, a one-way drive between two cities kept only its pick-up, + // and the road ran from that desk to the next car's instead of to its own return + // desk (#2894). + const arrDayId = last?.arr_day_id ?? booking.end_day_id ?? depDayId; const depClock = carrierClock(first?.dep_time ?? booking.reservation_time); const arrClock = carrierClock(last?.arr_time ?? booking.reservation_end_time); const bookingPosition = (dayId: number): number | null => @@ -597,11 +600,30 @@ export function seatCarrierStops( minutes: item.minutes, point: item.kind === 'stop' ? { lat: stops[item.index]!.lat, lng: stops[item.index]!.lng } : null, }); + // Just ahead of the next car's pick-up behind this car's own, or null when no car + // follows and the return closes the day. Closing the day, the first of two one-way + // cars was handed back after the second was collected. + const handOverKey = (seam: CarrierSeam): number | null => { + const pickups = items.filter( + (item): item is Extract => item.kind === 'end' && item.role === 'pickup', + ); + const own = pickups.find((item) => item.seam === seam); + if (!own) return null; + const next = pickups.filter((item) => item.seam !== seam && item.key > own.key).map((item) => item.key); + return next.length ? Math.min(...next) - 0.0001 : null; + }; + // A car handed back on the day it was picked up, at no stated hour, is seated once every + // other desk of the day is: it goes back before the next car is collected (`handOverKey`). + const untimedSameDayReturn = (e: { seam: CarrierSeam; role: CarrierTerminal['role']; minutes: number | null }) => + e.role === 'return' && e.minutes === null && e.seam.departure.dayId === dayId; ends .map((e) => ({ ...e, minutes: parseClock(e.end.clock) })) - .sort((a, b) => (a.minutes ?? 0) - (b.minutes ?? 0)) + .sort( + (a, b) => + Number(untimedSameDayReturn(a)) - Number(untimedSameDayReturn(b)) || (a.minutes ?? 0) - (b.minutes ?? 0), + ) .forEach((e, ti) => { - let key = e.end.position; + let key = e.end.position ?? (untimedSameDayReturn(e) ? handOverKey(e.seam) : null); if (key === null) { const minutes = e.minutes ?? 0; let after = -Infinity; diff --git a/wiki/Road-Trip.md b/wiki/Road-Trip.md index 965c3a196..5a66bd54a 100644 --- a/wiki/Road-Trip.md +++ b/wiki/Road-Trip.md @@ -332,7 +332,7 @@ What the seam does not do: it never plans the ride itself, and a booking without ### Hire cars -A **Car** booking is not a seam, you drive it, but its desks are where the drive starts and ends. The pick-up desk stands on the day card as a row with the car icon and *Pick-up 09:00* under its name, and it opens the day when nothing timed comes before it. The return desk closes the day it is handed back on, with *Return 11:30*. The road runs through both like any other stop, the range budget starts from a full tank at the pick-up, and both rows open the booking. The return only appears when the booking names a return day and a located return desk; nothing is guessed. +A **Car** booking is not a seam, you drive it, but its desks are where the drive starts and ends. The pick-up desk stands on the day card as a row with the car icon and *Pick-up 09:00* under its name, and it opens the day when nothing timed comes before it. The return desk closes the day it is handed back on, with *Return 11:30*. The road runs through both like any other stop, the range budget starts from a full tank at the pick-up, and both rows open the booking. The return only appears when the booking has a located return desk; nothing is guessed. A booking without a return day is handed back on the day it is picked up, as **Days** shows it, so two one-way cars on one day each drive their own road, and one handed back without a time goes back before the next car of that day is picked up. ### Other bookings on the day