diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index f564d3136..7fd3038fa 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -30,7 +30,7 @@ import { useTranslation } from '../../i18n' import { isDayInAccommodationRange, getAccommodationAnchors, getDayBookendHotels, shouldDrawMorningLeg, shouldDrawEveningLeg, type CarrierEdge } from '../../utils/dayOrder' import { TRANSPORT_TYPES, parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getDisplayTimeForDay, getTransportRouteEndpoints, - getTransportForDay as _getTransportForDay, getMergedItems as _getMergedItems, isCarrierTransport, + getTransportForDay as _getTransportForDay, getMergedItems as _getMergedItems, isCarrierTransport, hasCarrierEndpointOnDay, type MergedItem, } from '../../utils/dayMerge' import { withinDriveRange } from '../../utils/geo' @@ -592,8 +592,11 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const reachable = (h: { place_lat?: number | null; place_lng?: number | null } | undefined, w: typeof firstWay) => !h || !w || w.isPlace || h.place_lat == null || h.place_lng == null || withinDriveRange({ lat: h.place_lat, lng: h.place_lng }, w) - const wantTop = !!(startHotel && firstWay && bookends && day && shouldDrawMorningLeg(bookends, day, firstWay)) && reachable(startHotel, firstWay) - const wantBottom = !!(endHotel && lastWay && bookends && day && shouldDrawEveningLeg(bookends, day, lastWay)) && reachable(endHotel, lastWay) + // Same carrier evidence the map route uses (#2157): with a located carrier + // endpoint on the day, the no-time default must not open a hotel leg. + const dayHasCarrier = wayPts.some(w => w.carrierEdge != null) + const wantTop = !!(startHotel && firstWay && bookends && day && shouldDrawMorningLeg(bookends, day, firstWay, dayHasCarrier)) && reachable(startHotel, firstWay) + const wantBottom = !!(endHotel && lastWay && bookends && day && shouldDrawEveningLeg(bookends, day, lastWay, dayHasCarrier)) && reachable(endHotel, lastWay) return { runs, startHotel, endHotel, firstWay, lastWay, wantTop, wantBottom } } @@ -1582,11 +1585,14 @@ 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) 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 } : undefined + 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] - const last = lastAssignment ? { isPlace: true, time: lastAssignment.place?.place_time ?? null } : undefined - const drawMorning = !!routeBookends && shouldDrawMorningLeg(routeBookends, day, first) - const drawEvening = !!routeBookends && shouldDrawEveningLeg(routeBookends, day, last) + const last = lastAssignment ? { isPlace: true, time: lastAssignment.place?.place_time ?? null, lat: lastAssignment.place!.lat!, lng: lastAssignment.place!.lng! } : undefined + // Same carrier gate as the drawn route (#2157): the exported link must not + // start at a hotel you only reach tonight or lead back to one you left. + const dayHasCarrier = (mergedItemsMap[day.id] || []).some(i => i.type === 'transport' && hasCarrierEndpointOnDay(i.data, day.id)) + const drawMorning = !!routeBookends && shouldDrawMorningLeg(routeBookends, day, first, dayHasCarrier) + const drawEvening = !!routeBookends && shouldDrawEveningLeg(routeBookends, day, last, dayHasCarrier) const morning = drawMorning && routeBookends?.morning?.place_lat != null && routeBookends?.morning?.place_lng != null ? { lat: routeBookends.morning.place_lat, lng: routeBookends.morning.place_lng, name: routeBookends.morning.place_name } : null const evening = drawEvening && routeBookends?.evening?.place_lat != null && routeBookends?.evening?.place_lng != null diff --git a/client/src/hooks/useRouteCalculation.ts b/client/src/hooks/useRouteCalculation.ts index 702beaf2f..0aa69e49f 100644 --- a/client/src/hooks/useRouteCalculation.ts +++ b/client/src/hooks/useRouteCalculation.ts @@ -2,7 +2,7 @@ import { useState, useCallback, useRef, useEffect, useMemo } from 'react' import { useTripStore } from '../store/tripStore' import { useSettingsStore } from '../store/settingsStore' import { calculateRouteWithLegs, withHotelBookends, type RouteProfileKey } from '../components/Map/RouteCalculator' -import { getTransportRouteEndpoints, getTransportForDay, getMergedItems, isCarrierTransport } from '../utils/dayMerge' +import { getTransportRouteEndpoints, getTransportForDay, getMergedItems, isCarrierTransport, hasCarrierEndpointOnDay } from '../utils/dayMerge' import { getDayBookendHotels, shouldDrawMorningLeg, shouldDrawEveningLeg, type CarrierEdge } from '../utils/dayOrder' import { withinDriveRange } from '../utils/geo' import { resolveLegMode } from '../components/Planner/legMode' @@ -174,21 +174,25 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu const lastStop = [...entries].reverse().find(contributes) const edgeInfo = (e: Entry | undefined, side: 'first' | 'last') => { if (!e) return undefined - if (e.kind === 'place') return { isPlace: true, time: e.time } + if (e.kind === 'place') return { isPlace: true, time: e.time, lat: e.lat, lng: e.lng } const role: CarrierEdge = side === 'first' ? (e.from ? 'departure' : 'arrival') : (e.to ? 'arrival' : 'departure') return { isPlace: false, time: null, carrierEdge: e.carrier ? role : null } } + // A carrier with a located endpoint today means the day changes geography by + // air/rail/sea — the check-in hotel is then the day's destination and the + // check-out hotel its origin, which the no-time bookend default respects (#2157). + 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) // Same reachability test as the run builder: a hotel is not joined to a point no // one could have driven between. - const drawMorning = !!bookends && !!day && shouldDrawMorningLeg(bookends, day, edgeInfo(firstStop, 'first')) + 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')) + const drawEvening = !!bookends && !!day && shouldDrawEveningLeg(bookends, day, edgeInfo(lastStop, 'last'), dayHasCarrier) && (!eveningHotel || !lastWay || lastWay.isPlace || withinDriveRange(eveningHotel, lastWay)) const runsWithHotel: RunPoint[][] = withHotelBookends( runs, diff --git a/client/src/mobile/screens/trip/lib/dayRoute.test.ts b/client/src/mobile/screens/trip/lib/dayRoute.test.ts index 716e27554..506d7f4b2 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-005 +// FE-MOBILE-DAYROUTE-001 to FE-MOBILE-DAYROUTE-009 const days = [ { id: 10, day_number: 1 }, @@ -41,6 +41,34 @@ describe('dayExportStops', () => { const stops = dayExportStops(days[1], days, [louvre, nowhere], [], false) expect(stops.map(s => s.name)).toEqual(['Louvre']) }) + + // The stay above records neither check-in nor check-out time — the #2157 shape. + const homeNear = stop(4, 'Home', 48.9, 2.4) + + it('FE-MOBILE-DAYROUTE-006: with a carrier on the day, the not-yet-reached check-in hotel is not prepended (#2157)', () => { + // Arrival day: you fly in, so the hotel may end the list but must not start it. + const stops = dayExportStops(days[0], days, [homeNear], [hotel], true, true) + expect(stops.map(s => s.name)).toEqual(['Home', 'Hotel Lutetia']) + }) + + it('FE-MOBILE-DAYROUTE-007: with a carrier on the day, the already-left check-out hotel is not appended (#2157)', () => { + // Check-out day: the hotel starts the day, nothing routes back to it after the flight home. + const stops = dayExportStops(days[2], days, [homeNear], [hotel], true, true) + expect(stops.map(s => s.name)).toEqual(['Hotel Lutetia', 'Home']) + }) + + it('FE-MOBILE-DAYROUTE-008: without a carrier the no-time loop still closes (#2009 preserved)', () => { + const stops = dayExportStops(days[0], days, [louvre], [hotel], true, false) + expect(stops.map(s => s.name)).toEqual(['Hotel Lutetia', 'Louvre', 'Hotel Lutetia']) + }) + + it('FE-MOBILE-DAYROUTE-009: an edge stop out of drive range keeps the no-time hotel out even without a carrier (#2157)', () => { + // No flight recorded, but "Home" is an ocean away — the loop default is a guess + // and does not get to route back across it. + const farHome = stop(5, 'Home', 21.28, -157.83) + const stops = dayExportStops(days[2], days, [farHome], [hotel], true) + expect(stops.map(s => s.name)).toEqual(['Hotel Lutetia', 'Home']) + }) }) 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 1c16dd6ee..75e13d409 100644 --- a/client/src/mobile/screens/trip/lib/dayRoute.ts +++ b/client/src/mobile/screens/trip/lib/dayRoute.ts @@ -59,7 +59,10 @@ export function optimizeDayOrder( /** * The day's located stops in planned order, bookended by the morning/evening * accommodation exactly like the drawn route. Names ride along for the deep - * links that can label a pin with one. + * 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. */ export function dayExportStops( day: Day, @@ -67,17 +70,22 @@ export function dayExportStops( dayAssignments: Assignment[], accommodations: Accommodation[], bookendFromAccommodation: boolean, + dayHasCarrier?: boolean, ): NamedWaypoint[] { const located = dayAssignments.filter(a => a.place?.lat != null && a.place?.lng != null) 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] ? { isPlace: true, time: located[0].place?.place_time ?? null } : undefined + const firstStop = located[0] + ? { isPlace: true, time: located[0].place?.place_time ?? null, lat: located[0].place!.lat!, lng: located[0].place!.lng! } + : undefined const last = located[located.length - 1] - const lastStop = last ? { isPlace: true, time: last.place?.place_time ?? null } : undefined - const morning = bookends && shouldDrawMorningLeg(bookends, day, firstStop) + const lastStop = last + ? { isPlace: true, time: last.place?.place_time ?? null, lat: last.place!.lat!, lng: last.place!.lng! } + : undefined + const morning = bookends && shouldDrawMorningLeg(bookends, day, firstStop, dayHasCarrier) && bookends.morning?.place_lat != null && bookends.morning?.place_lng != null ? { lat: bookends.morning.place_lat, lng: bookends.morning.place_lng, name: bookends.morning.place_name } : null - const evening = bookends && shouldDrawEveningLeg(bookends, day, lastStop) + const evening = bookends && shouldDrawEveningLeg(bookends, day, lastStop, dayHasCarrier) && bookends.evening?.place_lat != null && bookends.evening?.place_lng != null ? { lat: bookends.evening.place_lat, lng: bookends.evening.place_lng, name: bookends.evening.place_name } : null return [...(morning ? [morning] : []), ...stops, ...(evening ? [evening] : [])] @@ -90,9 +98,10 @@ export function dayGoogleMapsUrl( dayAssignments: Assignment[], accommodations: Accommodation[], bookendFromAccommodation: boolean, + dayHasCarrier?: boolean, ): string | null { return generateGoogleMapsUrl( - dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation), + dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation, dayHasCarrier), ) || null } @@ -104,9 +113,10 @@ export function dayCoMapsUrl( accommodations: Accommodation[], bookendFromAccommodation: boolean, profile: RouteProfileKey, + dayHasCarrier?: boolean, ): string | null { return generateCoMapsUrl( - dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation), + dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation, dayHasCarrier), profile, ) || null } diff --git a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts index 1e12ca616..07a8ca463 100644 --- a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts +++ b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts @@ -4,7 +4,7 @@ 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 } from '../../../../utils/dayMerge' +import { getDisplayTimeForDay, getMergedItems, getTransportForDay, hasCarrierEndpointOnDay } from '../../../../utils/dayMerge' import { dayCoMapsUrl, dayGoogleMapsUrl, optimizeDayOrder } from '../lib/dayRoute' import { buildPlanRows, breaksChronology, findUpNext, hotelChipsForDay, hotelLegsForDay, itemHasTime, @@ -53,6 +53,13 @@ export function useMPlanTimeline(planner: TripPlanner) { }) }, [day, dayAssignments, dayNotes, reservations, days]) + // A carrier with a located endpoint today — the exported deep links need it so + // their no-time hotel bookends match the drawn route (#2157). + const dayHasCarrier = useMemo( + () => !!day && merged.some(it => it.type === 'transport' && hasCarrierEndpointOnDay(it.data, day.id)), + [day, merged], + ) + // Travel-time connectors (walk · distance · drive between consecutive places) // are shown permanently on the mobile timeline. The planner's route instance // only computes segments while the manual "show route" toggle is on (map), so @@ -316,18 +323,19 @@ export function useMPlanTimeline(planner: TripPlanner) { // drawn route does — only when the leg is real (#1372, #1465). const url = dayGoogleMapsUrl( day, days, dayAssignments, tripAccommodations, settings.optimize_from_accommodation !== false, + dayHasCarrier, ) if (url) window.open(url, '_blank', 'noopener,noreferrer') - }, [day, dayAssignments, days, tripAccommodations, settings]) + }, [day, dayAssignments, days, tripAccommodations, settings, dayHasCarrier]) const exportCoMaps = useCallback(() => { if (!day) return const url = dayCoMapsUrl( day, days, dayAssignments, tripAccommodations, settings.optimize_from_accommodation !== false, - day.default_transport_mode ?? routeProfile, + day.default_transport_mode ?? routeProfile, dayHasCarrier, ) if (url) window.open(url, '_blank', 'noopener,noreferrer') - }, [day, dayAssignments, days, tripAccommodations, settings, routeProfile]) + }, [day, dayAssignments, days, tripAccommodations, settings, routeProfile, dayHasCarrier]) const renameDay = useCallback((title: string) => { if (!day) return diff --git a/client/src/mobile/screens/trip/sheets/MDaySheet.tsx b/client/src/mobile/screens/trip/sheets/MDaySheet.tsx index 5fc2d4dc8..901023815 100644 --- a/client/src/mobile/screens/trip/sheets/MDaySheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MDaySheet.tsx @@ -13,6 +13,7 @@ import { usePluginStore } from '../../../../store/pluginStore' import { useDayNotes } from '../../../../hooks/useDayNotes' import { RES_ICONS, getNoteIcon } from '../../../../components/Planner/DayPlanSidebar.constants' import { getDayBookendHotels, isDayInAccommodationRange } from '../../../../utils/dayOrder' +import { getTransportForDay, hasCarrierEndpointOnDay } from '../../../../utils/dayMerge' import { splitReservationDateTime } from '../../../../utils/formatters' import { dayCoMapsUrl, dayGoogleMapsUrl, optimizeDayOrder } from '../lib/dayRoute' import GoogleMapsIcon from '../../../../components/shared/GoogleMapsIcon' @@ -174,11 +175,25 @@ export default function MDaySheet({ planner, shell }: MTripSheetsProps) { : t('dayplan.toast.routeOptimized')) } + // Carrier evidence for the export bookends (#2157) — via the span-aware transport + // filter the timeline uses, not dayReservations above, which matches on day_id + // only and would miss an overnight carrier that merely arrives today. + const dayHasCarrier = useMemo(() => { + if (!day) return false + return getTransportForDay({ + reservations: planner.reservations, + dayId: day.id, + dayAssignmentIds: dayAssignments.map(a => a.id), + days: planner.days, + }).some(r => hasCarrierEndpointOnDay(r, day.id)) + }, [day, planner.reservations, planner.days, dayAssignments]) + // Google-Maps export of the day's stops, hotel-bookended like the drawn route (#1372/#1465). const openInGoogleMaps = () => { if (!day) return const url = dayGoogleMapsUrl( day, planner.days, dayAssignments, planner.tripAccommodations, optimizeFromAccommodation !== false, + dayHasCarrier, ) if (url) window.open(url, '_blank', 'noopener,noreferrer') } @@ -188,7 +203,7 @@ export default function MDaySheet({ planner, shell }: MTripSheetsProps) { if (!day) return const url = dayCoMapsUrl( day, planner.days, dayAssignments, planner.tripAccommodations, optimizeFromAccommodation !== false, - day.default_transport_mode ?? planner.routeProfile, + day.default_transport_mode ?? planner.routeProfile, dayHasCarrier, ) if (url) window.open(url, '_blank', 'noopener,noreferrer') } diff --git a/client/src/utils/dayMerge.ts b/client/src/utils/dayMerge.ts index 457340e8b..157f27ae6 100644 --- a/client/src/utils/dayMerge.ts +++ b/client/src/utils/dayMerge.ts @@ -116,6 +116,21 @@ export function getTransportRouteEndpoints( } } +/** + * Whether this booking is a carrier that contributes a located route endpoint on the + * given day — the signal that the day's geography changes by riding something rather + * than driving, which the hotel-bookend rules need on days without recorded + * check-in/check-out times (#2157). The span rules above make an overnight carrier + * count only on the days it actually touches. A carrier saved without coordinates + * deliberately does not count: it contributes nothing to the drawn route, so treating + * it as proof of travel would flip hotel legs on data the map cannot see. + */ +export function hasCarrierEndpointOnDay(r: any, dayId: number): boolean { + if (!isCarrierTransport(r)) return false + const { from, to } = getTransportRouteEndpoints(r, dayId) + return from != null || to != null +} + export function getDisplayTimeForDay( r: { day_id?: number | null; end_day_id?: number | null; reservation_time?: string | null; reservation_end_time?: string | null }, dayId: number diff --git a/client/src/utils/dayOrder.test.ts b/client/src/utils/dayOrder.test.ts index 494ff72a7..84f99e4ae 100644 --- a/client/src/utils/dayOrder.test.ts +++ b/client/src/utils/dayOrder.test.ts @@ -225,6 +225,42 @@ describe('shouldDrawMorningLeg', () => { const bookends = { morning: into({ start_day_id: 20, check_in: '15:00' }), morningIsSleptHere: false } expect(shouldDrawMorningLeg(bookends, checkInDay, { isPlace: true, time: '19:00' })).toBe(false) }) + + it('does NOT draw on a no-check-in-time day when a carrier arrives or leaves that day (#2157)', () => { + // You fly in, so the hotel is where the day ends — the no-time loop default + // must not start the day from a hotel you only reach tonight. + const noCheckIn = { morning: into({ check_in: null }), morningIsSleptHere: false } + expect(shouldDrawMorningLeg(noCheckIn, checkInDay, { isPlace: true, time: null }, true)).toBe(false) + expect(shouldDrawMorningLeg(noCheckIn, checkInDay, { isPlace: true, time: '19:00' }, true)).toBe(false) + }) + + it('still closes the no-time loop when the day has no carrier (#2009 preserved)', () => { + const noCheckIn = { morning: into({ check_in: null }), morningIsSleptHere: false } + expect(shouldDrawMorningLeg(noCheckIn, checkInDay, { isPlace: true, time: null }, false)).toBe(true) + // Callers that don't know about the flag keep the old behaviour. + expect(shouldDrawMorningLeg(noCheckIn, checkInDay, { isPlace: true, time: null })).toBe(true) + }) + + it('a recorded check-in time keeps the timed rule even on a carrier day (#2157)', () => { + const bookends = { morning: into({ check_in: '15:00' }), morningIsSleptHere: false } + expect(shouldDrawMorningLeg(bookends, checkInDay, { isPlace: true, time: '19:00' }, true)).toBe(true) + expect(shouldDrawMorningLeg(bookends, checkInDay, { isPlace: true, time: '10:00' }, true)).toBe(false) + }) + + it('a carrier does not touch the slept-here morning leg (#2157)', () => { + // Mid-stay day with a day-trip by train: you still woke up in this hotel. + const bookends = { morning: into(), morningIsSleptHere: true } + expect(shouldDrawMorningLeg(bookends, checkInDay, { isPlace: true, time: '06:00' }, true)).toBe(true) + }) + + it('the no-time default does not reach an edge place out of drive range (#2157)', () => { + // "Home", an ocean away from the hotel, with no carrier recorded at all: the + // closed loop is a guess, and a guess does not get a 2400 km leg. + const noCheckIn = { morning: into({ check_in: null }), morningIsSleptHere: false } + expect(shouldDrawMorningLeg(noCheckIn, checkInDay, { isPlace: true, time: null, lat: 21.28, lng: -157.83 })).toBe(false) + // A nearby untimed place keeps the #2009 loop. + expect(shouldDrawMorningLeg(noCheckIn, checkInDay, { isPlace: true, time: null, lat: 48.2, lng: 11.6 })).toBe(true) + }) }) describe('shouldDrawEveningLeg', () => { @@ -285,4 +321,36 @@ describe('shouldDrawEveningLeg', () => { const bookends = { evening: out({ check_out: '11:00' }), eveningIsOvernight: false } expect(shouldDrawEveningLeg(bookends, checkOutDay, { isPlace: false, time: '09:00' })).toBe(false) }) + + it('does NOT draw on a no-check-out-time day when a carrier arrives or leaves that day (#2157)', () => { + // The reported last day: fly home, then a "Home" entry — nothing drives from + // Home back to the hotel you left that morning. + const noCheckOut = { evening: out({ check_out: null }), eveningIsOvernight: false } + expect(shouldDrawEveningLeg(noCheckOut, checkOutDay, { isPlace: true, time: null }, true)).toBe(false) + expect(shouldDrawEveningLeg(noCheckOut, checkOutDay, { isPlace: true, time: '09:00' }, true)).toBe(false) + }) + + it('still closes the no-time loop when the day has no carrier (#2009 preserved)', () => { + const noCheckOut = { evening: out({ check_out: null }), eveningIsOvernight: false } + expect(shouldDrawEveningLeg(noCheckOut, checkOutDay, { isPlace: true, time: null }, false)).toBe(true) + expect(shouldDrawEveningLeg(noCheckOut, checkOutDay, { isPlace: true, time: null })).toBe(true) + }) + + it('a recorded check-out time keeps the timed rule even on a carrier day (#2157)', () => { + const bookends = { evening: out({ check_out: '11:00' }), eveningIsOvernight: false } + expect(shouldDrawEveningLeg(bookends, checkOutDay, { isPlace: true, time: '09:00' }, true)).toBe(true) + expect(shouldDrawEveningLeg(bookends, checkOutDay, { isPlace: true, time: '18:00' }, true)).toBe(false) + }) + + it('a carrier does not touch the overnight evening leg (#2157)', () => { + // You sleep here tonight — the day-trip train changes nothing about that. + const bookends = { evening: out(), eveningIsOvernight: true } + expect(shouldDrawEveningLeg(bookends, checkOutDay, { isPlace: true, time: '23:00' }, true)).toBe(true) + }) + + it('the no-time default does not reach an edge place out of drive range (#2157)', () => { + const noCheckOut = { evening: out({ check_out: null }), eveningIsOvernight: false } + expect(shouldDrawEveningLeg(noCheckOut, checkOutDay, { isPlace: true, time: null, lat: 21.28, lng: -157.83 })).toBe(false) + expect(shouldDrawEveningLeg(noCheckOut, checkOutDay, { isPlace: true, time: null, lat: 48.2, lng: 11.6 })).toBe(true) + }) }) diff --git a/client/src/utils/dayOrder.ts b/client/src/utils/dayOrder.ts index 209f9e429..558f4665c 100644 --- a/client/src/utils/dayOrder.ts +++ b/client/src/utils/dayOrder.ts @@ -1,5 +1,6 @@ import type { Day, Accommodation, RouteAnchors } from '../types' import { parseTimeToMinutes } from './dayMerge' +import { withinDriveRange } from './geo' /** * Set on an edge waypoint that is the endpoint of a booking which CARRIES you out of @@ -71,13 +72,15 @@ export const getAccommodationAnchors = ( // morning hotel (a normal home-base day). On that hotel's check-in day you were traveling TO the // hotel, so the leg is drawn only when the first stop is a PLACE provably timed at/after check-in // (you dropped your bags first). A place before check-in (an airport you reach first, #1465), a -// transport arrival (you flew in, weren't at the hotel yet, #1321), an un-timed place ("Home" -// before driving out, #1597), or a missing check-in time all mean no leg — mirroring the -// evening rule below. +// transport arrival (you flew in, weren't at the hotel yet, #1321), or an un-timed place ("Home" +// before driving out, #1597) all mean no leg — mirroring the evening rule below. With no +// check-in time recorded the loop closes by default (#2009), except on a day a carrier +// takes you there or towards an edge place out of drive range (#2157). export const shouldDrawMorningLeg = ( bookends: { morning?: Accommodation; morningIsSleptHere?: boolean }, day: Day, - firstStop?: { isPlace: boolean; time?: string | null; carrierEdge?: CarrierEdge }, + firstStop?: { isPlace: boolean; time?: string | null; carrierEdge?: CarrierEdge; lat?: number; lng?: number }, + dayHasCarrier?: boolean, ): boolean => { // You landed here. Whatever hotel the day belongs to, nobody drove out of it to the // airport they arrived at — so there is no morning leg, not even on a night you @@ -92,7 +95,17 @@ export const shouldDrawMorningLeg = ( // loop at the start of every arrival day, since the hotel picker leaves the // time blank by default (#2009). With a time set, #1465 still holds: an // earlier stop is a place you reached before the hotel and draws no leg. - if (checkIn == null) return true + if (checkIn == null) { + // ...but the closed loop is a guess, and two things disprove it (#2157): a + // carrier on the day means you arrive at this hotel by air/rail/sea — on its + // check-in day the hotel is where the day ENDS, not where it starts — and an + // edge place no drive of the hotel could reach ("Home", an ocean away) was + // where you came FROM, not somewhere the hotel sent you. + if (dayHasCarrier) return false + if (firstStop.lat != null && firstStop.lng != null && m.place_lat != null && m.place_lng != null + && !withinDriveRange({ lat: m.place_lat, lng: m.place_lng }, { lat: firstStop.lat, lng: firstStop.lng })) return false + return true + } const stop = parseTimeToMinutes(firstStop.time) return stop != null && stop >= checkIn } @@ -105,7 +118,8 @@ export const shouldDrawMorningLeg = ( export const shouldDrawEveningLeg = ( bookends: { evening?: Accommodation; eveningIsOvernight?: boolean }, day: Day, - lastStop?: { isPlace: boolean; time?: string | null; carrierEdge?: CarrierEdge }, + lastStop?: { isPlace: boolean; time?: string | null; carrierEdge?: CarrierEdge; lat?: number; lng?: number }, + dayHasCarrier?: boolean, ): boolean => { // Mirror: you took off from here, so no drive leads from it back to tonight's hotel — // the reported "flight starting airport connected to the accommodation" (#2133). @@ -116,7 +130,15 @@ export const shouldDrawEveningLeg = ( const checkOut = parseTimeToMinutes(e.check_out) // Mirror of the morning rule: with no check-out time recorded there is nothing // to have missed, so the return leg is drawn and the loop closes (#2009). - if (checkOut == null) return true + if (checkOut == null) { + // Same two disproofs as the morning (#2157): a carrier on the check-out day + // means the hotel is where the day STARTS and nothing drives back to it, and + // an edge place out of drive range is where you went instead of returning. + if (dayHasCarrier) return false + if (lastStop.lat != null && lastStop.lng != null && e.place_lat != null && e.place_lng != null + && !withinDriveRange({ lat: e.place_lat, lng: e.place_lng }, { lat: lastStop.lat, lng: lastStop.lng })) return false + return true + } const stop = parseTimeToMinutes(lastStop.time) return stop != null && stop <= checkOut } diff --git a/client/tests/integration/hooks/useRouteCalculation.test.ts b/client/tests/integration/hooks/useRouteCalculation.test.ts index f131003b6..52a87d826 100644 --- a/client/tests/integration/hooks/useRouteCalculation.test.ts +++ b/client/tests/integration/hooks/useRouteCalculation.test.ts @@ -457,9 +457,10 @@ describe('useRouteCalculation', () => { it('FE-HOOK-ROUTE-022: #1597 check-in day with an un-timed place and a transport starts at the place, not the hotel', async () => { // Day 1 of a driving holiday: leave "Home" (no time set), cross by tunnel/ferry, and - // check into a hotel near the arrival port tonight. The hotel is only reached at the - // end of the day, so no hotel → Home leg may be drawn no matter the check-in time — - // the route starts at Home and still ends at the hotel. + // check into a hotel near the arrival port tonight. With a 19:00 check-in recorded, + // an un-timed Home cannot prove it was reached after check-in, so no hotel → Home + // leg — the route starts at Home and still ends at the hotel. (The same day with + // NO check-in time recorded is FE-HOOK-ROUTE-030.) const home = buildPlace({ lat: 52.48, lng: -1.90 }); // un-timed "Home" const dep = { lat: 51.09, lng: 1.12 }; // Folkestone terminal const arr = { lat: 50.94, lng: 1.81 }; // Calais terminal @@ -805,4 +806,112 @@ describe('useRouteCalculation', () => { const legs = (result.current.route ?? []).map(run => run.map(p => `${p[0]},${p[1]}`)); expect(legs).toContainEqual([`${hotel.lat},${hotel.lng}`, `${muc.lat},${muc.lng}`]); }); + + it('FE-HOOK-ROUTE-030: #2157 arrival day with no check-in time draws no hotel → Home leg when you fly in', async () => { + // The reported day 1: "Home" (untimed), the outbound flight, and a hotel that + // checks in tonight with neither check-in nor check-out recorded. The no-time + // default was closing the loop from a hotel 2400 km away — the 47-hour leg in + // the screenshot. Flying means the hotel is the day's destination, not its start. + const home = buildPlace({ lat: 50.11, lng: 8.68 }); // untimed "Home" + const dep = { lat: 50.03, lng: 8.57 }; // Frankfurt airport + const arr = { lat: 63.99, lng: -22.61 }; // Keflavík + const hotel = { lat: 64.14, lng: -21.94 }; // Reykjavík + + const flight = { + id: 400, type: 'flight', day_id: 1, end_day_id: 1, day_plan_position: 1, + endpoints: [ + { role: 'from', lat: dep.lat, lng: dep.lng }, + { role: 'to', lat: arr.lat, lng: arr.lng }, + ], + }; + const a1 = buildAssignment({ day_id: 1, order_index: 0, place: home }); + const accommodations = [{ id: 1, start_day_id: 1, end_day_id: 2, place_lat: hotel.lat, place_lng: hotel.lng }]; + const store = { assignments: { '1': [a1] } } as unknown as TripStoreState; + useTripStore.setState({ + assignments: store.assignments, + reservations: [flight], + days: [{ id: 1, day_number: 1 }, { id: 2, day_number: 2 }], + } as any); + + const { result } = renderHook(() => + useRouteCalculation(store, 1, true, 'driving', accommodations as any) + ); + await act(async () => {}); + + const legs = (result.current.route ?? []).map(run => run.map(p => `${p[0]},${p[1]}`)); + // No phantom morning bookend [hotel → Home]. + expect(legs).not.toContainEqual([`${hotel.lat},${hotel.lng}`, `${home.lat},${home.lng}`]); + // The day starts at Home and drives to the departure airport... + expect(result.current.route?.[0]?.[0]).toEqual([home.lat, home.lng]); + expect(legs).toContainEqual([`${home.lat},${home.lng}`, `${dep.lat},${dep.lng}`]); + // ...and the evening leg [arrival airport → hotel] is still drawn. + expect(legs).toContainEqual([`${arr.lat},${arr.lng}`, `${hotel.lat},${hotel.lng}`]); + }); + + it('FE-HOOK-ROUTE-031: #2157 departure day with no check-out time draws no Home → hotel leg after you fly out', async () => { + // The reported last day: fly home, then "Home" (untimed). The hotel you left that + // morning must not be routed back to from Home — but the morning drive + // [hotel → departure airport] is real and stays. + const hotel = { lat: 64.14, lng: -21.94 }; // Reykjavík + const dep = { lat: 63.99, lng: -22.61 }; // Keflavík + const arr = { lat: 50.03, lng: 8.57 }; // Frankfurt airport + const home = buildPlace({ lat: 50.11, lng: 8.68 }); // untimed "Home" + + const flight = { + id: 401, type: 'flight', day_id: 2, end_day_id: 2, day_plan_position: 0, + endpoints: [ + { role: 'from', lat: dep.lat, lng: dep.lng }, + { role: 'to', lat: arr.lat, lng: arr.lng }, + ], + }; + const a1 = buildAssignment({ day_id: 2, order_index: 1, place: home }); + const accommodations = [{ id: 1, start_day_id: 1, end_day_id: 2, place_lat: hotel.lat, place_lng: hotel.lng }]; + const store = { assignments: { '2': [a1] } } as unknown as TripStoreState; + useTripStore.setState({ + assignments: store.assignments, + reservations: [flight], + days: [{ id: 1, day_number: 1 }, { id: 2, day_number: 2 }], + } as any); + + const { result } = renderHook(() => + useRouteCalculation(store, 2, true, 'driving', accommodations as any) + ); + await act(async () => {}); + + const legs = (result.current.route ?? []).map(run => run.map(p => `${p[0]},${p[1]}`)); + // No phantom evening bookend [Home → hotel]. + expect(legs).not.toContainEqual([`${home.lat},${home.lng}`, `${hotel.lat},${hotel.lng}`]); + // The real morning drive [hotel → departure airport] stays... + expect(legs).toContainEqual([`${hotel.lat},${hotel.lng}`, `${dep.lat},${dep.lng}`]); + // ...and the day ends at Home. + const runs = result.current.route ?? []; + expect(runs[runs.length - 1]?.slice(-1)[0]).toEqual([home.lat, home.lng]); + }); + + it('FE-HOOK-ROUTE-032: #2157 no-time check-out day without any transport still draws no ocean leg back', async () => { + // The issue's literal steps mention no flight at all: just "Home" on the last + // day. With no carrier recorded, an edge place no drive of the hotel could + // reach must still not close the loop across the ocean. + const hotel = { lat: 64.14, lng: -21.94 }; // Reykjavík + const home = buildPlace({ lat: 50.11, lng: 8.68 }); // untimed "Home", ~2400 km away + const a1 = buildAssignment({ day_id: 2, order_index: 0, place: home }); + const accommodations = [{ id: 1, start_day_id: 1, end_day_id: 2, place_lat: hotel.lat, place_lng: hotel.lng }]; + const store = { assignments: { '2': [a1] } } as unknown as TripStoreState; + useTripStore.setState({ + assignments: store.assignments, + reservations: [], + days: [{ id: 1, day_number: 1 }, { id: 2, day_number: 2 }], + } as any); + + const { result } = renderHook(() => + useRouteCalculation(store, 2, true, 'driving', accommodations as any) + ); + await act(async () => {}); + + const legs = (result.current.route ?? []).map(run => run.map(p => `${p[0]},${p[1]}`)); + // No return leg [Home → hotel] after checking out. + expect(legs).not.toContainEqual([`${home.lat},${home.lng}`, `${hotel.lat},${hotel.lng}`]); + // The one real journey — you woke at the hotel and travelled home — stays. + expect(legs).toContainEqual([`${hotel.lat},${hotel.lng}`, `${home.lat},${home.lng}`]); + }); });