mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-22 06:56:35 +00:00
fix(dayplan): resolve per-leg transit departure time within its own day
Review of the previous commit surfaced that the departure-time lookup keyed a trip-wide coordinate index, so a POI revisited on another day (a supported pattern) leaked the other day's time into the prefill. Resolve the departure time from the leg's own day instead; the name index stays trip-wide (a name is coordinate-stable). Also from review: wire the per-leg option on the mobile sidebar too (it was desktop-only), drop the now-dead route-leg fallback in the outgoing menu opener, and accept single-digit hours in the time normalisation. Adds a regression test for the revisited-stop case and pins the hotel-bookend fallback time.
This commit is contained in:
Ida Rogalska
@@ -3496,9 +3496,37 @@ describe('DayPlanSidebar', () => {
|
||||
dayId: 11,
|
||||
from: { name: 'Hotel Lutetia', lat: 48.85, lng: 2.35 },
|
||||
to: { name: 'Louvre', lat: 48.86, lng: 2.34 },
|
||||
// The hotel has no place_time, so the departure time falls back to null (the
|
||||
// panel then uses its own 09:00 default).
|
||||
time: null,
|
||||
}))
|
||||
})
|
||||
|
||||
it('FE-PLANNER-DAYPLAN-171e: a revisited stop seeds THIS day\'s departure time, not another day\'s', async () => {
|
||||
const user = userEvent.setup()
|
||||
const onPlanTransitLeg = vi.fn()
|
||||
const days = [
|
||||
buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }),
|
||||
buildDay({ id: 11, date: '2025-06-02', title: 'Day 2' }),
|
||||
]
|
||||
// The SAME located POI (identical coords) is visited on both days at different
|
||||
// times. Day 10 has it alone (no leg); day 11 pairs it with Rodin (one leg).
|
||||
const assignments = {
|
||||
'10': [
|
||||
buildAssignment({ id: 11, day_id: 10, order_index: 0, place: buildPlace({ id: 1, name: 'Louvre', place_time: '09:00', lat: 48.86, lng: 2.34 }) }),
|
||||
],
|
||||
'11': [
|
||||
buildAssignment({ id: 21, day_id: 11, order_index: 0, place: buildPlace({ id: 1, name: 'Louvre', place_time: '16:30', lat: 48.86, lng: 2.34 }) }),
|
||||
buildAssignment({ id: 22, day_id: 11, order_index: 1, place: buildPlace({ id: 3, name: 'Rodin', lat: 48.855, lng: 2.315 }) }),
|
||||
],
|
||||
}
|
||||
render(<DayPlanSidebar {...makeDefaultProps({ days, assignments, selectedDayId: 11, routeShown: true, onPlanTransitLeg })} />)
|
||||
await user.click(await screen.findByTitle('Change travel mode'))
|
||||
await user.click(contextMenu().getByRole('button', { name: 'Public transit' }))
|
||||
// Day 11's own 16:30, not day 10's 09:00 (a trip-wide coord index would leak it).
|
||||
expect(onPlanTransitLeg).toHaveBeenCalledWith(expect.objectContaining({ dayId: 11, time: '16:30' }))
|
||||
})
|
||||
|
||||
it('FE-PLANNER-DAYPLAN-172: a failing per-segment save is reported and the days are refetched', async () => {
|
||||
const user = userEvent.setup()
|
||||
const { assignmentsApi } = await import('../../api/client')
|
||||
|
||||
@@ -1295,42 +1295,60 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
|
||||
}
|
||||
|
||||
// A connector's endpoints carry only coordinates (RouteSegment.from/to, both
|
||||
// [lat, lng]); resolve them back to the named, timed points the transit search
|
||||
// wants by indexing every located place, hotel and booking endpoint on the trip.
|
||||
// A connector only renders when both ends are located, so a coordinate that misses
|
||||
// the index is rare — fall back to a nameless pick (the search still runs off the
|
||||
// coordinates alone).
|
||||
const transitPointIndex = useMemo(() => {
|
||||
const m = new Map<string, { name: string; time: string | null }>()
|
||||
const put = (lat?: number | null, lng?: number | null, name?: string | null, time?: string | null) => {
|
||||
if (lat == null || lng == null) return
|
||||
// [lat, lng]); resolve them back to the NAMES the transit search shows by indexing
|
||||
// every located place, hotel and booking endpoint on the trip. Name is a property
|
||||
// of a coordinate, so a trip-wide index is fine; a connector only renders when both
|
||||
// ends are located, so a coordinate that misses the index is rare (nameless pick).
|
||||
const transitNameIndex = useMemo(() => {
|
||||
const m = new Map<string, string>()
|
||||
const put = (lat?: number | null, lng?: number | null, name?: string | null) => {
|
||||
if (lat == null || lng == null || !name) return
|
||||
const k = `${lat},${lng}`
|
||||
if (!m.has(k)) m.set(k, { name: name || '', time: time ?? null })
|
||||
if (!m.has(k)) m.set(k, name)
|
||||
}
|
||||
for (const list of Object.values(assignments)) {
|
||||
for (const a of list) put(a.place?.lat, a.place?.lng, a.place?.name, a.place?.place_time)
|
||||
for (const a of list) put(a.place?.lat, a.place?.lng, a.place?.name)
|
||||
}
|
||||
for (const acc of accommodations) put(acc.place_lat, acc.place_lng, acc.place_name, null)
|
||||
for (const acc of accommodations) put(acc.place_lat, acc.place_lng, acc.place_name)
|
||||
for (const r of reservations) {
|
||||
for (const ep of (r.endpoints || [])) put(ep.lat, ep.lng, ep.name, ep.local_time)
|
||||
for (const ep of (r.endpoints || [])) put(ep.lat, ep.lng, ep.name)
|
||||
}
|
||||
return m
|
||||
}, [assignments, accommodations, reservations])
|
||||
|
||||
// The leg's departure time must be resolved WITHIN its day: the same located POI can
|
||||
// be revisited on another day at a different time (a supported pattern), so a
|
||||
// trip-wide coordinate index would return the wrong day's time. Scope to this day's
|
||||
// assignments (and the reservations that touch it) so a revisit keeps its own time.
|
||||
const originDepartureTime = (originKey: string, dayId: number): string | null => {
|
||||
for (const a of assignments[String(dayId)] ?? []) {
|
||||
if (a.place?.lat != null && a.place?.lng != null && `${a.place.lat},${a.place.lng}` === originKey) return a.place.place_time ?? null
|
||||
}
|
||||
for (const r of reservations) {
|
||||
if (r.day_id !== dayId && r.end_day_id !== dayId) continue
|
||||
for (const ep of (r.endpoints || [])) {
|
||||
if (ep.lat != null && ep.lng != null && `${ep.lat},${ep.lng}` === originKey) return ep.local_time ?? null
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
// Build the "public transport" prefill for a leg from its RouteSegment: MOTIS is
|
||||
// driven off the coordinates, the names + departure time come from the index above.
|
||||
const buildTransitLeg = (seg?: RouteSegment): { from: PickedPlace; to: PickedPlace; time: string | null } | null => {
|
||||
// driven off the coordinates, the names come from the index and the departure time
|
||||
// from the origin's own day. Accepts 'H:mm' or 'HH:mm[:ss]', normalised to 'HH:mm'.
|
||||
const buildTransitLeg = (seg: RouteSegment | undefined, dayId: number): { from: PickedPlace; to: PickedPlace; time: string | null } | null => {
|
||||
if (!seg?.from || !seg?.to) return null
|
||||
const pick = (c: [number, number]): PickedPlace => ({ name: transitPointIndex.get(`${c[0]},${c[1]}`)?.name || '', lat: c[0], lng: c[1] })
|
||||
const rawTime = transitPointIndex.get(`${seg.from[0]},${seg.from[1]}`)?.time
|
||||
return { from: pick(seg.from), to: pick(seg.to), time: rawTime && /^\d{2}:\d{2}/.test(rawTime) ? rawTime.slice(0, 5) : null }
|
||||
const pick = (c: [number, number]): PickedPlace => ({ name: transitNameIndex.get(`${c[0]},${c[1]}`) || '', lat: c[0], lng: c[1] })
|
||||
const rawTime = originDepartureTime(`${seg.from[0]},${seg.from[1]}`, dayId)
|
||||
const m = rawTime ? /^(\d{1,2}):(\d{2})/.exec(rawTime) : null
|
||||
return { from: pick(seg.from), to: pick(seg.to), time: m ? `${m[1].padStart(2, '0')}:${m[2]}` : null }
|
||||
}
|
||||
|
||||
// The extra connector-menu entry (#1281 follow-up): search public transit for this
|
||||
// leg instead of drawing a road route. Only when a handler is wired (day has dates).
|
||||
const transitLegMenuItem = (dayId: number, seg?: RouteSegment) => {
|
||||
if (!onPlanTransitLeg) return []
|
||||
const leg = buildTransitLeg(seg)
|
||||
const leg = buildTransitLeg(seg, dayId)
|
||||
if (!leg) return []
|
||||
return [{ label: t('transit.title'), icon: TramFront, onClick: () => onPlanTransitLeg({ dayId, from: leg.from, to: leg.to, time: leg.time }) }]
|
||||
}
|
||||
@@ -1340,7 +1358,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
|
||||
const openLegModeMenu = (e: React.MouseEvent, assignmentId: number, dayId: number, seg?: RouteSegment) => {
|
||||
ctxMenu.open(e, [
|
||||
...routeProfileOptions.map(o => ({ label: o.label, icon: modeIcon(o.key), onClick: () => setLegMode(assignmentId, dayId, o.key) })),
|
||||
...transitLegMenuItem(dayId, seg ?? routeLegs[dayId]?.[assignmentId]),
|
||||
...transitLegMenuItem(dayId, seg),
|
||||
{ divider: true },
|
||||
{ label: t('dayplan.transportMode.useDefault'), icon: RotateCcw, onClick: () => setLegMode(assignmentId, dayId, null) },
|
||||
])
|
||||
|
||||
@@ -684,7 +684,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null {
|
||||
</div>
|
||||
<div style={{ flex: 1, overflow: 'auto', paddingBottom: 'env(safe-area-inset-bottom, 0px)' }}>
|
||||
{mobileSidebarOpen === 'left'
|
||||
? <DayPlanSidebar tripId={tripId} trip={trip} days={days} places={places} categories={categories} assignments={assignments} selectedDayId={selectedDayId} selectedPlaceId={selectedPlaceId} selectedAssignmentId={selectedAssignmentId} onSelectDay={(id) => { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId) }} onReorder={handleReorder} onReorderDays={handleReorderDays} onAddDay={handleAddDay} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} onRouteCalculated={(r) => { if (r) { setRoute([r.coordinates]); setRouteInfo(r) } else { setRoute(null); setRouteInfo(null) } }} reservations={reservations} visibleConnectionIds={visibleConnections} onToggleConnection={toggleConnection} allConnectionsShown={allConnectionsShown} onToggleAllConnections={toggleAllConnections} onAddReservation={(dayId) => { setEditingReservation(null); tripActions.setSelectedDay(dayId); setShowReservationModal(true); setMobileSidebarOpen(null) }} onAddTransport={can('day_edit', trip) ? (dayId) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(false); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onOpenTransit={(r) => { setTransitJourney(r); setMobileSidebarOpen(null) }} onPlanTransit={can('day_edit', trip) && tripHasDates ? (dayId) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(true); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null) }} onRemoveAssignment={handleRemoveAssignment} onEditPlace={(place, assignmentId) => { setEditingPlace(place); setEditingAssignmentId(assignmentId || null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} accommodations={tripAccommodations} routeShown={routeShown} routeProfile={routeProfile} onToggleRoute={() => setRouteShown(v => !v)} onSetRouteProfile={setRouteProfile} onNavigateToFiles={() => { setMobileSidebarOpen(null); handleTabChange('dateien') }} onExpandedDaysChange={setExpandedDayIds} pushUndo={pushUndo} canUndo={canUndo} lastActionLabel={lastActionLabel} onUndo={handleUndo} onEditTransport={can('day_edit', trip) ? (reservation) => { setEditingTransport(reservation); setTransportModalDayId(reservation.day_id ?? null); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onEditReservation={can('reservation_edit', trip) ? (r) => { setEditingReservation(r); setShowReservationModal(true); setMobileSidebarOpen(null) } : undefined} initialScrollTop={mobilePlanScrollTopRef.current} onScrollTopChange={(top) => { mobilePlanScrollTopRef.current = top }} showRouteToolsWhenExpanded isMobile />
|
||||
? <DayPlanSidebar tripId={tripId} trip={trip} days={days} places={places} categories={categories} assignments={assignments} selectedDayId={selectedDayId} selectedPlaceId={selectedPlaceId} selectedAssignmentId={selectedAssignmentId} onSelectDay={(id) => { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId) }} onReorder={handleReorder} onReorderDays={handleReorderDays} onAddDay={handleAddDay} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} onRouteCalculated={(r) => { if (r) { setRoute([r.coordinates]); setRouteInfo(r) } else { setRoute(null); setRouteInfo(null) } }} reservations={reservations} visibleConnectionIds={visibleConnections} onToggleConnection={toggleConnection} allConnectionsShown={allConnectionsShown} onToggleAllConnections={toggleAllConnections} onAddReservation={(dayId) => { setEditingReservation(null); tripActions.setSelectedDay(dayId); setShowReservationModal(true); setMobileSidebarOpen(null) }} onAddTransport={can('day_edit', trip) ? (dayId) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(false); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onOpenTransit={(r) => { setTransitJourney(r); setMobileSidebarOpen(null) }} onPlanTransit={can('day_edit', trip) && tripHasDates ? (dayId) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(true); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onPlanTransitLeg={can('day_edit', trip) && tripHasDates ? ({ dayId, from, to, time }) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill({ from, to, time }); setTransportModalAutomated(true); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null) }} onRemoveAssignment={handleRemoveAssignment} onEditPlace={(place, assignmentId) => { setEditingPlace(place); setEditingAssignmentId(assignmentId || null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} accommodations={tripAccommodations} routeShown={routeShown} routeProfile={routeProfile} onToggleRoute={() => setRouteShown(v => !v)} onSetRouteProfile={setRouteProfile} onNavigateToFiles={() => { setMobileSidebarOpen(null); handleTabChange('dateien') }} onExpandedDaysChange={setExpandedDayIds} pushUndo={pushUndo} canUndo={canUndo} lastActionLabel={lastActionLabel} onUndo={handleUndo} onEditTransport={can('day_edit', trip) ? (reservation) => { setEditingTransport(reservation); setTransportModalDayId(reservation.day_id ?? null); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onEditReservation={can('reservation_edit', trip) ? (r) => { setEditingReservation(r); setShowReservationModal(true); setMobileSidebarOpen(null) } : undefined} initialScrollTop={mobilePlanScrollTopRef.current} onScrollTopChange={(top) => { mobilePlanScrollTopRef.current = top }} showRouteToolsWhenExpanded isMobile />
|
||||
: <PlacesSidebar tripId={tripId} places={places} categories={categories} assignments={assignments} selectedDayId={selectedDayId} selectedPlaceId={selectedPlaceId} onPlaceClick={(placeId) => { handlePlaceClick(placeId); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onAssignToDay={handleAssignToDay} onEditPlace={(place) => { openPlaceEditor(place); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} onBulkDeletePlaces={(ids) => setDeletePlaceIds(ids)} onBulkDeleteConfirm={(ids) => confirmDeletePlaces(ids)} onBulkChangeCategory={(ids, catId) => confirmChangeCategory(ids, catId)} days={days} isMobile pushUndo={pushUndo} initialScrollTop={mobilePlacesScrollTopRef.current} onScrollTopChange={(top) => { mobilePlacesScrollTopRef.current = top }} />
|
||||
}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user