diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 8c168915..1159a876 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -411,25 +411,30 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { // 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 { morning: startHotel, evening: endHotel } = - day && optimizeFromAccommodation !== false ? getDayBookendHotels(day, days, accommodations) : {} + const bookends = day && optimizeFromAccommodation !== false + ? getDayBookendHotels(day, days, accommodations) + : null + const startHotel = bookends?.morning + const endHotel = bookends?.evening const hotelName = (a: Accommodation) => (a as any).place_name || (a as any).reservation_title || '' // 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 }[] = [] + // legs connect even when the day starts or ends with a booking rather than a place. Track + // whether each is a place so we can skip a hotel↔transport leg that isn't real: on a day-1 + // arrival the check-in hotel never drove to the departure airport (#1321). + const wayPts: { lat: number; lng: number; isPlace: boolean }[] = [] for (const it of merged) { if (it.type === 'place' && it.data.place?.lat && it.data.place?.lng) { - wayPts.push({ lat: it.data.place.lat, lng: it.data.place.lng }) + wayPts.push({ lat: it.data.place.lat, lng: it.data.place.lng, isPlace: true }) } 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 }) + if (from) wayPts.push({ lat: from.lat, lng: from.lng, isPlace: false }) + if (to) wayPts.push({ lat: to.lat, lng: to.lng, isPlace: false }) } } const firstWay = wayPts[0] const lastWay = wayPts[wayPts.length - 1] - const wantTop = !!(startHotel && firstWay) - const wantBottom = !!(endHotel && lastWay) + const wantTop = !!(startHotel && firstWay && (firstWay.isPlace || bookends?.morningIsSleptHere)) + const wantBottom = !!(endHotel && lastWay && (lastWay.isPlace || bookends?.eveningIsOvernight)) if (runs.length === 0 && !wantTop && !wantBottom) { setRouteLegs({}); setHotelLegs({}); return } diff --git a/client/src/hooks/useRouteCalculation.ts b/client/src/hooks/useRouteCalculation.ts index fa55b755..7089577a 100644 --- a/client/src/hooks/useRouteCalculation.ts +++ b/client/src/hooks/useRouteCalculation.ts @@ -105,8 +105,9 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu // getDayBookendHotels returns the morning/evening hotel (they differ only on a // transfer day) and already filters to accommodations that have coordinates. const day = allDays.find(d => d.id === dayId) - const { morning: startHotel, evening: endHotel } = - day && optimizeFromAccommodation !== false ? getDayBookendHotels(day, allDays, accommodations) : {} + const bookends = day && optimizeFromAccommodation !== false + ? getDayBookendHotels(day, allDays, accommodations) + : null const flatPts: { lat: number; lng: number }[] = [] for (const e of entries) { if (e.kind === 'place') flatPts.push({ lat: e.lat, lng: e.lng }) @@ -114,7 +115,24 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu } const hotelPt = (a?: Accommodation) => a && a.place_lat != null && a.place_lng != null ? { lat: a.place_lat, lng: a.place_lng } : null - const runsWithHotel = withHotelBookends(runs, flatPts[0], flatPts[flatPts.length - 1], hotelPt(startHotel), hotelPt(endHotel)) + // Only draw a hotel bookend when the leg is real. A hotel → first-stop leg holds + // if the first stop is a place, or if you actually slept in that hotel last night; + // on a day-1 arrival the morning hotel is just a check-in fallback and the first + // waypoint is the transport's departure point, so [hotel → departure] is dropped + // (#1321). Symmetrically, [last-stop → hotel] is dropped when you leave on a transport + // in the evening and don't sleep in that hotel tonight. + const contributes = (e: Entry) => e.kind === 'place' || !!e.from || !!e.to + const firstStop = entries.find(contributes) + const lastStop = [...entries].reverse().find(contributes) + const drawMorning = firstStop?.kind === 'place' || !!bookends?.morningIsSleptHere + const drawEvening = lastStop?.kind === 'place' || !!bookends?.eveningIsOvernight + const runsWithHotel = withHotelBookends( + runs, + flatPts[0], + flatPts[flatPts.length - 1], + drawMorning ? hotelPt(bookends?.morning) : null, + drawEvening ? hotelPt(bookends?.evening) : null, + ) const straightLines = (): [number, number][][] => runsWithHotel.map(r => r.map(p => [p.lat, p.lng] as [number, number])) diff --git a/client/src/utils/dayOrder.test.ts b/client/src/utils/dayOrder.test.ts index 4aac45a8..ba961596 100644 --- a/client/src/utils/dayOrder.test.ts +++ b/client/src/utils/dayOrder.test.ts @@ -117,4 +117,40 @@ describe('getDayBookendHotels', () => { const h = hotel({ place_lat: null, place_lng: null }) expect(getDayBookendHotels(days[1], days, [h])).toEqual({}) }) + + it('flags an arrival/check-in day as not slept-here in the morning (#1321)', () => { + // Day 1: you arrive from home and check in tonight, so the morning hotel is only a + // check-in fallback — no hotel → departure leg should be drawn. + const into = hotel({ start_day_id: 10, end_day_id: 30, place_lat: 3, place_lng: 4 }) + const r = getDayBookendHotels(days[0], days, [into]) + expect(r.morning).toBe(into) + expect(r.morningIsSleptHere).toBe(false) + expect(r.eveningIsOvernight).toBe(true) + // The optimizer anchor must stay a loop on the check-in day (values unchanged). + expect(getAccommodationAnchors(days[0], days, [into])).toEqual({ start: { lat: 3, lng: 4 }, end: { lat: 3, lng: 4 } }) + }) + + it('flags a mid-stay day as slept-here and overnight', () => { + const h = hotel({ start_day_id: 10, end_day_id: 30 }) + const r = getDayBookendHotels(days[1], days, [h]) + expect(r.morningIsSleptHere).toBe(true) + expect(r.eveningIsOvernight).toBe(true) + }) + + it('an evening departure with no replacement check-in is not overnight (S7 mirror)', () => { + // You woke up here but check out today and board an evening transport — you do not + // sleep here tonight, so the last-stop → hotel leg must be droppable. + const h = hotel({ start_day_id: 10, end_day_id: 20, place_lat: 1, place_lng: 1 }) + const r = getDayBookendHotels(days[1], days, [h]) + expect(r.morningIsSleptHere).toBe(true) + expect(r.eveningIsOvernight).toBe(false) + }) + + it('flags a transfer day as slept-here in the morning and overnight in the evening', () => { + 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 r = getDayBookendHotels(days[1], days, [out, into]) + expect(r.morningIsSleptHere).toBe(true) + expect(r.eveningIsOvernight).toBe(true) + }) }) diff --git a/client/src/utils/dayOrder.ts b/client/src/utils/dayOrder.ts index 1332a7b0..aa69350f 100644 --- a/client/src/utils/dayOrder.ts +++ b/client/src/utils/dayOrder.ts @@ -12,7 +12,7 @@ export const getDayBookendHotels = ( day: Day, days: Day[], accommodations: Accommodation[], -): { morning?: Accommodation; evening?: Accommodation } => { +): { morning?: Accommodation; evening?: Accommodation; morningIsSleptHere?: boolean; eveningIsOvernight?: boolean } => { const inRange = accommodations.filter(a => a.place_lat != null && a.place_lng != null && isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days), @@ -30,6 +30,13 @@ export const getDayBookendHotels = ( return { morning: sleptHere ?? checkIn ?? inRange[0], evening: checkIn ?? sleptHere ?? inRange[0], + // Provenance for the drawing consumers (map + sidebar). A hotel↔transport bookend + // is only real when you actually used the hotel: morningIsSleptHere is true only + // when you woke up there (not a check-in fallback on an arrival day), and + // eveningIsOvernight is true only when you sleep there tonight (you check in today, + // or an earlier stay continues past today). The optimizer keeps using the values. + morningIsSleptHere: sleptHere != null, + eveningIsOvernight: checkIn != null || (sleptHere != null && orderOf(sleptHere.end_day_id) > dayOrd), } } diff --git a/client/tests/integration/hooks/useRouteCalculation.test.ts b/client/tests/integration/hooks/useRouteCalculation.test.ts index 637e39f8..a1ea6835 100644 --- a/client/tests/integration/hooks/useRouteCalculation.test.ts +++ b/client/tests/integration/hooks/useRouteCalculation.test.ts @@ -251,6 +251,77 @@ describe('useRouteCalculation', () => { expect(result.current.routeSegments).toEqual([]); }); + it('FE-HOOK-ROUTE-014: #1321 day-1 arrival draws no check-in-hotel → departure leg', async () => { + // Day 1 = arrival from home: a flight (departure → arrival airport) then two activities, + // checking into a hotel tonight. The morning hotel is only a check-in fallback, so the + // hotel must NOT be bookended to the flight's departure point; the evening leg stays. + const dep = { lat: 50.03, lng: 8.57 }; // home/departure airport + const arr = { lat: 41.30, lng: 2.08 }; // destination airport + const actA = buildPlace({ lat: 41.38, lng: 2.17 }); + const actB = buildPlace({ lat: 41.40, lng: 2.19 }); + const hotel = { lat: 41.39, lng: 2.16 }; + + const flight = { + id: 100, type: 'flight', day_id: 1, end_day_id: 1, 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: 1, order_index: 1, place: actA }); + const a2 = buildAssignment({ day_id: 1, order_index: 2, place: actB }); + const accommodations = [{ id: 1, start_day_id: 1, end_day_id: 2, place_lat: hotel.lat, place_lng: hotel.lng }]; + // A single stable store reference (like buildMockStore) so selectedDayAssignments + // keeps its identity across renders and the effect doesn't loop. + const store = { assignments: { '1': [a1, a2] } } 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]}`)); + // The spurious morning bookend [hotel → departure airport] must be gone. + expect(legs).not.toContainEqual([`${hotel.lat},${hotel.lng}`, `${dep.lat},${dep.lng}`]); + // The route starts the day's run at the arrival airport, not the hotel. + expect(result.current.route?.[0]?.[0]).toEqual([arr.lat, arr.lng]); + // The evening leg [last activity → hotel] is still drawn. + expect(legs).toContainEqual([`${actB.lat},${actB.lng}`, `${hotel.lat},${hotel.lng}`]); + }); + + it('FE-HOOK-ROUTE-015: day-1 with no transport keeps the hotel → first-activity leg', async () => { + // Guard against over-suppression: with no arrival transport, the check-in day is a + // home-base loop and the hotel → first-stop leg must remain. + const actA = buildPlace({ lat: 41.38, lng: 2.17 }); + const actB = buildPlace({ lat: 41.40, lng: 2.19 }); + const hotel = { lat: 41.39, lng: 2.16 }; + const a1 = buildAssignment({ day_id: 1, order_index: 0, place: actA }); + const a2 = buildAssignment({ day_id: 1, order_index: 1, place: actB }); + 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, a2] } } 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, 1, true, 'driving', accommodations as any) + ); + + await act(async () => {}); + + const legs = (result.current.route ?? []).map(run => run.map(p => `${p[0]},${p[1]}`)); + expect(legs).toContainEqual([`${hotel.lat},${hotel.lng}`, `${actA.lat},${actA.lng}`]); + expect(legs).toContainEqual([`${actB.lat},${actB.lng}`, `${hotel.lat},${hotel.lng}`]); + }); + it('FE-HOOK-ROUTE-012: setRoute and setRouteInfo are exposed', () => { const store = buildMockStore({}); const { result } = renderHook(() =>