From 8d78c988c3f66f9ffa263eb182d7af7cfc767384 Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 23 Sep 2026 16:27:41 +0200 Subject: [PATCH] fix(dayplan): a moving day with a flight draws no drive from one stay to the next A day that checks out of one hotel and into another, with nothing else planned, falls back to a drive from the first hotel to the second (#1297). A flight, train, ferry or coach booked on that day is the move itself, but one saved without its stations leaves no waypoint behind, so the day map, the trip overview and the phone plan all drew the full road between the two hotels, and the phone showed it above and below the flight (#2476). With its stations the day was already right. - The fallback drive between the two hotels is no longer drawn when any such booking is on the day, located or not. A day without a booking keeps it. No line beats a wrong one. - The phone plan takes the evening hotel leg from the last drive that reaches the hotel, not the first, so an earlier drive to the same spot no longer stands in for the one that closes the day. - Google Maps and CoMaps exports of such a day, which could only ever describe that same drive, return nothing, and on the desktop and the phone the two buttons are left out whenever fewer than two stops remain, instead of doing nothing on a tap. A hotel stop planned by hand ahead of the flight already sorts behind it through the ride seating of #2474; a test now pins that for the days view as well. --- .../src/components/Map/dayRoutePlan.test.ts | 54 +++++++++++++++++ client/src/components/Map/dayRoutePlan.ts | 7 ++- .../components/Map/tripRouteGeometry.test.ts | 22 ++++++- .../Planner/DayPlanSidebar.test.tsx | 46 +++++++++++++++ .../src/components/Planner/DayPlanSidebar.tsx | 26 ++++++--- .../mobile/screens/trip/lib/dayRoute.test.ts | 21 ++++++- .../src/mobile/screens/trip/lib/dayRoute.ts | 13 ++++- .../screens/trip/plan/MPlanTimeline.tsx | 8 ++- .../screens/trip/plan/planTimelineModel.ts | 8 ++- .../screens/trip/plan/useMPlanTimeline.ts | 30 +++++++--- .../hooks/useRouteCalculation.test.ts | 30 ++++++++++ .../unit/mobile/trip/MPlanTimeline.test.tsx | 14 ++++- .../mobile/trip/planTimelineModel.test.ts | 58 ++++++++++++++++++- .../unit/mobile/trip/useMPlanTimeline.test.ts | 25 +++++++- 14 files changed, 335 insertions(+), 27 deletions(-) diff --git a/client/src/components/Map/dayRoutePlan.test.ts b/client/src/components/Map/dayRoutePlan.test.ts index 1395591d5..13141f286 100644 --- a/client/src/components/Map/dayRoutePlan.test.ts +++ b/client/src/components/Map/dayRoutePlan.test.ts @@ -145,4 +145,58 @@ describe('buildDayRouteRuns', () => { 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([]) }) + + describe('a moving day from one stay to the next (#2476)', () => { + // Day 3 checks out of a hotel in Munich and into one in Hamburg, with a flight + // in between. The booked Hamburg night wrote its own (hidden) stop onto day 3. + const HOTEL_A = { lat: 48.137, lng: 11.575 } + const HOTEL_B = { lat: 53.551, lng: 9.993 } + const MUC = { lat: 48.353, lng: 11.786 } + const HAM = { lat: 53.63, lng: 9.988 } + const days = [1, 2, 3, 4, 5].map(n => buildDay({ id: n, day_number: n })) + const stays = [ + { id: 1, trip_id: 1, place_lat: HOTEL_A.lat, place_lng: HOTEL_A.lng, start_day_id: 1, end_day_id: 3, check_in: '15:00', check_out: '11:00' }, + { id: 2, trip_id: 1, place_lat: HOTEL_B.lat, place_lng: HOTEL_B.lng, start_day_id: 3, end_day_id: 5, check_in: '15:00', check_out: '11:00' }, + ] as unknown as Accommodation[] + const bookedStop = buildAssignment({ day_id: 3, order_index: 0, accommodation_id: 2, place: buildPlace({ ...HOTEL_B }) }) + const flight = (located: boolean) => ({ + id: 7, trip_id: 1, type: 'flight', title: 'LH 2078', status: 'confirmed', day_id: 3, end_day_id: 3, + reservation_time: '2026-11-04T15:15', reservation_end_time: '2026-11-04T17:20', + endpoints: located + ? [{ role: 'from', sequence: 0, name: 'MUC', ...MUC }, { role: 'to', sequence: 1, name: 'HAM', ...HAM }] + : [], + }) as unknown as Reservation + const movingDay = (reservations: Reservation[], dayStops = [bookedStop]) => + buildDayRouteRuns(3, inputs({ + days, assignments: { '3': dayStops }, reservations, accommodations: stays, optimizeFromAccommodation: true, + })).map(run => run.map(p => [p.lat, p.lng])) + + it('FE-MAP-DRP-011: a flight saved without airports draws no drive from one hotel to the other', () => { + // Nothing located is left on the day, and the flight is still the move: a road + // from Munich to Hamburg is exactly the trip that did not happen. + expect(movingDay([flight(false)])).toEqual([]) + }) + + it('FE-MAP-DRP-012: with its airports the flight splits the day at them, as reported', () => { + expect(movingDay([flight(true)])).toEqual([ + [[HOTEL_A.lat, HOTEL_A.lng], [MUC.lat, MUC.lng]], + [[HAM.lat, HAM.lng], [HOTEL_B.lat, HOTEL_B.lng]], + ]) + }) + + it('FE-MAP-DRP-013: without any booking the move is still the drive between the two hotels (#1297)', () => { + expect(movingDay([])).toEqual([[[HOTEL_A.lat, HOTEL_A.lng], [HOTEL_B.lat, HOTEL_B.lng]]]) + }) + + it('FE-MAP-DRP-014: a hotel stop planned by hand ahead of the flight is reached after landing', () => { + // The traveller put the Hamburg hotel on the day before booking it, so the stop + // stays visible and carries no time. The flight still seats itself in front of + // the place it lands next to, and no road runs from Munich to Hamburg. + const handPlaced = at(HOTEL_B.lat, HOTEL_B.lng, 0, { day_id: 3 }) + expect(movingDay([flight(true)], [handPlaced])).toEqual([ + [[HOTEL_A.lat, HOTEL_A.lng], [MUC.lat, MUC.lng]], + [[HAM.lat, HAM.lng], [HOTEL_B.lat, HOTEL_B.lng]], + ]) + }) + }) }) diff --git a/client/src/components/Map/dayRoutePlan.ts b/client/src/components/Map/dayRoutePlan.ts index 6c0c9a937..4b0402988 100644 --- a/client/src/components/Map/dayRoutePlan.ts +++ b/client/src/components/Map/dayRoutePlan.ts @@ -170,7 +170,12 @@ export function buildDayRouteRuns(dayId: number, input: DayRouteInputs): DayRout // Transfer day with no activities: you check out of one accommodation and into // another, so there are no waypoints for withHotelBookends to attach a leg to. - if (runsWithHotel.length === 0 && drawMorning && drawEvening) { + // Not when a flight, train, ferry or coach is booked on the day, located or not: + // that booking IS the move, and the road from one hotel to the other is exactly + // the stretch nobody drove (#2476). One saved without its stations leaves no + // 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) if (m && e && (m.lat !== e.lat || m.lng !== e.lng)) runsWithHotel.push([m, e]) diff --git a/client/src/components/Map/tripRouteGeometry.test.ts b/client/src/components/Map/tripRouteGeometry.test.ts index 8df07b8cc..8b34ac30d 100644 --- a/client/src/components/Map/tripRouteGeometry.test.ts +++ b/client/src/components/Map/tripRouteGeometry.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { dayColor } from '../Roadtrip/dayColors' import { dayRouteColor, planTripRoute, routeTrip, summariseTripRoute } from './tripRouteGeometry' import { buildAssignment, buildDay, buildPlace } from '../../../tests/helpers/factories' -import type { AssignmentsMap, RouteSegment } from '../../types' +import type { Accommodation, AssignmentsMap, Reservation, RouteSegment } from '../../types' vi.mock('./RouteCalculator', async (importActual) => { const actual = await importActual() @@ -212,4 +212,24 @@ describe('tripRouteGeometry', () => { vi.useRealTimers() } }) + + it('FE-MAP-TRG-012: the overview draws no drive between two stays on the day a flight connects them (#2476)', () => { + // Day 2 moves from a Munich hotel to a Hamburg one. The overview reads the same + // day plan as the map, so the flight saved without airports keeps the road off it. + const days = [1, 2, 3].map(n => buildDay({ id: n, day_number: n })) + const accommodations = [ + { 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 flight = { + id: 80, trip_id: 1, type: 'flight', title: 'LH 2078', day_id: 2, end_day_id: 2, + reservation_time: '2026-11-04T15:15', reservation_end_time: '2026-11-04T17:20', endpoints: [], + } as unknown as Reservation + const moving = { days, assignments: {}, accommodations, optimizeFromAccommodation: true } + + expect(planTripRoute({ ...moving, reservations: [flight] }, 'driving')).toEqual([]) + // Without the booking the day is a drive from one hotel to the other, as before (#1297). + const byRoad = planTripRoute({ ...moving, reservations: [] }, 'driving') + expect(byRoad.map(d => d.day.id)).toEqual([2]) + }) }) diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index 4eb009a13..2b77772de 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -3934,6 +3934,52 @@ describe('DayPlanSidebar', () => { openSpy.mockRestore() }) + describe('a moving day with a flight between the two stays (#2476)', () => { + // Day 2 checks out of a Munich hotel and into a Hamburg one, and nothing else is + // planned on it. The booked night's own stop never reaches the list. + const movingDays = [ + buildDay({ id: 10, date: '2026-11-03', title: 'Day 1' }), + buildDay({ id: 11, date: '2026-11-04', title: 'Day 2' }), + buildDay({ id: 12, date: '2026-11-05', title: 'Day 3' }), + ] + const stays: Accommodation[] = [ + { id: 1, trip_id: 1, start_day_id: 10, end_day_id: 11, place_lat: 48.137, place_lng: 11.575, place_name: 'Hotel A' }, + { id: 2, trip_id: 1, start_day_id: 11, end_day_id: 12, place_lat: 53.551, place_lng: 9.993, place_name: 'Hotel B' }, + ] + const airport = (role: 'from' | 'to', name: string, lat: number, lng: number) => + ({ role, sequence: role === 'from' ? 0 : 1, name, code: null, lat, lng, timezone: null, local_date: null, local_time: null }) + const flight = (located: boolean) => buildReservation({ + id: 77, type: 'flight', title: 'LH 2078', day_id: 11, end_day_id: 11, + reservation_time: '2026-11-04T15:15:00', reservation_end_time: '2026-11-04T17:20:00', + endpoints: located ? [airport('from', 'MUC', 48.353, 11.786), airport('to', 'HAM', 53.63, 9.988)] : [], + }) + + it('FE-PLANNER-DAYPLAN-222: offers no Google Maps or CoMaps route from one hotel to the other', () => { + for (const located of [true, false]) { + const { unmount } = render() + // The route itself can still be shown; only the hand-offs, which could only + // ever describe a drive from Munich to Hamburg, are gone. + expect(screen.getByRole('button', { name: 'Route' })).toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Open in Google Maps' })).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Open in CoMaps' })).not.toBeInTheDocument() + unmount() + } + }) + + it('FE-PLANNER-DAYPLAN-223: the hotel legs run to the departure airport and from the arrival airport', async () => { + render() + await waitFor(() => expect(vi.mocked(calculateRouteWithLegs)).toHaveBeenCalledTimes(2)) + const pairs = vi.mocked(calculateRouteWithLegs).mock.calls.map(c => c[0]) + expect(pairs).toContainEqual([{ lat: 48.137, lng: 11.575 }, { lat: 48.353, lng: 11.786 }]) + expect(pairs).toContainEqual([{ lat: 53.63, lng: 9.988 }, { lat: 53.551, lng: 9.993 }]) + expect(pairs).not.toContainEqual([{ lat: 48.137, lng: 11.575 }, { lat: 53.551, lng: 9.993 }]) + }) + }) + it('FE-PLANNER-DAYPLAN-169: picking a whole-day travel mode persists it and redraws the map', async () => { const user = userEvent.setup() const { daysApi } = await import('../../api/client') diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index e98d3cf98..bd4f23bc5 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -1627,7 +1627,9 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP // useRouteCalculation) even with zero places. Mirror that exact gate here — two // distinct bookend hotels you actually slept in / sleep in tonight — so the route // tools appear when you click the day (#1297). A same-hotel rest day or a plain - // arrival/departure day has morning === evening and stays excluded. + // arrival/departure day has morning === evening and stays excluded. With a flight + // or train booked on it the map drops that leg and keeps only the drives to and + // from located stations, and the exports below have nothing to hand over (#2476). const transferMorning = routeBookends?.morning const transferEvening = routeBookends?.evening const hasHotelTransfer = !!( @@ -1647,6 +1649,10 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP */ const dayExportStops = (): NamedWaypoint[] => { const dayStops = getDayAssignments(day.id).filter(a => a.place?.lat != null && a.place?.lng != null) + // A flight, train, ferry or coach on a day without stops is the move itself, + // located or not: the hotels at either end are joined by it, not by a road + // worth handing to a map app (#2476). + if (dayStops.length === 0 && (mergedItemsMap[day.id] || []).some(i => i.type === 'transport' && isCarrierTransport(i.data))) return [] const stops = dayStops.map(a => ({ lat: a.place!.lat!, lng: a.place!.lng!, name: a.place!.name })) const first = dayStops[0] ? { isPlace: true, time: dayStops[0].place?.place_time ?? null, lat: dayStops[0].place!.lat!, lng: dayStops[0].place!.lng! } : undefined const lastAssignment = dayStops[dayStops.length - 1] @@ -1662,6 +1668,10 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP ? { lat: routeBookends.evening.place_lat, lng: routeBookends.evening.place_lng, name: routeBookends.evening.place_name } : null return [...(morning ? [morning] : []), ...stops, ...(evening ? [evening] : [])] } + const showRouteTools = (isSelected || (showRouteToolsWhenExpanded && isExpanded)) && routeToolsRoutable + // Built once, for the day the tools show on. A route needs two ends: with + // fewer the hand-offs would open a lone pin or nothing at all (#2476). + const exportStops = showRouteTools ? dayExportStops() : [] // Is this day's inline route currently on? Mobile toggles it per day (its // own expandedRouteDayIds entry); desktop uses the global Route toggle on // the selected day (#1374). @@ -2832,7 +2842,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP {/* Routen-Werkzeuge (ausgewählter Tag, 2+ Orte — oder 1 Ort mit Hotel-Bookend #1330 — oder Hotel-zu-Hotel-Transfertag ohne Orte #1297) */} - {(isSelected || (showRouteToolsWhenExpanded && isExpanded)) && routeToolsRoutable && ( + {showRouteTools && (
- + } {/* The same day, handed to CoMaps for offline navigation (#1904). The day's own travel mode rides along, so the route it builds walks when the plan walks. */} - + {exportStops.length >= 2 && - + } {/* Icon-only, like the two map hand-offs beside it (#1981). It was the one button here carrying a label with no room for it: `flex: 1` alongside `padding: '6px 0'` meant the text diff --git a/client/src/mobile/screens/trip/lib/dayRoute.test.ts b/client/src/mobile/screens/trip/lib/dayRoute.test.ts index 506d7f4b2..d89f90058 100644 --- a/client/src/mobile/screens/trip/lib/dayRoute.test.ts +++ b/client/src/mobile/screens/trip/lib/dayRoute.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import { dayCoMapsUrl, dayExportStops, dayGoogleMapsUrl } from './dayRoute' import type { Accommodation, Assignment, Day } from '../../../../types' -// FE-MOBILE-DAYROUTE-001 to FE-MOBILE-DAYROUTE-009 +// FE-MOBILE-DAYROUTE-001 to FE-MOBILE-DAYROUTE-011 const days = [ { id: 10, day_number: 1 }, @@ -69,6 +69,25 @@ describe('dayExportStops', () => { const stops = dayExportStops(days[2], days, [farHome], [hotel], true) expect(stops.map(s => s.name)).toEqual(['Hotel Lutetia', 'Home']) }) + + // Day 2 checks out of one stay and into the next, with no stop of its own. + const hotelA = { ...hotel, place_name: 'Hotel A', start_day_id: 10, end_day_id: 20 } as Accommodation + const hotelB = { ...hotel, place_lat: 53.55, place_lng: 9.99, place_name: 'Hotel B', start_day_id: 20, end_day_id: 30 } as Accommodation + + it('FE-MOBILE-DAYROUTE-010: a flight between the two stays leaves nothing to export (#2476)', () => { + // Located or not, the flight is how the day gets from one hotel to the other; a + // driving route between them is the stretch nobody drives. + expect(dayExportStops(days[1], days, [], [hotelA, hotelB], true, true, true)).toEqual([]) + expect(dayExportStops(days[1], days, [], [hotelA, hotelB], true, false, true)).toEqual([]) + expect(dayGoogleMapsUrl(days[1], days, [], [hotelA, hotelB], true, false, true)).toBeNull() + expect(dayCoMapsUrl(days[1], days, [], [hotelA, hotelB], true, 'driving', false, true)).toBeNull() + }) + + it('FE-MOBILE-DAYROUTE-011: without a booking the move is still the drive from one stay to the next (#1297)', () => { + expect(dayExportStops(days[1], days, [], [hotelA, hotelB], true).map(s => s.name)).toEqual(['Hotel A', 'Hotel B']) + // A stop of its own keeps the day exportable even with the flight booked. + expect(dayExportStops(days[1], days, [louvre], [hotelA, hotelB], true, true, true).map(s => s.name)).toContain('Louvre') + }) }) describe('day map links', () => { diff --git a/client/src/mobile/screens/trip/lib/dayRoute.ts b/client/src/mobile/screens/trip/lib/dayRoute.ts index 5b17dec99..76aa6f882 100644 --- a/client/src/mobile/screens/trip/lib/dayRoute.ts +++ b/client/src/mobile/screens/trip/lib/dayRoute.ts @@ -66,7 +66,10 @@ export function optimizeDayOrder( * links that can label a pin with one. `dayHasCarrier` — a carrier transport * with a located endpoint on the day — keeps the no-time bookend default from * opening a leg out of a hotel only reached tonight or back into one already - * left (#2157), same as the drawn route. + * left (#2157), same as the drawn route. `dayHasCarrierBooking`, any carrier + * booked on the day with or without coordinates, leaves a day without stops + * with nothing to export: the hotels at either end are joined by that booking, + * not by a road (#2476). */ export function dayExportStops( day: Day, @@ -75,8 +78,10 @@ export function dayExportStops( accommodations: Accommodation[], bookendFromAccommodation: boolean, dayHasCarrier?: boolean, + dayHasCarrierBooking?: boolean, ): NamedWaypoint[] { const located = dayAssignments.filter(a => a.place?.lat != null && a.place?.lng != null) + if (located.length === 0 && dayHasCarrierBooking) return [] const stops = located.map(a => ({ lat: a.place!.lat!, lng: a.place!.lng!, name: a.place!.name })) const bookends = bookendFromAccommodation ? getDayBookendHotels(day, days, accommodations) : null const firstStop = located[0] @@ -103,9 +108,10 @@ export function dayGoogleMapsUrl( accommodations: Accommodation[], bookendFromAccommodation: boolean, dayHasCarrier?: boolean, + dayHasCarrierBooking?: boolean, ): string | null { return generateGoogleMapsUrl( - dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation, dayHasCarrier), + dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation, dayHasCarrier, dayHasCarrierBooking), ) || null } @@ -118,9 +124,10 @@ export function dayCoMapsUrl( bookendFromAccommodation: boolean, profile: RouteProfileKey, dayHasCarrier?: boolean, + dayHasCarrierBooking?: boolean, ): string | null { return generateCoMapsUrl( - dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation, dayHasCarrier), + dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation, dayHasCarrier, dayHasCarrierBooking), profile, ) || null } diff --git a/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx b/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx index a118a2b30..2005ed16b 100644 --- a/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx +++ b/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx @@ -282,8 +282,12 @@ export default function MPlanTimeline({ planner, shell }: MPlanTimelineProps) { void tl.optimize()} /> - - + {tl.canExportRoute && ( + <> + + + + )}
)}
diff --git a/client/src/mobile/screens/trip/plan/planTimelineModel.ts b/client/src/mobile/screens/trip/plan/planTimelineModel.ts index 47b2f8d30..938293549 100644 --- a/client/src/mobile/screens/trip/plan/planTimelineModel.ts +++ b/client/src/mobile/screens/trip/plan/planTimelineModel.ts @@ -199,7 +199,13 @@ export function hotelLegsForDay( const legAt = (a: Accommodation | undefined, end: 'from' | 'to'): HotelLeg | null => { if (!a || a.place_lat == null || a.place_lng == null) return null const coord: [number, number] = [a.place_lat, a.place_lng] - const seg = routeSegments.find(s => sameCoord(end === 'from' ? s.from : s.to, coord)) + // 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)) return seg ? { seg, name: accommodationName(a) } : null } return { top: legAt(morning, 'from'), bottom: legAt(evening, 'to') } diff --git a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts index 4bc28ea88..8b98be4de 100644 --- a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts +++ b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts @@ -4,8 +4,8 @@ import { useRouteCalculation } from '../../../../hooks/useRouteCalculation' import { assignmentsApi, reservationsApi, weatherApi } from '../../../../api/client' import { usePluginStore } from '../../../../store/pluginStore' import { getDayBookendHotels } from '../../../../utils/dayOrder' -import { getDisplayTimeForDay, getMergedItems, getTransportForDay, hasCarrierEndpointOnDay } from '../../../../utils/dayMerge' -import { dayCoMapsUrl, dayGoogleMapsUrl, optimizeDayOrder } from '../lib/dayRoute' +import { getDisplayTimeForDay, getMergedItems, getTransportForDay, hasCarrierEndpointOnDay, isCarrierTransport } from '../../../../utils/dayMerge' +import { dayCoMapsUrl, dayExportStops, dayGoogleMapsUrl, optimizeDayOrder } from '../lib/dayRoute' import { buildTransitLeg, buildTransitNameIndex, type TransitLeg } from '../../../../components/Planner/transitLeg' import { buildPlanRows, breaksChronology, findUpNext, hotelChipsForDay, hotelLegsForDay, itemHasTime, @@ -60,6 +60,12 @@ export function useMPlanTimeline(planner: TripPlanner) { () => !!day && merged.some(it => it.type === 'transport' && hasCarrierEndpointOnDay(it.data, day.id)), [day, merged], ) + // Any carrier booked today, located or not. On a day without stops it is the move + // itself, so the exports have no road to hand over (#2476). + const dayHasCarrierBooking = useMemo( + () => merged.some(it => it.type === 'transport' && isCarrierTransport(it.data)), + [merged], + ) // Travel-time connectors (walk · distance · drive between consecutive places) // are shown permanently on the mobile timeline. The planner's route instance @@ -318,25 +324,35 @@ export function useMPlanTimeline(planner: TripPlanner) { } }, [day, dayAssignments, days, tripAccommodations, settings, dayHasCarrier, tripActions, tripId, pushUndo, updateRouteForDay, toast, t]) + // A route needs two ends. With fewer the map hand-offs would open a lone pin or + // nothing at all, so the plan does not offer them (#2476). + const canExportRoute = useMemo( + () => !!day && dayExportStops( + day, days, dayAssignments, tripAccommodations, settings.optimize_from_accommodation !== false, + dayHasCarrier, dayHasCarrierBooking, + ).length >= 2, + [day, days, dayAssignments, tripAccommodations, settings, dayHasCarrier, dayHasCarrierBooking], + ) + const exportGoogleMaps = useCallback(() => { if (!day) return // Bookend the exported route with the day's accommodation the same way the // drawn route does — only when the leg is real (#1372, #1465). const url = dayGoogleMapsUrl( day, days, dayAssignments, tripAccommodations, settings.optimize_from_accommodation !== false, - dayHasCarrier, + dayHasCarrier, dayHasCarrierBooking, ) if (url) window.open(url, '_blank', 'noopener,noreferrer') - }, [day, dayAssignments, days, tripAccommodations, settings, dayHasCarrier]) + }, [day, dayAssignments, days, tripAccommodations, settings, dayHasCarrier, dayHasCarrierBooking]) const exportCoMaps = useCallback(() => { if (!day) return const url = dayCoMapsUrl( day, days, dayAssignments, tripAccommodations, settings.optimize_from_accommodation !== false, - day.default_transport_mode ?? routeProfile, dayHasCarrier, + day.default_transport_mode ?? routeProfile, dayHasCarrier, dayHasCarrierBooking, ) if (url) window.open(url, '_blank', 'noopener,noreferrer') - }, [day, dayAssignments, days, tripAccommodations, settings, routeProfile, dayHasCarrier]) + }, [day, dayAssignments, days, tripAccommodations, settings, routeProfile, dayHasCarrier, dayHasCarrierBooking]) const renameDay = useCallback((title: string) => { if (!day) return @@ -408,7 +424,7 @@ export function useMPlanTimeline(planner: TripPlanner) { moveRow, removeAssignment, editAssignment, editTransport, openTransitJourney, moveRowTo, addPlace, addBooking, addTransport, - optimize, exportGoogleMaps, exportCoMaps, renameDay, fullPlaceOf, + optimize, canExportRoute, exportGoogleMaps, exportCoMaps, renameDay, fullPlaceOf, routeModeOptions, setLegMode, transitLegFor, planTransitLeg, } } diff --git a/client/tests/integration/hooks/useRouteCalculation.test.ts b/client/tests/integration/hooks/useRouteCalculation.test.ts index 636f13988..a4cfdd292 100644 --- a/client/tests/integration/hooks/useRouteCalculation.test.ts +++ b/client/tests/integration/hooks/useRouteCalculation.test.ts @@ -966,4 +966,34 @@ describe('useRouteCalculation', () => { // Tonight's hotel is still where the day ends. expect(legs).toContainEqual([`${home.lat},${home.lng}`, `${hotel.lat},${hotel.lng}`]); }); + + it('FE-HOOK-ROUTE-035: #2476 a moving day with a flight saved without airports draws no hotel → hotel drive', async () => { + // FE-HOOK-ROUTE-016 with the flight the day was actually travelled on. It carries + // no coordinates, so nothing is left to draw, and the fallback drive from one + // hotel to the other is exactly the stretch that was flown. + const hotelA = { lat: 48.137, lng: 11.575 }; + const hotelB = { lat: 53.551, lng: 9.993 }; + const accommodations = [ + { id: 1, start_day_id: 1, end_day_id: 2, place_lat: hotelA.lat, place_lng: hotelA.lng }, + { id: 2, start_day_id: 2, end_day_id: 3, place_lat: hotelB.lat, place_lng: hotelB.lng }, + ]; + const flight = { + id: 80, type: 'flight', title: 'LH 2078', day_id: 2, end_day_id: 2, + reservation_time: '2026-11-04T15:15', reservation_end_time: '2026-11-04T17:20', endpoints: [], + }; + const store = { assignments: {} } as unknown as TripStoreState; + useTripStore.setState({ + assignments: {}, + reservations: [flight], + days: [{ id: 1, day_number: 1 }, { id: 2, day_number: 2 }, { id: 3, day_number: 3 }], + } as unknown as Partial); + + const { result } = renderHook(() => + useRouteCalculation(store, 2, true, 'driving', accommodations as unknown as Parameters[4]) + ); + await act(async () => {}); + + expect(result.current.route).toBeNull(); + expect(calculateRouteWithLegs).not.toHaveBeenCalled(); + }); }); diff --git a/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx b/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx index 33319c6a0..d6205892d 100644 --- a/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx +++ b/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx @@ -9,7 +9,7 @@ import type { MergedItem } from '../../../../src/utils/dayMerge' import type { Assignment, Day, DayNote, Place, RouteSegment } from '../../../../src/types' import MPlanTimeline from '../../../../src/mobile/screens/trip/plan/MPlanTimeline' -// FE-MOB-PLTL-001 to FE-MOB-PLTL-047 +// FE-MOB-PLTL-001 to FE-MOB-PLTL-049 const mocks = vi.hoisted(() => ({ tl: {} as Record, @@ -110,7 +110,9 @@ function buildTl(over: Record = {}): MPlanTimelineController { addBooking: vi.fn(), addTransport: vi.fn(), optimize: vi.fn(async () => undefined), + canExportRoute: true, exportGoogleMaps: vi.fn(), + exportCoMaps: vi.fn(), renameDay: vi.fn(), fullPlaceOf: vi.fn(() => undefined as Place | undefined), routeModeOptions: [ @@ -583,6 +585,16 @@ describe('MPlanTimeline', () => { expect(mocks.tl.exportGoogleMaps).toHaveBeenCalledTimes(1) }) + it('FE-MOB-PLTL-049: a day with no route to hand over offers no Google Maps or CoMaps tile (#2476)', () => { + // A moving day whose only content is the flight: the export would be a drive + // from one hotel to the other, so the two tiles are left out, not left dead. + renderTimeline({ canExportRoute: false }, {}, { mode: 'edit' }) + + expect(screen.queryByText('mobileTrip.googleMaps')).not.toBeInTheDocument() + expect(screen.queryByText('mobileTrip.coMaps')).not.toBeInTheDocument() + expect(screen.getByText('dayplan.optimize')).toBeInTheDocument() + }) + it('FE-MOB-PLTL-034: the note tile is inert while no day is selected', () => { const { shell } = renderTimeline({ day: undefined, rows: [], merged: [] }, {}, { mode: 'edit' }) diff --git a/client/tests/unit/mobile/trip/planTimelineModel.test.ts b/client/tests/unit/mobile/trip/planTimelineModel.test.ts index d7fc2dfd8..df2c3408a 100644 --- a/client/tests/unit/mobile/trip/planTimelineModel.test.ts +++ b/client/tests/unit/mobile/trip/planTimelineModel.test.ts @@ -6,12 +6,13 @@ import { 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 { 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-044 +// FE-MOB-PTLM-001 to FE-MOB-PTLM-046 const DAYS = [ { id: 1, trip_id: 1, day_number: 1, date: '2026-05-01', title: null }, @@ -309,6 +310,61 @@ describe('planTimelineModel — hotel chips and legs', () => { 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 }) }) + + it('FE-MOB-PTLM-045: the evening leg is the last drive into the hotel, not the first (#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 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 legs = hotelLegsForDay(DAY2, DAYS, [hotel], [out, toHotelSpot, onward, back]) + expect(legs.top?.seg).toBe(out) + expect(legs.bottom?.seg).toBe(back) + }) + + describe('a moving day with a flight between the two stays (#2476)', () => { + // Day 2 checks out of Munich and into Hamburg; the flight sits between them. + const HOTEL_A = { lat: 48.137, lng: 11.575 } + const HOTEL_B = { lat: 53.551, lng: 9.993 } + const MUC = { lat: 48.353, lng: 11.786 } + const HAM = { lat: 53.63, lng: 9.988 } + const stays = [ + accommodation({ id: 1, start_day_id: 1, end_day_id: 2, place_name: 'Hotel A', place_lat: HOTEL_A.lat, place_lng: HOTEL_A.lng }), + accommodation({ id: 2, start_day_id: 2, end_day_id: 3, place_name: 'Hotel B', place_lat: HOTEL_B.lat, place_lng: HOTEL_B.lng }), + ] + const flight = (located: boolean) => buildReservation({ + id: 7, type: 'flight', title: 'LH 2078', day_id: 2, end_day_id: 2, + reservation_time: '2026-05-02T15:15', reservation_end_time: '2026-05-02T17:20', + endpoints: located + ? [ + { role: 'from', sequence: 0, name: 'MUC', code: null, ...MUC, 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 }, + ] + : [], + }) + // 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) + } + + it('FE-MOB-PTLM-046: shows no drive from one hotel to the other, above or below the flight', () => { + // Saved without its airports, the flight leaves nothing to connect: the plan + // used to show the whole Munich to Hamburg drive twice, around the flight. + expect(legsOf([flight(false)])).toEqual({ top: null, bottom: null }) + // With them, the morning drive goes to the departure airport and the evening + // one comes from the arrival airport. + const located = legsOf([flight(true)]) + expect(located.top).toMatchObject({ name: 'Hotel A', seg: { from: [HOTEL_A.lat, HOTEL_A.lng], to: [MUC.lat, MUC.lng] } }) + expect(located.bottom).toMatchObject({ name: 'Hotel B', seg: { from: [HAM.lat, HAM.lng], to: [HOTEL_B.lat, HOTEL_B.lng] } }) + }) + }) }) describe('planTimelineModel — cityPillsForDay', () => { diff --git a/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts b/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts index f7b03f419..aeb8ba377 100644 --- a/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts +++ b/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts @@ -12,7 +12,7 @@ import { buildPlanner, buildTripActions } from '../../../helpers/mobileTrip' import { resetAllStores, seedStore } from '../../../helpers/store' import { act, renderHook, waitFor } from '../../../helpers/render' -// FE-MOB-PLTL-001 to FE-MOB-PLTL-045 +// FE-MOB-PLTL-001 to FE-MOB-PLTL-050 // The connector calculation is its own hook with real OSRM calls — stubbed here so // the timeline sees exactly the legs a test wants to match against. @@ -604,6 +604,29 @@ describe('useMPlanTimeline', () => { expect(window.open).not.toHaveBeenCalled() }) + it('FE-MOB-PLTL-050: a moving day that only holds its flight has no route to export (#2476)', async () => { + // Day 2 checks out of one hotel and into another, the flight in between saved + // without its airports. The exports used to open a drive between the two hotels. + const stays = [ + { ...HOTEL, id: 72, start_day_id: 1, end_day_id: 2, place_name: 'Hotel A' }, + { ...HOTEL, id: 73, start_day_id: 2, end_day_id: 3, place_name: 'Hotel B', place_lat: 53.55, place_lng: 9.99 }, + ] as Accommodation[] + const flight = buildReservation({ id: 77, type: 'flight', title: 'LH 2078', day_id: 2, end_day_id: 2, reservation_time: '2026-05-02T15:15' }) + const { result } = await renderTimeline(makePlanner({ assignments: {}, reservations: [flight], tripAccommodations: stays })) + expect(result.current.canExportRoute).toBe(false) + act(() => { result.current.exportGoogleMaps() }) + act(() => { result.current.exportCoMaps() }) + expect(window.open).not.toHaveBeenCalled() + + // The same day without the booking is a drive from one hotel to the other. + const byRoad = await renderTimeline(makePlanner({ assignments: {}, reservations: [], tripAccommodations: stays })) + expect(byRoad.result.current.canExportRoute).toBe(true) + act(() => { byRoad.result.current.exportGoogleMaps() }) + expect(window.open).toHaveBeenCalledWith( + 'https://www.google.com/maps/dir/48,16.05/53.55,9.99', '_blank', 'noopener,noreferrer', + ) + }) + it('FE-MOB-PLTL-039: renames the day with a trimmed title', async () => { const planner = makePlanner() const { result } = await renderTimeline(planner)