fix(planner): a timed booking dropped against its time goes back alone instead of taking the stops behind the drop along

The day list draws a row with a time by that time, and an untimed stop takes the
time of the timed row before it. A transit ride dropped behind a later stop was
stored there, drawn back at its time on the next render, and every untimed stop
behind the drop went with it; the next drag then stored that shuffle. A dropped
booking or timed note now lands in the slot nearest the drop the list draws it
in, so nothing else moves and what is stored is what is shown. Rides dropped
where their time allows stay on the drop, and the rule for untimed stops behind
a booking is unchanged.

The chronology check for a timed stop reads a booking's time for the day (the
arrival on the last day of a multi-day ride) instead of its departure, and the
three copies of the remove and insert step share one helper.

Refs #2631

client/scripts/size-baseline.json

@@ -12,7 +12,7 @@
   "src/components/PDF/TripPDF.tsx": 1097,
   "src/components/Planner/DayDetailPanel.test.tsx": 2333,
   "src/components/Planner/DayPlanSidebar.test.tsx": 5381,
-  "src/components/Planner/DayPlanSidebar.tsx": 3000,
+  "src/components/Planner/DayPlanSidebar.tsx": 2979,
   "src/components/Planner/PlaceFormModal.test.tsx": 2864,
   "src/components/Planner/PlaceFormModal.tsx": 1250,
   "src/components/Planner/PlaceInspector.tsx": 1308,

client/src/components/Planner/DayPlanSidebar.dragData.test.tsx

@@ -1,10 +1,19 @@
-// FE-PLANNER-DAYPLAN-236: the drag data a booking drop leaves behind. The mocks
-// are the ones DayPlanSidebar.test.tsx renders the sidebar with.
+// FE-PLANNER-DAYPLAN-236: the drag data a booking drop leaves behind, and
+// FE-PLANNER-DAYPLAN-237/238: where a timed booking dropped in the day lands. The
+// mocks are the ones DayPlanSidebar.test.tsx renders the sidebar with.
 import { http, HttpResponse } from 'msw';
-import { buildDay, buildReservation, buildTrip, buildUser } from '../../../tests/helpers/factories';
+import {
+  buildAssignment,
+  buildDay,
+  buildPlace,
+  buildReservation,
+  buildTrip,
+  buildUser,
+} from '../../../tests/helpers/factories';
 import { server } from '../../../tests/helpers/msw/server';
-import { fireEvent, render, screen } from '../../../tests/helpers/render';
+import { fireEvent, render, screen, waitFor } from '../../../tests/helpers/render';
 import { resetAllStores, seedStore } from '../../../tests/helpers/store';
+import { reservationsApi } from '../../api/client';
 import { clearExchangeRateCache } from '../../hooks/useExchangeRates';
 import { useAuthStore } from '../../store/authStore';
 import { useSettingsStore } from '../../store/settingsStore';
@@ -183,3 +192,61 @@ describe('DayPlanSidebar drag data', () => {
     expect(window.__dragData).toBeNull();
   });
 });
+
+// #2631: a booking with a time is drawn by it, and the untimed stops behind it take
+// that time. Dropped against its time it went back on the next render and took the
+// stops behind the drop with it, so the day reshuffled on every drag.
+describe('DayPlanSidebar timed booking drop', () => {
+  const renderDay = (busTime: string, busSlot: number) => {
+    const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' });
+    const stops = [
+      buildPlace({ id: 1, name: 'Breakfast', place_time: '08:00' }),
+      buildPlace({ id: 2, name: 'Museum', place_time: '14:00' }),
+      buildPlace({ id: 3, name: 'Old town' }),
+      buildPlace({ id: 4, name: 'Harbour' }),
+    ];
+    const assignments = {
+      '10': stops.map((place, i) => buildAssignment({ id: 11 + i, day_id: 10, order_index: i, place })),
+    };
+    const bus = buildReservation({
+      id: 500,
+      type: 'bus',
+      title: 'City bus',
+      day_id: 10,
+      reservation_time: `2025-06-01T${busTime}`,
+      day_plan_position: busSlot,
+      day_positions: { 10: busSlot },
+    });
+    seedStore(useTripStore, { reservations: [bus] });
+    const onReorder = vi.fn(async () => undefined);
+    render(
+      <DayPlanSidebar
+        {...makeDefaultProps({ days: [day], places: stops, assignments, reservations: [bus], onReorder })}
+      />
+    );
+    const row = (text: string) => screen.getByText(text).closest('[draggable="true"]') as HTMLElement;
+    fireEvent.dragStart(row('City bus'), {
+      dataTransfer: { setData: vi.fn(), effectAllowed: '', getData: vi.fn(() => '') },
+    });
+    fireEvent.drop(row('Harbour'), { dataTransfer: { getData: vi.fn(() => '') } });
+    return { onReorder };
+  };
+
+  it('FE-PLANNER-DAYPLAN-237: a booking dropped against its time stays at its time and moves no stop', async () => {
+    // The 12:00 bus dropped ahead of Harbour, behind the 14:00 museum.
+    const { onReorder } = renderDay('12:00', 0.5);
+    await waitFor(() =>
+      expect(reservationsApi.updatePositions).toHaveBeenCalledWith(1, [{ id: 500, day_plan_position: 0.5 }], 10)
+    );
+    expect(onReorder).toHaveBeenCalledWith(10, [11, 12, 13, 14]);
+    expect(useTripStore.getState().reservations[0].day_positions).toEqual({ 10: 0.5 });
+  });
+
+  it('FE-PLANNER-DAYPLAN-238: a booking dropped where its time allows lands on the drop', async () => {
+    const { onReorder } = renderDay('16:00', 1.5);
+    await waitFor(() =>
+      expect(reservationsApi.updatePositions).toHaveBeenCalledWith(1, [{ id: 500, day_plan_position: 2.5 }], 10)
+    );
+    expect(onReorder).toHaveBeenCalledWith(10, [11, 12, 13, 14]);
+  });
+});

