From 20d7e62afc479df80ba0ee08fdc77760d5043386 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 21:03:54 +0000 Subject: [PATCH] refactor(planner): share the day plan reorder writes between desktop and phone The desktop day plan and the phone timeline derive the place order, the note and transport positions and the per leg positions of a reordered day from mergedOrder, and write the moved bookings back through the same helpers. --- .../src/components/Planner/DayPlanSidebar.tsx | 58 ++-------- .../screens/trip/plan/useMPlanTimeline.ts | 51 +-------- client/src/utils/mergedOrder.test.ts | 83 ++++++++++++++ client/src/utils/mergedOrder.ts | 103 ++++++++++++++++++ 4 files changed, 198 insertions(+), 97 deletions(-) create mode 100644 client/src/utils/mergedOrder.test.ts create mode 100644 client/src/utils/mergedOrder.ts 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 }; +}