From 3b933470d3ddc89d163db3e5020eac4537b4248e Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 12 Oct 2026 20:42:30 +0200 Subject: [PATCH] fix(planner): a booking pinned to a stop on another day is shown at that stop only, not on two days A booking linked to a stop has two days to go by, its own date and the stop's. They part when the stop is moved to another day or the trip is shifted with "keep bookings on their dates", and the plan then listed the booking at the stop and again in the timeline of its own day. The stop now wins wherever both are drawn: the desktop day plan and day detail, the phone's plan tab and day sheet, and the road trip rail on both. The booking's date is left as it is; a stop on one of the booking's own days changes nothing, so a rental picked up at a stop still runs across its other days. Refs #2604 --- .../src/components/Planner/DayDetailPanel.tsx | 7 +-- .../src/components/Planner/DayPlanSidebar.tsx | 6 ++- .../components/Planner/dayDetailModel.test.ts | 9 +++- .../src/components/Planner/dayDetailModel.ts | 8 +-- .../components/Roadtrip/RoadtripSidebar.tsx | 10 ++-- .../components/Roadtrip/stopBookings.test.ts | 9 ++++ .../src/components/Roadtrip/stopBookings.ts | 14 ++++- .../screens/trip/plan/useMPlanTimeline.ts | 8 ++- .../screens/trip/roadtrip/MRoadtripTab.tsx | 7 ++- .../mobile/screens/trip/sheets/MDaySheet.tsx | 9 ++-- client/src/utils/dayMerge.test.ts | 44 ++++++++++++++- client/src/utils/dayMerge.ts | 54 ++++++++++++++++++- .../unit/mobile/trip/useMPlanTimeline.test.ts | 15 +++++- 13 files changed, 175 insertions(+), 25 deletions(-) diff --git a/client/src/components/Planner/DayDetailPanel.tsx b/client/src/components/Planner/DayDetailPanel.tsx index 5135dfae8..441c2466c 100644 --- a/client/src/components/Planner/DayDetailPanel.tsx +++ b/client/src/components/Planner/DayDetailPanel.tsx @@ -19,6 +19,7 @@ import type { Day, Place, Category, Reservation, AssignmentsMap, Accommodation } import { formatClockTime, splitReservationDateTime } from '../../utils/formatters' import { useDayDetail, useDayRename, type HotelDayRange, type HotelForm, type HotelPickerMode } from './useDayDetail' import { dayBookings, stayDayLabel, toDisplayTemp } from './dayDetailModel' +import { pinnedAwayIn } from '../../utils/dayMerge' import { stayDayOptions, stayFormFrom, stayPlaceChoices, stayRangeFromEnd, stayRangeFromStart } from './stayFormModel' import { DialogShell, DialogHeader, DialogSection, DialogFooter, DialogButton, FooterSpacer, NEUTRAL_TINT, PILL, fs } from '../shared/DialogShell' import { INPUT, PANEL, EditorField, AddRowButton, PillSelect } from '../shared/dialogParts' @@ -230,7 +231,7 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri {dayContributions.length > 0 && } - +
@@ -374,10 +375,10 @@ function DayWeather({ weather, loading, isFahrenheit, placeName }: { weather: We const OPENS_BOOKING = 'w-full cursor-pointer text-start transition-shadow hover:shadow-md focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-1 focus-visible:outline-[color:var(--text-primary)]' /** The bookings of the day, hotels aside (they have their own block). With `onOpen` each row opens its booking. */ -function DayReservations({ day, assignments, reservations, is12h, onOpen }: { day: Day; assignments: AssignmentsMap; reservations: Reservation[]; is12h: boolean; onOpen?: (r: Reservation) => void }) { +function DayReservations({ day, days, assignments, reservations, is12h, onOpen }: { day: Day; days: Day[]; assignments: AssignmentsMap; reservations: Reservation[]; is12h: boolean; onOpen?: (r: Reservation) => void }) { const { t } = useTranslation() const dayAssignments = assignments[String(day.id)] || [] - const dayReservations = dayBookings(day.id, dayAssignments, reservations) + const dayReservations = dayBookings(day.id, dayAssignments, reservations, pinnedAwayIn(assignments, days)) if (dayReservations.length === 0) return null return ( diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index cd2c502cd..8d9a22e98 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -45,7 +45,7 @@ import { getAccommodationAnchors, getDayBookendHotels, shouldDrawMorningLeg, sho import { TRANSPORT_TYPES, parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getDisplayTimeForDay, getTransportRouteEndpoints, getTransportForDay as _getTransportForDay, getMergedItems as _getMergedItems, isCarrierTransport, hasCarrierEndpointOnDay, - getAssignmentReservations, timedSlot, storedRideSlot, seatTimedDrop, + getAssignmentReservations, timedSlot, storedRideSlot, seatTimedDrop, stopDayIds, type MergedItem, } from '../../utils/dayMerge' import { withinDriveRange } from '@trek/shared/roadtrip' @@ -434,8 +434,10 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { return { day_id: startId, end_day_id: targetDayId } } + // A booking pinned to a stop on another day shows at that stop only (#2604). + const stopDays = useMemo(() => stopDayIds(assignments), [assignments]) const getTransportForDay = (dayId: number) => - _getTransportForDay({ reservations, dayId, dayAssignmentIds: (assignments[String(dayId)] || []).map(a => a.id), days }) + _getTransportForDay({ reservations, dayId, dayAssignmentIds: (assignments[String(dayId)] || []).map(a => a.id), days, stopDays }) // Get car rentals that are in "active" (middle) phase for a day — shown in day header, not timeline const getActiveRentalsForDay = (dayId: number) => { diff --git a/client/src/components/Planner/dayDetailModel.test.ts b/client/src/components/Planner/dayDetailModel.test.ts index acd07f3d0..8a04e078f 100644 --- a/client/src/components/Planner/dayDetailModel.test.ts +++ b/client/src/components/Planner/dayDetailModel.test.ts @@ -1,4 +1,4 @@ -// FE-PLANNER-DAYDETAILMODEL-001 to -005: the day detail derivations the desktop day +// FE-PLANNER-DAYDETAILMODEL-001 to -006: the day detail derivations the desktop day // panel and the phone day sheet share. import { describe, expect, it } from 'vitest'; @@ -22,6 +22,13 @@ describe('dayBookings', () => { expect(dayBookings(4, [{ id: 11 }], [moved]).map((r) => r.id)).toEqual([5]); expect(dayBookings(6, [{ id: 11 }], [moved])).toEqual([]); }); + + it('FE-PLANNER-DAYDETAILMODEL-006: a booking whose stop moved to another day is listed at the stop only (#2604)', () => { + const dinner = buildReservation({ id: 5, type: 'restaurant', assignment_id: 77, day_id: 4 }); + const away = (r: { assignment_id?: number | null }) => r.assignment_id === 77; + expect(dayBookings(4, [{ id: 11 }], [dinner], away)).toEqual([]); + expect(dayBookings(6, [{ id: 77 }], [dinner], away).map((r) => r.id)).toEqual([5]); + }); }); describe('toDisplayTemp', () => { diff --git a/client/src/components/Planner/dayDetailModel.ts b/client/src/components/Planner/dayDetailModel.ts index f8536b3a3..da9134248 100644 --- a/client/src/components/Planner/dayDetailModel.ts +++ b/client/src/components/Planner/dayDetailModel.ts @@ -4,17 +4,19 @@ type Translate = (key: string, params?: Record) => stri /** * The bookings of a day, hotels aside (they have their own block): the ones hanging - * on one of the day's stops and the ones booked on the day itself. + * on one of the day's stops and the ones booked on the day itself. `pinnedAway` leaves + * a booking whose stop sits on another day to that stop (#2604). */ export function dayBookings>( dayId: number, dayAssignments: { id: number }[], - reservations: R[] + reservations: R[], + pinnedAway?: (r: Pick) => boolean ): R[] { return reservations.filter((r) => { if (r.type === 'hotel') return false; if (r.assignment_id && dayAssignments.some((a) => a.id === r.assignment_id)) return true; - return r.day_id === dayId; + return r.day_id === dayId && !pinnedAway?.(r); }); } diff --git a/client/src/components/Roadtrip/RoadtripSidebar.tsx b/client/src/components/Roadtrip/RoadtripSidebar.tsx index c971b5bef..d16dd52cc 100644 --- a/client/src/components/Roadtrip/RoadtripSidebar.tsx +++ b/client/src/components/Roadtrip/RoadtripSidebar.tsx @@ -18,7 +18,7 @@ import { ARRIVING_DRIVE, openOn, type LegAlternatives, type RailDrive } from './ import { FS } from './typeScale' import EmptyState from '../shared/EmptyState' import AutomaticDayStop from './AutomaticDayStop' -import { dayBookings } from './stopBookings' +import { dayBookings, stopDaysOf } from './stopBookings' import type { Reservation } from '../../types' import type { StayDraft } from './RoadtripStayModal' import { missedLeaveOf, stayDraftOf } from './stayReading' @@ -138,7 +138,7 @@ const RAIL_NARROW_PX = 330 * move, a stay edit or a refuel offer still names the day the server knows it by. See * `nightSpill.ts`. */ -function DaySection({ day, selectedAssignmentId, onSelectStop, onOpenBooking, canEditBookings, reservations, dayOrder, onReorderStop, onMoveStopToDay, drag, onAskAlternatives, openAlternatives, onEditStay, onSetStopKind, onSetStopFill, onFollowTrack, viaCount, trackName, refuel, onAskRefuel, onAcceptRefuel, loading, collapsed, onToggle, onFocusPoint, narrow }: { +function DaySection({ day, selectedAssignmentId, onSelectStop, onOpenBooking, canEditBookings, reservations, dayOrder, stopDays, onReorderStop, onMoveStopToDay, drag, onAskAlternatives, openAlternatives, onEditStay, onSetStopKind, onSetStopFill, onFollowTrack, viaCount, trackName, refuel, onAskRefuel, onAcceptRefuel, loading, collapsed, onToggle, onFocusPoint, narrow }: { onFocusPoint?: RoadtripSidebarProps['onFocusPoint'] /** The rail is pulled too narrow for every badge; the stop count gives way. */ narrow?: boolean @@ -159,6 +159,7 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onOpenBooking, ca reservations?: Reservation[] /** A day's place in the trip by its id, so a booking spanning days is seen on the days between. */ dayOrder?: (dayId: number) => number | null + stopDays?: ReadonlyMap onEditStay?: RoadtripSidebarProps['onEditStay'] onSetStopKind?: RoadtripSidebarProps['onSetStopKind'] onSetStopFill?: RoadtripSidebarProps['onSetStopFill'] @@ -175,7 +176,7 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onOpenBooking, ca // Which disc the picker hangs under, and for which stop. One at a time: two open // popovers over the same rail is two answers to one question. const [picking, setPicking] = useState<{ anchor: HTMLElement; stop: RoadtripStop } | null>(null) - const bookings = useMemo(() => dayBookings(day, reservations ?? [], dayOrder), [day, reservations, dayOrder]) + const bookings = useMemo(() => dayBookings(day, reservations ?? [], dayOrder, stopDays), [day, reservations, dayOrder, stopDays]) const [filling, setFilling] = useState<{ anchor: HTMLElement; stop: RoadtripStop } | null>(null) const { t } = useTranslation() const last = day.stops.length - 1 @@ -622,6 +623,8 @@ export default function RoadtripSidebar({ const numbers = new Map([...routes.days, ...routes.quietDays].map(d => [d.dayId, d.dayNumber])) return (dayId: number): number | null => numbers.get(dayId) ?? null }, [routes.days, routes.quietDays]) + // A booking pinned to a stop on another day is listed at that stop only (#2604). + const stopDays = useMemo(() => stopDaysOf(routes.days), [routes.days]) const undated = useMemo(() => undatedRides(reservations ?? []), [reservations]) // Nothing to total up, so nothing pretends to: no "0 km" standing above "No route yet". @@ -677,6 +680,7 @@ export default function RoadtripSidebar({ canEditBookings={canEditBookings} reservations={reservations} dayOrder={dayOrder} + stopDays={stopDays} onEditStay={onEditStay} onSetStopKind={onSetStopKind} onSetStopFill={onSetStopFill} diff --git a/client/src/components/Roadtrip/stopBookings.test.ts b/client/src/components/Roadtrip/stopBookings.test.ts index a273eb067..bdfc36484 100644 --- a/client/src/components/Roadtrip/stopBookings.test.ts +++ b/client/src/components/Roadtrip/stopBookings.test.ts @@ -17,6 +17,15 @@ const day = { } describe('dayBookings (#2428)', () => { + it('FE-STOPBOOKINGS-007: a booking pinned to a stop on another day is left to that stop, not listed loose here (#2604)', () => { + const order = (dayId: number) => dayId + const stopDays = new Map([[11, 1], [77, 3]]) + const dinner = booking({ id: 9, title: 'Dinner', day_id: 1, assignment_id: 77 }) + expect(dayBookings(day, [dinner], order, stopDays).loose).toEqual([]) + // Without the trip's stops it stays where it was. + expect(dayBookings(day, [dinner], order).loose.map(r => r.title)).toEqual(['Dinner']) + }) + it('FE-STOPBOOKINGS-001: a booking hangs under the stop it is pinned to, or under the stop at its place on its day, in clock order', () => { const { atStop, loose } = dayBookings(day, [ booking({ id: 3, title: 'Late', reservation_time: '2026-10-05T20:00', assignment_id: 11 }), diff --git a/client/src/components/Roadtrip/stopBookings.ts b/client/src/components/Roadtrip/stopBookings.ts index f0c7218ee..e2708a923 100644 --- a/client/src/components/Roadtrip/stopBookings.ts +++ b/client/src/components/Roadtrip/stopBookings.ts @@ -1,7 +1,7 @@ import { Ticket, type LucideIcon } from 'lucide-react' import { isCarrierType, isRentalType, isStoredStop } from '@trek/shared/roadtrip' import { RES_ICONS } from '../Planner/DayPlanSidebar.constants' -import { hidesOnMiddleDay } from '../../utils/dayMerge' +import { hidesOnMiddleDay, pinnedAway } from '../../utils/dayMerge' import { splitReservationDateTime } from '../../utils/formatters' import type { Reservation } from '../../types' import type { RoadtripStop } from '@trek/shared/roadtrip' @@ -64,6 +64,8 @@ export function dayBookings( reservations: readonly Reservation[], /** A day's position in the trip by its id, so a booking spanning days is on the days between too. */ dayOrder?: (dayId: number) => number | null | undefined, + /** The day each stop of the trip sits on, so a booking pinned to one elsewhere stays there. */ + stopDays?: ReadonlyMap, ): DayBookings { if (!reservations.length) return EMPTY const atStop = new Map() @@ -77,6 +79,7 @@ export function dayBookings( // hotel's own stop, or under the day. const real = (i: number): boolean => isStoredStop(stops[i]!) let index = r.assignment_id ? stops.findIndex((s, i) => real(i) && s.assignmentId === r.assignment_id) : -1 + if (index < 0 && dayOrder && pinnedAway(r, stopDays, dayOrder)) continue const onDay = spansDay(r, day.dayId, dayOrder) if (index < 0 && onDay && r.place_id) index = stops.findIndex((s, i) => real(i) && s.placeId === r.place_id) if (index >= 0) { @@ -92,6 +95,15 @@ export function dayBookings( return atStop.size || loose.length ? { atStop, loose } : EMPTY } +/** The day each stored stop of the drive sits on, by its assignment id, for {@link dayBookings}. */ +export function stopDaysOf( + days: readonly { dayId: number; stops: readonly Pick[] }[], +): Map { + const out = new Map() + for (const d of days) for (const s of d.stops) if (isStoredStop(s)) out.set(s.assignmentId, d.dayId) + return out +} + /** The booking panel's icon for the type, so a table reads as a table here too. */ export function bookingIcon(type: string): LucideIcon { return (RES_ICONS as Record)[type] ?? Ticket diff --git a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts index e1efd8203..9d404f119 100644 --- a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts +++ b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts @@ -7,7 +7,7 @@ import { toDisplayTemp } from '../../../../components/Planner/dayDetailModel' import { getDayBookendHotels } from '../../../../utils/dayOrder' import { dayWeatherAnchor } from '../../../../utils/dayWeather' import { metadataWithLegPositions, planMergedOrder, withTransportPositions } from '../../../../utils/mergedOrder' -import { getDisplayTimeForDay, getMergedItems, getTransportForDay, hasCarrierEndpointOnDay, isCarrierTransport } from '../../../../utils/dayMerge' +import { getDisplayTimeForDay, getMergedItems, getTransportForDay, hasCarrierEndpointOnDay, isCarrierTransport, stopDayIds } from '../../../../utils/dayMerge' import { dayCoMapsUrl, dayExportStops, dayGoogleMapsUrl, optimizeDayOrder, type DayCarrier } from '../../../../components/Planner/dayRoute' import { buildTransitLeg, buildTransitNameIndex, type TransitLeg } from '../../../../components/Planner/transitLeg' import { @@ -45,6 +45,9 @@ export function useMPlanTimeline(planner: TripPlanner) { return [...(assignments[String(day.id)] || [])].sort((a, b) => a.order_index - b.order_index) }, [assignments, day]) + // A booking pinned to a stop on another day shows at that stop only (#2604). + const stopDays = useMemo(() => stopDayIds(assignments), [assignments]) + const merged = useMemo(() => { if (!day) return [] const dayTransports = getTransportForDay({ @@ -52,6 +55,7 @@ export function useMPlanTimeline(planner: TripPlanner) { dayId: day.id, dayAssignmentIds: dayAssignments.map(a => a.id), days, + stopDays, }) return getMergedItems({ dayAssignments, @@ -59,7 +63,7 @@ export function useMPlanTimeline(planner: TripPlanner) { dayTransports, dayId: day.id, }) - }, [day, dayAssignments, dayNotes, reservations, days]) + }, [day, dayAssignments, dayNotes, reservations, days, stopDays]) // A carrier with a located endpoint today — the exported deep links need it so // their no-time hotel bookends match the drawn route (#2157). diff --git a/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx b/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx index 0c3a33e3b..0c9f2477e 100644 --- a/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx +++ b/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx @@ -19,6 +19,7 @@ import type { MTripTabPanelProps } from '../MTripShell' import { arrivingReroutable, legReroutable, type StopRow } from '../../../../components/Roadtrip/roadtripRowModel' import { ARRIVING_DRIVE, type RailDrive } from '../../../../components/Roadtrip/useRouteAlternatives' import { dayBookings } from '../../../../components/Roadtrip/stopBookings' +import { stopDayIds } from '../../../../utils/dayMerge' import { bookendBooking } from '../../../../components/Roadtrip/nightBookend' import { getDayOrder } from '../../../../utils/dayOrder' import { undatedRides } from '@trek/shared/roadtrip' @@ -99,12 +100,14 @@ export default function MRoadtripTab({ planner, shell }: MTripTabPanelProps) { // by the stop's assignment because the rows carry no index. Opened the way the place // sheet opens its linked bookings: a transport in its sheet, anything else in its // editor. + // A booking pinned to a stop on another day is listed at that stop only (#2604). + const stopDays = useMemo(() => stopDayIds(planner.assignments), [planner.assignments]) const bookings = useMemo(() => { - const byIndex = stage ? dayBookings(stage, planner.reservations, dayOrder) : null + const byIndex = stage ? dayBookings(stage, planner.reservations, dayOrder, stopDays) : null const atStop = new Map() if (stage && byIndex) for (const [i, list] of byIndex.atStop) atStop.set(stage.stops[i]!.assignmentId, list) return { atStop, loose: byIndex?.loose ?? [] } - }, [stage, planner.reservations, dayOrder]) + }, [stage, planner.reservations, dayOrder, stopDays]) // The rides on no day, which the drive leaves out: listed above the stage, whichever it is. const undated = useMemo(() => undatedRides(planner.reservations), [planner.reservations]) // A chip that may not be opened is not a button in the first place (`bookingOpens`), diff --git a/client/src/mobile/screens/trip/sheets/MDaySheet.tsx b/client/src/mobile/screens/trip/sheets/MDaySheet.tsx index a637e37bd..3a1c06acd 100644 --- a/client/src/mobile/screens/trip/sheets/MDaySheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MDaySheet.tsx @@ -12,7 +12,7 @@ import { useDayForecast, useDayRename } from '../../../../components/Planner/use import { dayBookings, stayDayLabel, toDisplayTemp } from '../../../../components/Planner/dayDetailModel' import { RES_ICONS, getNoteIcon } from '../../../../components/Planner/DayPlanSidebar.constants' import { getDayBookendHotels, isDayInAccommodationRange } from '../../../../utils/dayOrder' -import { getTransportForDay, hasCarrierEndpointOnDay } from '../../../../utils/dayMerge' +import { getTransportForDay, hasCarrierEndpointOnDay, pinnedAwayIn, stopDayIds } from '../../../../utils/dayMerge' import { splitReservationDateTime } from '../../../../utils/formatters' import { dayCoMapsUrl, dayGoogleMapsUrl, optimizeDayOrder } from '../../../../components/Planner/dayRoute' import GoogleMapsIcon from '../../../../components/shared/GoogleMapsIcon' @@ -84,8 +84,8 @@ export default function MDaySheet({ planner, shell }: MTripSheetsProps) { const dayReservations = useMemo(() => { if (!day) return [] - return dayBookings(day.id, dayAssignments, planner.reservations) - }, [day, planner.reservations, dayAssignments]) + return dayBookings(day.id, dayAssignments, planner.reservations, pinnedAwayIn(planner.assignments, planner.days)) + }, [day, planner.reservations, dayAssignments, planner.assignments, planner.days]) const dayAccommodations = useMemo(() => { if (!day) return [] @@ -160,8 +160,9 @@ export default function MDaySheet({ planner, shell }: MTripSheetsProps) { dayId: day.id, dayAssignmentIds: dayAssignments.map(a => a.id), days: planner.days, + stopDays: stopDayIds(planner.assignments), }).some(r => hasCarrierEndpointOnDay(r, day.id)) - }, [day, planner.reservations, planner.days, dayAssignments]) + }, [day, planner.reservations, planner.days, dayAssignments, planner.assignments]) // Google-Maps export of the day's stops, hotel-bookended like the drawn route (#1372/#1465). const openInGoogleMaps = () => { diff --git a/client/src/utils/dayMerge.test.ts b/client/src/utils/dayMerge.test.ts index e832594a6..5f99670af 100644 --- a/client/src/utils/dayMerge.test.ts +++ b/client/src/utils/dayMerge.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getTransportRouteEndpoints, getDisplayTimeForDay, getTransportForDay, getAssignmentReservations, getPlaceBookings, getMergedItems, nightRowsOf, rideSeatKey, seatTimedDrop, stayStartingOn, storedRideSlot, timedSlot, type MergedItem } from './dayMerge' +import { parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getTransportRouteEndpoints, getDisplayTimeForDay, getTransportForDay, getAssignmentReservations, pinnedAway, stopDayIds, getPlaceBookings, getMergedItems, nightRowsOf, rideSeatKey, seatTimedDrop, stayStartingOn, storedRideSlot, timedSlot, type MergedItem } from './dayMerge' import { planMergedOrder, withTransportPositions } from './mergedOrder' describe('parseTimeToMinutes', () => { @@ -200,6 +200,48 @@ describe('getTransportForDay', () => { }) }) +describe('a booking pinned to a stop on another day (#2604)', () => { + const days = [ + { id: 1, day_number: 1 }, + { id: 2, day_number: 2 }, + { id: 3, day_number: 3 }, + { id: 4, day_number: 4 }, + ] + const order = (id: number) => days.find(d => d.id === id)?.day_number ?? null + const stopDays = stopDayIds({ '1': [{ id: 41 }], '3': [{ id: 43 }, { id: 44 }], '4': undefined }) + + it('maps every stop to its day', () => { + expect([...stopDays.entries()]).toEqual([[41, 1], [43, 3], [44, 3]]) + }) + + it("a stop outside the booking's own days wins, a stop on one of them or a gone stop does not", () => { + expect(pinnedAway({ assignment_id: 43, day_id: 1 }, stopDays, order)).toBe(true) + expect(pinnedAway({ assignment_id: 41, day_id: 1 }, stopDays, order)).toBe(false) + // A rental picked up at a stop still runs across the rest of its days. + expect(pinnedAway({ assignment_id: 43, day_id: 2, end_day_id: 4 }, stopDays, order)).toBe(false) + expect(pinnedAway({ assignment_id: 99, day_id: 1 }, stopDays, order)).toBe(false) + expect(pinnedAway({ assignment_id: null, day_id: 1 }, stopDays, order)).toBe(false) + expect(pinnedAway({ assignment_id: 43, day_id: null }, stopDays, order)).toBe(false) + expect(pinnedAway({ assignment_id: 43, day_id: 1 }, undefined, order)).toBe(false) + }) + + it("the timeline of the booking's own day leaves it to the stop, so it is listed once", () => { + const dinner = { id: 10, type: 'restaurant', day_id: 1, assignment_id: 43 } + const reservations = [dinner] + expect(getTransportForDay({ reservations, dayId: 1, dayAssignmentIds: [41], days, stopDays })).toHaveLength(0) + expect(getTransportForDay({ reservations, dayId: 3, dayAssignmentIds: [43, 44], days, stopDays })).toHaveLength(0) + expect(getAssignmentReservations(reservations, 43)).toEqual([dinner]) + // Without the stops of the trip the old rule stands. + expect(getTransportForDay({ reservations, dayId: 1, dayAssignmentIds: [41], days })).toHaveLength(1) + }) + + it('a multi-day booking pinned to a stop on one of its days keeps its other days', () => { + const rental = { id: 11, type: 'event', day_id: 2, end_day_id: 4, assignment_id: 43 } + expect(getTransportForDay({ reservations: [rental], dayId: 2, dayAssignmentIds: [], days, stopDays })).toHaveLength(1) + expect(getTransportForDay({ reservations: [rental], dayId: 3, dayAssignmentIds: [43, 44], days, stopDays })).toHaveLength(0) + }) +}) + describe('getPlaceBookings (#2363)', () => { const reservations = [ { id: 1, assignment_id: 42, reservation_time: '2025-06-01T10:00:00' }, diff --git a/client/src/utils/dayMerge.ts b/client/src/utils/dayMerge.ts index 25e4dce3d..e94ef0014 100644 --- a/client/src/utils/dayMerge.ts +++ b/client/src/utils/dayMerge.ts @@ -204,14 +204,63 @@ export function expandFlightLegsForDay( return out } -/** Filter reservations that are active transports for the given day, excluding assignment-linked ones. */ +/** The day each stop of the trip sits on, by its assignment id, for {@link pinnedAway}. */ +export function stopDayIds(assignments: Record): Map { + const out = new Map() + for (const [dayId, list] of Object.entries(assignments)) { + for (const a of list || []) out.set(a.id, Number(dayId)) + } + return out +} + +/** + * Whether a booking pinned to a stop is shown at that stop and not on its own day. + * Such a booking has two days to go by, its own date and the stop's, and they part + * when the stop is moved to another day or the trip's dates shift while bookings keep + * theirs. The stop wins then, so the booking is listed once instead of on two days, + * and its date stays as it is (#2604). A stop on one of the booking's own days changes + * nothing: a rental picked up at a stop still runs across the rest of its days. + */ +export function pinnedAway( + r: { assignment_id?: number | null; day_id?: number | null; end_day_id?: number | null }, + stopDays: ReadonlyMap | undefined, + dayOrder: (dayId: number) => number | null | undefined +): boolean { + if (!r.assignment_id || !stopDays || r.day_id == null) return false + const stopDay = stopDays.get(r.assignment_id) + if (stopDay == null) return false + const at = dayOrder(stopDay) + const first = dayOrder(r.day_id) + const last = dayOrder(r.end_day_id ?? r.day_id) + if (at == null || first == null || last == null) return false + return at < Math.min(first, last) || at > Math.max(first, last) +} + +/** {@link pinnedAway} over the whole trip: its stops from `assignments`, its days in plan order. */ +export function pinnedAwayIn( + assignments: Record, + days: Array<{ id: number; day_number?: number }> +): (r: { assignment_id?: number | null; day_id?: number | null; end_day_id?: number | null }) => boolean { + const stopDays = stopDayIds(assignments) + const order = (id: number): number | null => { + const d = days.find(x => x.id === id) + return d ? (d.day_number ?? days.indexOf(d)) : null + } + return r => pinnedAway(r, stopDays, order) +} + +/** + * Filter reservations that are active transports for the given day, excluding assignment-linked ones. + * With `stopDays` a booking pinned to a stop on another day is left to that stop as well. + */ export function getTransportForDay(opts: { reservations: any[] dayId: number dayAssignmentIds: number[] days: Array<{ id: number; day_number?: number; date?: string | null }> + stopDays?: ReadonlyMap }): any[] { - const { reservations, dayId, dayAssignmentIds, days } = opts + const { reservations, dayId, dayAssignmentIds, days, stopDays } = opts const getDayOrder = (id: number): number => { const d = days.find(x => x.id === id) @@ -222,6 +271,7 @@ export function getTransportForDay(opts: { return reservations.filter(r => { if (r.type === 'hotel') return false if (r.assignment_id && dayAssignmentIds.includes(r.assignment_id)) return false + if (pinnedAway(r, stopDays, getDayOrder)) return false const startDayId = r.day_id const endDayId = r.end_day_id ?? startDayId diff --git a/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts b/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts index 96b3b91d4..f71a29709 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-051 +// FE-MOB-PLTL-001 to FE-MOB-PLTL-052 // 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. @@ -699,4 +699,17 @@ describe('useMPlanTimeline', () => { await act(async () => { result.current.setLegMode(11, 'driving') }) await waitFor(() => expect(planner.toast.error).toHaveBeenCalledWith('common.unknownError')) }) + + it('FE-MOB-PLTL-052: a booking pinned to a stop on another day shows at that stop only (#2604)', async () => { + const zoo = buildAssignment({ id: 31, day_id: 3, order_index: 0, place_id: 101, place: MUSEUM }) + const tickets = buildReservation({ id: 80, type: 'event', title: 'Zoo tickets', day_id: 2, assignment_id: 31 }) + const assignments = { '2': [A_MUSEUM], '3': [zoo] } + const ownDay = await renderTimeline(makePlanner({ assignments, reservations: [tickets] })) + expect(ownDay.result.current.rows.map(r => r.key)).toEqual(['pl-11']) + ownDay.unmount() + + const stopDay = await renderTimeline(makePlanner({ assignments, reservations: [tickets], selectedDayId: 3 })) + const row = stopDay.result.current.rows.find(r => r.key === 'pl-31') + expect(row && row.kind === 'place' ? row.linkedReservations.map(r => r.id) : []).toEqual([80]) + }) })