client/src/components/Planner/DayPlanSidebar.tsx

@@ -46,7 +46,7 @@ import { getAccommodationAnchors, getDayBookendHotels, shouldDrawMorningLeg, sho
 import {
   TRANSPORT_TYPES, parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getDisplayTimeForDay, getTransportRouteEndpoints,
   getTransportForDay as _getTransportForDay, getMergedItems as _getMergedItems, isCarrierTransport, hasCarrierEndpointOnDay,
-  getAssignmentReservations, timedSlot, storedRideSlot,
+  getAssignmentReservations, timedSlot, storedRideSlot, seatTimedDrop,
   type MergedItem,
 } from '../../utils/dayMerge'
 import { withinDriveRange } from '@trek/shared/roadtrip'
@@ -54,7 +54,7 @@ import { formatDate, formatTime, formatMoneySum, splitReservationDateTime } from
 import { dayHeadingParts } from '../../utils/dayLabel'
 import { pendingStayIds } from '../../utils/pendingStays'
 import { dayWeatherAnchor } from '../../utils/dayWeather'
-import { metadataWithLegPositions, planMergedOrder, withTransportPositions } from '../../utils/mergedOrder'
+import { metadataWithLegPositions, moveRow, planMergedOrder, withTransportPositions } from '../../utils/mergedOrder'
 import { planCosts } from './planCosts'
 import { useDayNotes } from '../../hooks/useDayNotes'
 import { useExchangeRates } from '../../hooks/useExchangeRates'
@@ -939,16 +939,9 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
         const fromIdx = m.findIndex(i => i.type === fromType && i.data.id === fromId)
         const toIdx = m.findIndex(matchTo)
         if (fromIdx !== -1 && toIdx !== -1) {
-          const simulated = [...m]
-          const [moved] = simulated.splice(fromIdx, 1)
-          let insertIdx = simulated.findIndex(matchTo)
-          if (insertIdx === -1) insertIdx = simulated.length
-          if (insertAfter) insertIdx += 1
-          simulated.splice(insertIdx, 0, moved)
-
-          const timedInOrder = simulated
+          const timedInOrder = moveRow(m, fromIdx, matchTo, insertAfter).order
             .map(i => {
-              if (i.type === 'transport') return parseTimeToMinutes(i.data?.reservation_time)
+              if (i.type === 'transport') return parseTimeToMinutes(getDisplayTimeForDay(i.data, dayId))
               if (i.type === 'place') return parseTimeToMinutes(i.data?.place?.place_time)
               return null
             })
@@ -974,17 +967,9 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
       return
     }
 
-    const newOrder = [...m]
-    const [moved] = newOrder.splice(fromIdx, 1)
-    let adjustedTo = newOrder.findIndex(matchTo)
-    if (adjustedTo === -1) adjustedTo = newOrder.length
-    if (insertAfter) adjustedTo += 1
-    newOrder.splice(adjustedTo, 0, moved)
-
-    await applyMergedOrder(dayId, newOrder)
-    setDraggingId(null)
-    setDropTargetKey(null)
-    dragDataRef.current = null
+    const dropped = moveRow(m, fromIdx, matchTo, insertAfter)
+    await applyMergedOrder(dayId, seatTimedDrop(dropped.order, dropped.index, dayId, getDisplayTimeForDay)) // #2631
+    setDraggingId(null); setDropTargetKey(null); dragDataRef.current = null
   }
 
   const confirmTimeRemoval = async () => {
@@ -1011,14 +996,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
       const toIdx = m.findIndex(matchTo)
       if (fromIdx === -1 || toIdx === -1 || fromIdx === toIdx) return
 
-      const newOrder = [...m]
-      const [moved] = newOrder.splice(fromIdx, 1)
-      let adjustedTo = newOrder.findIndex(matchTo)
-      if (adjustedTo === -1) adjustedTo = newOrder.length
-      if (insertAfter) adjustedTo += 1
-      newOrder.splice(adjustedTo, 0, moved)
-
-      await applyMergedOrder(dayId, newOrder)
+      await applyMergedOrder(dayId, moveRow(m, fromIdx, matchTo, insertAfter).order)
     }
   }
 
