diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index dc9f6c591..264ec2476 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -3344,6 +3344,64 @@ describe('DayPlanSidebar', () => { expect(updateReservation).toHaveBeenCalledWith(1, 501, { day_id: 11, end_day_id: 11 }) }) + // #2455. The list draws a timed stop by its start, the road trip drives the stored + // order. A stop with a start moved onto another day has to be stored where the list + // will draw it, or the road trip visits it somewhere else and, behind a later start, + // reaches it late. + describe('a timed stop moved onto another day is stored where the list shows it (#2455)', () => { + const setup = (extra: Record = {}) => { + const moveAssignment = vi.fn(async () => undefined) + stubTripActions({ moveAssignment }) + const days = [ + buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }), + buildDay({ id: 11, date: '2025-06-02', title: 'Day 2' }), + ] + const moved = buildPlace({ id: 1, name: 'Travemuende Strand', place_time: '10:00' }) + const early = buildPlace({ id: 2, name: 'Luebeck Dom', place_time: '09:00' }) + const middle = buildPlace({ id: 3, name: 'Wismar Hafen', place_time: '11:00' }) + const late = buildPlace({ id: 4, name: 'Rostock Markt', place_time: '14:00' }) + const assignments = { + '10': [buildAssignment({ id: 21, day_id: 10, order_index: 0, place: moved })], + '11': [ + buildAssignment({ id: 31, day_id: 11, order_index: 0, place: early }), + buildAssignment({ id: 32, day_id: 11, order_index: 1, place: middle }), + buildAssignment({ id: 33, day_id: 11, order_index: 2, place: late }), + ], + } + render() + return moveAssignment + } + + it('FE-PLANNER-DAYPLAN-215: dropped on the day body, it lands between the stops its start falls between', () => { + const moveAssignment = setup() + fireEvent.dragStart(dragRow(screen.getByText('Travemuende Strand')), { dataTransfer: emptyDataTransfer }) + fireEvent.drop(document.querySelectorAll('[style*="padding-top: 6px"]')[1], { dataTransfer: { getData: vi.fn(() => '') } }) + // 10:00 sits between 09:00 (index 0) and 11:00, so index 1, not the end of the day. + expect(moveAssignment).toHaveBeenCalledWith(1, 21, 10, 11, 1) + }) + + it('FE-PLANNER-DAYPLAN-216: dropped on a later stop, it still lands where its start puts it', () => { + const moveAssignment = setup() + fireEvent.dragStart(dragRow(screen.getByText('Travemuende Strand')), { dataTransfer: emptyDataTransfer }) + fireEvent.drop(dragRow(screen.getByText('Rostock Markt')), { dataTransfer: { getData: vi.fn(() => '') } }) + expect(moveAssignment).toHaveBeenCalledWith(1, 21, 10, 11, 1) + }) + + it('FE-PLANNER-DAYPLAN-217: with the planner wired in, the move goes through it so the day keeps its vias', async () => { + // Landing in the middle of a day shifts the legs behind it, which only the planner + // can correct: it holds the vias. The list still decides where the stop goes. + const onMoveToDay = vi.fn(async () => undefined) + const pushUndo = vi.fn() + const moveAssignment = setup({ onMoveToDay, pushUndo }) + fireEvent.dragStart(dragRow(screen.getByText('Travemuende Strand')), { dataTransfer: emptyDataTransfer }) + fireEvent.drop(dayHeader('Day 2'), { dataTransfer: { getData: vi.fn(() => '') } }) + expect(onMoveToDay).toHaveBeenCalledWith(21, 10, 11, 1) + expect(moveAssignment).not.toHaveBeenCalled() + // Still offered back once the planner has made the move. + await waitFor(() => expect(pushUndo).toHaveBeenCalledTimes(1)) + }) + }) + it('FE-PLANNER-DAYPLAN-149: the note context menu edits and asks before deleting', async () => { const user = userEvent.setup() const note = buildDayNote({ id: 70, day_id: 10, text: 'A note' }) diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 749b8b561..01bee0669 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -35,7 +35,7 @@ import { isDayInAccommodationRange, getAccommodationAnchors, getDayBookendHotels import { TRANSPORT_TYPES, parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getDisplayTimeForDay, getTransportRouteEndpoints, getTransportForDay as _getTransportForDay, getMergedItems as _getMergedItems, isCarrierTransport, hasCarrierEndpointOnDay, - getAssignmentReservations, + getAssignmentReservations, timedSlot, type MergedItem, } from '../../utils/dayMerge' import { withinDriveRange } from '../../utils/geo' @@ -81,6 +81,12 @@ interface DayPlanSidebarProps { /** The day route is computed by the planner page itself; kept for the existing call sites. */ onRouteCalculated: (route: RouteResult | null) => void onAssignToDay: (placeId: number, dayId: number, position?: number) => void + /** + * Moves a stop over from another day, at a row index of that day or at its end, and + * keeps the vias drawn on that day on their legs. Without it the list moves the stop + * in the store and leaves the vias as they are. + */ + onMoveToDay?: (assignmentId: number, fromDayId: number, toDayId: number, position?: number) => Promise onRemoveAssignment: (dayId: number, assignmentId: number) => void onEditPlace: (place: Place, assignmentId?: number) => void onDeletePlace: (placeId: number) => void @@ -141,7 +147,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { selectedDayId, selectedPlaceId, selectedAssignmentId, onSelectDay, onPlaceClick, onDayDetail, accommodations = [], onReorder, onReorderDays, onAddDay, - onAssignToDay, onRemoveAssignment, onEditPlace, onDeletePlace, + onAssignToDay, onMoveToDay, onRemoveAssignment, onEditPlace, onDeletePlace, reservations = [], visibleConnectionIds = [], onToggleConnection, @@ -510,6 +516,19 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { return idx >= 0 ? idx : stored.length } + // Moving a stop over from another day. One with a start is drawn by it wherever it + // is stored, and the road trip drives the stored order, so it is stored where the + // list will draw it rather than at the drop. One without a start makes the same call + // it always has, dropped where it was dropped or at the end of the day. + const moveToDay = (assignmentId: number, fromDayId: number, toDayId: number, dropAt?: number) => { + const moving = (assignments[String(fromDayId)] || []).find(a => a.id === assignmentId) + const slot = timedSlot(assignments[String(toDayId)] || [], accommodations, moving?.place?.place_time, dropAt) ?? dropAt + if (onMoveToDay) return onMoveToDay(assignmentId, fromDayId, toDayId, slot) + return slot === undefined + ? tripActions.moveAssignment(tripId, assignmentId, fromDayId, toDayId) + : tripActions.moveAssignment(tripId, assignmentId, fromDayId, toDayId, slot) + } + // The stop a drop on a note lands ahead of: the next place below the note. const placeBelowNote = (dayId: number, noteId: number): Assignment | undefined => { const tm = getMergedItems(dayId) @@ -1061,7 +1080,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const srcAssignment = (useTripStore.getState().assignments[String(fromDayId)] || []).find(a => a.id === Number(assignmentId)) const capturedFromDayId = fromDayId const capturedOrderIndex = srcAssignment?.order_index ?? 0 - tripActions.moveAssignment(tripId, Number(assignmentId), fromDayId, dayId) + moveToDay(Number(assignmentId), fromDayId, dayId) .then(() => { pushUndo?.(t('undo.moveDay'), async () => { await tripActions.moveAssignment(tripId, Number(assignmentId), dayId, capturedFromDayId, capturedOrderIndex) @@ -1216,6 +1235,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { getMergedItems, storedPositionBefore, placeBelowNote, + moveToDay, mergedItemsMap, applyMergedOrder, handleMergedDrop, @@ -1417,6 +1437,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP getMergedItems, storedPositionBefore, placeBelowNote, + moveToDay, mergedItemsMap, applyMergedOrder, handleMergedDrop, @@ -1876,7 +1897,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP } else if (fromReservationId) { handleMergedDrop(day.id, 'transport', Number(fromReservationId), 'transport', transportId, isAfter, toLegIndex) } else if (assignmentId && fromDayId !== day.id) { - tripActions.moveAssignment(tripId, Number(assignmentId), fromDayId, day.id).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) + moveToDay(Number(assignmentId), fromDayId, day.id).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) } else if (assignmentId) { handleMergedDrop(day.id, 'place', Number(assignmentId), 'transport', transportId, isAfter, toLegIndex) } else if (noteId && fromDayId !== day.id) { @@ -1899,7 +1920,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP setDropTargetKey(null); window.__dragData = null; return } if (assignmentId && fromDayId !== day.id) { - tripActions.moveAssignment(tripId, Number(assignmentId), fromDayId, day.id).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) + moveToDay(Number(assignmentId), fromDayId, day.id).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) setDraggingId(null); setDropTargetKey(null); dragDataRef.current = null; return } if (noteId && fromDayId !== day.id) { @@ -2059,7 +2080,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP } else if (fromReservationId) { handleMergedDrop(day.id, 'transport', Number(fromReservationId), 'place', assignment.id) } else if (fromAssignmentId && fromDayId !== day.id) { - tripActions.moveAssignment(tripId, Number(fromAssignmentId), fromDayId, day.id, storedPositionBefore(day.id, assignment)).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) + moveToDay(Number(fromAssignmentId), fromDayId, day.id, storedPositionBefore(day.id, assignment)).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) setDraggingId(null); setDropTargetKey(null); dragDataRef.current = null } else if (fromAssignmentId) { handleMergedDrop(day.id, 'place', Number(fromAssignmentId), 'place', assignment.id) @@ -2483,7 +2504,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP } else if (fromReservationId) { handleMergedDrop(day.id, 'transport', Number(fromReservationId), 'transport', res.id, insertAfter, res.__leg?.index ?? null) } else if (fromAssignmentId && fromDayId !== day.id) { - tripActions.moveAssignment(tripId, Number(fromAssignmentId), fromDayId, day.id).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) + moveToDay(Number(fromAssignmentId), fromDayId, day.id).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) } else if (fromAssignmentId) { handleMergedDrop(day.id, 'place', Number(fromAssignmentId), 'transport', res.id, insertAfter, res.__leg?.index ?? null) } else if (noteId && fromDayId !== day.id) { @@ -2667,7 +2688,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP } else if (fromNoteId && fromNoteId !== String(note.id)) { handleMergedDrop(day.id, 'note', Number(fromNoteId), 'note', note.id) } else if (fromAssignmentId && fromDayId !== day.id) { - tripActions.moveAssignment(tripId, Number(fromAssignmentId), fromDayId, day.id, storedPositionBefore(day.id, placeBelowNote(day.id, note.id))).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) + moveToDay(Number(fromAssignmentId), fromDayId, day.id, storedPositionBefore(day.id, placeBelowNote(day.id, note.id))).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) setDraggingId(null); setDropTargetKey(null) } else if (fromAssignmentId) { handleMergedDrop(day.id, 'place', Number(fromAssignmentId), 'note', note.id) @@ -2778,7 +2799,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP } if (!assignmentId && !noteId && !fromReservationId) { dragDataRef.current = null; window.__dragData = null; return } if (assignmentId && fromDayId !== day.id) { - tripActions.moveAssignment(tripId, Number(assignmentId), fromDayId, day.id).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) + moveToDay(Number(assignmentId), fromDayId, day.id).catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) setDraggingId(null); setDropTargetKey(null); dragDataRef.current = null; return } if (noteId && fromDayId !== day.id) { diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.dayOrder.test.ts b/client/src/components/Roadtrip/useRoadtripRoutes.dayOrder.test.ts new file mode 100644 index 000000000..98499e6a7 --- /dev/null +++ b/client/src/components/Roadtrip/useRoadtripRoutes.dayOrder.test.ts @@ -0,0 +1,172 @@ +import { renderHook, waitFor } from '@testing-library/react' +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { Assignment, AssignmentsMap, Day } from '../../types' + +// #2455: "the road trip plan should align with the days ordering". The road trip drives a +// day in its stored order (order_index); the Days list draws the same stored day by time. +// Where the two agree the rail must read exactly like the list, and where a day is stored +// against its own times the rail must at least keep every stop on its own card. + +const { calculateRouteWithLegs, RoutingRefusedError } = vi.hoisted(() => { + class RoutingRefusedError extends Error { + constructor(readonly status: number, readonly retryAfterMs: number | null) { + super('refused') + this.name = 'RoutingRefusedError' + } + get isRateLimit(): boolean { return this.status === 429 || this.status === 503 } + } + return { calculateRouteWithLegs: vi.fn(), RoutingRefusedError } +}) +vi.mock('../Map/RouteCalculator', () => ({ calculateRouteWithLegs, RoutingRefusedError })) + +import { useRoadtripRoutes } from './useRoadtripRoutes' +import { getMergedItems } from '../../utils/dayMerge' + +interface Visit { + id: number + name: string + at: [number, number] + /** The visit's start, as the day list shows it. */ + time?: string | null +} + +const day = (id: number, number: number): Day => + ({ id, day_number: number, date: null, title: null }) as unknown as Day + +/** + * A stored visit shaped the way the server hands it over: `place.place_time` is the + * visit's own start folded over the place's (COALESCE in AssignmentsService), which is + * the field the Days list sorts by. + */ +const visit = (v: Visit, orderIndex: number, dayId: number): Assignment => + ({ + id: v.id, + day_id: dayId, + place_id: v.id * 10, + order_index: orderIndex, + assignment_time: v.time ?? null, + assignment_end_time: null, + leg_transport_mode: null, + incoming_leg_transport_mode: null, + place: { + id: v.id * 10, + name: v.name, + lat: v.at[0], + lng: v.at[1], + place_time: v.time ?? null, + end_time: null, + duration_minutes: 30, + stop_type: null, + }, + }) as unknown as Assignment + +/** A day as stored: `order_index` is the position in the array. */ +const stored = (dayId: number, visits: Visit[]): AssignmentsMap => + ({ [String(dayId)]: visits.map((v, i) => visit(v, i, dayId)) }) as unknown as AssignmentsMap + +/** The places of one day in the order the Days list draws them (DayPlanSidebar, MPlanTimeline). */ +const daysListOrder = (assignments: AssignmentsMap, dayId: number): string[] => + getMergedItems({ + dayAssignments: [...(assignments[String(dayId)] ?? [])].sort((a, b) => a.order_index - b.order_index), + dayNotes: [], + dayTransports: [], + dayId, + }) + .filter(item => item.type === 'place') + .map(item => (item.data as Assignment).place!.name) + +type Card = { dayId: number; stops: { ownerDayId: number; name: string; automaticNight?: unknown }[] } + +/** What one card of the rail draws, by name. */ +const cardOf = (days: Card[], dayId: number): string[] => + days.find(d => d.dayId === dayId)?.stops.filter(s => !s.automaticNight).map(s => s.name) ?? [] + +/** The cards that draw a stop stored on `dayId`. */ +const cardsHolding = (days: Card[], dayId: number): number[] => + days.filter(d => d.stops.some(s => !s.automaticNight && s.ownerDayId === dayId)).map(d => d.dayId) + +beforeEach(() => { + calculateRouteWithLegs.mockReset() + // Twenty minutes and 20 km between any two waypoints, one leg per pair, the way OSRM answers. + calculateRouteWithLegs.mockImplementation(async (waypoints: { lat: number; lng: number }[]) => ({ + coordinates: waypoints.map(w => [w.lat, w.lng] as [number, number]), + distance: 20000 * (waypoints.length - 1), + duration: 1200 * (waypoints.length - 1), + legs: waypoints.slice(1).map(() => ({ distance: 20000, duration: 1200, text: '20 km' })), + })) +}) + +describe('the road trip reads a day the way the Days list does (#2455)', () => { + it('FE-ROADTRIP-ORDER-001: a day arranged by hand drives in the order the list shows', async () => { + const days = [day(1, 1)] + // Handed over out of array order on purpose: order_index decides, not the array. + const assignments = { + '1': [ + visit({ id: 3, name: 'Schwerin', at: [53.63, 11.41] }, 2, 1), + visit({ id: 1, name: 'Lübeck', at: [53.87, 10.69] }, 0, 1), + visit({ id: 2, name: 'Wismar', at: [53.89, 11.46], time: '11:00' }, 1, 1), + ], + } as unknown as AssignmentsMap + + const { result } = renderHook(() => useRoadtripRoutes(7, days, assignments)) + await waitFor(() => expect(result.current.loading).toBe(false)) + + expect(daysListOrder(assignments, 1)).toEqual(['Lübeck', 'Wismar', 'Schwerin']) + expect(cardOf(result.current.days, 1)).toEqual(daysListOrder(assignments, 1)) + }) + + it('FE-ROADTRIP-ORDER-002: a stop stored behind later starts stays on its own card, flagged late', async () => { + // What a Days drop onto a day header used to leave behind, and what a trip moved + // that way still holds: a 10:00 stop stored after a 14:00 one. The schedule reaches + // it late on day one; the rail used to take that for a night and open day two with it. + const days = [day(1, 1), day(2, 2)] + const assignments = { + ...stored(1, [ + { id: 1, name: 'Lübeck', at: [53.87, 10.69], time: '09:00' }, + { id: 2, name: 'Wismar', at: [53.89, 11.46], time: '11:00' }, + { id: 3, name: 'Rostock', at: [54.09, 12.10], time: '14:00' }, + { id: 4, name: 'Travemünde', at: [53.96, 10.87], time: '10:00' }, + ]), + ...stored(2, [ + { id: 5, name: 'Stralsund', at: [54.31, 13.09] }, + { id: 6, name: 'Greifswald', at: [54.09, 13.38] }, + ]), + } as AssignmentsMap + + const { result } = renderHook(() => useRoadtripRoutes(7, days, assignments)) + await waitFor(() => expect(result.current.loading).toBe(false)) + + expect(cardOf(result.current.days, 1)).toEqual(['Lübeck', 'Wismar', 'Rostock', 'Travemünde']) + expect(cardsHolding(result.current.days, 1)).toEqual([1]) + expect(cardOf(result.current.days, 2)).toEqual(['Stralsund', 'Greifswald']) + const dayOne = result.current.days.find(d => d.dayId === 1)! + expect(dayOne.schedule.warnings).toContainEqual(expect.objectContaining({ index: 3, code: 'late' })) + expect(result.current.days.find(d => d.dayId === 2)!.spills).toEqual([]) + }) + + it('FE-ROADTRIP-ORDER-003: a day stored against its times is not torn across two cards', async () => { + // However it came about (a cross-day move, a start set on the place through the MCP + // tools, a trip planned before the time sort), day one keeps its card: it was demoted + // to a one-stop placeholder while its other stops opened day two. + const days = [day(1, 1), day(2, 2)] + const assignments = { + ...stored(1, [ + { id: 1, name: 'Lübeck', at: [53.87, 10.69], time: '14:00' }, + { id: 2, name: 'Wismar', at: [53.89, 11.46], time: '10:00' }, + { id: 3, name: 'Rostock', at: [54.09, 12.10], time: '16:00' }, + ]), + ...stored(2, [ + { id: 4, name: 'Stralsund', at: [54.31, 13.09] }, + { id: 5, name: 'Greifswald', at: [54.09, 13.38] }, + ]), + } as AssignmentsMap + + const { result } = renderHook(() => useRoadtripRoutes(7, days, assignments)) + await waitFor(() => expect(result.current.loading).toBe(false)) + + expect(cardOf(result.current.days, 1)).toEqual(['Lübeck', 'Wismar', 'Rostock']) + expect(cardsHolding(result.current.days, 1)).toEqual([1]) + expect(cardOf(result.current.days, 2)).toEqual(['Stralsund', 'Greifswald']) + expect(result.current.quietDays.map(d => d.dayId)).toEqual([]) + }) +}) diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index 35a8b7de0..f4e6a1b61 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -303,7 +303,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null { route, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay, handleSelectDay, handlePlaceClick, handleMarkerClick, handleMapClick, handleMapContextMenu, handlePoiClick, handleSavePlace, openPlaceEditor, handleDeletePlace, confirmDeletePlace, confirmDeletePlaces, confirmChangeCategory, - handleAssignToDay, handleRemoveAssignment, handleReorder, handleReorderDays, handleAddDay, handleUpdateDayTitle, + handleAssignToDay, handleMoveToDay, handleRemoveAssignment, handleReorder, handleReorderDays, handleAddDay, handleUpdateDayTitle, handleSaveReservation, handleSaveTransport, handleDeleteReservation, selectedPlace, dayOrderMap, dayPlaces, mapTileUrl, fontStyle, splashDone, @@ -657,6 +657,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null { onAddDay={handleAddDay} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} + onMoveToDay={handleMoveToDay} onRouteCalculated={(r) => { if (r) { setRoute([r.coordinates]); setRouteInfo(r) } else { setRoute(null); setRouteInfo(null) } }} reservations={reservations} visibleConnectionIds={visibleConnections} @@ -946,7 +947,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null {
{mobileSidebarOpen === 'left' - ? { 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); setPlaceFormDayId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onCreatePlaceForDay={can('place_edit', trip) ? (dayId) => { setEditingPlace(null); setPlaceFormDayId(dayId); setShowPlaceForm(true); setMobileSidebarOpen(null) } : undefined} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null) }} onRemoveAssignment={handleRemoveAssignment} onEditPlace={(place, assignmentId) => { setEditingPlace(place); setEditingAssignmentId(assignmentId || null); setPlaceFormDayId(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 /> + ? { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId) }} onReorder={handleReorder} onReorderDays={handleReorderDays} onAddDay={handleAddDay} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} onMoveToDay={handleMoveToDay} 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); setPlaceFormDayId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onCreatePlaceForDay={can('place_edit', trip) ? (dayId) => { setEditingPlace(null); setPlaceFormDayId(dayId); setShowPlaceForm(true); setMobileSidebarOpen(null) } : undefined} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null) }} onRemoveAssignment={handleRemoveAssignment} onEditPlace={(place, assignmentId) => { setEditingPlace(place); setEditingAssignmentId(assignmentId || null); setPlaceFormDayId(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 /> : handleSelectDay(null)} selectedPlaceId={selectedPlaceId} onPlaceClick={(placeId) => { handlePlaceClick(placeId); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setPlaceFormDayId(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 }} /> }
diff --git a/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx b/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx index 9f624a9da..f815d46c6 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx +++ b/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx @@ -1,5 +1,5 @@ import { roadtripPreferencesRepo } from '../../repo/roadtripPreferencesRepo' -// FE-TP-ROAD-001 to FE-TP-ROAD-112 +// FE-TP-ROAD-001 to FE-TP-ROAD-115 import React from 'react' import { renderHook, act, waitFor } from '@testing-library/react' import { TranslationProvider } from '../../i18n/TranslationContext' @@ -2265,6 +2265,60 @@ describe('useTripPlanner road trip: corrections measured on the stored day', () }) }) +describe('useTripPlanner road trip: a stop moved over under Days', () => { + const twoDays = () => { + seedTrip({ + days: [buildDay({ id: 5, day_number: 1 }), buildDay({ id: 6, day_number: 2 })], + assignments: { + '5': [stopAt(11, 5, 0), stopAt(13, 5, 1)], + '6': [stopAt(21, 6, 0), stopAt(22, 6, 1), stopAt(23, 6, 2)], + }, + }) + rt.corridor.day = { dayId: 6, dayNumber: 2 } + // One via on each leg of the day the stop moves onto. + rt.vias.byDay = { 6: [via(1, 6, 0), via(2, 6, 1)] } + } + + it('FE-TP-ROAD-113: landing in the middle of a day moves the vias behind it along', async () => { + // A stop with a start is stored among the stops its hour falls between, which is + // no longer the end of the day. Left alone, the via drawn for the second leg would + // shape the first leg out of the new stop. + twoDays() + const { result } = await renderRoadtrip() + + await act(async () => { await result.current.handleMoveToDay(13, 5, 6, 1) }) + + expect(actions.moveAssignment).toHaveBeenCalledWith(42, 13, 5, 6, 1) + expect(rt.vias.reanchor).toHaveBeenCalledWith(6, { vias: [{ id: 2, after_order_index: 2 }], remove: [] }) + const moved = actions.moveAssignment.mock.invocationCallOrder[0] ?? 0 + const corrected = rt.vias.reanchor.mock.invocationCallOrder[0] ?? 0 + expect(moved).toBeLessThan(corrected) + }) + + it('FE-TP-ROAD-114: landing at the end of a day moves no via', async () => { + twoDays() + const { result } = await renderRoadtrip() + + await act(async () => { await result.current.handleMoveToDay(13, 5, 6) }) + + expect(actions.moveAssignment).toHaveBeenCalledWith(42, 13, 5, 6, undefined) + expect(rt.vias.reanchor).not.toHaveBeenCalled() + }) + + it('FE-TP-ROAD-115: a move that fails is handed back and corrects nothing', async () => { + // The list reports it and leaves the undo out, so the planner must not swallow it. + twoDays() + actions.moveAssignment.mockRejectedValue(new Error('gone')) + const { result } = await renderRoadtrip() + + await act(async () => { + await expect(result.current.handleMoveToDay(13, 5, 6, 1)).rejects.toThrow('gone') + }) + + expect(rt.vias.reanchor).not.toHaveBeenCalled() + }) +}) + describe('useTripPlanner road trip: a booked night in the popup', () => { const hotel = () => buildPlace({ id: 102, name: 'Hotel Fjord', stop_type: 'hotel', lat: 60.39, lng: 5.32 }) diff --git a/client/src/pages/tripPlanner/useTripPlanner.test.tsx b/client/src/pages/tripPlanner/useTripPlanner.test.tsx index c6428278b..ba28fe201 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.test.tsx +++ b/client/src/pages/tripPlanner/useTripPlanner.test.tsx @@ -1400,6 +1400,30 @@ describe('useTripPlanner — day plan CRUD', () => { expect(toasts.some(t => t.message === 'day is full')).toBe(true) }) + it('FE-TP-HOOK-121: a place with a start of its own is stored where the day list draws it', async () => { + // The list sorts the day by start and the road trip drives the stored order, so + // 10:00 goes between 09:00 and 11:00 whether it was dropped at the end or on top. + const place = buildPlace({ id: 1, place_time: '10:00' }) + const early = buildPlace({ id: 2, place_time: '09:00' }) + const late = buildPlace({ id: 3, place_time: '11:00' }) + seedTrip({ + places: [place, early, late], + assignments: { + '7': [ + buildAssignment({ id: 20, day_id: 7, place: early, order_index: 0 }), + buildAssignment({ id: 21, day_id: 7, place: late, order_index: 1 }), + ], + }, + }) + + const { result } = await renderPlanner() + await act(async () => { await result.current.handleAssignToDay(1, 7) }) + await act(async () => { await result.current.handleAssignToDay(1, 7, 0) }) + + expect(actions.assignPlaceToDay).toHaveBeenNthCalledWith(1, 42, 7, 1, 1) + expect(actions.assignPlaceToDay).toHaveBeenNthCalledWith(2, 42, 7, 1, 1) + }) + it('FE-TP-HOOK-066: removing an assignment can be undone back to its old position', async () => { const place = buildPlace({ id: 1, lat: 1, lng: 2 }) seedTrip({ diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index bd72ec974..8ffdda4bf 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -14,7 +14,7 @@ import { resolvePluginIcon } from '../../components/shared/PluginIcon' import { useTranslation, translateApiError } from '../../i18n' import { addonsApi, accommodationsApi, authApi, tripsApi, assignmentsApi, healthApi, airtrailApi, mapsApi, placesApi } from '../../api/client' import { getDayOrder } from '../../utils/dayOrder' -import { TRANSPORT_TYPES } from '../../utils/dayMerge' +import { TRANSPORT_TYPES, timedSlot } from '../../utils/dayMerge' import { isOvernightCategory } from '../../components/Roadtrip/stopKinds' import { parsedItemToDraft, isTransportItem, isUnplaceableItem, type BookingReviewDraft } from '../../components/Planner/parsedItemToDraft' import type { BookingImportPreviewItem } from '@trek/shared' @@ -1008,6 +1008,24 @@ export function useTripPlanner() { } }, [roadtripRoutes.days]) + /** + * How a day's vias move when a place at `at` lands on it at row `position`, or at its + * end without one. Null when none do: appending moves nothing, and a place without + * coordinates is never a stop. + * + * Worked out before the stop lands, the same way the road-trip popup does it: once the + * list has shifted there is no record of which leg each via was drawn for. The + * predicate decides which side of the new stop a via falls on when it is dropped into + * the middle of a leg. + */ + const viasAfterInsert = useCallback((dayId: number, position: number | undefined, at: { lat?: number | null; lng?: number | null } | undefined) => { + const stopsBefore = roadtripStopsOf(dayId) + // The position is a row index in the day list, the anchors count stops. + const insertAt = position === undefined ? stopsBefore.length : roadtripIndexOf(dayId, position) + if (insertAt >= stopsBefore.length || typeof at?.lat !== 'number' || typeof at?.lng !== 'number') return null + return reanchorAfterInsert(roadtripVias.byDay[dayId] ?? [], insertAt, viaLiesBefore(dayId, { lat: at.lat, lng: at.lng })) + }, [roadtripStopsOf, roadtripIndexOf, roadtripVias.byDay, viaLiesBefore]) + /** * Saves a corridor hit as a stop: the place itself, then its position in the day. * @@ -2234,24 +2252,13 @@ export function useTripPlanner() { const handleAssignToDay = useCallback(async (placeId: number, dayId?: number, position?: number) => { const target = dayId || selectedDayId if (!target) { toast.error(t('trip.toast.selectDay')); return } - // Worked out before the stop lands, the same way the road-trip popup does it: - // once the list has shifted there is no record of which leg each via was - // drawn for. Appending to the end moves nothing, so only a real insert needs - // the correction — and the predicate decides which side of the new stop a - // via falls on when it is dropped into the middle of a leg. - const stopsBefore = roadtripStopsOf(target) - // The position is a row index in the day list, the anchors count stops. - const insertAt = position === undefined ? stopsBefore.length : roadtripIndexOf(target, position) const place = places.find(p => p.id === placeId) - const plan = insertAt >= stopsBefore.length || typeof place?.lat !== 'number' || typeof place?.lng !== 'number' - ? null - : reanchorAfterInsert( - roadtripVias.byDay[target] ?? [], - insertAt, - viaLiesBefore(target, { lat: place.lat, lng: place.lng }), - ) + // A place with a start of its own is drawn by it, so it is stored there too, the + // way a stop moved over from another day is. Without one it goes where it was put. + const slot = timedSlot(storedAssignments[String(target)] ?? [], tripAccommodations, place?.place_time, position) ?? position + const plan = viasAfterInsert(target, slot, place) try { - const assignment = await tripActions.assignPlaceToDay(tripId, target, placeId, position) + const assignment = await tripActions.assignPlaceToDay(tripId, target, placeId, slot) toast.success(t('trip.toast.assignedToDay')) if (plan) await roadtripVias.reanchor(target, plan) updateRouteForDay(target) @@ -2263,7 +2270,24 @@ export function useTripPlanner() { }) } } catch (err: unknown) { toast.error(err instanceof Error ? err.message : t('common.unknownError')) } - }, [selectedDayId, tripId, toast, updateRouteForDay, pushUndo, t, places, roadtripVias, roadtripStopsOf, roadtripIndexOf, viaLiesBefore]) + }, [selectedDayId, tripId, toast, updateRouteForDay, pushUndo, t, places, storedAssignments, tripAccommodations, roadtripVias, viasAfterInsert]) + + /** + * Moves a stop from the day list onto another day, at a row of that day or at its + * end without one. + * + * It can land in the middle of a day whose road has been drawn, on the row it was + * dropped on or among the stops its start falls between. The vias of that day count + * stops, so every one behind the new stop would shape the leg before the one it was + * drawn on. They are moved the way a place added to the day moves them. Rejects when + * a write fails, so the list can say so and leave its undo out. + */ + const handleMoveToDay = useCallback(async (assignmentId: number, fromDayId: number, toDayId: number, position?: number) => { + const place = (storedAssignments[String(fromDayId)] ?? []).find(a => a.id === assignmentId)?.place + const plan = viasAfterInsert(toDayId, position, place) + await tripActions.moveAssignment(tripId, assignmentId, fromDayId, toDayId, position) + if (plan) await roadtripVias.reanchor(toDayId, plan) + }, [tripId, tripActions, storedAssignments, roadtripVias, viasAfterInsert]) const handleRemoveAssignment = useCallback(async (dayId: number, assignmentId: number) => { const state = useTripStore.getState() @@ -2671,7 +2695,7 @@ export function useTripPlanner() { route, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay, handleSelectDay, handlePlaceClick, handleMarkerClick, handleMapClick, handleMapContextMenu, openAddPlaceFromPoi, handlePoiClick, handleSavePlace, openPlaceEditor, handleDeletePlace, confirmDeletePlace, confirmDeletePlaces, confirmChangeCategory, - handleAssignToDay, handleRemoveAssignment, handleReorder, handleReorderDays, handleAddDay, handleUpdateDayTitle, + handleAssignToDay, handleMoveToDay, handleRemoveAssignment, handleReorder, handleReorderDays, handleAddDay, handleUpdateDayTitle, handleSaveReservation, handleSaveTransport, handleDeleteReservation, selectedPlace, dayOrderMap, dayPlaces, mapTileUrl, fontStyle, splashDone, diff --git a/client/src/utils/dayMerge.test.ts b/client/src/utils/dayMerge.test.ts index 935bb2c57..6f3e0c3f4 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, getMergedItems } from './dayMerge' +import { parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getTransportRouteEndpoints, getDisplayTimeForDay, getTransportForDay, getAssignmentReservations, getMergedItems, timedSlot } from './dayMerge' describe('parseTimeToMinutes', () => { it('parses HH:MM string', () => { @@ -303,3 +303,52 @@ describe('getMergedItems', () => { expect(result.map(i => i.data.id)).toEqual([1, 3, 10, 2]) }) }) + +// A stop with a start is drawn by it, and the road trip drives the stored order, so a +// stop joining a day from elsewhere is stored where the list will draw it. +describe('timedSlot', () => { + const visit = (order_index: number, place_time: string | null, accommodation_id: number | null = null) => + ({ order_index, accommodation_id, place: { place_time } }) + const day = [visit(0, '09:00'), visit(1, '11:00'), visit(2, '14:00')] + + it('leaves a stop without a start to the drop', () => { + expect(timedSlot(day, [], null, 1)).toBeNull() + expect(timedSlot(day, [], undefined)).toBeNull() + expect(timedSlot(day, [], 'soon')).toBeNull() + }) + + it('puts a stop between the stored stops its start falls between, wherever it was dropped', () => { + expect(timedSlot(day, [], '10:00')).toBe(1) + expect(timedSlot(day, [], '10:00', 2)).toBe(1) + expect(timedSlot(day, [], '10:00', 0)).toBe(1) + expect(timedSlot(day, [], '08:00')).toBe(0) + expect(timedSlot(day, [], '15:00', 0)).toBe(3) + }) + + it('keeps the drop among the stops without a time behind the same start', () => { + const loose = [visit(0, '09:00'), visit(1, null), visit(2, '11:00')] + expect(timedSlot(loose, [], '10:00')).toBe(2) + expect(timedSlot(loose, [], '10:00', 1)).toBe(1) + }) + + it('reads the day by order_index, not by the order the rows come in', () => { + const shuffled = [visit(2, '14:00'), visit(0, '09:00'), visit(1, '11:00')] + expect(timedSlot(shuffled, [], '12:00')).toBe(2) + expect(timedSlot([], [], '12:00')).toBe(0) + }) + + it('stores it right behind the stop the list draws it after on a day stored against its hours', () => { + // Stored 11:00 then 09:00: the list draws 09:00, 10:00, 11:00, so 10:00 goes behind 09:00. + expect(timedSlot([visit(0, '11:00'), visit(1, '09:00')], [], '10:00')).toBe(2) + }) + + it('counts the booked night the list hides, timed by its check-in', () => { + const nights = [{ id: 7, check_in: '15:00' }, { id: 8, check_in: null }] + const withCheckIn = [visit(0, '09:00'), visit(1, null, 7), visit(2, '16:00')] + expect(timedSlot(withCheckIn, nights, '12:00')).toBe(1) + expect(timedSlot(withCheckIn, nights, '15:30')).toBe(2) + // Without a check-in the night leads the day and stays in front. + const leading = [visit(0, null, 8), visit(1, '09:00')] + expect(timedSlot(leading, nights, '08:00')).toBe(1) + }) +}) diff --git a/client/src/utils/dayMerge.ts b/client/src/utils/dayMerge.ts index 70395498c..128e0c74b 100644 --- a/client/src/utils/dayMerge.ts +++ b/client/src/utils/dayMerge.ts @@ -356,3 +356,49 @@ export function getMergedItems(opts: { return applyChronoOrder(result.sort((a, b) => a.sortKey - b.sortKey), dayId, getDisplayTime) } + +/** A stored day row, as far as `timedSlot` reads it. */ +interface StoredVisit { + order_index: number + accommodation_id?: number | null + place?: { place_time?: string | null } | null +} + +/** + * Where a stop joining a day is stored when it carries a start of its own, as an index + * into the day's stored rows sorted by order_index. Null when it has none: that stop + * goes where it was dropped, as it always has. + * + * The list draws a day by time (`applyChronoOrder`) and the road trip drives it in the + * order it is stored. Put at the end of the day, or wherever it was dropped, a stop with + * a start sat in one place on the list and in another on the road, and behind a later + * start the road trip reached it late. So it is stored right behind the row it will be + * drawn after. Among stops without a time it keeps the spot it was dropped on; where the + * drop and the start disagree, the start wins. + * + * Read over every stored row, the ones the list hides included, because those are what + * the index counts. A booked night is timed by its check-in, the way the server's time + * sort reads it, so a stop pinned before the check-in lands ahead of the hotel and one + * pinned after it behind. A night without one counts as a stop without a time, so a + * night that leads its day keeps leading it. + */ +export function timedSlot( + day: readonly StoredVisit[], + nights: readonly { id: number; check_in?: string | null }[], + start: string | null | undefined, + dropAt?: number | null, +): number | null { + const minutes = parseTimeToMinutes(start) + if (minutes === null) return null + const stored = [...day].sort((a, b) => a.order_index - b.order_index) + const at = dropAt == null ? stored.length : Math.min(Math.max(dropAt, 0), stored.length) + const joining: StoredVisit = { order_index: -1 } + const startOf = (row: StoredVisit): number | null => { + if (row === joining) return minutes + const checkIn = row.accommodation_id == null ? null : nights.find(n => n.id === row.accommodation_id)?.check_in + return parseTimeToMinutes(row.place?.place_time ?? checkIn) + } + const drawn = chronoOrder([...stored.slice(0, at), joining, ...stored.slice(at)], startOf) + const before = drawn[drawn.indexOf(joining) - 1] + return before ? stored.indexOf(before) + 1 : 0 +} diff --git a/shared/src/roadtrip/nightSpill.spec.ts b/shared/src/roadtrip/nightSpill.spec.ts index 17c3ed238..df7c9b0f5 100644 --- a/shared/src/roadtrip/nightSpill.spec.ts +++ b/shared/src/roadtrip/nightSpill.spec.ts @@ -357,6 +357,132 @@ describe('spillChains', () => { expect(chains[1]!.spills).toEqual([]); }); + // A pinned stop the drive reaches late reads EARLIER on the clock than the stop before + // it, because the schedule keeps the traveller's hour and flags the lateness. That is + // not a night: the schedule itself files the stop on day 0. Counted as one, the stop + // and every stop behind it left for the top of the next day's card, which reads as the + // road trip shuffling the day. + it('FE-NIGHTSPILL-022: a pinned stop reached late behind a stop without an hour stays on its day', () => { + // 09:00 + 1 h, a three hour drive to Celle (13:00), three more to a 12:00 in Hannover. + const plan = [ + day(1, 1, [ + stop({ assignmentId: 1, name: 'Hamburg', time: '09:00', dwellMinutes: 60 }), + stop({ assignmentId: 2, name: 'Celle' }), + stop({ assignmentId: 3, name: 'Hannover', time: '12:00' }), + ]), + day(2, 2, [stop({ assignmentId: 4, name: 'Goslar' })]), + ]; + + const chains = spillChains(plan, [], everyLeg(180)); + + expect(chains[0]!.stops.map((s) => s.name)).toEqual(['Hamburg', 'Celle', 'Hannover']); + expect(chains[0]!.schedule.warnings).toContainEqual({ index: 2, code: 'late', minutes: 240 }); + expect(chains[1]!.stops.map((s) => s.name)).toEqual(['Goslar']); + expect(chains[1]!.spills).toEqual([]); + }); + + it('FE-NIGHTSPILL-023: a day stored against its hours keeps every stop on its own card', () => { + // The order a stop dragged onto the day header used to be stored in: at the end, + // behind stops with later hours. 10:00 after 14:00 is a stop reached late, not tomorrow. + const plan = [ + day(1, 1, [ + stop({ assignmentId: 1, name: 'Lübeck', time: '14:00', dwellMinutes: 60 }), + stop({ assignmentId: 2, name: 'Wismar', time: '10:00' }), + stop({ assignmentId: 3, name: 'Rostock', time: '16:00' }), + ]), + day(2, 2, [stop({ assignmentId: 4, name: 'Stralsund' })]), + ]; + + const chains = spillChains(plan, [], everyLeg(30)); + + expect(chains[0]!.stops.map((s) => s.name)).toEqual(['Lübeck', 'Wismar', 'Rostock']); + expect(chains[0]!.schedule.warnings).toContainEqual({ index: 1, code: 'late', minutes: 330 }); + expect(chains[1]!.stops.map((s) => s.name)).toEqual(['Stralsund']); + expect(chains[1]!.spills).toEqual([]); + }); + + it('FE-NIGHTSPILL-024: the stop behind one reached late is timed on from its pinned hour', () => { + // The schedule carries on from Wismar's 10:00, so Kühlungsborn comes out at 10:30: + // earlier than Lübeck's 14:00, later than the hour the drive just went on from. + const plan = [ + day(1, 1, [ + stop({ assignmentId: 1, name: 'Lübeck', time: '14:00', dwellMinutes: 60 }), + stop({ assignmentId: 2, name: 'Wismar', time: '10:00' }), + stop({ assignmentId: 3, name: 'Kühlungsborn' }), + ]), + day(2, 2, [stop({ assignmentId: 4, name: 'Stralsund' })]), + ]; + + const chains = spillChains(plan, [], everyLeg(30)); + + expect(chains[0]!.stops.map((s) => s.name)).toEqual(['Lübeck', 'Wismar', 'Kühlungsborn']); + expect(chains[0]!.schedule.entries[2]!.arrival).toBe('10:30'); + expect(chains[1]!.stops.map((s) => s.name)).toEqual(['Stralsund']); + expect(chains[1]!.spills).toEqual([]); + }); + + it('FE-NIGHTSPILL-025: a stop reached late after midnight still takes the stops behind it along', () => { + // 20:00 + 1 h, four hours to a 00:30 in Berlin: late, and tomorrow. The leg out of + // Berlin never routed, so Potsdam has no clock of its own and goes where Berlin goes. + const plan = [ + day(1, 1, [ + stop({ assignmentId: 1, name: 'Hamburg', time: '20:00', dwellMinutes: 60 }), + stop({ assignmentId: 2, name: 'Berlin', time: '00:30' }), + stop({ assignmentId: 3, name: 'Potsdam' }), + ]), + day(2, 2, [stop({ assignmentId: 4, name: 'Dresden', time: '12:00' })]), + ]; + const legFor = (from: RoadtripStop) => (from.name === 'Berlin' ? undefined : everyLeg(240)()); + + const chains = spillChains(plan, [], legFor); + + expect(chains[0]!.stops.map((s) => s.name)).toEqual(['Hamburg']); + expect(chains[1]!.stops.map((s) => s.name)).toEqual(['Berlin', 'Potsdam', 'Dresden']); + expect(chains[1]!.schedule.warnings).toContainEqual({ index: 0, code: 'late', minutes: 30 }); + }); + + it('FE-NIGHTSPILL-026: the clock going back past a late hour is still a night', () => { + // FE-NIGHTSPILL-006 with a night at Wittenberg: 23:00, reached late, eight hours on + // and fifty minutes to Leipzig at 07:50. That is later than Cobbelsdorf's 07:00 but + // earlier than the 23:00 it was timed on from, so it is tomorrow morning. + const plan = [ + day(1, 1, [ + stop({ assignmentId: 1, name: 'Cobbelsdorf', time: '07:00', dwellMinutes: 45 }), + stop({ assignmentId: 2, name: 'Wittenberg', time: '23:00', dwellMinutes: 480 }), + stop({ assignmentId: 3, name: 'Leipzig' }), + ]), + day(2, 2, [stop({ assignmentId: 4, name: 'Dresden', time: '12:00' })]), + ]; + + const chains = spillChains(plan, [], everyLeg(50)); + + expect(chains[0]!.stops.map((s) => s.name)).toEqual(['Cobbelsdorf', 'Wittenberg']); + expect(chains[1]!.stops.map((s) => s.name)).toEqual(['Leipzig', 'Dresden']); + expect(chains[1]!.schedule.entries[0]!.arrival).toBe('07:50'); + }); + + it('FE-NIGHTSPILL-027: a stop without a clock follows a night drive out of a late hour', () => { + // 06:00 + 12 h, three hours to a 20:00 in Kassel: late. An hour there, ten hours + // through the night to Munich at 07:00, and Garmisch behind it never routed. + const plan = [ + day(1, 1, [ + stop({ assignmentId: 1, name: 'Hamburg', time: '06:00', dwellMinutes: 720 }), + stop({ assignmentId: 2, name: 'Kassel', time: '20:00', dwellMinutes: 60 }), + stop({ assignmentId: 3, name: 'München' }), + stop({ assignmentId: 4, name: 'Garmisch' }), + ]), + day(2, 2, [stop({ assignmentId: 5, name: 'Innsbruck' })]), + ]; + const legMinutes: Record = { Hamburg: 180, Kassel: 600 }; + const legFor = (from: RoadtripStop) => (legMinutes[from.name] ? everyLeg(legMinutes[from.name]!)() : undefined); + + const chains = spillChains(plan, [], legFor); + + expect(chains[0]!.stops.map((s) => s.name)).toEqual(['Hamburg', 'Kassel']); + expect(chains[0]!.schedule.warnings).toContainEqual({ index: 1, code: 'late', minutes: 60 }); + expect(chains[1]!.stops.map((s) => s.name)).toEqual(['München', 'Garmisch', 'Innsbruck']); + }); + describe('a stay that runs past midnight carries the clock into the next day', () => { // Standing somewhere for twenty-four hours is not over when the date changes. The // day after used to begin at nothing, so a trip whose second day had no pinned time diff --git a/shared/src/roadtrip/nightSpill.ts b/shared/src/roadtrip/nightSpill.ts index b1f4053a8..31a9767e9 100644 --- a/shared/src/roadtrip/nightSpill.ts +++ b/shared/src/roadtrip/nightSpill.ts @@ -91,12 +91,12 @@ export function spillChains(plan: PlanDay[], quietDays: QuietDay[], legFor: LegL // Plus the drive from where the stay ends to where this day starts. The carried // minute is a DEPARTURE, and arriving at the same instant would mean the road // between them takes no time at all. - const arriving = carriedInto?.dayNumber === d.dayNumber && d.stops[0] - ? legFor(carriedInto.from, d.stops[0])?.seg.duration - : undefined; - const notBefore = carriedInto?.dayNumber === d.dayNumber - ? carriedInto.minute + Math.round((arriving ?? 0) / 60) - : null; + const arriving = + carriedInto?.dayNumber === d.dayNumber && d.stops[0] + ? legFor(carriedInto.from, d.stops[0])?.seg.duration + : undefined; + const notBefore = + carriedInto?.dayNumber === d.dayNumber ? carriedInto.minute + Math.round((arriving ?? 0) / 60) : null; const routed = d.stops.slice(0, -1).map((s, i) => legFor(s, d.stops[i + 1]!)); const legs = routed.map((l) => l?.seg); const schedule = computeSchedule( @@ -107,7 +107,11 @@ export function spillChains(plan: PlanDay[], quietDays: QuietDay[], legFor: LegL // Where this day's last clock lands, carried to whichever day that turns out to be. const ends = schedule.endsAt ?? null; if (ends !== null && ends > 1440) { - carriedInto = { dayNumber: d.dayNumber + Math.floor(ends / 1440), minute: ends % 1440, from: d.stops[d.stops.length - 1]! }; + carriedInto = { + dayNumber: d.dayNumber + Math.floor(ends / 1440), + minute: ends % 1440, + from: d.stops[d.stops.length - 1]!, + }; } else if (carriedInto && carriedInto.dayNumber <= d.dayNumber) { // Spent: this is the day it was pointing at. carriedInto = null; @@ -122,8 +126,18 @@ export function spillChains(plan: PlanDay[], quietDays: QuietDay[], legFor: LegL d.stops.forEach((stop, i) => { const entry = schedule.entries[i]!; const clock = parseClock(entry?.arrival); + const marks = schedule.warnings.filter((w) => w.index === i); if (clock !== null) { - if (previous !== null && clock < previous) day += 1; + if (marks.some((w) => w.code === 'late')) { + // A pinned stop the drive reaches late shows the hour the traveller set, not + // the minute they get there, so it can read earlier than the stop before it + // without a night in between. Counted as one, it took the stop and every stop + // behind it to the top of the next day's card. The schedule has already filed + // it on its day, so that is the day it counts for. + day = Math.max(day, entry.dayOffset - baseOffset); + } else if (previous !== null && clock < previous) day += 1; + // Always the reading just made, the late hour included: the stops after it are + // timed on from that hour, so a clock that goes back past it is a real night. previous = clock; } // How many days on from where this one's clock STARTS, not the schedule's absolute @@ -136,7 +150,6 @@ export function spillChains(plan: PlanDay[], quietDays: QuietDay[], legFor: LegL // ran through the night: twenty-four hours on puts you in tomorrow at the same // reading on the clock, never an earlier one. const offset = entry?.arrival != null ? Math.max(entry.dayOffset - baseOffset, day) : day; - const marks = schedule.warnings.filter((w) => w.index === i); const reachable = offset > 0 && numbers.has(d.dayNumber + offset); const target = reachable ? d.dayNumber + offset : d.dayNumber;