From 7b0333a0c1f826f5abe4e509bb59f620d73d8dfc Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 29 Sep 2026 20:53:54 +0200 Subject: [PATCH] fix(roadtrip): a booked night no longer shows up mid-day when the day already ends at the stay Booking a night writes the hotel onto its check-in day as a stop, seated first or at its check-in, so the drive reaches it even without hotel bookends. With "Start and end each day at your stay" on, the evening row already ends that day at the same hotel, so the road trip drove check-out, tonight's hotel, the day's places and back to the hotel again. Days hides that stop and draws the stay at the day's edges only. seatNightBookends now passes over the stop tonight's booking wrote whenever it seats that stay as the day's evening, in the browser and in calculate_roadtrip alike. The stops keep their stored index, a hotel the traveller placed stays, and the booking's stop stays while it is the day's last one or no evening is seated. With the switch off nothing changes. Closes #2546 --- .../Roadtrip/useRoadtripRoutes.test.ts | 28 +++++++- .../components/Roadtrip/useRoadtripRoutes.ts | 1 + .../nest/roadtrip/roadtrip-plan.service.ts | 5 +- .../unit/nest/roadtrip-plan.service.test.ts | 22 ++++++ shared/src/roadtrip/nightBookends.spec.ts | 59 +++++++++++++++- shared/src/roadtrip/nightBookends.ts | 70 ++++++++++++++----- shared/src/roadtrip/planning-types.ts | 7 ++ wiki/Road-Trip.md | 5 +- 8 files changed, 174 insertions(+), 23 deletions(-) diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.test.ts b/client/src/components/Roadtrip/useRoadtripRoutes.test.ts index 4f8de1291..c30e4442e 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.test.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.test.ts @@ -1166,7 +1166,7 @@ describe('a leg asked for again on demand', () => { }) /** - * FE-ROADTRIP-ROUTES-052..061: a booked night at both ends of the days around it. + * FE-ROADTRIP-ROUTES-052..061, 064: a booked night at both ends of the days around it. * * The first cases here that hand the hook the trip's stays. What is pinned is what goes to * the router: the drive from the hotel slept in and to tonight's is part of the day's own @@ -1404,6 +1404,32 @@ describe('a booked night at both ends of its days', () => { expect(runs).toContainEqual(['driving', [P[2][0], P[3][0], HOTEL[0]]]) expect(runs).toContainEqual(['driving', [HOTEL[0], P[4][0], P[5][0], HOTEL[0]]]) }) + + it('FE-ROADTRIP-ROUTES-064: the stop tonight’s booking put first on its day gives way to the evening at the stay (#2546)', async () => { + const { days } = cam() + const stays = [stayOf(1, 1, 1, 2, GETAWAY), stayOf(2, 4, 2, 3, WALLINGA)] + const assignments = { + ...map(1, [{ id: 1, at: GETAWAY }, { id: 2, at: LOOKOUT }]), + ...map(2, [{ id: 4, at: WALLINGA }, { id: 5, at: P[0] }, { id: 6, at: P[1] }]), + } as AssignmentsMap + // The booking wrote the Wallinga stop, seated first because it names no check-in. + Object.assign(assignments['2'][0], { accommodation_id: 2 }) + const { result } = renderHook(() => useRoadtripRoutes(7, days, assignments, 'driving', {}, [], stays)) + await waitFor(() => expect(result.current.loading).toBe(false)) + + expect(result.current.days.map(d => shape(d.stops))).toEqual([ + ['Stop 1', 'Stop 2', 'evening:1'], + ['morning:1', 'Stop 5', 'Stop 6', 'evening:2'], + ]) + expect(asked()).toContainEqual([GETAWAY[0], P[0][0], P[1][0], WALLINGA[0]]) + expect(result.current.days[1].stops[3].bookend).toMatchObject({ checkingIn: true, accommodationId: 2 }) + // Writes still address the stored order: the places keep the index they are stored at. + expect(result.current.days[1].stops.map(s => s.ownerIndex)).toEqual([0, 1, 2, 3]) + + // Switched off, the booking's stop is the only way the drive reaches the hotel. + switchTo(false) + await waitFor(() => expect(result.current.days[1].stops.map(s => s.name)).toEqual(['Stop 4', 'Stop 5', 'Stop 6'])) + }) }) /** diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.ts b/client/src/components/Roadtrip/useRoadtripRoutes.ts index 8ff23ba9d..c1fbc712f 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.ts @@ -176,6 +176,7 @@ const asStop = (a: Assignment, ownerDayId: number, ownerIndex: number, accommoda leaveAt: a.assignment_end_time ?? p.end_time ?? null, checkInTime: stay?.check_in ?? null, night: stay !== undefined, + bookedNightId: a.accommodation_id ?? null, dwellMinutes: typeof p.duration_minutes === 'number' ? p.duration_minutes : null, endDay: a.end_day === true, legMode: a.leg_transport_mode ?? null, diff --git a/server/src/nest/roadtrip/roadtrip-plan.service.ts b/server/src/nest/roadtrip/roadtrip-plan.service.ts index 216e22273..d480c5d0d 100644 --- a/server/src/nest/roadtrip/roadtrip-plan.service.ts +++ b/server/src/nest/roadtrip/roadtrip-plan.service.ts @@ -62,6 +62,8 @@ interface CarrierRow { interface VisitRow { /** The booking this stop stands for on its check-in day, when there is one. */ stay_id: number | null; + /** The stay whose booking wrote this stop, null for one the traveller placed. */ + accommodation_id: number | null; check_in: string | null; /** The drive does not read these two (#2357): they are the booking as get_roadtrip_context reports it. */ check_out: string | null; @@ -113,7 +115,7 @@ export class RoadtripPlanService { `SELECT a.id, a.day_id, a.order_index, a.place_id, p.name, p.lat, p.lng, COALESCE(a.assignment_time, p.place_time) AS time, COALESCE(a.assignment_end_time, p.end_time) AS end_time, p.duration_minutes, a.end_day, - a.leg_transport_mode, a.incoming_leg_transport_mode, p.stop_type, p.fill_percent, stay.id AS stay_id, stay.check_in, stay.check_out, checkout.day_number AS checkout_day + a.leg_transport_mode, a.incoming_leg_transport_mode, p.stop_type, p.fill_percent, a.accommodation_id, stay.id AS stay_id, stay.check_in, stay.check_out, checkout.day_number AS checkout_day FROM day_assignments a JOIN days d ON d.id = a.day_id JOIN places p ON p.id = a.place_id LEFT JOIN day_accommodations stay ON stay.id = (SELECT id FROM day_accommodations WHERE place_id = p.id AND start_day_id = d.id ORDER BY id LIMIT 1) LEFT JOIN days checkout ON checkout.id = stay.end_day_id @@ -239,6 +241,7 @@ export class RoadtripPlanService { leaveAt: v.end_time, checkInTime: v.check_in, night: v.stay_id !== null, + bookedNightId: v.accommodation_id, dwellMinutes: v.duration_minutes, endDay: v.end_day === 1, legMode: v.leg_transport_mode, diff --git a/server/tests/unit/nest/roadtrip-plan.service.test.ts b/server/tests/unit/nest/roadtrip-plan.service.test.ts index d0b5f2801..d412f4605 100644 --- a/server/tests/unit/nest/roadtrip-plan.service.test.ts +++ b/server/tests/unit/nest/roadtrip-plan.service.test.ts @@ -530,6 +530,28 @@ describe('a booked night at both ends of its days', () => { expect(runs).toContainEqual(['driving', [HOTEL.lat, 45.5, 45.6, HOTEL.lat]]); }); + it('drops the stop tonight’s booking put first on its check-in day, as the browser does, and keeps it while off (#2546)', async () => { + const t = trip(); + const [d1, d2, d3] = [createDay(db, t.trip.id), createDay(db, t.trip.id), createDay(db, t.trip.id)]; + const getaway = visit(t.trip.id, d1.id, 'Getaway', GETAWAY); + visit(t.trip.id, d1.id, 'Lookout', { lat: -33.73, lng: 150.35 }); + const wallinga = visit(t.trip.id, d2.id, 'Wallinga', WALLINGA); + visit(t.trip.id, d2.id, 'Cave', { lat: -34.2, lng: 150.8 }); + visit(t.trip.id, d2.id, 'Bakery', { lat: -34.3, lng: 150.7 }); + const stayA = createDayAccommodation(db, t.trip.id, getaway.place.id, d1.id, d2.id); + const stayB = createDayAccommodation(db, t.trip.id, wallinga.place.id, d2.id, d3.id); + // The Wallinga stop is the one the booking wrote, seated first without a check-in. + db.prepare('UPDATE day_assignments SET accommodation_id = ? WHERE id = ?').run(stayB.id, wallinga.assignment.id); + + const { calculated } = await t.plans.calculate(t.trip.id, t.user.id); + expect(shape(calculated.days[1].stops)).toEqual([`morning:${stayA.id}`, 'Cave', 'Bakery', `evening:${stayB.id}`]); + expect(calculated.days[1].stops[3].bookend).toMatchObject({ checkingIn: true }); + expect(lats(t.router)).toContainEqual([GETAWAY.lat, -34.2, -34.3, WALLINGA.lat]); + + const off = await t.plans.calculate(t.trip.id, t.user.id, { roadtrip_hotel_bookends: false }); + expect(shape(off.calculated.days[1].stops)).toEqual(['Wallinga', 'Cave', 'Bakery']); + }); + it('drives the stored days while the trip has it off, and a preview can switch it either way', async () => { const off = simeon({}); const stored = await off.plans.calculate(off.trip.id, off.user.id); diff --git a/shared/src/roadtrip/nightBookends.spec.ts b/shared/src/roadtrip/nightBookends.spec.ts index f0d7a015a..ce50dd04e 100644 --- a/shared/src/roadtrip/nightBookends.spec.ts +++ b/shared/src/roadtrip/nightBookends.spec.ts @@ -1,5 +1,5 @@ /** - * ROADTRIP-BOOKENDS-001..032: a booked night stands at both ends of the days around it. + * ROADTRIP-BOOKENDS-001..035: a booked night stands at both ends of the days around it. * * Pinned here: which hotel a day wakes up in and which it sleeps in, where the two are * seated and where they are not (the hotel already there, a landing, a departure, a hire @@ -7,7 +7,8 @@ * does (a clock), that the choice does not depend on the order the stays come in, and * that a night spent at one hotel is a leg going nowhere. From 029 on: a day moved by a * ride the road does not see, a terminal out of a drive's reach, and the flight day of - * the report read from the stored day to its nights. + * the report read from the stored day to its nights. From 033 on: the stop a booking put + * on its check-in day gives way to the evening at that stay. */ import { assembleRoadtrip } from './assemble'; import { carrierLegsFor, carrierSeam, seatCarrierStops, terminalAssignmentId, type CarrierBooking } from './carriers'; @@ -760,3 +761,57 @@ describe('the flight from Hamburg to the Munich hotel (trip 45, day 3)', () => { expect(timed(seated!).schedule.entries[0]).toMatchObject({ arrival: '13:45', departure: '13:45' }); }); }); + +describe('the stop a booking put on its check-in day (#2546)', () => { + const a = stay(1, D1, D2, GETAWAY); + const b = stay(2, D2, D3, WALLINGA); + /** The Wallinga stop the booking wrote, which Days hides. */ + const booked = (ownerIndex: number, over: Partial = {}) => + stayStop(D2, ownerIndex, WALLINGA, { name: 'B', bookedNightId: 2, ...over }); + + it('ROADTRIP-BOOKENDS-033: seated first without a check-in, it gives way to the evening, which checks in', () => { + const [day] = seatNightBookends( + [planDay(D2, [booked(0), visit(D2, 1, { name: 'P1' }), visit(D2, 2, { name: 'P2', time: '18:00' })])], + days, + [a, b], + ); + // Not check out, the hotel, the places and the hotel again: the day ends where it sleeps. + expect(shape(day)).toEqual(['morning:1', 'P1', 'P2', 'evening:2']); + expect(day!.stops[3]!.bookend).toMatchObject({ checkingIn: true, accommodationId: 2 }); + // The places keep their stored index, and the evening the one behind the last stored stop. + expect(day!.stops.map((s) => s.ownerIndex)).toEqual([0, 1, 2, 3]); + }); + + it('ROADTRIP-BOOKENDS-034: seated at its check-in in the middle of the day, it gives way too', () => { + const [day] = seatNightBookends( + [ + planDay(D2, [ + visit(D2, 0, { name: 'P0', time: '11:00' }), + booked(1, { checkInTime: '15:00' }), + visit(D2, 2, { name: 'P1' }), + ]), + ], + days, + [a, { ...b, check_in: '15:00' }], + ); + expect(shape(day)).toEqual(['morning:1', 'P0', 'P1', 'evening:2']); + // The hour the room is ready stays on the evening's row, as a label. + expect(day!.stops[3]!.bookend).toMatchObject({ checkingIn: true, checkIn: '15:00' }); + }); + + it('ROADTRIP-BOOKENDS-035: the hotel stays where the traveller placed it, and the booked stop where nothing replaces it', () => { + const p = () => visit(D2, 1, { name: 'P' }); + // Placed by the traveller: theirs, and the day comes back to it in the evening. + const placed = seatNightBookends([planDay(D2, [booked(0, { bookedNightId: null }), p()])], days, [a, b]); + expect(shape(placed[0])).toEqual(['morning:1', 'B', 'P', 'evening:2']); + // Already the day's last stop: it is the evening, and nothing is seated behind it. + const last = seatNightBookends([planDay(D2, [p(), booked(1)])], days, [a, b]); + expect(shape(last[0])).toEqual(['morning:1', 'P', 'B']); + // A departure closes the day, so no evening is seated and the stop stays the hotel's visit. + const flight = seatNightBookends([planDay(D2, [booked(0), p(), terminal(D2, 2, 'departure')])], days, [a, b]); + expect(shape(flight[0])).toEqual(['morning:1', 'B', 'P', 'departure']); + // Another stay's stop is not tonight's, and is left alone. + const other = seatNightBookends([planDay(D2, [booked(0, { bookedNightId: 9 }), p()])], days, [a, b]); + expect(shape(other[0])).toEqual(['morning:1', 'B', 'P', 'evening:2']); + }); +}); diff --git a/shared/src/roadtrip/nightBookends.ts b/shared/src/roadtrip/nightBookends.ts index 5764dd7b1..5adf32623 100644 --- a/shared/src/roadtrip/nightBookends.ts +++ b/shared/src/roadtrip/nightBookends.ts @@ -219,29 +219,36 @@ export function seatNightBookends( const out = plan.map((day) => { const ref = dayOf(day.dayId); if (!ref) return day; - const stops = day.stops; - if (stops.every((stop) => stop.night) && ridden(ref)) return day; + if (day.stops.every((stop) => stop.night) && ridden(ref)) return day; const hotels = getDayBookendHotels(ref, days, nights); - const first = stops[0]; - const last = stops[stops.length - 1]; - let head = - !!hotels.morning && - !!hotels.morningIsSleptHere && - !(first && (hotelIsTheStop(hotels.morning, first) || opensTheDay(first.carrier?.role))) && - withinReach(hotels.morning, first); - let tail = - !!hotels.evening && - !!hotels.eveningIsOvernight && - !(last && (hotelIsTheStop(hotels.evening, last) || closesTheDay(last.carrier?.role))) && - withinReach(hotels.evening, last); - if (!stops.length) head = tail = head && tail && !sameSpot(hotels.morning, hotels.evening); + const edges = (stops: RoadtripStop[]): { head: boolean; tail: boolean } => { + const first = stops[0]; + const last = stops[stops.length - 1]; + const head = + !!hotels.morning && + !!hotels.morningIsSleptHere && + !(first && (hotelIsTheStop(hotels.morning, first) || opensTheDay(first.carrier?.role))) && + withinReach(hotels.morning, first); + const tail = + !!hotels.evening && + !!hotels.eveningIsOvernight && + !(last && (hotelIsTheStop(hotels.evening, last) || closesTheDay(last.carrier?.role))) && + withinReach(hotels.evening, last); + if (!stops.length) { + const both = head && tail && !sameSpot(hotels.morning, hotels.evening); + return { head: both, tail: both }; + } + return { head, tail }; + }; + const stops = withoutBookedNight(day.stops, hotels.evening, day.dayId, edges); + const { head, tail } = edges(stops); if (!head && !tail) return day; changed = true; - const stored = stops.filter((stop) => !stop.carrier).length; + const stored = day.stops.filter((stop) => !stop.carrier).length; return { ...day, stops: [ - ...(head ? [bookendStop(day, 'morning', hotels.morning!, 0, first?.incomingLegMode ?? null)] : []), + ...(head ? [bookendStop(day, 'morning', hotels.morning!, 0, stops[0]?.incomingLegMode ?? null)] : []), ...stops, ...(tail ? [bookendStop(day, 'evening', hotels.evening!, stored, null)] : []), ], @@ -249,3 +256,32 @@ export function seatNightBookends( }); return changed ? out : plan; } + +/** + * The day's stops without the one tonight's booking put on it, when the evening seats that + * stay at the end of the day instead (#2546). + * + * Booking a night writes the hotel onto its check-in day as a stop (#2354), seated first or + * at its check-in, so a trip that routes without bookends still reaches the hotel. With + * bookends the evening already ends the day there, and the booking's stop drew the same + * hotel a second time in the middle of the day: check out, drive to tonight's hotel, the + * day's places, and back to the hotel for the night. Days hides that stop and draws the + * stay at the day's edges, and so does the road trip now. A hotel the traveller placed on + * the day themselves is theirs and stays, and so does the booking's stop while it is the + * day's last one, or while the evening is not seated after all (a departure closes the day, + * or the hotel is out of reach of the stop before it). The stops keep their `ownerIndex`, + * so every write still addresses the stored order, and the evening keeps the index behind + * the day's last stored stop. + */ +function withoutBookedNight( + stops: RoadtripStop[], + evening: BookendStay | undefined, + dayId: number, + edges: (stops: RoadtripStop[]) => { tail: boolean }, +): RoadtripStop[] { + if (!evening || evening.start_day_id !== dayId || !stops.length) return stops; + const own = (stop: RoadtripStop): boolean => stop.bookedNightId === evening.id && !stop.carrier; + if (!stops.some(own) || own(stops[stops.length - 1]!)) return stops; + const rest = stops.filter((stop) => !own(stop)); + return rest.length && edges(rest).tail ? rest : stops; +} diff --git a/shared/src/roadtrip/planning-types.ts b/shared/src/roadtrip/planning-types.ts index 109e98a88..c59ac38bf 100644 --- a/shared/src/roadtrip/planning-types.ts +++ b/shared/src/roadtrip/planning-types.ts @@ -84,6 +84,13 @@ export interface RoadtripStop { checkInTime?: string | null; /** A booked night's stop on its check-in day, whether or not the booking names an hour. */ night?: boolean; + /** + * The stay whose booking wrote this stop (`day_assignments.accommodation_id`), rather + * than the traveller placing the hotel on the day. Days hides such a stop and draws the + * stay at the day's edges instead; `seatNightBookends` does the same once it seats that + * stay at the end of the day. + */ + bookedNightId?: number | null; endDay?: boolean; legMode: string | null; diff --git a/wiki/Road-Trip.md b/wiki/Road-Trip.md index 69b473d9b..5745ba8d5 100644 --- a/wiki/Road-Trip.md +++ b/wiki/Road-Trip.md @@ -281,7 +281,7 @@ A night booked anywhere, whether in the day panel, the booking form, on the phon - Moving the booking to another day moves its stop along; deleting the booking removes the stop it created. A stop you placed yourself is left standing. - The drive uses the booking's **Check-in** to anchor the stop, the way a pinned time anchors any other, and the place's stay as its length. A drive that cannot make the check-in is reported late. The check-out stays a booking detail. - **The night leads its day.** Its stop is seated first, behind only a stop whose own time is at or before the check-in, and the stops that carry no hour follow it: a hotel booked for ten in the morning is reached at ten, not at a quarter past twelve behind a whole day of untimed stops. A night without a check-in is seated first as well, a new check-in seats the stop afresh, and two nights booked on one day settle by their check-ins. A day that is nothing but its booked night stands among the days with the hotel and its check-in, instead of under the *Only {name} so far* placeholder. -- One consequence to be aware of: a night booked for the end of a driving day heads that day too when its stops carry no time of their own, so the drive reads hotel first. To put the hotel back at the end, drag it down the rail: an edit to the booking that leaves the check-in alone will not undo that. A **Start** at or before the check-in does it as well, as long as it goes on the *first* stop after the hotel: the day re-sorts by time, and the untimed stops behind that one follow it. On the last stop alone it is not enough, because the untimed stops in front of it keep counting as being at the check-in and stay behind the hotel. +- One consequence to be aware of: a night booked for the end of a driving day heads that day too when its stops carry no time of their own, so the drive reads hotel first. To put the hotel back at the end, drag it down the rail: an edit to the booking that leaves the check-in alone will not undo that. A **Start** at or before the check-in does it as well, as long as it goes on the *first* stop after the hotel: the day re-sorts by time, and the untimed stops behind that one follow it. On the last stop alone it is not enough, because the untimed stops in front of it keep counting as being at the check-in and stay behind the hotel. With [Start and end each day at your stay](#starting-and-ending-the-day-at-the-stay) switched on, none of this is needed: the day ends at tonight's hotel by itself. - **Add as an overnight stay** from the search books the night and adds the stop in one go. The night runs to the next day; change the dates under **Days**. On the phone, booking a night works in the desktop planner only. Trips planned before 4.3.1 were seated the same way when the server upgraded, see [Upgrading to 4.3.1](Updating#upgrading-to-431). @@ -294,10 +294,11 @@ Normally a day drives from its first stop to its last, and the hotel you slept i With it on: -- The hotel stands at the edge of the day card as a row of its own, with a bed icon and no number. In the morning it reads *Check-out · Hotel Adler* with *until 10:00* on the day you leave, and *From Hotel Adler* on a day you come back to. When the plan sets out after the check-out hour, the row adds *Leaves after check-out*. In the evening it reads *Back to Hotel Adler*, or *Check-in · Hotel Adler* on the day the stay begins when the hotel's own stop is not on the card before it, for example because you removed it. +- The hotel stands at the edge of the day card as a row of its own, with a bed icon and no number. In the morning it reads *Check-out · Hotel Adler* with *until 10:00* on the day you leave, and *From Hotel Adler* on a day you come back to. When the plan sets out after the check-out hour, the row adds *Leaves after check-out*. In the evening it reads *Check-in · Hotel Adler* on the day the stay begins, and *Back to Hotel Adler* on a day you come back to, or when you placed the hotel on the day yourself earlier on. - The drive to and from the hotel counts towards the day's distance, driving time, driving limits and range. The hotel does not count as a stop and does not refill the tank. - These rows are not stops you planned, and nothing is stored for them: they cannot be dragged, removed or given a stay. Clicking one opens the night's booking, or the hotel's place when there is no booking to open. On the phone, a night without a booking opens the stay for somebody who may edit days. - Only real nights count: a stay that checks in and out on the same day adds no row, and neither does a hotel without coordinates. Where two stays overlap, the choice is the one **Days** makes: the day ends at a stay checked in that day, otherwise at the one it started from, and it starts at a stay checked in on an earlier day, the one entered first where there are two. A drive between two different hotels is then a real drive. +- The stop a booking put on its check-in day gives way to that evening's row, the way **Days** hides it: the day checks out, drives its places and ends at tonight's hotel, instead of visiting the hotel in the middle of the day and coming back to it at night. It stays while it is already the day's last stop, or when the day ends at a departure terminal instead. A hotel you placed on the day yourself always stays. - A side of the day that already starts or ends at the hotel gets no row. Neither does a day that starts where a flight, train or ferry lands or at a hire car's pick-up desk, or ends at a departure terminal or where the hire car goes back. - A day with one place becomes a drive from the hotel to it and back. A day between two nights in the same hotel with no place planned has no drive, and a check-out day with no places is the drive from one hotel to the next. - The drive from and to the hotel keeps its road. Clicking it on the map places no via point and says *No via point on the drive to or from your stay. Add a stop there instead.*, and its drive band offers no **Other ways**. Where the day drives the same road again between two of its places, a click there places the via point on that leg as usual. A place added from the search along that drive goes to the start or the end of the day.