@@ -1866,7 +1844,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
                           if (parseTimeToMinutes(placeTime) !== null) {
                             const timedInNewOrder = newOrder
                               .map(i => {
-                                if (i.type === 'transport') return parseTimeToMinutes(i.data?.reservation_time)
+                                if (i.type === 'transport') return parseTimeToMinutes(getDisplayTimeForDay(i.data, day.id))
                                 if (i.type === 'place') return parseTimeToMinutes(i.data?.place?.place_time)
                                 return null
                               })

client/src/utils/dayMerge.test.ts

@@ -1,5 +1,6 @@
 import { describe, it, expect } from 'vitest'
-import { parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getTransportRouteEndpoints, getDisplayTimeForDay, getTransportForDay, getAssignmentReservations, getPlaceBookings, getMergedItems, rideSeatKey, stayStartingOn, storedRideSlot, timedSlot } from './dayMerge'
+import { parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getTransportRouteEndpoints, getDisplayTimeForDay, getTransportForDay, getAssignmentReservations, getPlaceBookings, getMergedItems, rideSeatKey, seatTimedDrop, stayStartingOn, storedRideSlot, timedSlot, type MergedItem } from './dayMerge'
+import { planMergedOrder, withTransportPositions } from './mergedOrder'
 
 describe('parseTimeToMinutes', () => {
   it('parses HH:MM string', () => {
@@ -491,3 +492,105 @@ describe('timedSlot', () => {
     expect(timedSlot(leading, nights, '08:00')).toBe(1)
   })
 })
+
+// #2631: a day with timed and untimed stops between timed transit rides. Dropping a
+// ride against its time used to draw it back at its time with every untimed stop
+// behind the drop, and the next drag stored that shuffle.
+describe('seatTimedDrop', () => {
+  const DAY = 1
+  const stop = (id: number, name: string, time: string | null = null) =>
+    ({ id, order_index: 0, place: { name, place_time: time } })
+  const ride = (id: number, time: string, pos: number) =>
+    ({ id, type: 'transit', title: `ride ${time}`, day_id: DAY, reservation_time: `2026-11-20T${time}`, day_positions: { [DAY]: pos } })
+  const note = (id: number, time: string | null, sort_order: number) => ({ id, text: `note ${id}`, time, sort_order })
+
+  const setup = () => {
+    const stops = [
+      stop(1, 'Palacio', '07:30'), stop(2, 'Jardin', '10:30'), stop(3, 'Asamkirche', '14:00'), stop(4, 'StPeter', '14:30'),
+      stop(5, 'Sendlinger'), stop(6, 'Ayuntamiento'), stop(7, 'Isartor'), stop(8, 'Juliet'),
+    ].map((a, i) => ({ ...a, order_index: i }))
+    return { stops, rides: [ride(101, '07:01', -0.5), ride(102, '12:01', 1.5), ride(103, '16:55', 3.5)], notes: [] as ReturnType<typeof note>[] }
+  }
+  type Day = ReturnType<typeof setup>
+  const draw = (day: Day) => getMergedItems({ dayAssignments: day.stops, dayNotes: day.notes, dayTransports: day.rides, dayId: DAY })
+  const label = (it: MergedItem) => it.type === 'place' ? it.data.place.name : it.type === 'note' ? it.data.text : it.data.title
+
+  /** What the day list does with a drop: insert, seat, store, and draw again. */
+  const drop = (day: Day, from: (it: MergedItem) => boolean, to: (it: MergedItem) => boolean, after = true) => {
+    const m = draw(day)
+    const order = [...m]
+    const [moved] = order.splice(order.findIndex(from), 1)
+    const at = order.findIndex(to) + (after ? 1 : 0)
+    order.splice(at, 0, moved)
+    const seated = seatTimedDrop(order, at, DAY)
+    const plan = planMergedOrder(seated)
+    const stored: Day = {
+      stops: day.stops.map(a => ({ ...a, order_index: plan.assignmentIds.indexOf(a.id) })),
+      rides: withTransportPositions(day.rides, plan.transportUpdates, DAY),
+      notes: day.notes.map(n => ({ ...n, sort_order: plan.noteUpdates.find(u => u.id === n.id)?.sort_order ?? n.sort_order })),
+    }
+    return { seated: seated.map(label), drawn: draw(stored).map(label), stored }
+  }
+  const isRide = (id: number) => (it: MergedItem) => it.type === 'transport' && it.data.id === id
+  const isStop = (name: string) => (it: MergedItem) => it.type === 'place' && it.data.place.name === name
+  const stopsOf = (labels: string[]) => labels.filter(l => !l.startsWith('ride') && !l.startsWith('note'))
+
+  it('draws the day as the setup expects', () => {
+    expect(draw(setup()).map(label)).toEqual([
+      'ride 07:01', 'Palacio', 'Jardin', 'ride 12:01', 'Asamkirche', 'StPeter', 'ride 16:55', 'Sendlinger', 'Ayuntamiento', 'Isartor', 'Juliet',
+    ])
+  })
+
+  it('a ride dropped against its time goes back alone and the stops keep their order', () => {
+    const day = setup()
+    const before = stopsOf(draw(day).map(label))
+    const { seated, drawn } = drop(day, isRide(102), isStop('Sendlinger'))
+    expect(drawn).toEqual(seated)
+    expect(stopsOf(drawn)).toEqual(before)
+    expect(drawn.indexOf('ride 12:01')).toBe(drawn.indexOf('Asamkirche') - 1)
+  })
+
+  it('a later drag of another ride does not reshuffle the stops either', () => {
+    const first = drop(setup(), isRide(102), isStop('Sendlinger'))
+    const { seated, drawn } = drop(first.stored, isRide(101), isStop('Ayuntamiento'))
+    expect(drawn).toEqual(seated)
+    expect(stopsOf(drawn)).toEqual(['Palacio', 'Jardin', 'Asamkirche', 'StPeter', 'Sendlinger', 'Ayuntamiento', 'Isartor', 'Juliet'])
+    expect(drawn[0]).toBe('ride 07:01')
+  })
+
+  it('a ride dropped between untimed stops its time allows stays where it was dropped', () => {
+    const { seated, drawn } = drop(setup(), isRide(103), isStop('Isartor'))
+    expect(drawn).toEqual(seated)
+    expect(drawn.slice(-4)).toEqual(['Ayuntamiento', 'Isartor', 'ride 16:55', 'Juliet'])
+  })
+
+  it('leaves the untimed stops behind a ride where they are, as before', () => {
+    const day: Day = {
+      stops: [stop(1, 'Hotel'), stop(2, 'Museum'), stop(3, 'Dinner')].map((a, i) => ({ ...a, order_index: i })),
+      rides: [ride(101, '09:00', 0.5)],
+      notes: [],
+    }
+    expect(draw(day).map(label)).toEqual(['Hotel', 'ride 09:00', 'Museum', 'Dinner'])
+    const { seated, drawn } = drop(day, isRide(101), isStop('Museum'))
+    expect(drawn).toEqual(seated)
+    expect(drawn).toEqual(['Hotel', 'Museum', 'ride 09:00', 'Dinner'])
+  })
+
+  it('seats a timed note the same way and leaves an untimed one where it was dropped', () => {
+    const day = setup()
+    day.notes = [note(201, '12:30', 1.6), note(202, null, 6.5)]
+    const isNote = (id: number) => (it: MergedItem) => it.type === 'note' && it.data.id === id
+    const timed = drop(day, isNote(201), isStop('Isartor'))
+    expect(timed.drawn).toEqual(timed.seated)
+    expect(timed.drawn.indexOf('note 201')).toBe(timed.drawn.indexOf('Asamkirche') - 1)
+    const untimed = drop(day, isNote(202), isStop('Palacio'))
+    expect(untimed.drawn).toEqual(untimed.seated)
+    expect(untimed.drawn.indexOf('note 202')).toBe(untimed.drawn.indexOf('Palacio') + 1)
+  })
+
+  it('returns a dropped stop as it came', () => {
+    const order = draw(setup())
+    expect(seatTimedDrop(order, 1, DAY)).toEqual(order)
+  })
+})
+

client/src/utils/dayMerge.ts

@@ -309,6 +309,43 @@ function applyChronoOrder(
   })
 }
 
+/**
+ * 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.
+ */
+export function seatTimedDrop(
+  order: readonly MergedItem[],
+  index: number,
+  dayId: number,
+  getDisplayTime: (r: MergedItem['data'], dayId: number) => string | null = getDisplayTimeForDay
+): MergedItem[] {
+  const moved = order[index]
+  if (!moved || moved.type === 'place') return [...order]
+  const minutes = moved.type === 'note'
+    ? parseTimeToMinutes(moved.data?.time)
+    : parseTimeToMinutes(getDisplayTime(moved.data, dayId))
+  if (minutes === null) return [...order]
+  const rest = order.filter((_, i) => i !== index)
+  const drawnAsIs = (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
+  }
+  for (let dist = 0; dist <= rest.length; dist++) {
+    for (const at of dist === 0 ? [index] : [index - dist, index + dist]) {
+      if (at < 0 || at > rest.length) continue
+      const seated = drawnAsIs(at)
+      if (seated) return seated
+    }
+  }
+  return [...order]
+}
+
 /** A row of the day list as the seat of a ride reads it; a note is none (see rideSeatKey). */
 function seatItemsOf(row: MergedItem): SeatItem[] {
   if (row.type === 'place') {

client/src/utils/mergedOrder.test.ts

@@ -1,8 +1,8 @@
-// FE-UTIL-MERGEDORDER-001 to -007: the reorder writes the desktop day plan and the
+// FE-UTIL-MERGEDORDER-001 to -008: the reorder writes the desktop day plan and the
 // phone's plan timeline both derive from a day's new timeline order.
 import { describe, expect, it } from 'vitest';
 
-import { metadataWithLegPositions, planMergedOrder, withTransportPositions } from './mergedOrder';
+import { metadataWithLegPositions, moveRow, planMergedOrder, withTransportPositions } from './mergedOrder';
 
 const place = (id: number) => ({ type: 'place', data: { id } });
 const note = (id: number) => ({ type: 'note', data: { id } });
@@ -81,3 +81,13 @@ describe('metadataWithLegPositions', () => {
     expect(metadataWithLegPositions('', { 0: 1 }, 4)).toBeNull();
   });
 });
+
+describe('moveRow', () => {
+  it('FE-UTIL-MERGEDORDER-008: puts the row in front of the target, behind it with after, at the end without one', () => {
+    const rows = ['a', 'b', 'c', 'd'];
+    expect(moveRow(rows, 3, (r) => r === 'b', false)).toEqual({ order: ['a', 'd', 'b', 'c'], index: 1 });
+    expect(moveRow(rows, 0, (r) => r === 'c', true)).toEqual({ order: ['b', 'c', 'a', 'd'], index: 2 });
+    expect(moveRow(rows, 1, () => false, false)).toEqual({ order: ['a', 'c', 'd', 'b'], index: 3 });
+    expect(rows).toEqual(['a', 'b', 'c', 'd']);
+  });
+});

client/src/utils/mergedOrder.ts

@@ -23,6 +23,25 @@ export interface MergedOrderPlan {
   legPosUpdates: Record<number, Record<number, number>>;
 }
 
+/**
+ * The day order with the row at `from` taken out and put back in front of the first
+ * row `isTarget` picks, or behind it with `after`; at the end when no row is picked.
+ * `index` is where the row landed.
+ */
+export function moveRow<T>(
+  order: readonly T[],
+  from: number,
+  isTarget: (row: T) => boolean,
+  after = false
+): { order: T[]; index: number } {
+  const moved = [...order];
+  const [row] = moved.splice(from, 1);
+  const target = moved.findIndex(isTarget);
+  const index = (target === -1 ? moved.length : target) + (after ? 1 : 0);
+  moved.splice(index, 0, row);
+  return { order: moved, index };
+}
+
 export function planMergedOrder(newOrder: readonly MergedOrderItem[]): MergedOrderPlan {
   const plan: MergedOrderPlan = { assignmentIds: [], noteUpdates: [], transportUpdates: [], legPosUpdates: {} };
   let placeCount = 0;