From 3c7f6e69dd17dc801fde1b6f5a5b992f98e7cf98 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 25 Sep 2026 19:46:29 +0200 Subject: [PATCH] fix(mobile): pick the plan timeline's hotel legs by what the route drew (#2501) On a day that opens with a flight, where the traveller also put the hotel on the day as a stop of their own, the phone timeline started the day with a hotel row and the drive from that stop to the next place, above the flight, and then showed the same drive again in its own spot. hotelLegsForDay guessed the bookends from coordinates: the first segment leaving the hotel's spot was the morning leg. The route had correctly drawn no morning leg on that day, but the stop on the hotel's spot starts a leg there too, so that hop was taken for it. The route calculation now tags the two legs it draws from and to the accommodation, and the timeline reads that tag instead. The hotel end of a tagged leg still has to be the day's own stay, because the calculation keeps the previous day's legs until the new day has routed. The stop to stop connectors leave tagged legs alone, so one leg can no longer show up in both places. --- .../src/components/Map/dayRoutePlan.test.ts | 26 +++- client/src/components/Map/dayRoutePlan.ts | 28 +++- client/src/hooks/useRouteCalculation.ts | 9 +- .../screens/trip/plan/planTimelineModel.ts | 45 +++--- client/src/types.ts | 6 + .../hooks/useRouteCalculation.test.ts | 128 +++++++++++++++++- .../mobile/trip/planTimelineModel.test.ts | 118 +++++++++++++--- .../unit/mobile/trip/useMPlanTimeline.test.ts | 5 +- 8 files changed, 314 insertions(+), 51 deletions(-) diff --git a/client/src/components/Map/dayRoutePlan.test.ts b/client/src/components/Map/dayRoutePlan.test.ts index 13141f286..5c808e70d 100644 --- a/client/src/components/Map/dayRoutePlan.test.ts +++ b/client/src/components/Map/dayRoutePlan.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { buildDayRouteRuns } from './dayRoutePlan' +import { buildDayRouteRuns, hotelBookendOf } from './dayRoutePlan' import { buildAssignment, buildDay, buildPlace } from '../../../tests/helpers/factories' import type { Accommodation, AssignmentsMap, Day, Reservation } from '../../types' @@ -142,6 +142,30 @@ describe('buildDayRouteRuns', () => { expect(runs[0].map(p => p.lat)).toEqual([53.5503, 53.5465]) }) + it('FE-MAP-DRP-015: the hotel points say which end of the day they are, a stop on the same spot does not (#2501)', () => { + const hotel = { id: 1, trip_id: 1, place_lat: 48.80, place_lng: 2.30, start_day_id: 1, end_day_id: 3 } as unknown as Accommodation + const runs = buildDayRouteRuns(2, inputs({ + days: [buildDay({ id: 1, day_number: 1 }), buildDay({ id: 2, day_number: 2 }), buildDay({ id: 3, day_number: 3 })], + assignments: { '2': [at(48.86, 2.35, 0), at(48.80, 2.30, 1), at(48.88, 2.36, 2)] }, + accommodations: [hotel], + optimizeFromAccommodation: true, + })) + const legs = runs.flatMap(run => run.slice(1).map((p, i) => hotelBookendOf(run[i], p) ?? null)) + // Out of the hotel, three stops (one of them on the hotel's own spot), back to it. + expect(legs).toEqual(['morning', null, null, 'evening']) + }) + + it('FE-MAP-DRP-016: the one drive of a moving day without stops is its morning leg (#2476)', () => { + const days = [1, 2, 3].map(n => buildDay({ id: n, day_number: n })) + const stays = [ + { id: 1, trip_id: 1, place_lat: 48.137, place_lng: 11.575, start_day_id: 1, end_day_id: 2 }, + { id: 2, trip_id: 1, place_lat: 53.551, place_lng: 9.993, start_day_id: 2, end_day_id: 3 }, + ] as unknown as Accommodation[] + const [run] = buildDayRouteRuns(2, inputs({ days, accommodations: stays, optimizeFromAccommodation: true })) + expect(run.map(p => p.hotel)).toEqual(['morning', 'evening']) + expect(hotelBookendOf(run[0], run[1])).toBe('morning') + }) + it('FE-MAP-DRP-008: a day that is not in the trip has no route', () => { expect(buildDayRouteRuns(99, inputs({ assignments: { '1': [at(48.86, 2.35, 0)] } }))).toEqual([]) }) diff --git a/client/src/components/Map/dayRoutePlan.ts b/client/src/components/Map/dayRoutePlan.ts index 567446c10..033ae72b2 100644 --- a/client/src/components/Map/dayRoutePlan.ts +++ b/client/src/components/Map/dayRoutePlan.ts @@ -13,6 +13,19 @@ export interface DayRoutePoint { isPlace: boolean leg_transport_mode?: string | null incoming_leg_transport_mode?: string | null + /** Only on an accommodation bookend: the stay the day starts from or ends at. */ + hotel?: 'morning' | 'evening' +} + +/** + * Which accommodation bookend the leg between two neighbouring run points is, if any. + * A day that is nothing but the drive from one stay to the next counts as its morning + * leg, so it shows once at the top rather than at both ends (#2476). + */ +export function hotelBookendOf(from: DayRoutePoint, to: DayRoutePoint): 'morning' | 'evening' | undefined { + if (from.hotel === 'morning') return 'morning' + if (to.hotel === 'evening') return 'evening' + return undefined } /** Everything the plan depends on, passed in rather than read from the store, so the @@ -131,9 +144,10 @@ export function buildDayRouteRuns(dayId: number, input: DayRouteInputs): DayRout } // A hotel bookend point is not a place-assignment, so isPlace: false — resolveLegMode // falls through to the day default for hotel-adjacent legs unless the place endpoint - // carries its own override. - const hotelPt = (a?: Accommodation): DayRoutePoint | null => - a && a.place_lat != null && a.place_lng != null ? { lat: a.place_lat, lng: a.place_lng, isPlace: false } : null + // carries its own override. It says which end of the day it is, so the legs routed + // from it can be told apart from a stop planned on the hotel's own spot (#2501). + const hotelPt = (a: Accommodation | undefined, hotel: 'morning' | 'evening'): DayRoutePoint | null => + a && a.place_lat != null && a.place_lng != null ? { lat: a.place_lat, lng: a.place_lng, isPlace: false, hotel } : null // Only draw a hotel bookend when the leg is a real drive: a place before check-in // (#1465), a later "home" stop on the checkout day (#1465), or a transport endpoint on // an arrival/departure day (#1321, #2133) all draw no bookend. @@ -154,8 +168,8 @@ export function buildDayRouteRuns(dayId: number, input: DayRouteInputs): DayRout const dayHasCarrier = dayTransports.some(r => hasCarrierEndpointOnDay(r, dayId)) const firstWay = flatPts[0] const lastWay = flatPts[flatPts.length - 1] - const morningHotel = hotelPt(bookends?.morning) - const eveningHotel = hotelPt(bookends?.evening) + const morningHotel = hotelPt(bookends?.morning, 'morning') + const eveningHotel = hotelPt(bookends?.evening, 'evening') const drawMorning = !!bookends && !!day && shouldDrawMorningLeg(bookends, day, edgeInfo(firstStop, 'first'), dayHasCarrier) && (!morningHotel || !firstWay || firstWay.isPlace || withinDriveRange(morningHotel, firstWay)) const drawEvening = !!bookends && !!day && shouldDrawEveningLeg(bookends, day, edgeInfo(lastStop, 'last'), dayHasCarrier) @@ -176,8 +190,8 @@ export function buildDayRouteRuns(dayId: number, input: DayRouteInputs): DayRout // waypoint behind, so the gates above never see it; no line beats a wrong one. const dayHasCarrierBooking = dayTransports.some(r => isCarrierTransport(r)) if (runsWithHotel.length === 0 && drawMorning && drawEvening && !dayHasCarrierBooking) { - const m = hotelPt(bookends?.morning) - const e = hotelPt(bookends?.evening) + const m = hotelPt(bookends?.morning, 'morning') + const e = hotelPt(bookends?.evening, 'evening') if (m && e && (m.lat !== e.lat || m.lng !== e.lng)) runsWithHotel.push([m, e]) } diff --git a/client/src/hooks/useRouteCalculation.ts b/client/src/hooks/useRouteCalculation.ts index 06e0a358f..ae9bdd09d 100644 --- a/client/src/hooks/useRouteCalculation.ts +++ b/client/src/hooks/useRouteCalculation.ts @@ -4,7 +4,7 @@ import { useState, useCallback, useRef, useEffect, useMemo } from 'react' import { useTripStore } from '../store/tripStore' import { useSettingsStore } from '../store/settingsStore' import { calculateRouteWithLegs, type RouteProfileKey } from '../components/Map/RouteCalculator' -import { buildDayRouteRuns, TRANSPORT_TYPES } from '../components/Map/dayRoutePlan' +import { buildDayRouteRuns, hotelBookendOf, TRANSPORT_TYPES } from '../components/Map/dayRoutePlan' import { resolveLegMode } from '../components/Planner/legMode' import type { TripStoreState } from '../store/tripStore' import type { RouteSegment, RouteResult, RouteVia, Accommodation } from '../types' @@ -100,7 +100,12 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu try { const r = await calculateRouteWithLegs(chunk.map(p => ({ lat: p.lat, lng: p.lng })), { signal: controller.signal, profile: mode, tripId, dayId }) pushCoords(r.coordinates.length >= 2 ? r.coordinates : straight()) - for (const leg of r.legs) allLegs.push({ ...leg, mode }) + // Leg k runs from chunk[k] to chunk[k + 1]; a hotel bookend says so on the + // segment, which is how the phone timeline finds it (#2501). + r.legs.forEach((leg, k) => { + const hotelBookend = chunk[k + 1] ? hotelBookendOf(chunk[k], chunk[k + 1]) : undefined + allLegs.push(hotelBookend ? { ...leg, mode, hotelBookend } : { ...leg, mode }) + }) if (r.vias) allVias.push(...r.vias) } catch (err) { if (err instanceof Error && err.name === 'AbortError') throw err diff --git a/client/src/mobile/screens/trip/plan/planTimelineModel.ts b/client/src/mobile/screens/trip/plan/planTimelineModel.ts index 5c20e6d42..2a3a2d9eb 100644 --- a/client/src/mobile/screens/trip/plan/planTimelineModel.ts +++ b/client/src/mobile/screens/trip/plan/planTimelineModel.ts @@ -75,8 +75,10 @@ const sameCoord = (a: [number, number], b: [number, number]): boolean => * Map the day's merged items to render rows and slot a travel-time connector * after every located place whose next located stop is again a place. Segments * are matched by their exact waypoint coordinates (the calculator echoes the - * input waypoints back on each leg), so hotel-bookend or transport legs in the - * pool simply never match and no index bookkeeping is needed. + * input waypoints back on each leg), so transport legs in the pool simply never + * match and no index bookkeeping is needed. The hotel bookends are left out up + * front: they belong to the day's edges, and a stop planned on the hotel's own + * spot would otherwise pair with them (#2501). */ export function buildPlanRows(opts: { merged: MergedItem[] @@ -85,7 +87,7 @@ export function buildPlanRows(opts: { dayId: number }): PlanRow[] { const { merged, reservations, routeSegments, dayId } = opts - const pool = [...routeSegments] + const pool = routeSegments.filter(s => !s.hotelBookend) const takeSegment = (from: [number, number], to: [number, number]): RouteSegment | null => { const idx = pool.findIndex(s => sameCoord(s.from, from) && sameCoord(s.to, to)) return idx >= 0 ? pool.splice(idx, 1)[0] : null @@ -184,10 +186,18 @@ export interface HotelLegs { top: HotelLeg | null; bottom: HotelLeg | null } /** * The two accommodation bookend legs of a day: the drive from the day's hotel to * the first stop (top) and from the last stop back to the hotel (bottom). The - * route calc already produced these via withHotelBookends — honouring the - * optimize-from-accommodation setting and the should-draw gates — so we just - * locate the pooled segment that starts (top) / ends (bottom) at the hotel's - * coordinates. Its presence is exactly the signal that the leg should be drawn. + * route calc already produced these via withHotelBookends, honouring the + * optimize-from-accommodation setting and the should-draw gates, and tagged + * them, so we just pick the tagged segments. Their presence is exactly the signal + * that the leg should be drawn. + * + * Not by coordinates alone: a stop planned on the hotel's own spot starts and + * ends legs there too. Matched that way, the drive from that stop to the next one + * sat above a day that opens with a flight, and showed a second time in its own + * place (#2501); an earlier drive onto that spot sat at the day's end (#2476). + * The tag says which end of the day a leg is, and its hotel end still has to be + * this stay: the calc keeps the last day's legs until the new day has routed, so + * right after a day switch the tagged legs can belong to another hotel. */ export function hotelLegsForDay( day: Day, @@ -196,24 +206,15 @@ export function hotelLegsForDay( routeSegments: RouteSegment[], ): HotelLegs { const { morning, evening } = getDayBookendHotels(day, days, accommodations) - const legAt = (a: Accommodation | undefined, end: 'from' | 'to'): HotelLeg | null => { + const legOf = (a: Accommodation | undefined, bookend: 'morning' | 'evening'): HotelLeg | null => { if (!a || a.place_lat == null || a.place_lng == null) return null - const coord: [number, number] = [a.place_lat, a.place_lng] - // The morning leg is the first segment leaving the hotel, the evening leg the - // LAST one reaching it. Taking the first for both put an earlier drive that - // happens to end on the evening hotel's spot under the day's end, instead of - // the drive that closes the day (#2476). - const seg = end === 'from' - ? routeSegments.find(s => sameCoord(s.from, coord)) - : [...routeSegments].reverse().find(s => sameCoord(s.to, coord)) + const at: [number, number] = [a.place_lat, a.place_lng] + const seg = routeSegments.find(s => s.hotelBookend === bookend && sameCoord(bookend === 'morning' ? s.from : s.to, at)) return seg ? { seg, name: accommodationName(a) } : null } - const top = legAt(morning, 'from') - const bottom = legAt(evening, 'to') - // A moving day without stops is one drive from one hotel to the next (#1297): it - // both leaves the morning hotel and reaches the evening one, so it shows once, at - // the top, instead of again at the bottom (#2476). - return { top, bottom: top && bottom && bottom.seg === top.seg ? null : bottom } + // A moving day without stops is one drive from one hotel to the next (#1297). The + // calc tags it as the morning leg, so it shows once, at the top (#2476). + return { top: legOf(morning, 'morning'), bottom: legOf(evening, 'evening') } } /** The day headline as city pills — a "Tokyo → Kyoto" title becomes two pills with an arrow. */ diff --git a/client/src/types.ts b/client/src/types.ts index efbec7712..38d77a50c 100644 --- a/client/src/types.ts +++ b/client/src/types.ts @@ -255,6 +255,12 @@ export interface RouteSegment { noteText?: string /** The travel mode this leg was routed with (#1281) — drives the connector icon. */ mode?: string + /** + * Set on the drive from the day's accommodation to its first stop ('morning') or + * from its last stop back to it ('evening'), so a list can show that leg at the + * day's edge without guessing it from coordinates a stop may share (#2501). + */ + hotelBookend?: 'morning' | 'evening' } /** An intermediate stop a plugin route places on the drawn line (charging stop, rest area). */ diff --git a/client/tests/integration/hooks/useRouteCalculation.test.ts b/client/tests/integration/hooks/useRouteCalculation.test.ts index a4cfdd292..a6c0130da 100644 --- a/client/tests/integration/hooks/useRouteCalculation.test.ts +++ b/client/tests/integration/hooks/useRouteCalculation.test.ts @@ -2,9 +2,10 @@ import { renderHook, act } from '@testing-library/react'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { useRouteCalculation } from '../../../src/hooks/useRouteCalculation'; import { useTripStore } from '../../../src/store/tripStore'; +import { hotelLegsForDay } from '../../../src/mobile/screens/trip/plan/planTimelineModel'; import { buildAssignment, buildPlace } from '../../helpers/factories'; import type { TripStoreState } from '../../../src/store/tripStore'; -import type { RouteSegment } from '../../../src/types'; +import type { Day, RouteSegment } from '../../../src/types'; vi.mock('../../../src/components/Map/RouteCalculator', async (importActual) => { const actual = await importActual(); @@ -996,4 +997,129 @@ describe('useRouteCalculation', () => { expect(result.current.route).toBeNull(); expect(calculateRouteWithLegs).not.toHaveBeenCalled(); }); + + describe('hotel bookend legs are tagged on the segments (#2501)', () => { + // Every request answers one leg per neighbouring waypoint pair, echoing the + // waypoints the way OSRM's legs do. + const echoLegs = () => (calculateRouteWithLegs as ReturnType).mockImplementation( + (waypoints: { lat: number; lng: number }[]) => Promise.resolve({ + coordinates: [] as [number, number][], + distance: 0, + duration: 0, + legs: waypoints.slice(0, -1).map((w, i) => ({ + ...MOCK_SEGMENTS[0], + from: [w.lat, w.lng] as [number, number], + to: [waypoints[i + 1].lat, waypoints[i + 1].lng] as [number, number], + })), + }), + ); + const tagsOf = (segments: RouteSegment[]) => + segments.map(s => [`${s.from[0]},${s.from[1]} > ${s.to[0]},${s.to[1]}`, s.hotelBookend ?? null]); + + it('FE-HOOK-ROUTE-036: a day out of the hotel tags the drive out and the drive back, nothing else', async () => { + echoLegs(); + const hotel = { lat: 41.39, lng: 2.16 }; + const actA = buildPlace({ lat: 41.38, lng: 2.17 }); + const actB = buildPlace({ lat: 41.40, lng: 2.19 }); + const store = { assignments: { '2': [ + buildAssignment({ day_id: 2, order_index: 0, place: actA }), + buildAssignment({ day_id: 2, order_index: 1, place: actB }), + ] } } as unknown as TripStoreState; + useTripStore.setState({ + assignments: store.assignments, + reservations: [], + days: [{ id: 1, day_number: 1 }, { id: 2, day_number: 2 }, { id: 3, day_number: 3 }], + } as unknown as Partial); + const accommodations = [{ id: 1, start_day_id: 1, end_day_id: 3, place_lat: hotel.lat, place_lng: hotel.lng }]; + + const { result } = renderHook(() => + useRouteCalculation(store, 2, true, 'driving', accommodations as unknown as Parameters[4]) + ); + await act(async () => {}); + + expect(tagsOf(result.current.routeSegments)).toEqual([ + [`${hotel.lat},${hotel.lng} > ${actA.lat},${actA.lng}`, 'morning'], + [`${actA.lat},${actA.lng} > ${actB.lat},${actB.lng}`, null], + [`${actB.lat},${actB.lng} > ${hotel.lat},${hotel.lng}`, 'evening'], + ]); + }); + + it('FE-HOOK-ROUTE-037: a hotel stop of your own after landing is no morning bookend', async () => { + // The reported day: fly in, drive to the hotel you put on the day yourself, then + // on to the town hall. Only the drive back to the hotel tonight is a bookend. + echoLegs(); + const hotel = { lat: 53.5465, lng: 9.9727 }; + const ams = { lat: 52.3105, lng: 4.7683 }; + const ham = { lat: 53.6304, lng: 9.9882 }; + const hotelStop = buildPlace({ lat: hotel.lat, lng: hotel.lng }); + const townHall = buildPlace({ lat: 53.5503, lng: 9.9937 }); + const flight = { + id: 90, type: 'flight', day_id: 1, end_day_id: 1, day_plan_position: -0.5, + reservation_time: '2026-10-19T10:00', reservation_end_time: '2026-10-19T13:00', + endpoints: [ + { role: 'from', sequence: 0, lat: ams.lat, lng: ams.lng }, + { role: 'to', sequence: 1, lat: ham.lat, lng: ham.lng }, + ], + }; + const store = { assignments: { '1': [ + buildAssignment({ day_id: 1, order_index: 0, place: hotelStop }), + buildAssignment({ day_id: 1, order_index: 1, place: townHall }), + ] } } as unknown as TripStoreState; + useTripStore.setState({ + assignments: store.assignments, + reservations: [flight], + days: [{ id: 1, day_number: 1 }, { id: 2, day_number: 2 }, { id: 3, day_number: 3 }], + } as unknown as Partial); + const accommodations = [{ id: 1, start_day_id: 1, end_day_id: 3, check_in: '15:00', check_out: '11:00', place_lat: hotel.lat, place_lng: hotel.lng }]; + + const { result } = renderHook(() => + useRouteCalculation(store, 1, true, 'driving', accommodations as unknown as Parameters[4]) + ); + await act(async () => {}); + + expect(tagsOf(result.current.routeSegments)).toEqual([ + [`${ham.lat},${ham.lng} > ${hotel.lat},${hotel.lng}`, null], + [`${hotel.lat},${hotel.lng} > ${townHall.lat},${townHall.lng}`, null], + [`${townHall.lat},${townHall.lng} > ${hotel.lat},${hotel.lng}`, 'evening'], + ]); + }); + + it('FE-HOOK-ROUTE-038: after a day switch the last day\'s bookends are no legs of the new day\'s hotel', async () => { + // Day 1 sleeps in Munich, day 3 in Hamburg. The segments of day 1 stay in place + // until day 3 has routed, and that request may take a while or never answer. + echoLegs(); + const days = [1, 2, 3, 4].map(n => ({ id: n, trip_id: 1, day_number: n, date: `2026-05-0${n}`, title: null })); + const munich = { id: 1, trip_id: 1, place_name: 'Munich Inn', place_lat: 48.137, place_lng: 11.575, start_day_id: 1, end_day_id: 2 }; + const hamburg = { id: 2, trip_id: 1, place_name: 'Hamburg Inn', place_lat: 53.551, place_lng: 9.993, start_day_id: 2, end_day_id: 4 }; + const stays = [munich, hamburg] as unknown as Parameters[4]; + const store = { assignments: { + '1': [ + buildAssignment({ day_id: 1, order_index: 0, place: buildPlace({ lat: 48.14, lng: 11.58 }) }), + buildAssignment({ day_id: 1, order_index: 1, place: buildPlace({ lat: 48.15, lng: 11.59 }) }), + ], + '3': [ + buildAssignment({ day_id: 3, order_index: 0, place: buildPlace({ lat: 53.56, lng: 9.99 }) }), + buildAssignment({ day_id: 3, order_index: 1, place: buildPlace({ lat: 53.57, lng: 10.0 }) }), + ], + } } as unknown as TripStoreState; + useTripStore.setState({ assignments: store.assignments, reservations: [], days } as unknown as Partial); + const legsOf = (dayId: number, segments: RouteSegment[]) => + hotelLegsForDay(days[dayId - 1] as unknown as Day, days as unknown as Day[], stays, segments); + + const { result, rerender } = renderHook( + ({ dayId }: { dayId: number }) => useRouteCalculation(store, dayId, true, 'driving', stays), + { initialProps: { dayId: 1 } }, + ); + await act(async () => {}); + expect(legsOf(1, result.current.routeSegments)).toMatchObject({ top: { name: 'Munich Inn' }, bottom: { name: 'Munich Inn' } }); + + (calculateRouteWithLegs as ReturnType).mockImplementation(() => new Promise(() => {})); + rerender({ dayId: 3 }); + await act(async () => {}); + + // Day 1's legs are still there, and Hamburg shows none of them. + expect(result.current.routeSegments.some(s => s.hotelBookend)).toBe(true); + expect(legsOf(3, result.current.routeSegments)).toEqual({ top: null, bottom: null }); + }); + }); }); diff --git a/client/tests/unit/mobile/trip/planTimelineModel.test.ts b/client/tests/unit/mobile/trip/planTimelineModel.test.ts index 2784d87f3..bd5a2a2cb 100644 --- a/client/tests/unit/mobile/trip/planTimelineModel.test.ts +++ b/client/tests/unit/mobile/trip/planTimelineModel.test.ts @@ -5,14 +5,14 @@ import { hotelLegsForDay, itemHasTime, parseReservationMeta, transportSubtitle, weatherIconFor, type TransportEntry, } from '../../../../src/mobile/screens/trip/plan/planTimelineModel' -import { getDisplayTimeForDay, type MergedItem } from '../../../../src/utils/dayMerge' -import { buildDayRouteRuns } from '../../../../src/components/Map/dayRoutePlan' +import { getDisplayTimeForDay, getMergedItems, getTransportForDay, type MergedItem } from '../../../../src/utils/dayMerge' +import { buildDayRouteRuns, hotelBookendOf, type DayRoutePoint } from '../../../../src/components/Map/dayRoutePlan' import { buildAssignment, buildDayNote, buildPlace, buildReservation } from '../../../helpers/factories' import type { Accommodation, Assignment, Day, DayNote, Reservation, RouteSegment, TranslationFn, } from '../../../../src/types' -// FE-MOB-PTLM-001 to FE-MOB-PTLM-047 +// FE-MOB-PTLM-001 to FE-MOB-PTLM-050 const DAYS = [ { id: 1, trip_id: 1, day_number: 1, date: '2026-05-01', title: null }, @@ -44,6 +44,14 @@ function seg(from: [number, number], to: [number, number], overrides: Partial + runs.flatMap(run => run.slice(1).map((p, i) => { + const hotelBookend = hotelBookendOf(run[i], p) + return seg([run[i].lat, run[i].lng], [p.lat, p.lng], hotelBookend ? { hotelBookend } : {}) + })) + const placeItem = (a: Assignment): MergedItem => ({ type: 'place', sortKey: a.order_index, data: a }) const noteItem = (n: DayNote): MergedItem => ({ type: 'note', sortKey: n.sort_order ?? 0, data: n }) const transportItem = (r: TransportEntry): MergedItem => ({ type: 'transport', sortKey: 0, data: r }) @@ -295,30 +303,46 @@ describe('planTimelineModel — hotel chips and legs', () => { it('FE-MOB-PTLM-025: picks the hotel bookend legs out of the calculated segments', () => { const hotel = accommodation({ id: 7, start_day_id: 1, end_day_id: 3, place_lat: 48.0, place_lng: 16.0 }) - const out = seg([48.0, 16.0], [48.1, 16.1]) - const back = seg([48.2, 16.2], [48.0, 16.0]) - const legs = hotelLegsForDay(DAY2, DAYS, [hotel], [out, back, seg([48.1, 16.1], [48.2, 16.2])]) + const out = seg([48.0, 16.0], [48.1, 16.1], { hotelBookend: 'morning' }) + const back = seg([48.2, 16.2], [48.0, 16.0], { hotelBookend: 'evening' }) + const legs = hotelLegsForDay(DAY2, DAYS, [hotel], [out, seg([48.1, 16.1], [48.2, 16.2]), back]) expect(legs.top).toEqual({ seg: out, name: 'Hotel Sacher' }) expect(legs.bottom).toEqual({ seg: back, name: 'Hotel Sacher' }) }) - it('FE-MOB-PTLM-026: returns no legs when no segment touches the hotel', () => { + it('FE-MOB-PTLM-026: returns no legs when the calculation drew no bookend, whatever touches the hotel (#2501)', () => { + // A stop on the hotel's own spot starts and ends legs at its coordinates too. const hotel = accommodation({ id: 8, start_day_id: 1, end_day_id: 3, place_lat: 48.0, place_lng: 16.0 }) - expect(hotelLegsForDay(DAY2, DAYS, [hotel], [seg([48.1, 16.1], [48.2, 16.2])])).toEqual({ top: null, bottom: null }) + const legs = hotelLegsForDay(DAY2, DAYS, [hotel], [seg([48.1, 16.1], [48.0, 16.0]), seg([48.0, 16.0], [48.2, 16.2])]) + expect(legs).toEqual({ top: null, bottom: null }) }) it('FE-MOB-PTLM-027: returns no legs without an accommodation on the day', () => { - expect(hotelLegsForDay(DAY2, DAYS, [], [seg([48.1, 16.1], [48.2, 16.2])])).toEqual({ top: null, bottom: null }) + const tagged = [seg([48.0, 16.0], [48.1, 16.1], { hotelBookend: 'morning' }), seg([48.1, 16.1], [48.0, 16.0], { hotelBookend: 'evening' })] + expect(hotelLegsForDay(DAY2, DAYS, [], tagged)).toEqual({ top: null, bottom: null }) }) - it('FE-MOB-PTLM-045: the evening leg is the last drive into the hotel, not the first (#2476)', () => { + it('FE-MOB-PTLM-050: another stay\'s bookends are not this day\'s, as right after a day switch (#2501)', () => { + // The calc still holds the day before's legs, out of and back to Munich, while + // the new day, a night in Hamburg, is being routed. + const munich = { lat: 48.137, lng: 11.575 } + const hamburg = accommodation({ id: 9, start_day_id: 2, end_day_id: 3, place_name: 'Hamburg Inn', place_lat: 53.551, place_lng: 9.993 }) + const stale = [ + seg([munich.lat, munich.lng], [48.14, 11.58], { hotelBookend: 'morning' }), + seg([48.14, 11.58], [48.15, 11.59]), + seg([48.15, 11.59], [munich.lat, munich.lng], { hotelBookend: 'evening' }), + ] + expect(hotelLegsForDay(DAY2, DAYS, [hamburg], stale)).toEqual({ top: null, bottom: null }) + }) + + it('FE-MOB-PTLM-045: the evening leg is the drive that closes the day, not an earlier one onto the hotel spot (#2476)', () => { // A stop planned on the hotel's own spot early in the day: the drive there reaches // the hotel's coordinates first, but the day ends with the drive back from the park. const hotel = accommodation({ id: 7, start_day_id: 1, end_day_id: 3, place_lat: 48.0, place_lng: 16.0 }) - const out = seg([48.0, 16.0], [48.1, 16.1]) + const out = seg([48.0, 16.0], [48.1, 16.1], { hotelBookend: 'morning' }) const toHotelSpot = seg([48.1, 16.1], [48.0, 16.0]) const onward = seg([48.0, 16.0], [48.2, 16.2]) - const back = seg([48.2, 16.2], [48.0, 16.0]) + const back = seg([48.2, 16.2], [48.0, 16.0], { hotelBookend: 'evening' }) const legs = hotelLegsForDay(DAY2, DAYS, [hotel], [out, toHotelSpot, onward, back]) expect(legs.top?.seg).toBe(out) expect(legs.bottom?.seg).toBe(back) @@ -344,14 +368,11 @@ describe('planTimelineModel — hotel chips and legs', () => { ] : [], }) - // What the connector calculation hands the timeline: one segment per pair of - // neighbouring waypoints in each drawn run. const legsOf = (reservations: Reservation[]) => { const runs = buildDayRouteRuns(2, { days: DAYS, assignments: {}, reservations, accommodations: stays, optimizeFromAccommodation: true, }) - const segments = runs.flatMap(run => run.slice(1).map((p, i) => seg([run[i].lat, run[i].lng], [p.lat, p.lng]))) - return hotelLegsForDay(DAY2, DAYS, stays, segments) + return hotelLegsForDay(DAY2, DAYS, stays, segmentsOf(runs)) } it('FE-MOB-PTLM-046: shows no drive from one hotel to the other, above or below the flight', () => { @@ -375,6 +396,71 @@ describe('planTimelineModel — hotel chips and legs', () => { }) }) +describe('planTimelineModel: a day that lands and then drives to the hotel (#2501)', () => { + // Day 1 checks into a Hamburg hotel at three. The flight lands at one, and the + // traveller put the hotel on the day as a stop of their own, then the town hall. + const DAY1 = DAYS[0] + const HOTEL = { lat: 53.5465, lng: 9.9727 } + const AMS = { lat: 52.3105, lng: 4.7683 } + const HAM = { lat: 53.6304, lng: 9.9882 } + const TOWN_HALL = { lat: 53.5503, lng: 9.9937 } + const stay = accommodation({ + id: 30, place_id: 900, place_name: 'Hotel Hafen', place_lat: HOTEL.lat, place_lng: HOTEL.lng, + start_day_id: 1, end_day_id: 3, check_in: '15:00', check_out: '11:00', + }) + const flight = buildReservation({ + id: 7, type: 'flight', title: 'KL 1783', day_id: 1, end_day_id: 1, day_plan_position: -0.5, + reservation_time: '2026-05-01T10:00', reservation_end_time: '2026-05-01T13:00', + endpoints: [ + { role: 'from', sequence: 0, name: 'AMS', code: null, ...AMS, timezone: null, local_date: null, local_time: null }, + { role: 'to', sequence: 1, name: 'HAM', code: null, ...HAM, timezone: null, local_date: null, local_time: null }, + ], + }) + const hotelStop = buildAssignment({ id: 21, day_id: 1, order_index: 0, place: place(900, 'Hotel Hafen', HOTEL.lat, HOTEL.lng) }) + const townHall = buildAssignment({ id: 22, day_id: 1, order_index: 1, place: place(901, 'Town Hall', TOWN_HALL.lat, TOWN_HALL.lng) }) + + // The day as the phone timeline builds it: merged the way useMPlanTimeline merges, + // segments the way useRouteCalculation routes the same day. + const dayOne = () => { + const dayAssignments = [hotelStop, townHall] + const merged = getMergedItems({ + dayAssignments, + dayNotes: [], + dayTransports: getTransportForDay({ reservations: [flight], dayId: 1, dayAssignmentIds: dayAssignments.map(a => a.id), days: DAYS }), + dayId: 1, + }) + const segments = segmentsOf(buildDayRouteRuns(1, { + days: DAYS, assignments: { '1': dayAssignments }, reservations: [flight], accommodations: [stay], optimizeFromAccommodation: true, + })) + return { + rows: buildPlanRows({ merged, reservations: [flight], routeSegments: segments, dayId: 1 }), + legs: hotelLegsForDay(DAY1, DAYS, [stay], segments), + } + } + const ends = (s: RouteSegment) => [s.from, s.to] + + it('FE-MOB-PTLM-048: opens with the flight, shows the hop from the hotel stop once and ends at the hotel', () => { + const { rows, legs } = dayOne() + // No drive out of a hotel the traveller has not reached yet. + expect(legs.top).toBeNull() + const hops = rows.filter(r => r.kind === 'conn').map(r => r.kind === 'conn' && ends(r.seg)) + expect(hops.filter(h => JSON.stringify(h) === JSON.stringify([[HOTEL.lat, HOTEL.lng], [TOWN_HALL.lat, TOWN_HALL.lng]]))).toHaveLength(1) + expect(rows[0]).toMatchObject({ kind: 'transport', key: 'tr-7' }) + // The day closes with the drive back to the hotel. + expect(legs.bottom).toMatchObject({ name: 'Hotel Hafen', seg: { from: [TOWN_HALL.lat, TOWN_HALL.lng], to: [HOTEL.lat, HOTEL.lng] } }) + }) + + it('FE-MOB-PTLM-049: a bookend is never read as the hop between two stops that share its ends', () => { + // The tag decides, not the coordinates: two stops on the very spots a bookend + // joins get no connector out of it. + const bookend = seg([HOTEL.lat, HOTEL.lng], [TOWN_HALL.lat, TOWN_HALL.lng], { hotelBookend: 'morning' }) + const rows = buildPlanRows({ + merged: [placeItem(hotelStop), placeItem(townHall)], reservations: [], routeSegments: [bookend], dayId: 1, + }) + expect(rows.map(r => r.kind)).toEqual(['place', 'place']) + }) +}) + describe('planTimelineModel — cityPillsForDay', () => { const t = ((key: string, params?: Record) => params ? `${key}:${Object.values(params).join(',')}` : key) as unknown as TranslationFn diff --git a/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts b/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts index d4ea468ff..6570a83c6 100644 --- a/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts +++ b/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts @@ -126,8 +126,9 @@ describe('useMPlanTimeline', () => { }) it('FE-MOB-PLTL-004: exposes the hotel bookend legs and header chips', async () => { - const out = seg([48, 16.05], [48, 16.1]) - const back = seg([48, 16.2], [48, 16.05]) + // The route calculation tags the two bookends it drew (#2501). + const out: RouteSegment = { ...seg([48, 16.05], [48, 16.1]), hotelBookend: 'morning' } + const back: RouteSegment = { ...seg([48, 16.2], [48, 16.05]), hotelBookend: 'evening' } routeCalc.segments = [out, back] const { result } = await renderTimeline(makePlanner({ tripAccommodations: [HOTEL] })) expect(result.current.hotelLegs.top?.seg).toBe(out)