From 4bac447ac89fb93a006b230c80045f5488e10b41 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 23 Aug 2026 21:22:44 +0200 Subject: [PATCH] fix(planner): draw only the selected day's automated transports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #2019 The route toggle belongs to one day; the reservation list it filtered is the whole trip. So turning on a day's routing let every transit reservation in the trip draw its geometry at once, including the ones belonging to other days — on a day that has no transport of its own, which is what the reporter saw. Only automated transports rode it, and that is the distinction they drew: a manually entered train or flight is type 'train'/'flight' and reaches the map only through its own per-booking toggle. Only the transit search stores type 'transit'. Gate the transit branch on the day, and gate only that one — the per-booking toggle stays trip-wide, because switching a booking's route on is a deliberate act about that booking. The predicate is the day-order range, not the departure day: an overnight journey carries end_day_id and has to stay on both of its days, the same rule the sidebar timeline already applies. Day order rather than day id, since ids stop being monotonic once days are reordered. The day context is optional throughout, so a map without one — collections — keeps behaving exactly as it did. Not a 4.0.0 regression: it reproduces on 3.4.1 too. --- client/src/components/Map/MapView.tsx | 8 ++- client/src/components/Map/MapViewGL.tsx | 11 +++- .../src/mobile/screens/trip/map/MMapArea.tsx | 4 ++ client/src/pages/TripPlannerPage.tsx | 4 ++ .../src/pages/TripPlannerPage.wiring.test.tsx | 6 +- client/src/utils/reservationRoutes.test.ts | 64 ++++++++++++++++++- client/src/utils/reservationRoutes.ts | 30 ++++++++- 7 files changed, 116 insertions(+), 11 deletions(-) diff --git a/client/src/components/Map/MapView.tsx b/client/src/components/Map/MapView.tsx index 5c8ca3deb..f1b507b54 100644 --- a/client/src/components/Map/MapView.tsx +++ b/client/src/components/Map/MapView.tsx @@ -16,7 +16,7 @@ import { useTransportRoutes } from '../../hooks/useTransportRoutes' import { visibleRouteReservations } from '../../utils/reservationRoutes' import { safeHexColor } from '../../utils/safeColor' import { escapeHtml } from '@trek/shared' -import type { Reservation, RouteVia } from '../../types' +import type { Day, Reservation, RouteVia } from '../../types' import { POI_CATEGORY_BY_KEY, type Poi } from './poiCategories' import { resolveTrackColor, hasManualTrackColor } from './trackColors' import { DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM, SATELLITE_TILE_URL, SATELLITE_TILE_ATTRIBUTION, SATELLITE_TILE_MAXZOOM } from '../../constants/mapDefaults' @@ -526,6 +526,8 @@ export const MapView = memo(function MapView({ showReservationStats = false, visibleConnectionIds = [] as number[], showTransitRoutes = true, + days = [] as Day[], + selectedDayId = null, onReservationClick, pois = [] as Poi[], onPoiClick, @@ -545,8 +547,8 @@ export const MapView = memo(function MapView({ )), [pois, onPoiClick]) const visibleReservations = useMemo(() => ( - visibleRouteReservations(reservations, { visibleConnectionIds, showTransitRoutes }) - ), [reservations, visibleConnectionIds, showTransitRoutes]) + visibleRouteReservations(reservations, { visibleConnectionIds, showTransitRoutes, selectedDayId, days }) + ), [reservations, visibleConnectionIds, showTransitRoutes, selectedDayId, days]) // Real road geometry for car/bus/taxi/bicycle bookings (straight line until it loads/if it fails). const transportRoutes = useTransportRoutes(visibleReservations) // Dynamic padding: account for sidebars + bottom inspector + day detail panel diff --git a/client/src/components/Map/MapViewGL.tsx b/client/src/components/Map/MapViewGL.tsx index a2f3ab69a..4b4cf6241 100644 --- a/client/src/components/Map/MapViewGL.tsx +++ b/client/src/components/Map/MapViewGL.tsx @@ -17,7 +17,7 @@ import { escapeHtml } from '@trek/shared' import { MAPBOX_DEFAULT_STYLE, styleForActiveProvider, basemapLanguage, type GlMapProvider } from './glProviders' import LocationButton from './LocationButton' import { useGeolocation } from '../../hooks/useGeolocation' -import type { Place, Reservation, RouteVia } from '../../types' +import type { Day, Place, Reservation, RouteVia } from '../../types' import { POI_CATEGORY_BY_KEY, type Poi } from './poiCategories' import { resolveTrackColor, hasManualTrackColor } from './trackColors' import { buildPoiPopupHtml } from './placePopup' @@ -86,6 +86,7 @@ const NO_DAY_ORDER: Record = {} const NO_RESERVATIONS: Reservation[] = [] const NO_CONNECTION_IDS: number[] = [] const NO_POIS: Poi[] = [] +const NO_DAYS: Day[] = [] interface Props { places: Place[] @@ -114,6 +115,8 @@ interface Props { reservations?: Reservation[] visibleConnectionIds?: number[] showTransitRoutes?: boolean + days?: Day[] + selectedDayId?: number | null showReservationStats?: boolean onReservationClick?: (reservationId: number) => void pois?: Poi[] @@ -359,6 +362,8 @@ export function MapViewGL({ reservations = NO_RESERVATIONS, visibleConnectionIds = NO_CONNECTION_IDS, showTransitRoutes = true, + days = NO_DAYS, + selectedDayId = null, showReservationStats = false, onReservationClick, pois = NO_POIS, @@ -1270,8 +1275,8 @@ export function MapViewGL({ // DayPlanSidebar — nothing is rendered until the user enables a // booking's route, matching the Leaflet MapView's behaviour. const visibleReservations = useMemo(() => ( - visibleRouteReservations(reservations, { visibleConnectionIds, showTransitRoutes }) - ), [reservations, visibleConnectionIds, showTransitRoutes]) + visibleRouteReservations(reservations, { visibleConnectionIds, showTransitRoutes, selectedDayId, days }) + ), [reservations, visibleConnectionIds, showTransitRoutes, selectedDayId, days]) // Real road geometry for car/bus/taxi/bicycle bookings (straight line until it loads/if it fails). const transportRoutes = useTransportRoutes(visibleReservations) diff --git a/client/src/mobile/screens/trip/map/MMapArea.tsx b/client/src/mobile/screens/trip/map/MMapArea.tsx index 91636b0c7..86cd2253b 100644 --- a/client/src/mobile/screens/trip/map/MMapArea.tsx +++ b/client/src/mobile/screens/trip/map/MMapArea.tsx @@ -45,6 +45,10 @@ export default function MMapArea({ planner, shell }: MMapAreaProps) { route={planner.route} routeVias={planner.routeVias} showTransitRoutes={planner.routeShown} + // The route toggle belongs to one day, so the map needs that day to know + // which automated transports may ride it (#2019). + days={planner.days} + selectedDayId={planner.selectedDayId} routeSegments={planner.routeSegments} selectedPlaceId={planner.selectedPlaceId} onMarkerClick={planner.handleMarkerClick} diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index e0859d5ed..a78644845 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -366,6 +366,10 @@ function TripPlannerPageDesktop(): React.ReactElement | null { route={route} routeVias={routeVias} showTransitRoutes={routeShown} + // The route toggle belongs to one day, so the map needs that day to + // know which automated transports may ride it (#2019). + days={days} + selectedDayId={selectedDayId} routeSegments={routeSegments} selectedPlaceId={selectedPlaceId} onMarkerClick={handleMarkerClick} diff --git a/client/src/pages/TripPlannerPage.wiring.test.tsx b/client/src/pages/TripPlannerPage.wiring.test.tsx index f4a51c4b3..4ab76513e 100644 --- a/client/src/pages/TripPlannerPage.wiring.test.tsx +++ b/client/src/pages/TripPlannerPage.wiring.test.tsx @@ -351,13 +351,17 @@ describe('TripPlannerPage — shell', () => { }) describe('TripPlannerPage — plan tab', () => { - it('FE-PAGE-TPW-008: the map receives the filtered markers, the tile url and the panel widths', () => { + it('FE-PAGE-TPW-008: the map receives the filtered markers, the tile url, the panel widths and the selected day', () => { renderPage() expect(props('map').places).toEqual([place]) expect(props('map').tileUrl).toBe('https://tiles/{z}/{x}/{y}.png') expect(props('map').leftWidth).toBe(320) expect(props('map').center).toBeUndefined() + // Without the day the route toggle belongs to, the map draws every automated + // transport in the trip as soon as any day's route is on (#2019). + expect(props('map').days).toEqual([day]) + expect(props('map').selectedDayId).toBe(7) }) it('FE-PAGE-TPW-009: collapsed panels report a zero width to the map', () => { diff --git a/client/src/utils/reservationRoutes.test.ts b/client/src/utils/reservationRoutes.test.ts index d708411a9..1f0be69ec 100644 --- a/client/src/utils/reservationRoutes.test.ts +++ b/client/src/utils/reservationRoutes.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest' import { isRoutableReservation, visibleRouteReservations } from './reservationRoutes' -import type { Reservation, ReservationEndpoint } from '../types' +import type { Day, Reservation, ReservationEndpoint } from '../types' function endpoint(role: 'from' | 'to', lat: number, lng: number): ReservationEndpoint { return { role, sequence: role === 'from' ? 0 : 1, name: role, code: null, lat, lng, timezone: null, local_time: null, local_date: null } @@ -54,3 +54,65 @@ describe('visibleRouteReservations', () => { expect(result).toEqual([r]) }) }) + +describe('visibleRouteReservations with a selected day', () => { + const twoStop = [endpoint('from', 1, 2), endpoint('to', 3, 4)] + const days = [ + { id: 10, trip_id: 1, day_number: 1 }, + { id: 11, trip_id: 1, day_number: 2 }, + { id: 20, trip_id: 1, day_number: 3 }, + ] as Day[] + + function transit(overrides: Partial): Reservation { + return reservation({ type: 'transit', endpoints: twoStop, ...overrides }) + } + + it('excludes a transit journey that runs on another day', () => { + const r = transit({ id: 1, day_id: 20, end_day_id: 20 }) + expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, selectedDayId: 10, days })).toEqual([]) + }) + + it('includes a transit journey that runs on the selected day', () => { + const r = transit({ id: 1, day_id: 10, end_day_id: 10 }) + expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, selectedDayId: 10, days })).toEqual([r]) + }) + + it('keeps an overnight journey on both its departure and its arrival day', () => { + const r = transit({ id: 1, day_id: 10, end_day_id: 11 }) + expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, selectedDayId: 10, days })).toEqual([r]) + expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, selectedDayId: 11, days })).toEqual([r]) + }) + + it('keeps a multi-day journey on the days in between', () => { + const r = transit({ id: 1, day_id: 10, end_day_id: 20 }) + expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, selectedDayId: 11, days })).toEqual([r]) + }) + + it('goes by day order, not by day id', () => { + // Days dragged into a new order: id 20 is now the first day, id 10 the last. + const reordered = [ + { id: 20, trip_id: 1, day_number: 1 }, + { id: 11, trip_id: 1, day_number: 2 }, + { id: 10, trip_id: 1, day_number: 3 }, + ] as Day[] + const r = transit({ id: 1, day_id: 20, end_day_id: 11 }) + expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, selectedDayId: 11, days: reordered })).toEqual([r]) + expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, selectedDayId: 10, days: reordered })).toEqual([]) + }) + + it('keeps a transit journey that is bound to no day', () => { + const r = transit({ id: 1, day_id: null, end_day_id: null }) + expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, selectedDayId: 10, days })).toEqual([r]) + }) + + it('leaves the per-item route toggle day-agnostic', () => { + const r = reservation({ id: 5, type: 'train', endpoints: twoStop, day_id: 20, end_day_id: 20 }) + expect(visibleRouteReservations([r], { visibleConnectionIds: [5], showTransitRoutes: false, selectedDayId: 10, days })).toEqual([r]) + }) + + it('does not scope transit without a selected day', () => { + const r = transit({ id: 1, day_id: 20, end_day_id: 20 }) + expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, days })).toEqual([r]) + expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, selectedDayId: null, days })).toEqual([r]) + }) +}) diff --git a/client/src/utils/reservationRoutes.ts b/client/src/utils/reservationRoutes.ts index 312e0c12f..3f5cc5f2f 100644 --- a/client/src/utils/reservationRoutes.ts +++ b/client/src/utils/reservationRoutes.ts @@ -1,4 +1,5 @@ -import type { Reservation } from '../types' +import { isDayInAccommodationRange } from './dayOrder' +import type { Day, Reservation } from '../types' /** * A reservation is routable on the map once it has at least two ordered @@ -17,14 +18,37 @@ export interface RouteVisibilityOptions { visibleConnectionIds: number[] /** The separate manual day-route-calculator toggle — 'transit' reservations ride it (#1065). */ showTransitRoutes: boolean + /** The day that toggle belongs to. Optional: a map without a day context (collections) keeps drawing every transit, as before. */ + selectedDayId?: number | null + /** The trip's days, needed to order them — day ids are not monotonic once days have been reordered. */ + days?: Day[] +} + +/** + * Does this transit journey actually run on the selected day? `showTransitRoutes` is one + * day's toggle but the reservation list is trip-wide, so without this every automated + * transport in the trip drew its geometry as soon as any day's route was switched on (#2019). + * + * The span, not the departure day: an overnight journey carries end_day_id and has to stay + * on both of its days — same rule getTransportForDay applies to the sidebar timeline. + * A journey bound to no day keeps drawing; hiding something that legitimately belongs + * to today is the worse failure. + */ +function transitRunsOnDay(r: Reservation, selectedDayId: number | null | undefined, days: Day[]): boolean { + if (selectedDayId == null) return true + const startDayId = r.day_id ?? r.end_day_id + if (startDayId == null) return true + const day = days.find(d => d.id === selectedDayId) + if (!day) return true + return isDayInAccommodationRange(day, startDayId, r.end_day_id ?? startDayId, days) } /** Which reservations should draw a route on the map, combining the two independent toggles above. */ export function visibleRouteReservations(reservations: Reservation[], options: RouteVisibilityOptions): Reservation[] { - const { visibleConnectionIds, showTransitRoutes } = options + const { visibleConnectionIds, showTransitRoutes, selectedDayId, days } = options const set = new Set(visibleConnectionIds || []) return reservations.filter(r => - (r.type === 'transit' && showTransitRoutes) || + (r.type === 'transit' && showTransitRoutes && transitRunsOnDay(r, selectedDayId, days || [])) || set.has(r.id) ) }