diff --git a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts index 1c88767e3..92ea3ee91 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, stopDayIds } from '../../../../utils/dayMerge' +import { getDisplayTimeForDay, getMergedItems, getTransportForDay, hasCarrierEndpointOnDay, isCarrierTransport, stopDayIds, yieldTimedBookings } from '../../../../utils/dayMerge' import { dayCoMapsUrl, dayExportStops, dayGoogleMapsUrl, optimizeDayOrder, type DayCarrier } from '../../../../components/Planner/dayRoute' import type { PlannedTransitLeg, TransitLeg } from '../../../../components/Planner/transitLeg' import { useTransitLegPlanner } from '../../../../components/Planner/useTransitLegPlanner' @@ -208,10 +208,17 @@ export function useMPlanTimeline(planner: TripPlanner) { const newOrder = [...merged] newOrder.splice(idx, 1) newOrder.splice(toIndex, 0, item) - // Same guard as the desktop arrow buttons: a TIMED item may not jump past - // other timed items (untimed items move freely and keep their neighbours). + // A TIMED item may not jump past other timed items (untimed items move freely and + // keep their neighbours). Bookings it passes make room when they can, as on a desktop + // drop (#2983); a timed stop or note in the way, or a move that leaves the day as it + // was, is refused. if (itemHasTime(item, day.id) && breaksChronology(newOrder, day.id, getDisplayTimeForDay)) { - toast.info(t('dayplan.cannotBreakChronology')) + const yielded = yieldTimedBookings(newOrder, toIndex, day.id, getDisplayTimeForDay) + if (!yielded || yielded.every((it, i) => it === merged[i])) { + toast.info(t('dayplan.cannotBreakChronology')) + return + } + void applyMergedOrder(day.id, yielded) return } void applyMergedOrder(day.id, newOrder) diff --git a/client/src/utils/dayMerge.test.ts b/client/src/utils/dayMerge.test.ts index b62be0286..71c6307df 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, pinnedAway, stopDayIds, getPlaceBookings, getMergedItems, chronoKeyOf, nightRowsOf, rideSeatKey, seatTimedDrop, stayStartingOn, storedRideSlot, timedSlot, type MergedItem } from './dayMerge' +import { parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getTransportRouteEndpoints, getDisplayTimeForDay, getTransportForDay, getAssignmentReservations, pinnedAway, stopDayIds, getPlaceBookings, getMergedItems, chronoKeyOf, nightRowsOf, rideSeatKey, seatTimedDrop, yieldTimedBookings, stayStartingOn, storedRideSlot, timedSlot, type MergedItem } from './dayMerge' import { planMergedOrder, withTransportPositions } from './mergedOrder' describe('parseTimeToMinutes', () => { @@ -646,6 +646,52 @@ describe('seatTimedDrop', () => { const order = draw(setup()) expect(seatTimedDrop(order, 1, DAY)).toEqual(order) }) + + // #2983: a flight listed below the day's untimed stops held every slot among them. + const untimedDay = (rides: ReturnType[]): Day => ({ + stops: [stop(1, 'Hotel'), stop(2, 'Shrine'), stop(3, 'Crossing'), stop(4, 'Tower')].map((a, i) => ({ ...a, order_index: i })), + rides, + notes: [], + }) + + it('a ride dropped among untimed stops stays there, and an earlier booking below them makes room ahead of it', () => { + const day = untimedDay([ride(101, '06:30', 3.5), ride(102, '13:00', 3.6)]) + expect(draw(day).map(label)).toEqual(['Hotel', 'Shrine', 'Crossing', 'Tower', 'ride 06:30', 'ride 13:00']) + const { seated, drawn } = drop(day, isRide(102), isStop('Shrine')) + expect(drawn).toEqual(seated) + expect(drawn).toEqual(['Hotel', 'Shrine', 'ride 06:30', 'ride 13:00', 'Crossing', 'Tower']) + }) + + it('a later booking ahead of the drop moves behind it, and the stops keep their order', () => { + const day = untimedDay([ride(101, '09:00', -0.5), ride(102, '18:00', 1.5)]) + expect(draw(day).map(label)).toEqual(['ride 09:00', 'Hotel', 'Shrine', 'ride 18:00', 'Crossing', 'Tower']) + const { seated, drawn } = drop(day, isRide(101), isStop('Tower')) + expect(drawn).toEqual(seated) + expect(drawn).toEqual(['Hotel', 'Shrine', 'Crossing', 'Tower', 'ride 09:00', 'ride 18:00']) + }) + + it('a timed stop in the way still sends the ride back to its time, the bookings stay put', () => { + const day = untimedDay([ride(101, '06:30', 3.5), ride(102, '13:00', 3.6)]) + day.stops[2] = { ...day.stops[2], place: { name: 'Crossing', place_time: '12:00' } } + expect(draw(day).map(label)).toEqual(['Hotel', 'Shrine', 'ride 06:30', 'Crossing', 'Tower', 'ride 13:00']) + const { seated, drawn } = drop(day, isRide(102), isStop('Shrine')) + expect(drawn).toEqual(seated) + expect(drawn).toEqual(['Hotel', 'Shrine', 'ride 06:30', 'Crossing', 'ride 13:00', 'Tower']) + }) +}) + +describe('yieldTimedBookings', () => { + const DAY = 1 + const row = (type: MergedItem['type'], data: Record): MergedItem => ({ type, sortKey: 0, data }) + const place = (id: number) => row('place', { id, place: { place_time: null } }) + const booking = (id: number, time: string | null) => row('transport', { id, day_id: DAY, reservation_time: time }) + + it('leaves an untimed row, a stop and an order without a booking out of place to the caller', () => { + const order = [place(1), booking(2, null), booking(3, '2026-11-20T08:00')] + expect(yieldTimedBookings(order, 1, DAY)).toBeNull() + expect(yieldTimedBookings(order, 0, DAY)).toBeNull() + expect(yieldTimedBookings(order, 2, DAY)).toBeNull() + }) }) diff --git a/client/src/utils/dayMerge.ts b/client/src/utils/dayMerge.ts index 83a8036c4..b6d143b79 100644 --- a/client/src/utils/dayMerge.ts +++ b/client/src/utils/dayMerge.ts @@ -428,20 +428,65 @@ function zonedClockSlots( return slots } +/** Whether the list draws `order` as it stands. */ +function drawnAsIs(order: readonly MergedItem[], dayId: number, getDisplayTime: DisplayTimeOf): boolean { + return applyChronoOrder([...order], dayId, getDisplayTime).every((it, i) => it === order[i]) +} + +/** + * A day order with the row at `index` kept where it is, and the other timed bookings it + * puts out of time order moved next to it: the ones due earlier right ahead of it, the + * later ones right behind it, each group in the order it had. Stops and notes never move. + * Null when the row has no time, when no booking is out of order, or when the result is + * still not the order the list draws, which is when a timed stop or note stands against it. + * + * A booking nobody can drag otherwise blocks every slot on its far side: with a 06:30 + * flight listed below the day's untimed stops, a 13:00 transit dropped between two of + * them went back behind the flight on every drop (#2983). + */ +export function yieldTimedBookings( + order: readonly MergedItem[], + index: number, + dayId: number, + getDisplayTime: DisplayTimeOf = getDisplayTimeForDay +): MergedItem[] | null { + const moved = order[index] + if (!moved || moved.type === 'place') return null + const keyOf = chronoKeyOf(order, dayId, getDisplayTime) + const own = keyOf(moved) + if (own === null) return null + const ahead: MergedItem[] = [] + const behind: MergedItem[] = [] + order.forEach((it, i) => { + if (it === moved || it.type !== 'transport') return + const key = keyOf(it) + if (key === null) return + if (i > index && key < own) ahead.push(it) + else if (i < index && key > own) behind.push(it) + }) + if (!ahead.length && !behind.length) return null + const yielding = new Set([...ahead, ...behind]) + const kept = order.filter(it => !yielding.has(it)) + const at = kept.indexOf(moved) + const result = [...kept.slice(0, at), ...ahead, moved, ...behind, ...kept.slice(at + 1)] + return drawnAsIs(result, dayId, getDisplayTime) ? result : null +} + /** * A day order with a dragged booking or note at `index`, moved to the nearest slot the * list draws it in. A row with a time is drawn by it, and the untimed rows behind it * take that time too. Dropped against its time, a train went back to its time on the * next render and took every untimed stop behind the drop with it, and the next drag - * stored that order (#2631). So it lands as close to the drop as its time allows and - * nothing else moves. A row without a time, or one the list draws where it was - * dropped, is returned as it came. + * stored that order (#2631). So it stays where it was dropped when only other bookings + * stand against its time, and they make room (`yieldTimedBookings`); against a timed + * stop or note it lands as close to the drop as its time allows and nothing else moves. + * A row without a time, or one the list draws where it was dropped, is returned as it came. */ export function seatTimedDrop( order: readonly MergedItem[], index: number, dayId: number, - getDisplayTime: (r: MergedItem['data'], dayId: number) => string | null = getDisplayTimeForDay + getDisplayTime: DisplayTimeOf = getDisplayTimeForDay ): MergedItem[] { const moved = order[index] if (!moved || moved.type === 'place') return [...order] @@ -449,16 +494,18 @@ export function seatTimedDrop( ? parseTimeToMinutes(moved.data?.time) : parseTimeToMinutes(getDisplayTime(moved.data, dayId)) if (minutes === null) return [...order] + if (drawnAsIs(order, dayId, getDisplayTime)) return [...order] + const yielded = yieldTimedBookings(order, index, dayId, getDisplayTime) + if (yielded) return yielded const rest = order.filter((_, i) => i !== index) - const drawnAsIs = (at: number): MergedItem[] | null => { + const seatAt = (at: number): MergedItem[] | null => { const candidate = [...rest.slice(0, at), moved, ...rest.slice(at)] - const drawn = applyChronoOrder(candidate, dayId, getDisplayTime) - return drawn.every((it, i) => it === candidate[i]) ? candidate : null + return drawnAsIs(candidate, dayId, getDisplayTime) ? candidate : null } - for (let dist = 0; dist <= rest.length; dist++) { - for (const at of dist === 0 ? [index] : [index - dist, index + dist]) { + for (let dist = 1; dist <= rest.length; dist++) { + for (const at of [index - dist, index + dist]) { if (at < 0 || at > rest.length) continue - const seated = drawnAsIs(at) + const seated = seatAt(at) if (seated) return seated } } diff --git a/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts b/client/tests/unit/mobile/trip/useMPlanTimeline.test.ts index f71a29709..4e4b24599 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-052 +// FE-MOB-PLTL-001 to FE-MOB-PLTL-054 // 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. @@ -337,6 +337,41 @@ describe('useMPlanTimeline', () => { expect(planner.toast.info).not.toHaveBeenCalled() }) + // #2983: a 06:30 flight listed below the day's untimed stops blocked every slot of a + // 13:00 transit among them. The flight makes room ahead of the transit instead. + const FLIGHT_EARLY = buildReservation({ + id: 61, type: 'flight', title: 'Flight', day_id: 2, reservation_time: '2026-05-02T06:30', day_positions: { 2: 1.5 }, + }) + const TRANSIT_NOON = buildReservation({ + id: 62, type: 'transit', title: 'Transit', day_id: 2, reservation_time: '2026-05-02T13:00', day_positions: { 2: 1.6 }, + }) + + it('FE-MOB-PLTL-053: keeps a timed booking where it was moved and lets an earlier booking make room', async () => { + seedStore(useTripStore, { reservations: [FLIGHT_EARLY, TRANSIT_NOON] }) + const planner = makePlanner({ assignments: { '2': [A_MUSEUM, A_PARK] }, reservations: [FLIGHT_EARLY, TRANSIT_NOON] }) + const { result } = await renderTimeline(planner) + expect(result.current.merged.map(m => m.data.id)).toEqual([11, 12, 61, 62]) + + await act(async () => { result.current.moveRowTo(result.current.merged[3], 1) }) + + await waitFor(() => expect(planner.tripActions.updateReservationPositions).toHaveBeenCalledWith( + 1, [{ id: 61, day_plan_position: 1 / 3 }, { id: 62, day_plan_position: 2 / 3 }], 2, + )) + expect(planner.tripActions.reorderAssignments).toHaveBeenCalledWith(1, 2, [11, 12]) + expect(planner.toast.info).not.toHaveBeenCalled() + }) + + it('FE-MOB-PLTL-054: still refuses a move the earlier booking would only undo', async () => { + seedStore(useTripStore, { reservations: [FLIGHT_EARLY, TRANSIT_NOON] }) + const planner = makePlanner({ assignments: { '2': [A_MUSEUM, A_PARK] }, reservations: [FLIGHT_EARLY, TRANSIT_NOON] }) + const { result } = await renderTimeline(planner) + + await act(async () => { result.current.moveRow(result.current.merged[3], 'up') }) + + expect(planner.toast.info).toHaveBeenCalledWith('dayplan.cannotBreakChronology') + expect(planner.tripActions.updateReservationPositions).not.toHaveBeenCalled() + }) + it('FE-MOB-PLTL-021: persists note and transport positions alongside the reorder', async () => { const bus = buildReservation({ id: 51, type: 'bus', title: 'Bus 13A', day_id: 2, day_positions: { 2: 1.5 },