diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 0541d3c9b..9f995c8dc 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -52,6 +52,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 { planCosts } from './planCosts' import { useDayNotes } from '../../hooks/useDayNotes' import { useExchangeRates } from '../../hooks/useExchangeRates' @@ -913,39 +914,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { // Places get sequential integer positions (0, 1, 2, ...) // Non-place items between place N-1 and place N get fractional positions - const assignmentIds: number[] = [] - const noteUpdates: { id: number; sort_order: number }[] = [] - const transportUpdates: { id: number; day_plan_position: number }[] = [] - // Multi-leg flight legs share a reservation id, so their positions can't live in - // the single per-booking slot — collect them per leg, keyed reservationId → legIndex → pos. - const legPosUpdates: Record> = {} - - let placeCount = 0 - let i = 0 - while (i < newOrder.length) { - if (newOrder[i].type === 'place') { - assignmentIds.push(newOrder[i].data.id) - placeCount++ - i++ - } else { - // Collect consecutive non-place items - const group: { type: string; data: any }[] = [] - while (i < newOrder.length && newOrder[i].type !== 'place') { - group.push(newOrder[i]) - i++ - } - // Fractional positions between (placeCount-1) and placeCount - const base = placeCount > 0 ? placeCount - 1 : -1 - group.forEach((g, idx) => { - const pos = base + (idx + 1) / (group.length + 1) - if (g.type === 'note') noteUpdates.push({ id: g.data.id, sort_order: pos }) - else if (g.type === 'transport') { - if (g.data.__leg) ((legPosUpdates[g.data.id] ??= {})[g.data.__leg.index] = pos) - else transportUpdates.push({ id: g.data.id, day_plan_position: pos }) - } - }) - } - } + const { assignmentIds, noteUpdates, transportUpdates, legPosUpdates } = planMergedOrder(newOrder) try { // Update transport positions in store FIRST so the useEffect triggered by @@ -955,14 +924,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const r = useTripStore.getState().reservations.find(x => x.id === tu.id) if (r) pendingRollback.push({ id: r.id, day_plan_position: r.day_plan_position, day_positions: r.day_positions }) } - useTripStore.setState(state => ({ - reservations: state.reservations.map(r => { - const tu = transportUpdates.find(u => u.id === r.id) - if (!tu) return r - const day_positions = { ...(r.day_positions || {}), [dayId]: tu.day_plan_position } - return { ...r, day_plan_position: tu.day_plan_position, day_positions } - }) - })) + useTripStore.setState(state => ({ reservations: withTransportPositions(state.reservations, transportUpdates, dayId) })) setTransportPosVersion(v => v + 1) } // Per-leg positions of multi-leg flights live in metadata.legs[i].day_positions @@ -973,20 +935,14 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const rid = Number(ridStr) const r = useTripStore.getState().reservations.find(x => x.id === rid) if (!r) continue - let parsed: any = {} - try { parsed = typeof r.metadata === 'string' ? JSON.parse(r.metadata || '{}') : (r.metadata || {}) } catch { parsed = {} } - if (!Array.isArray(parsed.legs)) continue - const legs = parsed.legs.map((leg: any, i: number) => { - const pos = legPosUpdates[rid][i] - return pos == null ? leg : { ...leg, day_positions: { ...(leg.day_positions || {}), [dayId]: pos } } - }) + const newMeta = metadataWithLegPositions(r.metadata, legPosUpdates[rid], dayId) + if (!newMeta) continue // Send metadata as an OBJECT (like the form does) — passing a JSON string // here double-encodes it on the server, which wipes metadata.legs on read // and collapses the flight back to a single span. - const newMeta = { ...parsed, legs } pendingRollback.push({ id: rid, metadata: r.metadata }) - useTripStore.setState(state => ({ reservations: state.reservations.map(x => (x.id === rid ? { ...x, metadata: newMeta } : x)) })) - await tripActions.updateReservation(tripId, rid, { metadata: newMeta }) + useTripStore.setState(state => ({ reservations: state.reservations.map(x => (x.id === rid ? { ...x, metadata: newMeta as never } : x)) })) + await tripActions.updateReservation(tripId, rid, { metadata: newMeta as unknown as string }) // Stored, so a later step failing must not put the old legs back. dropRollback(rid) } diff --git a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts index 9ca690280..fc11691db 100644 --- a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts +++ b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts @@ -5,6 +5,7 @@ import { assignmentsApi, reservationsApi, weatherApi } from '../../../../api/cli import { useRouteModeOptions } from '../../../../components/Planner/routeModes' import { getDayBookendHotels } from '../../../../utils/dayOrder' import { dayWeatherAnchor } from '../../../../utils/dayWeather' +import { metadataWithLegPositions, planMergedOrder, withTransportPositions } from '../../../../utils/mergedOrder' import { getDisplayTimeForDay, getMergedItems, getTransportForDay, hasCarrierEndpointOnDay, isCarrierTransport } from '../../../../utils/dayMerge' import { dayCoMapsUrl, dayExportStops, dayGoogleMapsUrl, optimizeDayOrder, type DayCarrier } from '../../../../components/Planner/dayRoute' import { buildTransitLeg, buildTransitNameIndex, type TransitLeg } from '../../../../components/Planner/transitLeg' @@ -140,63 +141,21 @@ export function useMPlanTimeline(planner: TripPlanner) { // ── Reorder via buttons (#1432) — port of the desktop applyMergedOrder ── const applyMergedOrder = useCallback(async (dayId: number, newOrder: MergedItem[]) => { const prevAssignmentIds = dayAssignments.map(a => a.id) - const assignmentIds: number[] = [] - const noteUpdates: { id: number; sort_order: number }[] = [] - const transportUpdates: { id: number; day_plan_position: number }[] = [] - const legPosUpdates: Record> = {} - - let placeCount = 0 - let i = 0 - while (i < newOrder.length) { - if (newOrder[i].type === 'place') { - assignmentIds.push((newOrder[i].data as Assignment).id) - placeCount++ - i++ - continue - } - const group: MergedItem[] = [] - while (i < newOrder.length && newOrder[i].type !== 'place') { group.push(newOrder[i]); i++ } - const base = placeCount > 0 ? placeCount - 1 : -1 - group.forEach((g, idx) => { - const pos = base + (idx + 1) / (group.length + 1) - if (g.type === 'note') noteUpdates.push({ id: g.data.id, sort_order: pos }) - else if (g.type === 'transport') { - const res = g.data as TransportEntry - if (res.__leg) (legPosUpdates[res.id] ??= {})[res.__leg.index] = pos - else transportUpdates.push({ id: res.id, day_plan_position: pos }) - } - }) - } + const { assignmentIds, noteUpdates, transportUpdates, legPosUpdates } = planMergedOrder(newOrder) try { // Optimistic transport positions first, so the recomputed merge is stable // before the reorder round-trips (same order of operations as the desktop). if (transportUpdates.length) { - useTripStore.setState(state => ({ - reservations: state.reservations.map(r => { - const tu = transportUpdates.find(u => u.id === r.id) - if (!tu) return r - return { - ...r, - day_plan_position: tu.day_plan_position, - day_positions: { ...(r.day_positions || {}), [dayId]: tu.day_plan_position }, - } - }), - })) + useTripStore.setState(state => ({ reservations: withTransportPositions(state.reservations, transportUpdates, dayId) })) } // Per-leg positions of a multi-leg flight/train live in metadata.legs[i]. for (const ridStr of Object.keys(legPosUpdates)) { const rid = Number(ridStr) const r = useTripStore.getState().reservations.find(x => x.id === rid) if (!r) continue - let parsed: Record = {} - try { parsed = typeof r.metadata === 'string' ? JSON.parse(r.metadata || '{}') : ((r.metadata as never) || {}) } catch { parsed = {} } - if (!Array.isArray(parsed.legs)) continue - const legs = (parsed.legs as Record[]).map((leg, li) => { - const pos = legPosUpdates[rid][li] - return pos == null ? leg : { ...leg, day_positions: { ...((leg.day_positions as object) || {}), [dayId]: pos } } - }) - const newMeta = { ...parsed, legs } + const newMeta = metadataWithLegPositions(r.metadata, legPosUpdates[rid], dayId) + if (!newMeta) continue useTripStore.setState(state => ({ reservations: state.reservations.map(x => (x.id === rid ? { ...x, metadata: newMeta as never } : x)), })) diff --git a/client/src/utils/mergedOrder.test.ts b/client/src/utils/mergedOrder.test.ts new file mode 100644 index 000000000..62f83cf8d --- /dev/null +++ b/client/src/utils/mergedOrder.test.ts @@ -0,0 +1,83 @@ +// FE-UTIL-MERGEDORDER-001 to -007: 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'; + +const place = (id: number) => ({ type: 'place', data: { id } }); +const note = (id: number) => ({ type: 'note', data: { id } }); +const transport = (id: number, leg?: number) => ({ + type: 'transport', + data: { id, __leg: leg == null ? undefined : { index: leg } }, +}); + +describe('planMergedOrder', () => { + it('FE-UTIL-MERGEDORDER-001: places get sequential slots and what sits between two places shares the gap', () => { + const plan = planMergedOrder([place(1), note(7), transport(9), place(2), place(3)]); + expect(plan.assignmentIds).toEqual([1, 2, 3]); + expect(plan.noteUpdates).toEqual([{ id: 7, sort_order: 1 / 3 }]); + expect(plan.transportUpdates).toEqual([{ id: 9, day_plan_position: 2 / 3 }]); + expect(plan.legPosUpdates).toEqual({}); + }); + + it('FE-UTIL-MERGEDORDER-002: rows before the first place sit below zero', () => { + const plan = planMergedOrder([note(7), place(1)]); + expect(plan.noteUpdates).toEqual([{ id: 7, sort_order: -0.5 }]); + expect(plan.assignmentIds).toEqual([1]); + }); + + it('FE-UTIL-MERGEDORDER-003: the legs of one booking are positioned leg by leg', () => { + const plan = planMergedOrder([place(1), transport(5, 0), place(2), transport(5, 1)]); + expect(plan.legPosUpdates).toEqual({ 5: { 0: 0.5, 1: 1.5 } }); + expect(plan.transportUpdates).toEqual([]); + }); + + it('FE-UTIL-MERGEDORDER-004: a row of any other type takes a slot but writes nothing', () => { + const plan = planMergedOrder([place(1), { type: 'other', data: { id: 3 } }, note(4), place(2)]); + expect(plan.noteUpdates).toEqual([{ id: 4, sort_order: 2 / 3 }]); + expect(plan.transportUpdates).toEqual([]); + }); +}); + +describe('withTransportPositions', () => { + it('FE-UTIL-MERGEDORDER-005: writes the moved bookings for that day and leaves the rest as they were', () => { + const untouched = { id: 2, day_plan_position: 4, day_positions: { 3: 4 } }; + const moved = { id: 1, day_plan_position: 0, day_positions: { 2: 9 } }; + const fresh = { id: 3 }; + const result = withTransportPositions( + [moved, untouched, fresh], + [ + { id: 1, day_plan_position: 1.5 }, + { id: 3, day_plan_position: 0.25 }, + ], + 7 + ); + expect(result[0]).toEqual({ id: 1, day_plan_position: 1.5, day_positions: { 2: 9, 7: 1.5 } }); + expect(result[1]).toBe(untouched); + expect(result[2]).toEqual({ id: 3, day_plan_position: 0.25, day_positions: { 7: 0.25 } }); + }); +}); + +describe('metadataWithLegPositions', () => { + it('FE-UTIL-MERGEDORDER-006: writes each moved leg position for the day, from a string or an object', () => { + const legs = [{ from: 'FRA', day_positions: { 1: 0.2 } }, { from: 'DXB' }]; + const fromString = metadataWithLegPositions(JSON.stringify({ airline: 'LH', legs }), { 1: 2.5 }, 4); + expect(fromString).toEqual({ + airline: 'LH', + legs: [ + { from: 'FRA', day_positions: { 1: 0.2 } }, + { from: 'DXB', day_positions: { 4: 2.5 } }, + ], + }); + const fromObject = metadataWithLegPositions({ legs }, { 0: 1.5 }, 4); + expect(fromObject).toEqual({ legs: [{ from: 'FRA', day_positions: { 1: 0.2, 4: 1.5 } }, { from: 'DXB' }] }); + expect((fromObject?.legs as unknown[])[1]).toBe(legs[1]); + }); + + it('FE-UTIL-MERGEDORDER-007: metadata without legs, or that cannot be read, has nothing to move', () => { + expect(metadataWithLegPositions(JSON.stringify({ airline: 'LH' }), { 0: 1 }, 4)).toBeNull(); + expect(metadataWithLegPositions('{not json', { 0: 1 }, 4)).toBeNull(); + expect(metadataWithLegPositions(null, { 0: 1 }, 4)).toBeNull(); + expect(metadataWithLegPositions('', { 0: 1 }, 4)).toBeNull(); + }); +}); diff --git a/client/src/utils/mergedOrder.ts b/client/src/utils/mergedOrder.ts new file mode 100644 index 000000000..7562e084e --- /dev/null +++ b/client/src/utils/mergedOrder.ts @@ -0,0 +1,103 @@ +/** + * Turning a day's reordered timeline back into the writes that store it, shared by + * the desktop day plan and the phone's plan timeline. Places get sequential + * integer positions (0, 1, 2, ...); the notes and transports between place N-1 and + * place N get fractional positions between the two. + */ + +/** A timeline row as the reorder reads it: a place, a note or a transport (one leg of it, for a multi-leg booking). */ +export interface MergedOrderItem { + type: string; + data: { id: number; __leg?: { index: number } | null }; +} + +export interface MergedOrderPlan { + /** The day's assignments in their new order. */ + assignmentIds: number[]; + noteUpdates: { id: number; sort_order: number }[]; + transportUpdates: { id: number; day_plan_position: number }[]; + /** + * Multi-leg flight legs share a reservation id, so their positions cannot live in + * the single per-booking slot: reservation id, then leg index, then position. + */ + legPosUpdates: Record>; +} + +export function planMergedOrder(newOrder: readonly MergedOrderItem[]): MergedOrderPlan { + const plan: MergedOrderPlan = { assignmentIds: [], noteUpdates: [], transportUpdates: [], legPosUpdates: {} }; + let placeCount = 0; + let i = 0; + while (i < newOrder.length) { + if (newOrder[i].type === 'place') { + plan.assignmentIds.push(newOrder[i].data.id); + placeCount++; + i++; + continue; + } + // Consecutive non-place items share the gap after the place before them. + const group: MergedOrderItem[] = []; + while (i < newOrder.length && newOrder[i].type !== 'place') { + group.push(newOrder[i]); + i++; + } + const base = placeCount > 0 ? placeCount - 1 : -1; + group.forEach((g, idx) => { + const pos = base + (idx + 1) / (group.length + 1); + if (g.type === 'note') plan.noteUpdates.push({ id: g.data.id, sort_order: pos }); + else if (g.type === 'transport') { + if (g.data.__leg) (plan.legPosUpdates[g.data.id] ??= {})[g.data.__leg.index] = pos; + else plan.transportUpdates.push({ id: g.data.id, day_plan_position: pos }); + } + }); + } + return plan; +} + +interface PositionedBooking { + id: number; + day_plan_position?: number | null; + day_positions?: Record | null; +} + +/** The bookings with the new positions written in, for the day they were moved on. */ +export function withTransportPositions( + reservations: R[], + updates: MergedOrderPlan['transportUpdates'], + dayId: number +): R[] { + return reservations.map((r) => { + const tu = updates.find((u) => u.id === r.id); + if (!tu) return r; + return { + ...r, + day_plan_position: tu.day_plan_position, + day_positions: { ...(r.day_positions || {}), [dayId]: tu.day_plan_position }, + }; + }); +} + +/** + * A booking's metadata with each moved leg's position for the day written into + * metadata.legs[i].day_positions. Null when the metadata holds no legs to move. + * Metadata that cannot be parsed counts as empty. + */ +export function metadataWithLegPositions( + metadata: unknown, + positions: Record, + dayId: number +): Record | null { + let parsed: Record; + try { + parsed = typeof metadata === 'string' ? JSON.parse(metadata || '{}') : (metadata as Record) || {}; + } catch { + parsed = {}; + } + if (!Array.isArray(parsed.legs)) return null; + const legs = (parsed.legs as Record[]).map((leg, li) => { + const pos = positions[li]; + return pos == null + ? leg + : { ...leg, day_positions: { ...((leg.day_positions as Record) || {}), [dayId]: pos } }; + }); + return { ...parsed, legs }; +}