diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 91ad626d..0072eb7d 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -18,7 +18,7 @@ import { useTripStore } from '../../store/tripStore' import { useCanDo } from '../../store/permissionsStore' import { useSettingsStore } from '../../store/settingsStore' import { useTranslation } from '../../i18n' -import { isDayInAccommodationRange, getAccommodationAnchors } from '../../utils/dayOrder' +import { isDayInAccommodationRange, getAccommodationAnchors, getDayBookendHotels } from '../../utils/dayOrder' import { TRANSPORT_TYPES, parseTimeToMinutes, getSpanPhase, getDisplayTimeForDay, getTransportRouteEndpoints, getTransportForDay as _getTransportForDay, getMergedItems as _getMergedItems, @@ -407,30 +407,29 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { } if (cur.length >= 2) runs.push(cur) - // Hotel bookend legs: the drive from the day's accommodation to the first - // located place (morning) and from the last place back to it (evening). Only - // when the "optimize from accommodation" setting is on and the day has a hotel, - // mirroring the range logic the optimizer itself uses (getAccommodationAnchors). + // Hotel bookend legs: the drive from the day's accommodation to the first located + // waypoint of the day (morning) and from the last one back to it (evening). Only when + // the "optimize from accommodation" setting is on and the day has a hotel. const day = days.find(d => d.id === selectedDayId) - const dayAccs = day && optimizeFromAccommodation !== false - ? accommodations.filter(a => a.place_lat != null && a.place_lng != null && isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days)) - : [] - const checkOut = day ? dayAccs.find(a => a.end_day_id === day.id) : undefined - const checkIn = day ? dayAccs.find(a => a.start_day_id === day.id) : undefined - const transfer = !!(checkOut && checkIn && checkOut !== checkIn) - const startHotel = transfer ? checkOut : dayAccs[0] - const endHotel = transfer ? checkIn : dayAccs[0] + const { morning: startHotel, evening: endHotel } = + day && optimizeFromAccommodation !== false ? getDayBookendHotels(day, days, accommodations) : {} const hotelName = (a: Accommodation) => (a as any).place_name || (a as any).reservation_title || '' - const placePts: { lat: number; lng: number }[] = [] + // Waypoints include transport endpoints (a car return, a taxi/train arrival), so the hotel + // legs connect even when the day starts or ends with a booking rather than a place. + const wayPts: { lat: number; lng: number }[] = [] for (const it of merged) { if (it.type === 'place' && it.data.place?.lat && it.data.place?.lng) { - placePts.push({ lat: it.data.place.lat, lng: it.data.place.lng }) + wayPts.push({ lat: it.data.place.lat, lng: it.data.place.lng }) + } else if (it.type === 'transport') { + const { from, to } = getTransportRouteEndpoints(it.data, selectedDayId) + if (from) wayPts.push({ lat: from.lat, lng: from.lng }) + if (to) wayPts.push({ lat: to.lat, lng: to.lng }) } } - const firstPlace = placePts[0] - const lastPlace = placePts[placePts.length - 1] - const wantTop = !!(startHotel && firstPlace) - const wantBottom = !!(endHotel && lastPlace) + const firstWay = wayPts[0] + const lastWay = wayPts[wayPts.length - 1] + const wantTop = !!(startHotel && firstWay) + const wantBottom = !!(endHotel && lastWay) if (runs.length === 0 && !wantTop && !wantBottom) { setRouteLegs({}); setHotelLegs({}); return } @@ -456,11 +455,11 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { } const hotel: { top?: { seg: RouteSegment; name: string }; bottom?: { seg: RouteSegment; name: string } } = {} if (wantTop) { - const seg = await legBetween({ lat: startHotel!.place_lat as number, lng: startHotel!.place_lng as number }, { lat: firstPlace.lat, lng: firstPlace.lng }) + const seg = await legBetween({ lat: startHotel!.place_lat as number, lng: startHotel!.place_lng as number }, { lat: firstWay.lat, lng: firstWay.lng }) if (seg) hotel.top = { seg, name: hotelName(startHotel!) } } if (wantBottom) { - const seg = await legBetween({ lat: lastPlace.lat, lng: lastPlace.lng }, { lat: endHotel!.place_lat as number, lng: endHotel!.place_lng as number }) + const seg = await legBetween({ lat: lastWay.lat, lng: lastWay.lng }, { lat: endHotel!.place_lat as number, lng: endHotel!.place_lng as number }) if (seg) hotel.bottom = { seg, name: hotelName(endHotel!) } } diff --git a/client/src/utils/dayOrder.test.ts b/client/src/utils/dayOrder.test.ts index 8640d6b4..4aac45a8 100644 --- a/client/src/utils/dayOrder.test.ts +++ b/client/src/utils/dayOrder.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest' import type { Day, Accommodation } from '../types' -import { getDayOrder, isDayInAccommodationRange, getAccommodationAnchors } from './dayOrder' +import { getDayOrder, isDayInAccommodationRange, getAccommodationAnchors, getDayBookendHotels } from './dayOrder' const days = [ { id: 10, day_number: 1 }, @@ -70,4 +70,51 @@ describe('getAccommodationAnchors', () => { const accs = [hotel({ start_day_id: 10, end_day_id: 30, place_lat: null, place_lng: null })] expect(getAccommodationAnchors(days[1], days, accs)).toEqual({}) }) + + it('keeps morning/evening correct on a transfer day when the morning stay runs long (#887)', () => { + const accs = [ + hotel({ start_day_id: 10, end_day_id: 30, place_lat: 1, place_lng: 1 }), // slept here, checks out later + hotel({ start_day_id: 20, end_day_id: 30, place_lat: 9, place_lng: 9 }), // check-in today + ] + expect(getAccommodationAnchors(days[1], days, accs)).toEqual({ + start: { lat: 1, lng: 1 }, + end: { lat: 9, lng: 9 }, + }) + }) +}) + +describe('getDayBookendHotels', () => { + it('returns nothing when the day has no accommodation', () => { + expect(getDayBookendHotels(days[1], days, [])).toEqual({}) + }) + + it('bookends both ends with the single hotel on a normal stay day', () => { + const h = hotel({ start_day_id: 10, end_day_id: 30 }) + const { morning, evening } = getDayBookendHotels(days[1], days, [h]) + expect(morning).toBe(h) + expect(evening).toBe(h) + }) + + it('uses the checked-out hotel in the morning and the checked-in hotel in the evening on a transfer day', () => { + const out = hotel({ start_day_id: 10, end_day_id: 20, place_lat: 1, place_lng: 1 }) + const into = hotel({ start_day_id: 20, end_day_id: 30, place_lat: 9, place_lng: 9 }) + const { morning, evening } = getDayBookendHotels(days[1], days, [out, into]) + expect(morning).toBe(out) + expect(evening).toBe(into) + }) + + it('still picks the slept-in hotel for the morning when its stay does not end on the transfer day (#887)', () => { + // The morning hotel runs long (checks out day 3) so it is not flagged as "checks out today"; + // the old "ends today" rule collapsed both bookends onto the arriving hotel. + const stayed = hotel({ start_day_id: 10, end_day_id: 30, place_lat: 1, place_lng: 1 }) + const into = hotel({ start_day_id: 20, end_day_id: 30, place_lat: 9, place_lng: 9 }) + const { morning, evening } = getDayBookendHotels(days[1], days, [stayed, into]) + expect(morning).toBe(stayed) + expect(evening).toBe(into) + }) + + it('ignores accommodations without coordinates', () => { + const h = hotel({ place_lat: null, place_lng: null }) + expect(getDayBookendHotels(days[1], days, [h])).toEqual({}) + }) }) diff --git a/client/src/utils/dayOrder.ts b/client/src/utils/dayOrder.ts index c96b948b..1332a7b0 100644 --- a/client/src/utils/dayOrder.ts +++ b/client/src/utils/dayOrder.ts @@ -3,6 +3,36 @@ import type { Day, Accommodation, RouteAnchors } from '../types' export const getDayOrder = (day: Day, days: Day[]): number => day.day_number ?? days.indexOf(day) +// The two hotels that bookend a day: the one you woke up in (morning) and the one you sleep in +// tonight (evening). On a transfer day these differ; on any other day both are the single hotel. +// The morning hotel is keyed off "checked in on an earlier day and still in range" (i.e. you slept +// there) rather than "checks out today", so it stays correct when an overlapping or long stay does +// not end exactly on the transfer day. +export const getDayBookendHotels = ( + day: Day, + days: Day[], + accommodations: Accommodation[], +): { morning?: Accommodation; evening?: Accommodation } => { + const inRange = accommodations.filter(a => + a.place_lat != null && a.place_lng != null && + isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days), + ) + if (inRange.length === 0) return {} + + const dayOrd = getDayOrder(day, days) + const orderOf = (id: number) => { + const d = days.find(x => x.id === id) + return d ? getDayOrder(d, days) : dayOrd + } + const checkIn = inRange.find(a => a.start_day_id === day.id) // the hotel you arrive at tonight + const sleptHere = inRange.find(a => orderOf(a.start_day_id) < dayOrd) // the hotel you woke up in + + return { + morning: sleptHere ?? checkIn ?? inRange[0], + evening: checkIn ?? sleptHere ?? inRange[0], + } +} + // Derives route anchors from the accommodation(s) active on a day. A single hotel is the day's home // base, so the route is a loop that starts and ends there. A transfer day — checking out of one hotel // and into another — instead runs from the morning hotel to the evening one. @@ -11,22 +41,12 @@ export const getAccommodationAnchors = ( days: Day[], accommodations: Accommodation[], ): RouteAnchors => { - const located = accommodations.filter(a => - a.place_lat != null && a.place_lng != null && - isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days), - ) - if (located.length === 0) return {} - - const toAnchor = (a: Accommodation) => ({ lat: a.place_lat as number, lng: a.place_lng as number }) - - const checkOut = located.find(a => a.end_day_id === day.id) // the hotel you leave this morning - const checkIn = located.find(a => a.start_day_id === day.id) // the hotel you arrive at tonight - if (checkOut && checkIn && checkOut !== checkIn) { - return { start: toAnchor(checkOut), end: toAnchor(checkIn) } + const { morning, evening } = getDayBookendHotels(day, days, accommodations) + if (!morning || !evening) return {} + return { + start: { lat: morning.place_lat as number, lng: morning.place_lng as number }, + end: { lat: evening.place_lat as number, lng: evening.place_lng as number }, } - - const hotel = toAnchor(located[0]) - return { start: hotel, end: hotel } } export const isDayInAccommodationRange = (