diff --git a/client/scripts/eslint-baseline.json b/client/scripts/eslint-baseline.json index 6b322f5f2..ad35626f0 100644 --- a/client/scripts/eslint-baseline.json +++ b/client/scripts/eslint-baseline.json @@ -4,7 +4,7 @@ "@typescript-eslint/no-misused-promises": 876, "no-console": 88, "complexity": 231, - "@typescript-eslint/no-explicit-any": 149, + "@typescript-eslint/no-explicit-any": 148, "trek/no-swallowed-catch": 135, "max-params": 35, "trek/store-without-selector": 69, diff --git a/client/scripts/size-baseline.json b/client/scripts/size-baseline.json index 0686ec8b8..73a490e91 100644 --- a/client/scripts/size-baseline.json +++ b/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": 3001, + "src/components/Planner/DayPlanSidebar.tsx": 3000, "src/components/Planner/PlaceFormModal.test.tsx": 2864, "src/components/Planner/PlaceFormModal.tsx": 1250, "src/components/Planner/PlaceInspector.tsx": 1308, diff --git a/client/src/components/Map/dayRoutePlan.ts b/client/src/components/Map/dayRoutePlan.ts index e88d88769..5b0d6ccd7 100644 --- a/client/src/components/Map/dayRoutePlan.ts +++ b/client/src/components/Map/dayRoutePlan.ts @@ -16,6 +16,8 @@ export interface DayRoutePoint { incoming_leg_transport_mode?: string | null /** Only on an accommodation bookend: the stay the day starts from or ends at. */ hotel?: 'morning' | 'evening' + /** Only on a planned stop: its assignment, which the road trip's vias are filed against. */ + assignmentId?: number } /** @@ -84,7 +86,7 @@ export function buildDayRouteRuns(dayId: number, input: DayRouteInputs): DayRout const projected = new Map(projectDayItinerary(da, input.toursEnabled ?? false, input.places).map(item => [item.assignment.id, item])) type Entry = - | { kind: 'place'; lat: number; lng: number; pos: number; time: string | null; mode: string | null; incoming: string | null } + | { kind: 'place'; id: number; lat: number; lng: number; pos: number; time: string | null; mode: string | null; incoming: string | null } | { kind: 'tour'; start: { lat: number; lng: number } | null; end: { lat: number; lng: number } | null; pos: number; time: string | null; mode: string | null; incoming: string | null } | { kind: 'transport'; from: { lat: number; lng: number } | null; to: { lat: number; lng: number } | null; pos: number; carrier: boolean } const entries: Entry[] = merged.flatMap((item): Entry[] => { @@ -99,7 +101,7 @@ export function buildDayRouteRuns(dayId: number, input: DayRouteInputs): DayRout }] if (!a.place?.lat || !a.place?.lng) return [] return [{ - kind: 'place', lat: a.place.lat, lng: a.place.lng, pos: item.sortKey, time: a.place?.place_time ?? null, + kind: 'place', id: a.id, lat: a.place.lat, lng: a.place.lng, pos: item.sortKey, time: a.place?.place_time ?? null, // Per-segment travel mode (#1281): mode of the leg leaving this place. mode: (a as { leg_transport_mode?: string | null }).leg_transport_mode ?? null, // Boundary-leg mode (#1281 follow-up): mode of the leg arriving at this place. @@ -144,7 +146,7 @@ export function buildDayRouteRuns(dayId: number, input: DayRouteInputs): DayRout // The open run may be nothing but a far-away arrival endpoint — break rather // than draw the ocean. if (prev && !prev.isPlace && !withinDriveRange(prev, entry)) closeRun() - currentRun.push({ lat: entry.lat, lng: entry.lng, isPlace: true, leg_transport_mode: entry.mode, incoming_leg_transport_mode: entry.incoming }) + currentRun.push({ lat: entry.lat, lng: entry.lng, isPlace: true, leg_transport_mode: entry.mode, incoming_leg_transport_mode: entry.incoming, assignmentId: entry.id }) runHasPlace = true } else if (entry.from || entry.to) { const prev = currentRun[currentRun.length - 1] diff --git a/client/src/components/Map/dayRouteVias.test.ts b/client/src/components/Map/dayRouteVias.test.ts new file mode 100644 index 000000000..99548dbba --- /dev/null +++ b/client/src/components/Map/dayRouteVias.test.ts @@ -0,0 +1,185 @@ +import type { RoadtripVia } from '@trek/shared'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { buildAssignment, buildPlace } from '../../../tests/helpers/factories'; +import type { AssignmentsMap, RouteSegment } from '../../types'; +import { + dayViaLookup, + foldViaLegs, + roadtripStopList, + routeLeg, + threadVias, + viaKeyOf, + viasOnDayLeg, +} from './dayRouteVias'; + +vi.mock('./RouteCalculator', async (importActual) => { + const actual = await importActual(); + return { ...actual, calculateRouteWithLegs: vi.fn() }; +}); + +const { calculateRouteWithLegs } = await import('./RouteCalculator'); + +const leg = (distance: number): RouteSegment => ({ + mid: [0, 0], + from: [distance, 0], + to: [distance + 1, 0], + distance, + duration: distance / 10, + distanceText: '', + durationText: '', + walkingText: '', + drivingText: '', +}); + +const via = (dayId: number, afterOrderIndex: number, lat: number, sequence = 0): RoadtripVia => + ({ + id: lat, + day_id: dayId, + after_order_index: afterOrderIndex, + sequence, + lat, + lng: 0, + created_at: '', + }) as RoadtripVia; + +const stop = (id: number, order: number, extra: Record = {}) => + buildAssignment({ id, day_id: 1, order_index: order, place: buildPlace({ lat: 48 + order, lng: 2 }), ...extra }); + +// Stored order: hotel row, Paris, a place without coordinates, Lille, Brussels. +const STORED: AssignmentsMap = { + '1': [ + stop(14, 3), + stop(11, 1), + stop(10, 0, { accommodation_id: 5 }), + buildAssignment({ + id: 12, + day_id: 1, + order_index: 2, + place: buildPlace({ lat: null as unknown as number, lng: null as unknown as number }), + }), + stop(13, 2.5), + ], +}; + +/** The lookup of a trip that has vias, which is never null. */ +function lookupFor(viasByDay: Record) { + const lookup = dayViaLookup(STORED, viasByDay); + if (!lookup) throw new Error('expected a lookup'); + return lookup; +} + +beforeEach(() => { + vi.mocked(calculateRouteWithLegs).mockReset(); +}); + +describe('dayRouteVias (#2580)', () => { + it('FE-MAP-DRV-001: the stop list is the stored order with coordinates, hidden rows included', () => { + expect(roadtripStopList(STORED['1']).map((a) => a.id)).toEqual([10, 11, 13, 14]); + expect(roadtripStopList(undefined)).toEqual([]); + }); + + it('FE-MAP-DRV-002: a leg gets the vias filed behind its first stop, in sequence', () => { + const lookup = lookupFor({ 1: [via(1, 1, 49.6, 1), via(1, 1, 49.3, 0), via(1, 2, 50.5)] }); + expect(lookup(1, 11, 13).map((v) => v.lat)).toEqual([49.3, 49.6]); + expect(lookup(1, 13, 14).map((v) => v.lat)).toEqual([50.5]); + }); + + it('FE-MAP-DRV-003: two stops that do not follow each other in the stored list get none', () => { + const lookup = lookupFor({ 1: [via(1, 1, 49.3)] }); + // Paris to Brussels skips Lille: the via belongs to the leg to Lille. + expect(lookup(1, 11, 14)).toEqual([]); + expect(lookup(1, 13, 11)).toEqual([]); + expect(lookup(1, 999, 13)).toEqual([]); + expect(lookup(2, 11, 13)).toEqual([]); + }); + + it('FE-MAP-DRV-004: a trip without vias routes as before', () => { + expect(dayViaLookup(STORED, undefined)).toBeNull(); + expect(dayViaLookup(STORED, {})).toBeNull(); + expect(dayViaLookup(STORED, { 1: [] })).toBeNull(); + }); + + it('FE-MAP-DRV-005: a ride, a hotel bookend or a missing lookup bends nothing', () => { + const lookup = dayViaLookup(STORED, { 1: [via(1, 1, 49.3)] }); + expect(viasOnDayLeg(lookup, 1, 'driving', { assignmentId: 11 }, { assignmentId: 13 })).toHaveLength(1); + expect(viasOnDayLeg(lookup, 1, 'walking', { assignmentId: 11 }, { assignmentId: 13 })).toHaveLength(1); + expect(viasOnDayLeg(lookup, 1, 'train', { assignmentId: 11 }, { assignmentId: 13 })).toEqual([]); + expect(viasOnDayLeg(lookup, 1, 'driving', { assignmentId: 11 }, {})).toEqual([]); + expect(viasOnDayLeg(null, 1, 'driving', { assignmentId: 11 }, { assignmentId: 13 })).toEqual([]); + }); + + it('FE-MAP-DRV-006: threading remembers where each stop went, folding gives one leg per stop pair', () => { + const points = [ + { lat: 1, lng: 0, k: 'a' }, + { lat: 2, lng: 0, k: 'b' }, + { lat: 3, lng: 0, k: 'c' }, + ]; + const { waypoints, stopAt } = threadVias(points, (from) => + from.k === 'a' + ? [ + { lat: 1.5, lng: 0 }, + { lat: 1.7, lng: 0 }, + ] + : [] + ); + expect(waypoints.map((w) => w.lat)).toEqual([1, 1.5, 1.7, 2, 3]); + expect(stopAt).toEqual([0, 3, 4]); + + const folded = foldViaLegs([leg(1000), leg(2000), leg(3000), leg(4000)], stopAt, 'imperial'); + expect(folded.map((l) => l.distance)).toEqual([6000, 4000]); + expect(folded[0].from).toEqual([1000, 0]); + expect(folded[0].to).toEqual([3001, 0]); + expect(folded[0].distanceText).toMatch(/mi/); + }); + + it('FE-MAP-DRV-007: legs are left alone when nothing was threaded or the answer does not line up', () => { + const legs = [leg(1000), leg(2000)]; + expect(foldViaLegs(legs, [0, 1, 2], 'metric')).toBe(legs); + expect(foldViaLegs(legs, [0, 2, 3], 'metric')).toBe(legs); + }); + + it('FE-MAP-DRV-008: the via key changes with the road and not with the order of the list', () => { + const a = { 1: [via(1, 0, 49.3)], 2: [via(2, 0, 50.1)] }; + expect(viaKeyOf({ 2: a[2], 1: a[1] })).toBe(viaKeyOf(a)); + expect(viaKeyOf({ ...a, 1: [via(1, 0, 49.4)] })).not.toBe(viaKeyOf(a)); + expect(viaKeyOf(undefined)).toBe(''); + }); + + it('FE-MAP-DRV-009: one leg is routed through its vias and read back as one leg in its mode', async () => { + vi.mocked(calculateRouteWithLegs).mockResolvedValue({ + coordinates: [], + distance: 0, + duration: 0, + legs: [leg(1000), leg(2000)], + }); + const lookup = dayViaLookup(STORED, { 1: [via(1, 1, 49.3)] }); + const options = { profile: 'driving', tripId: 7, dayId: 1 }; + + const seg = await routeLeg( + { lat: 49, lng: 2, assignmentId: 11 }, + { lat: 50.5, lng: 2, assignmentId: 13 }, + lookup, + options, + 'metric' + ); + + expect(vi.mocked(calculateRouteWithLegs).mock.calls[0][0]).toEqual([ + { lat: 49, lng: 2 }, + { lat: 49.3, lng: 0 }, + { lat: 50.5, lng: 2 }, + ]); + expect(seg?.distance).toBe(3000); + expect(seg?.mode).toBe('driving'); + }); + + it('FE-MAP-DRV-010: a leg the router cannot answer is undefined', async () => { + vi.mocked(calculateRouteWithLegs).mockRejectedValueOnce(new Error('down')); + expect( + await routeLeg({ lat: 1, lng: 1 }, { lat: 2, lng: 2 }, null, { profile: 'driving', dayId: 1 }, 'metric') + ).toBeUndefined(); + vi.mocked(calculateRouteWithLegs).mockResolvedValueOnce({ coordinates: [], distance: 0, duration: 0, legs: [] }); + expect( + await routeLeg({ lat: 1, lng: 1 }, { lat: 2, lng: 2 }, null, { profile: 'driving', dayId: 1 }, 'metric') + ).toBeUndefined(); + }); +}); diff --git a/client/src/components/Map/dayRouteVias.ts b/client/src/components/Map/dayRouteVias.ts new file mode 100644 index 000000000..5f8e931e5 --- /dev/null +++ b/client/src/components/Map/dayRouteVias.ts @@ -0,0 +1,144 @@ +import type { RoadtripVia } from '@trek/shared'; +import { isCarrierMode, mergeRouteSegments, viasOnLeg } from '@trek/shared/roadtrip'; +import type { Assignment, AssignmentsMap, DistanceUnit, RouteSegment } from '../../types'; +import { calculateRouteWithLegs } from './RouteCalculator'; + +/** + * The stops of a day as the road trip counts them, in the order it drives them. + * + * This is the index space a via's `after_order_index` lives in: the STORED list, sorted + * by `order_index` and filtered to the rows with coordinates. The stop a lodging booking + * put on the day and the service stops are counted too, even where the day list hides + * them, because the rail and the server count them. + */ +export function roadtripStopList(dayAssignments: readonly Assignment[] | undefined): Assignment[] { + return (dayAssignments ?? []) + .slice() + .sort((a, b) => (a.order_index ?? 0) - (b.order_index ?? 0)) + .filter((a) => typeof a.place?.lat === 'number' && typeof a.place?.lng === 'number'); +} + +/** The trip's vias, keyed by the day they are filed against. */ +export type ViasByDay = Record; + +/** The vias shaping the drive between two assignments of a day, in driving order. */ +export type DayViaLookup = (dayId: number, fromAssignmentId: number, toAssignmentId: number) => readonly RoadtripVia[]; + +const NONE: readonly RoadtripVia[] = []; + +/** + * The road a traveller picked in the road trip, for the views that draw a day without it. + * + * A chosen alternative or a hand-bent drive is stored as vias filed against the stop + * they leave, so the day view asks for the same points between the same two stops. Only + * between two stops that follow each other in the stored list: with a stop the day list + * hides in between, the vias belong to a leg the day view does not draw, and bending the + * shortcut through them would be a road nobody chose. + * + * Null when the trip has no vias, so a trip without a road trip routes exactly as before. + */ +export function dayViaLookup(stored: AssignmentsMap, viasByDay: ViasByDay | undefined): DayViaLookup | null { + if (!viasByDay || !Object.values(viasByDay).some((list) => list.length)) return null; + const indexByDay = new Map>(); + return (dayId, fromAssignmentId, toAssignmentId) => { + const vias = viasByDay[dayId]; + if (!vias?.length) return NONE; + let index = indexByDay.get(dayId); + if (!index) { + index = new Map(roadtripStopList(stored[String(dayId)]).map((a, i) => [a.id, i])); + indexByDay.set(dayId, index); + } + const from = index.get(fromAssignmentId); + if (from === undefined || index.get(toAssignmentId) !== from + 1) return NONE; + return viasOnLeg({ ownerDayId: dayId, ownerIndex: from }, {}, vias); + }; +} + +/** + * The vias of one leg of a drawn day: none unless both ends are planned stops, and none + * on a ride, which is no road. + */ +export function viasOnDayLeg( + lookup: DayViaLookup | null, + dayId: number, + mode: string, + from: { assignmentId?: number }, + to: { assignmentId?: number } +): readonly RoadtripVia[] { + if (!lookup || isCarrierMode(mode) || from.assignmentId == null || to.assignmentId == null) return NONE; + return lookup(dayId, from.assignmentId, to.assignmentId); +} + +/** A stable key for the via list, so a routing effect re-runs when the road changes and only then. */ +export function viaKeyOf(viasByDay: ViasByDay | undefined): string { + return Object.entries(viasByDay ?? {}) + .map( + ([dayId, vias]) => + `${dayId}:${vias.map((v) => `${v.after_order_index}@${v.lat.toFixed(5)},${v.lng.toFixed(5)}`).join('|')}` + ) + .sort((a, b) => a.localeCompare(b)) + .join(';'); +} + +/** + * The waypoints to ask the router for: the stops with the vias of each leg threaded in + * between. `stopAt[i]` is the waypoint the i-th stop became, which is how the answer's + * legs (one per waypoint pair) are folded back to one per stop pair. + */ +export function threadVias

( + points: readonly P[], + viasBetween: (from: P, to: P) => readonly { lat: number; lng: number }[] +): { waypoints: { lat: number; lng: number }[]; stopAt: number[] } { + const waypoints: { lat: number; lng: number }[] = []; + const stopAt: number[] = []; + points.forEach((point, i) => { + stopAt.push(waypoints.length); + waypoints.push({ lat: point.lat, lng: point.lng }); + if (i < points.length - 1) + for (const via of viasBetween(point, points[i + 1])) waypoints.push({ lat: via.lat, lng: via.lng }); + }); + return { waypoints, stopAt }; +} + +/** + * The router's legs read as one leg per stop pair again. Untouched when nothing was + * threaded, or when the answer does not have a leg per waypoint pair (a plugin route may + * not), since the pieces could not be told apart then. + */ +export function foldViaLegs(legs: RouteSegment[], stopAt: number[], unit: DistanceUnit | undefined): RouteSegment[] { + const waypointCount = stopAt.length ? stopAt[stopAt.length - 1] + 1 : 0; + if (waypointCount === stopAt.length || legs.length !== waypointCount - 1) return legs; + const folded: RouteSegment[] = []; + for (let i = 0; i < stopAt.length - 1; i++) { + const parts = legs.slice(stopAt[i], stopAt[i + 1]); + if (parts.length) + folded.push(mergeRouteSegments(parts, unit === 'imperial' ? 'imperial' : 'metric') as RouteSegment); + } + return folded; +} + +/** One end of a leg: a planned stop carries its assignment, which the vias are filed against. */ +export type LegEnd = { lat: number; lng: number; assignmentId?: number }; + +/** + * The drive between two points of a day as one leg, bent through the road trip's vias + * between them and tagged with the mode it was routed in. Undefined when the router has + * no road for it. + */ +export async function routeLeg( + from: LegEnd, + to: LegEnd, + lookup: DayViaLookup | null, + options: Parameters[1] & { profile: string; dayId: number }, + unit: DistanceUnit | undefined +): Promise { + const vias = viasOnDayLeg(lookup, options.dayId, options.profile, from, to); + try { + const waypoints = [from, ...vias, to].map((p) => ({ lat: p.lat, lng: p.lng })); + const r = await calculateRouteWithLegs(waypoints, options); + const leg = foldViaLegs(r.legs, [0, vias.length + 1], unit)[0]; + return leg ? { ...leg, mode: options.profile } : undefined; + } catch { + return undefined; + } +} diff --git a/client/src/components/Map/tripRouteGeometry.test.ts b/client/src/components/Map/tripRouteGeometry.test.ts index 8b34ac30d..a87b91366 100644 --- a/client/src/components/Map/tripRouteGeometry.test.ts +++ b/client/src/components/Map/tripRouteGeometry.test.ts @@ -232,4 +232,26 @@ describe('tripRouteGeometry', () => { const byRoad = planTripRoute({ ...moving, reservations: [] }, 'driving') expect(byRoad.map(d => d.day.id)).toEqual([2]) }) + + it('FE-MAP-TRG-013: a road picked in the road trip bends the overview and still counts as one leg (#2580)', async () => { + // A lodging row hides in the plan but counts in the stored list, so the via behind + // the first stop of day 1 is filed at 1. + const hotel = buildAssignment({ day_id: 1, order_index: -1, accommodation_id: 3, place: buildPlace({ lat: 48.8, lng: 2.3 }) }) + const storedAssignments = { ...ASSIGNMENTS, '1': [hotel, ...ASSIGNMENTS['1']] } + const viasByDay = { 1: [{ id: 1, day_id: 1, after_order_index: 1, sequence: 0, lat: 48.95, lng: 2.6, created_at: '' }] } + vi.mocked(calculateRouteWithLegs).mockImplementation(async (points) => ({ + coordinates: points.map(p => [p.lat, p.lng] as [number, number]), + distance: 0, duration: 0, + legs: points.slice(1).map(() => leg(10000)), + })) + + const plan = planTripRoute({ ...input, storedAssignments, viasByDay }, 'driving') + expect(plan[0].runs[0][0].waypoints.map(p => p.lat)).toEqual([48.86, 48.95, 48.90]) + expect(plan[1].runs[0][0].waypoints.map(p => p.lat)).toEqual([45.76, 45.80]) + + const summary = await routeTrip({ ...input, storedAssignments, viasByDay }, { profile: 'driving', tripId: 7, unit: 'metric' }) + expect(summary.days[0].segments).toHaveLength(1) + expect(summary.days[0].distance).toBe(20000) + expect(summary.days[1].distance).toBe(10000) + }) }) diff --git a/client/src/components/Map/tripRouteGeometry.ts b/client/src/components/Map/tripRouteGeometry.ts index dbdd191b7..afa5dd228 100644 --- a/client/src/components/Map/tripRouteGeometry.ts +++ b/client/src/components/Map/tripRouteGeometry.ts @@ -1,9 +1,11 @@ import { calculateRouteWithLegs, RoutingRefusedError, type RouteProfileKey } from './RouteCalculator' import { buildDayRouteRuns, type DayRouteInputs, type DayRoutePoint } from './dayRoutePlan' +import { dayViaLookup, foldViaLegs, threadVias, viasOnDayLeg, type DayViaLookup } from './dayRouteVias' import { projectDayItinerary } from './dayTourProjection' import { resolveLegMode } from '../Planner/legMode' import { dayColor } from '../Roadtrip/dayColors' -import type { Day, RouteSegment } from '../../types' +import type { RoadtripVia } from '@trek/shared' +import type { AssignmentsMap, Day, DistanceUnit, RouteSegment } from '../../types' /** One travel day of the overview: the roads it covers, in its own colour. */ export interface TripOverviewDay { @@ -48,8 +50,16 @@ export interface TripRouteSummary { } /** Neighbouring legs of one run that resolve to the same mode travel as one request, - * exactly as the single-day route builds them. */ -interface Chunk { points: DayRoutePoint[]; mode: string } + * exactly as the single-day route builds them. `waypoints` are the points with the road + * trip's vias threaded in, and `stopAt` says which waypoint each point became. */ +interface Chunk { points: DayRoutePoint[]; mode: string; waypoints: { lat: number; lng: number }[]; stopAt: number[] } + +/** The day inputs plus the road trip's vias, which bend a day the way it was picked there (#2580). */ +interface TripRouteInputs extends DayRouteInputs { + /** The stored day lists the vias are anchored in, hidden rows included. Absent: `assignments`. */ + storedAssignments?: AssignmentsMap + viasByDay?: Record +} /** A day reduced to the routing requests it needs, in the order they draw. */ export interface TripRoutePlanDay { day: Day; runs: Chunk[][]; tourLines?: [number, number][][] } @@ -62,14 +72,16 @@ export type TripRouteAnswers = Answer[][][] export const dayRouteColor = (day: Day): { line: string; casing: string } => dayColor(day.day_number ?? 0) -function chunkRun(run: DayRoutePoint[], dayDefaultMode: string): Chunk[] { +function chunkRun(run: DayRoutePoint[], dayDefaultMode: string, dayId: number, viaLookup: DayViaLookup | null): Chunk[] { const chunks: Chunk[] = [] let i = 0 while (i < run.length - 1) { const mode = resolveLegMode(run[i], run[i + 1], dayDefaultMode) let end = i + 1 while (end < run.length - 1 && resolveLegMode(run[end], run[end + 1], dayDefaultMode) === mode) end++ - chunks.push({ points: run.slice(i, end + 1), mode }) + const points = run.slice(i, end + 1) + const { waypoints, stopAt } = threadVias(points, (from, to) => viasOnDayLeg(viaLookup, dayId, mode, from, to)) + chunks.push({ points, mode, waypoints, stopAt }) i = end } return chunks @@ -83,7 +95,8 @@ const straight = (points: DayRoutePoint[]): [number, number][] => points.map(p = * Pure and synchronous, so a caller can compare two plans (the map keys its routing * round on one) before spending a single request. */ -export function planTripRoute(input: DayRouteInputs, profile: RouteProfileKey): TripRoutePlanDay[] { +export function planTripRoute(input: TripRouteInputs, profile: RouteProfileKey): TripRoutePlanDay[] { + const viaLookup = dayViaLookup(input.storedAssignments ?? input.assignments, input.viasByDay) return [...input.days] .sort((a, b) => (a.day_number ?? 0) - (b.day_number ?? 0)) .map(day => ({ @@ -98,7 +111,7 @@ export function planTripRoute(input: DayRouteInputs, profile: RouteProfileKey): .filter(entry => entry.runs.length > 0 || entry.tourLines.length > 0) .map(({ day, runs, tourLines }) => ({ day, - runs: runs.map(run => chunkRun(run, day.default_transport_mode || profile)), + runs: runs.map(run => chunkRun(run, day.default_transport_mode || profile, day.id, viaLookup)), tourLines, })) } @@ -205,9 +218,11 @@ const sleep = (ms: number, signal?: AbortSignal): Promise => */ export async function routeTripLegs( plan: TripRoutePlanDay[], - { tripId, signal, onAnswer }: { + { tripId, signal, onAnswer, unit }: { tripId: number | null signal?: AbortSignal + /** The unit a leg bent through vias has its text written again in. */ + unit?: DistanceUnit /** Called after each answer, for a caller that wants to draw as they arrive. */ onAnswer?: (routed: Answer[][][]) => void }, @@ -218,15 +233,14 @@ export async function routeTripLegs( runs.forEach((chunks, r) => { chunks.forEach((chunk, c) => { tasks.push(async () => { - const waypoints = chunk.points.map(p => ({ lat: p.lat, lng: p.lng })) for (let attempt = 0; attempt <= RETRY_DELAYS_MS.length; attempt++) { if (signal?.aborted) return try { const answer = await calculateRouteWithLegs( - waypoints, + chunk.waypoints, { signal, profile: chunk.mode, tripId, dayId: day.id }, ) - routed[d][r][c] = { coordinates: answer.coordinates, legs: answer.legs } + routed[d][r][c] = { coordinates: answer.coordinates, legs: foldViaLegs(answer.legs, chunk.stopAt, unit) } onAnswer?.(routed) return } catch (err) { @@ -261,12 +275,13 @@ export async function routeTripLegs( * requests against a shared router, and no document is worth that wait. */ export async function routeTrip( - input: DayRouteInputs, - { profile, tripId, signal, timeoutMs }: { + input: TripRouteInputs, + { profile, tripId, signal, timeoutMs, unit }: { profile: RouteProfileKey tripId: number | null signal?: AbortSignal timeoutMs?: number + unit?: DistanceUnit }, ): Promise { const plan = planTripRoute(input, profile) @@ -275,7 +290,7 @@ export async function routeTrip( const timer = deadline ? setTimeout(() => deadline.abort(), timeoutMs) : null if (deadline && signal) signal.addEventListener('abort', () => deadline.abort(), { once: true }) try { - const routed = await routeTripLegs(plan, { tripId, signal: deadline?.signal ?? signal }) + const routed = await routeTripLegs(plan, { tripId, signal: deadline?.signal ?? signal, unit }) return summariseTripRoute(assembleTripRoute(plan, routed)) } finally { if (timer) clearTimeout(timer) diff --git a/client/src/components/Map/useTripRouteOverview.test.ts b/client/src/components/Map/useTripRouteOverview.test.ts index 52ec28739..fe209eac9 100644 --- a/client/src/components/Map/useTripRouteOverview.test.ts +++ b/client/src/components/Map/useTripRouteOverview.test.ts @@ -278,6 +278,22 @@ describe('useTripRouteOverview', () => { expect(result.current.focusPoints).toBe(frame) }) + it('FE-MAP-TRO-015: a road picked in the road trip re-routes the overview through it (#2580)', async () => { + const via = { id: 1, day_id: 1, after_order_index: 0, sequence: 0, lat: 48.95, lng: 2.6, created_at: '' } + const { result, rerender } = renderHook( + ({ viasByDay }: { viasByDay: Record }) => + useTripRouteOverview(7, DAYS, ASSIGNMENTS, [], [], 'driving', true, undefined, { storedAssignments: ASSIGNMENTS, viasByDay }), + { initialProps: { viasByDay: {} } }, + ) + await waitFor(() => expect(result.current.loading).toBe(false)) + const asked = () => vi.mocked(calculateRouteWithLegs).mock.calls.map(call => call[0].map(p => p.lat)) + expect(asked()).toEqual([[48.86, 48.88], [45.76, 45.78]]) + + rerender({ viasByDay: { 1: [via] } }) + await waitFor(() => expect(asked()).toHaveLength(3)) + expect(asked()[2]).toEqual([48.86, 48.95, 48.88]) + }) + it('FE-MAP-TRO-013: switching the overview back on publishes a fresh frame', async () => { const { result, rerender } = render() await waitFor(() => expect(result.current.loading).toBe(false)) diff --git a/client/src/components/Map/useTripRouteOverview.ts b/client/src/components/Map/useTripRouteOverview.ts index 25ec5d67e..02e73ca88 100644 --- a/client/src/components/Map/useTripRouteOverview.ts +++ b/client/src/components/Map/useTripRouteOverview.ts @@ -6,6 +6,7 @@ import { } from './tripRouteGeometry' import { useSettingsStore } from '../../store/settingsStore' import { useAddonStore } from '../../store/addonStore' +import type { RoadtripVia } from '@trek/shared' import type { Accommodation, AssignmentsMap, Day, Place, Reservation } from '../../types' export type { TripOverviewDay, TripRouteSummary } from './tripRouteGeometry' @@ -18,6 +19,12 @@ export interface TripRouteOverview extends TripRouteSummary { const EMPTY: TripRouteOverview = { ...summariseTripRoute([]), loading: false } const EMPTY_PLACES: Place[] = [] +/** The road trip's vias and the stored day lists their anchors count in. */ +interface TripRouteVias { + storedAssignments: AssignmentsMap + viasByDay: Record +} + /** * Every travel day's route at once, each day in its own colour, with the trip's total * distance (#1736). @@ -26,7 +33,8 @@ const EMPTY_PLACES: Place[] = [] * `useRouteCalculation` draws the selected day from, so a day looks identical whether * you are looking at it alone or at the whole trip. This hook is the React wrapper: * the planning and routing themselves live in `tripRouteGeometry`, so the PDF export - * can draw the same trip without a component to hang a hook on. + * can draw the same trip without a component to hang a hook on. A road picked in the road + * trip is drawn here too (#2580). */ export function useTripRouteOverview( tripId: number | null, @@ -37,6 +45,7 @@ export function useTripRouteOverview( profile: RouteProfileKey, enabled: boolean, places: Place[] = EMPTY_PLACES, + vias?: TripRouteVias, ): TripRouteOverview { const optimizeFromAccommodation = useSettingsStore(s => s.settings.optimize_from_accommodation) const toursEnabled = useAddonStore(s => s.isEnabled('tours')) @@ -53,18 +62,20 @@ export function useTripRouteOverview( const frameRef = useRef<{ tripId: number | null; points: [number, number][] } | null>(null) const wasEnabled = useRef(false) + const storedAssignments = vias?.storedAssignments + const viasByDay = vias?.viasByDay const plan = useMemo( () => (enabled - ? planTripRoute({ days, assignments, reservations, accommodations, optimizeFromAccommodation, toursEnabled, places }, profile) + ? planTripRoute({ days, assignments, reservations, accommodations, optimizeFromAccommodation, toursEnabled, places, storedAssignments, viasByDay }, profile) : []), - [enabled, days, assignments, reservations, accommodations, optimizeFromAccommodation, profile, toursEnabled, places], + [enabled, days, assignments, reservations, accommodations, optimizeFromAccommodation, profile, toursEnabled, places, storedAssignments, viasByDay], ) // Only geometry and mode decide whether legs have to be fetched again: renaming a // place or editing its notes must not fire a routing round. const planKey = useMemo( () => plan.map(({ day, runs, tourLines }) => `${day.id}@${day.default_transport_mode ?? ''}:${runs - .map(chunks => chunks.map(c => `${c.mode}>${c.points.map(p => `${p.lat.toFixed(5)},${p.lng.toFixed(5)}`).join('|')}`).join('+')) + .map(chunks => chunks.map(c => `${c.mode}>${c.waypoints.map(p => `${p.lat.toFixed(5)},${p.lng.toFixed(5)}`).join('|')}`).join('+')) .join('/')}:tours=${JSON.stringify(tourLines ?? [])}`).join(';'), [plan], ) @@ -108,7 +119,7 @@ export function useTripRouteOverview( if (!loading && reframe) frameRef.current = { tripId, points: next.focusPoints } setResult({ ...next, focusPoints: loading ? frame : frameRef.current!.points, loading }) } - void routeTripLegs(plan, { tripId, signal: controller.signal, onAnswer: routed => publish(routed, true) }) + void routeTripLegs(plan, { tripId, signal: controller.signal, unit: distanceUnit, onAnswer: routed => publish(routed, true) }) .then(routed => publish(routed, false)) return () => controller.abort() diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 07db1adbe..7cfc50e89 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -11,7 +11,8 @@ import { ChevronDown, ChevronRight, ChevronUp, Compass, RotateCcw, ExternalLink, import { type PickedPlace } from './TransitSearchPanel' import { buildTransitLeg, buildTransitNameIndex } from './transitLeg' import { reservationsApi, daysApi } from '../../api/client' -import { calculateRouteWithLegs, optimizeRoute, generateGoogleMapsUrl, generateCoMapsUrl } from '../Map/RouteCalculator' +import { optimizeRoute, generateGoogleMapsUrl, generateCoMapsUrl } from '../Map/RouteCalculator' +import { dayViaLookup, routeLeg, type LegEnd, type ViasByDay } from '../Map/dayRouteVias' import GoogleMapsIcon from '../shared/GoogleMapsIcon' import PlaceAvatar from '../shared/PlaceAvatar' import ConfirmDialog from '../shared/ConfirmDialog' @@ -131,6 +132,7 @@ export interface DayPlanSidebarProps { onNavigateToFiles?: () => void routeShown?: boolean routeProfile?: string + roadtripViasByDay?: ViasByDay onToggleRoute?: () => void onSetRouteProfile?: (profile: string) => void onAddPlace?: () => void @@ -194,7 +196,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { tripId, trip, days, places, assignments, selectedDayId, selectedPlaceId, selectedAssignmentId, - onSelectDay, accommodations, + onSelectDay, accommodations, roadtripViasByDay, onReorder, onAssignToDay, onMoveToDay, reservations, @@ -613,8 +615,8 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { // Each run point carries the ORIGIN assignment's per-segment travel mode // (#1281): the leg from this point to the next is routed with `mode` (null = // inherit the day default). Transport endpoints carry no mode → day default. - const runs: { id: number; lat: number; lng: number; isPlace: boolean; leg_transport_mode?: string | null; incoming_leg_transport_mode?: string | null }[][] = [] - let cur: { id: number; lat: number; lng: number; isPlace: boolean; leg_transport_mode?: string | null; incoming_leg_transport_mode?: string | null }[] = [] + const runs: { id: number; lat: number; lng: number; isPlace: boolean; leg_transport_mode?: string | null; incoming_leg_transport_mode?: string | null; assignmentId?: number }[][] = [] + let cur: { id: number; lat: number; lng: number; isPlace: boolean; leg_transport_mode?: string | null; incoming_leg_transport_mode?: string | null; assignmentId?: number }[] = [] // A run is only a real drive when it holds an actual place. Two back-to-back // transports (e.g. two flights on one day) would otherwise pair the first's // arrival with the second's departure into a phantom airport→airport leg — the @@ -648,7 +650,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { cur = [] curHasPlace = false } - cur.push({ id: it.data.id, lat: it.data.place.lat, lng: it.data.place.lng, isPlace: true, leg_transport_mode: it.data.leg_transport_mode ?? null, incoming_leg_transport_mode: it.data.incoming_leg_transport_mode ?? null }) + cur.push({ id: it.data.id, assignmentId: it.data.id, lat: it.data.place.lat, lng: it.data.place.lng, isPlace: true, leg_transport_mode: it.data.leg_transport_mode ?? null, incoming_leg_transport_mode: it.data.incoming_leg_transport_mode ?? null }) curHasPlace = true } else if (it.type === 'transport') { const r = it.data @@ -749,7 +751,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const legsByDay: Record> = {} const hotelByDay: Record = {} - // One cached OSRM/plugin call per waypoint pair; shares RouteCalculator's cache. + // One cached OSRM/plugin call per leg, through the road trip's vias (#2580); shares RouteCalculator's cache. // tripId/dayId ride along for plugin route profiles (the server access-checks them). // Resolve a leg's mode (#1281): an explicit per-segment override wins, then // the day's saved default, then the live picker value. Sticky by design — the @@ -757,12 +759,9 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const dayDefaultMode = (dayId: number): string => days.find(d => d.id === dayId)?.default_transport_mode || routeProfile - const legBetween = async (a: { lat: number; lng: number }, b: { lat: number; lng: number }, dayId: number, mode: string): Promise => { - try { - const r = await calculateRouteWithLegs([a, b], { signal: controller.signal, profile: mode, tripId, dayId }) - return r.legs[0] ? { ...r.legs[0], mode } : undefined - } catch { return undefined } - } + const viaLookup = dayViaLookup(assignments, roadtripViasByDay) + const legBetween = (a: LegEnd, b: LegEnd, dayId: number, mode: string) => + routeLeg(a, b, viaLookup, { signal: controller.signal, profile: mode, tripId, dayId }, distanceUnit) // Collect every leg of every day first, then fetch them a few at a time. // Each result is stored under its own key, so nothing here depends on the @@ -784,7 +783,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const to = run[i + 1] const mode = resolveLegMode(from, to, dfMode) tasks.push(async () => { - const seg = await legBetween({ lat: from.lat, lng: from.lng }, { lat: to.lat, lng: to.lng }, dayId, mode) + const seg = await legBetween(from, to, dayId, mode) if (seg) dayLegs[from.id] = seg }) } @@ -829,7 +828,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { // routeDayIds is memoized from the same inputs as routeDayKey below, so keying the // effect on the string is equivalent while staying stable across unrelated renders. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [routeDayKey, routeProfile, mergedItemsMap, accommodations, days, optimizeFromAccommodation, distanceUnit, toursEnabled, places]) + }, [routeDayKey, routeProfile, mergedItemsMap, accommodations, days, optimizeFromAccommodation, distanceUnit, toursEnabled, places, roadtripViasByDay]) const openAddNote = (dayId, e) => { e?.stopPropagation() diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.ts b/client/src/components/Roadtrip/useRoadtripRoutes.ts index 14b3b4bd9..a4c0ffd41 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.ts @@ -14,6 +14,7 @@ import { dayWindow } from './dayWindow' import { useTranslation } from '../../i18n/TranslationContext' import { stayStartingOn } from '../../utils/dayMerge' import { staysAtTheirPlaces } from './nightBookend' +import { viaKeyOf } from '../Map/dayRouteVias' export type { RoadtripStop, RoadtripDay, RoadtripRoutes, QuietDay, AccessSpur, RoutedLeg, SnappedPoint } from '@trek/shared/roadtrip' @@ -402,15 +403,7 @@ export function useRoadtripRoutes( // Only the geometry decides whether legs have to be re-fetched: renaming a place or // editing its notes must not fire a routing round. - const viaKey = useMemo( - () => Object.entries(viasByDay) - .map(([dayId, vias]) => `${dayId}:${vias.map(v => `${v.after_order_index}@${v.lat.toFixed(5)},${v.lng.toFixed(5)}`).join('|')}`) - // Only the order has to be stable — this is a cache key, not a list anybody reads - // — but it has to be stable on purpose rather than by default. - .sort((a, b) => a.localeCompare(b)) - .join(';'), - [viasByDay], - ) + const viaKey = useMemo(() => viaKeyOf(viasByDay), [viasByDay]) const planKey = useMemo( () => `${plan.map(d => `${d.dayId}:${d.stops.map(stopKey).join('|')}`).join(';')}#${viaKey}`, diff --git a/client/src/hooks/useRouteCalculation.ts b/client/src/hooks/useRouteCalculation.ts index a2efe0bec..a044a8b1a 100644 --- a/client/src/hooks/useRouteCalculation.ts +++ b/client/src/hooks/useRouteCalculation.ts @@ -6,18 +6,25 @@ import { useSettingsStore } from '../store/settingsStore' import { useAddonStore } from '../store/addonStore' import { calculateRouteWithLegs, type RouteProfileKey } from '../components/Map/RouteCalculator' import { buildDayRouteRuns, hotelBookendOf, TRANSPORT_TYPES } from '../components/Map/dayRoutePlan' +import { dayViaLookup, foldViaLegs, threadVias, viaKeyOf, viasOnDayLeg } from '../components/Map/dayRouteVias' import { resolveLegMode } from '../components/Planner/legMode' +import type { RoadtripVia } from '@trek/shared' import type { TripStoreState } from '../store/tripStore' import type { RouteSegment, RouteResult, RouteVia, Accommodation } from '../types' const NO_ACCOMMODATIONS: Accommodation[] = [] +const NO_VIAS: Record = {} /** * Manages route calculation state for a selected day. Extracts geo-coded waypoints from * day assignments, draws a straight-line route immediately, then upgrades it to real OSRM * road geometry with per-segment durations. Aborts in-flight requests when the day changes. + * + * `roadtripVias` are the road trip's vias by day: a road picked there is the road this + * day drives too (#2580). Absent, or empty while the road trip addon is off. */ -export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: number | null, enabled: boolean = true, profile: RouteProfileKey = 'driving', accommodations: Accommodation[] = NO_ACCOMMODATIONS) { +export function useRouteCalculation(tripStore: Pick & { roadtripVias?: Record }, selectedDayId: number | null, enabled: boolean = true, profile: RouteProfileKey = 'driving', accommodations: Accommodation[] = NO_ACCOMMODATIONS) { + const viasByDay = tripStore.roadtripVias ?? NO_VIAS const [route, setRoute] = useState<[number, number][][] | null>(null) const [routeInfo, setRouteInfo] = useState(null) const [routeSegments, setRouteSegments] = useState([]) @@ -42,6 +49,7 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu // Recompute when the user flips km↔mi so leg distances (formatted at compute time) // refresh instead of showing stale cached text (#1300). const distanceUnit = useSettingsStore((s) => s.settings.distance_unit) + const viaKey = useMemo(() => viaKeyOf(viasByDay), [viasByDay]) const updateRouteForDay = useCallback(async (dayId: number | null) => { if (routeAbortRef.current) routeAbortRef.current.abort() @@ -61,6 +69,8 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu places, }) const day = allDays.find(d => d.id === dayId) + // Anchors count the stored list, the hidden lodging and service rows included. + const viaLookup = dayViaLookup(state.assignments || {}, viasByDay) const straightLines = (): [number, number][][] => runsWithHotel.map(r => r.map(p => [p.lat, p.lng] as [number, number])) @@ -119,12 +129,13 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu while (end < run.length - 1 && resolveLegMode(run[end], run[end + 1], dayDefaultMode) === mode) end++ const chunk = run.slice(i, end + 1) const straight = (): [number, number][] => chunk.map(p => [p.lat, p.lng] as [number, number]) + const { waypoints, stopAt } = threadVias(chunk, (from, to) => viasOnDayLeg(viaLookup, dayId, mode, from, to)) try { - const r = await calculateRouteWithLegs(chunk.map(p => ({ lat: p.lat, lng: p.lng })), { signal: controller.signal, profile: mode, tripId, dayId }) + const r = await calculateRouteWithLegs(waypoints, { signal: controller.signal, profile: mode, tripId, dayId }) pushCoords(r.coordinates.length >= 2 ? r.coordinates : straight(), mode === 'walking') // Leg k runs from chunk[k] to chunk[k + 1]; a hotel bookend says so on the // segment, which is how the phone timeline finds it (#2501). - r.legs.forEach((leg, k) => { + foldViaLegs(r.legs, stopAt, distanceUnit).forEach((leg, k) => { const hotelBookend = chunk[k + 1] ? hotelBookendOf(chunk[k], chunk[k + 1]) : undefined allLegs.push(hotelBookend ? { ...leg, mode, hotelBookend } : { ...leg, mode }) }) @@ -143,7 +154,7 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu // Aborted (day changed) — newer call owns the state. Anything else: keep straight lines. if (!(err instanceof Error) || err.name !== 'AbortError') { setRouteSegments([]); setRouteVias([]) } } - }, [enabled, profile, accommodations, optimizeFromAccommodation, distanceUnit, selectedDayDefaultMode, mirrorServiceStops, toursEnabled, places]) + }, [enabled, profile, accommodations, optimizeFromAccommodation, distanceUnit, selectedDayDefaultMode, mirrorServiceStops, toursEnabled, places, viasByDay]) // Stable signature for transport reservations on the selected day — changes when a transport // is added, removed, or repositioned, ensuring route recalc fires even on transport-only reorders. @@ -167,7 +178,7 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu if (!selectedDayId) { setRoute(null); setRouteWalking([]); setRouteSegments([]); setRouteVias([]); return } updateRouteForDay(selectedDayId) // eslint-disable-next-line react-hooks/exhaustive-deps - }, [selectedDayId, selectedDayAssignments, transportSignature, enabled, profile, accommodations, optimizeFromAccommodation, distanceUnit, selectedDayDefaultMode, mirrorServiceStops, toursEnabled, places]) + }, [selectedDayId, selectedDayAssignments, transportSignature, enabled, profile, accommodations, optimizeFromAccommodation, distanceUnit, selectedDayDefaultMode, mirrorServiceStops, toursEnabled, places, viaKey]) return { route, routeWalking, routeSegments, routeVias, routeInfo, setRoute, setRouteInfo, updateRouteForDay } } diff --git a/client/src/hooks/useRouteCalculation.vias.test.ts b/client/src/hooks/useRouteCalculation.vias.test.ts new file mode 100644 index 000000000..0137db615 --- /dev/null +++ b/client/src/hooks/useRouteCalculation.vias.test.ts @@ -0,0 +1,163 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import type { RoadtripVia } from '@trek/shared'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { buildAssignment, buildDay, buildPlace } from '../../tests/helpers/factories'; +import { resetAllStores } from '../../tests/helpers/store'; +import { useSettingsStore } from '../store/settingsStore'; +import { useTripStore } from '../store/tripStore'; +import type { Assignment, RouteSegment } from '../types'; +import { useRouteCalculation } from './useRouteCalculation'; + +const mocks = vi.hoisted(() => ({ route: vi.fn(), mirrorServiceStops: true })); +vi.mock('../components/Map/RouteCalculator', async (importActual) => ({ + ...(await importActual()), + calculateRouteWithLegs: mocks.route, +})); +vi.mock('./useRoadtripSettings', () => ({ + useRoadtripSettings: (select: (settings: { roadtrip_service_stops_in_days: boolean }) => unknown) => + select({ roadtrip_service_stops_in_days: mocks.mirrorServiceStops }), +})); + +const segment = (distance: number): RouteSegment => ({ + mid: [0, 0], + from: [0, 0], + to: [0, 0], + distance, + duration: distance / 20, + distanceText: '', + durationText: '', + walkingText: '', + drivingText: '', +}); + +const via = (afterOrderIndex: number, lat: number, lng: number, sequence = 0, dayId = 1): RoadtripVia => + ({ + id: sequence + afterOrderIndex * 10, + day_id: dayId, + after_order_index: afterOrderIndex, + sequence, + lat, + lng, + created_at: '', + }) as RoadtripVia; + +/** Paris, Brussels; the chosen road bends east of the direct one. */ +function seed(extra: Assignment[] = []) { + const paris = buildAssignment({ + id: 101, + day_id: 1, + order_index: 0, + place: buildPlace({ id: 1, lat: 48.85, lng: 2.29 }), + }); + const brussels = buildAssignment({ + id: 102, + day_id: 1, + order_index: 1, + place: buildPlace({ id: 2, lat: 50.89, lng: 4.34 }), + }); + const assignments = { '1': [paris, brussels, ...extra] }; + useTripStore.setState({ days: [buildDay({ id: 1 })], assignments, places: [], reservations: [] }); + return assignments; +} + +const askedLats = () => mocks.route.mock.calls.map((call) => call[0].map((point: { lat: number }) => point.lat)); + +beforeEach(() => { + resetAllStores(); + mocks.mirrorServiceStops = true; + mocks.route.mockReset().mockImplementation(async (points: Array<{ lat: number; lng: number }>) => ({ + coordinates: points.map((point) => [point.lat, point.lng]), + legs: points.slice(1).map((_, i) => segment(100_000 + i * 10_000)), + })); + useSettingsStore.setState((state) => ({ + settings: { ...state.settings, optimize_from_accommodation: false, distance_unit: 'metric' }, + })); +}); + +describe('useRouteCalculation with the road trip vias (#2580)', () => { + it('FE-HOOK-ROUTE-VIA-001: routes the day through the road chosen in the road trip', async () => { + const assignments = seed(); + const vias = { 1: [via(0, 49.85, 3.29)] }; + const { result } = renderHook(() => useRouteCalculation({ assignments, roadtripVias: vias }, 1, true, 'driving')); + + await waitFor(() => expect(result.current.routeSegments).toHaveLength(1)); + expect(askedLats()).toEqual([[48.85, 49.85, 50.89]]); + // One connector per stop pair, carrying the whole bent drive. + expect(result.current.routeSegments[0].distance).toBe(210_000); + expect(result.current.routeSegments[0].distanceText).toBe('210 km'); + expect(result.current.route).toEqual([ + [ + [48.85, 2.29], + [49.85, 3.29], + [50.89, 4.34], + ], + ]); + }); + + it('FE-HOOK-ROUTE-VIA-002: several vias on one leg keep their sequence', async () => { + const assignments = seed(); + const vias = { 1: [via(0, 50.2, 3.9, 1), via(0, 49.5, 3.0, 0)] }; + renderHook(() => useRouteCalculation({ assignments, roadtripVias: vias }, 1, true, 'driving')); + + await waitFor(() => expect(mocks.route).toHaveBeenCalled()); + expect(askedLats()).toEqual([[48.85, 49.5, 50.2, 50.89]]); + }); + + it('FE-HOOK-ROUTE-VIA-003: without vias the day routes exactly as before', async () => { + const assignments = seed(); + const { result } = renderHook(() => useRouteCalculation({ assignments }, 1, true, 'driving')); + + await waitFor(() => expect(result.current.routeSegments).toHaveLength(1)); + expect(askedLats()).toEqual([[48.85, 50.89]]); + expect(result.current.routeSegments[0].distance).toBe(100_000); + }); + + it('FE-HOOK-ROUTE-VIA-004: a hidden service stop between two stops keeps the vias off the shortcut', async () => { + mocks.mirrorServiceStops = false; + // Stored order: Paris (0), fuel stop (1), Brussels (2). The via leaves Paris towards + // the fuel stop, a leg the day list does not draw. + const fuel = buildAssignment({ + id: 103, + day_id: 1, + order_index: 0.5, + place: buildPlace({ id: 3, lat: 49.4, lng: 2.8, stop_type: 'fuel' }), + }); + const assignments = seed([fuel]); + const vias = { 1: [via(0, 49.1, 2.5)] }; + renderHook(() => useRouteCalculation({ assignments, roadtripVias: vias }, 1, true, 'driving')); + + await waitFor(() => expect(mocks.route).toHaveBeenCalled()); + expect(askedLats()).toEqual([[48.85, 50.89]]); + }); + + it('FE-HOOK-ROUTE-VIA-005: a lodging row the day list hides still counts in the anchor index', async () => { + // Stored order: hotel row (0), Paris (1), Brussels (2). The via behind Paris is filed + // at 1, because the road trip counts the hotel row the day list leaves out. + const hotel = buildAssignment({ + id: 104, + day_id: 1, + order_index: -1, + accommodation_id: 9, + place: buildPlace({ id: 4, lat: 48.8, lng: 2.3 }), + }); + const assignments = seed([hotel]); + const vias = { 1: [via(1, 49.85, 3.29)] }; + renderHook(() => useRouteCalculation({ assignments, roadtripVias: vias }, 1, true, 'driving')); + + await waitFor(() => expect(mocks.route).toHaveBeenCalled()); + expect(askedLats()).toEqual([[48.85, 49.85, 50.89]]); + }); + + it('FE-HOOK-ROUTE-VIA-006: a new choice redraws the day', async () => { + const assignments = seed(); + let vias: Record = {}; + const { rerender } = renderHook(() => useRouteCalculation({ assignments, roadtripVias: vias }, 1, true, 'driving')); + await waitFor(() => expect(mocks.route).toHaveBeenCalledTimes(1)); + + vias = { 1: [via(0, 49.85, 3.29)] }; + rerender(); + + await waitFor(() => expect(mocks.route).toHaveBeenCalledTimes(2)); + expect(askedLats()[1]).toEqual([48.85, 49.85, 50.89]); + }); +}); diff --git a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts index 4530799b0..361b07a6f 100644 --- a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts +++ b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts @@ -75,8 +75,9 @@ export function useMPlanTimeline(planner: TripPlanner) { // only computes segments while the manual "show route" toggle is on (map), so // we run a dedicated, always-enabled calculation just for the connectors and // use only its segments — the polyline it produces is ignored here. + // The road trip's vias bend these connectors the way the road was picked (#2580). const { routeSegments: connSegments } = useRouteCalculation( - { assignments } as unknown as Parameters[0], + { assignments, roadtripVias: planner.roadtripVias.byDay }, selectedDayId, true, routeProfile, diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index f3688095e..0ad8922cc 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -403,7 +403,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null { onDeletePlace={(placeId) => handleDeletePlace(placeId)} accommodations={tripAccommodations} routeShown={routeShown} - routeProfile={routeProfile} + routeProfile={routeProfile} roadtripViasByDay={roadtripVias.byDay} onToggleRoute={() => setRouteShown(v => !v)} onSetRouteProfile={setRouteProfile} onNavigateToFiles={() => handleTabChange('dateien')} @@ -622,7 +622,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null {

{mobileSidebarOpen === 'left' - ? { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId) }} onReorder={handleReorder} onReorderDays={handleReorderDays} onAddDay={handleAddDay} dayAdd={dayAdd} onDeleteDay={handleDeleteDay} deleteDayQuestion={deleteDayQuestion} onClearDay={can('day_edit', trip) ? handleClearDay : undefined} 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(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId) }} onReorder={handleReorder} onReorderDays={handleReorderDays} onAddDay={handleAddDay} dayAdd={dayAdd} onDeleteDay={handleDeleteDay} deleteDayQuestion={deleteDayQuestion} onClearDay={can('day_edit', trip) ? handleClearDay : undefined} 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} roadtripViasByDay={roadtripVias.byDay} 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/useRoadtripFeed.test.ts b/client/src/pages/tripPlanner/useRoadtripFeed.test.ts index b99672dc8..caa42977e 100644 --- a/client/src/pages/tripPlanner/useRoadtripFeed.test.ts +++ b/client/src/pages/tripPlanner/useRoadtripFeed.test.ts @@ -29,7 +29,6 @@ const rt = vi.hoisted(() => ({ }, boundaryArgs: [] as unknown[], vias: { byDay: {} as Record> }, - viasArgs: [] as unknown[], routes: { days: [] as Array>, quietDays: [] as unknown[], @@ -52,9 +51,6 @@ vi.mock('../../hooks/useRoadtripSettings', () => ({ vi.mock('../../components/Roadtrip/useDayBoundaries', () => ({ useDayBoundaries: (...args: unknown[]) => { rt.boundaryArgs = args; return rt.boundaries }, })) -vi.mock('../../components/Roadtrip/useRoadtripVias', () => ({ - useRoadtripVias: (...args: unknown[]) => { rt.viasArgs = args; return rt.vias }, -})) vi.mock('../../components/Roadtrip/useRefuelSearch', () => ({ useRefuelSearch: () => rt.refuel })) vi.mock('../../components/Roadtrip/useRoadtripRoutes', () => ({ useRoadtripRoutes: (...args: unknown[]) => { rt.routesArgs = args; return rt.routes }, @@ -107,6 +103,7 @@ function baseOptions(over: Partial = {}): Options { return { tripId: 42, trip, can, toast, t, isMobile: false, activeTab: 'plan', enabledAddons: { roadtrip: true }, roadtripMode: true, roadtripSettings: {}, + roadtripVias: rt.vias as unknown as Options['roadtripVias'], days, storedAssignments: stored, assignments: plan, tripAccommodations: [], reservations: [], places: [lake, castle], visibleConnections: [], routeProfile: 'driving', mapPlaces: [], expandedDayIds: null, ...over, @@ -143,7 +140,6 @@ describe('useRoadtripFeed', () => { expect(result.current.roadtripActive).toBe(false) expect(result.current.roadtripFeedActive).toBe(false) expect(rt.prefsArgs).toEqual([42, false]) - expect(rt.viasArgs).toEqual([42, false]) expect(rt.boundaryArgs[1]).toBe(false) const [tripId, routedDays, routedAssignments, profile, , , , routedReservations] = rt.routesArgs expect(tripId).toBe(42) @@ -170,7 +166,6 @@ describe('useRoadtripFeed', () => { expect(result.current.roadtripFeedActive).toBe(true) expect(result.current.dailyTimesActive).toBe(true) expect(rt.prefsArgs).toEqual([42, true]) - expect(rt.viasArgs).toEqual([42, true]) expect(rt.boundaryArgs).toEqual([42, true, plan]) expect(rt.routesArgs[1]).toBe(days) expect(rt.routesArgs[2]).toBe(stored) diff --git a/client/src/pages/tripPlanner/useRoadtripFeed.ts b/client/src/pages/tripPlanner/useRoadtripFeed.ts index c07c108d3..b8fd91c70 100644 --- a/client/src/pages/tripPlanner/useRoadtripFeed.ts +++ b/client/src/pages/tripPlanner/useRoadtripFeed.ts @@ -13,7 +13,8 @@ import { useFollowTrack } from '../../components/Roadtrip/useFollowTrack' import { useRefuelSearch } from '../../components/Roadtrip/useRefuelSearch' import { useRoadtripCorridor } from '../../components/Roadtrip/useRoadtripCorridor' import { useRoadtripRoutes } from '../../components/Roadtrip/useRoadtripRoutes' -import { useRoadtripVias } from '../../components/Roadtrip/useRoadtripVias' +import type { RoadtripVias } from '../../components/Roadtrip/useRoadtripVias' +import { roadtripStopList } from '../../components/Map/dayRouteVias' import { useLoadRoadtripSettings } from '../../hooks/useRoadtripSettings' import { roadtripPreferencesRepo } from '../../repo/roadtripPreferencesRepo' import { useAuthStore } from '../../store/authStore' @@ -35,6 +36,8 @@ interface RoadtripFeedOptions extends Pick - (storedAssignments[String(dayId)] ?? []) - .slice() - .sort((a, b) => (a.order_index ?? 0) - (b.order_index ?? 0)) - .filter(a => typeof a.place?.lat === 'number' && typeof a.place?.lng === 'number'), + const roadtripStopsOf = useCallback((dayId: number) => roadtripStopList(storedAssignments[String(dayId)]), [storedAssignments]) /** diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 2193b56ee..a6488f70c 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -55,6 +55,7 @@ import { usePlaceFormOpeners } from './usePlaceFormOpeners' import { useDayPlanEdits } from './useDayPlanEdits' import { useBookingEdits } from './useBookingEdits' import { useIsPhone } from '../../mobile/useIsPhone' +import { useRoadtripVias } from '../../components/Roadtrip/useRoadtripVias' /** * Trip planner page logic, the big one. Owns the trip store wiring, addon @@ -422,20 +423,31 @@ export function useTripPlanner() { places, assignments, selectedDayId, days, tripAccommodations, reservations, toursEnabled, }) - const { route, routeWalking, routeSegments, routeVias, routeInfo, setRoute, setRouteInfo, updateRouteForDay } = useRouteCalculation({ assignments } as any, selectedDayId, routeShown, routeProfile, tripAccommodations) + // Fed whenever the addon is on, not only while the mode is being looked at. + // The three handlers that re-anchor a day's vias (assign, remove, reorder) + // are reachable from the place inspector in both modes, and with an empty list + // they computed an empty correction and wrote nothing while the server kept + // every via pointing at a position that had moved. Deleting a stop with the + // switch off then left the detour on the wrong leg, for the traveller and for + // a collaborator who never turned the mode on at all. The day route and the + // overview draw them too, so a road picked in the road trip is the road the + // day drives (#2580). + const roadtripVias = useRoadtripVias(tripId, !!enabledAddons.roadtrip) + const { route, routeWalking, routeSegments, routeVias, routeInfo, setRoute, setRouteInfo, updateRouteForDay } = useRouteCalculation({ assignments, roadtripVias: roadtripVias.byDay }, selectedDayId, routeShown, routeProfile, tripAccommodations) // Road trip mode already draws the whole trip its own way, so the overview stands // down there rather than drawing a second set of lines over it. const overviewActive = overviewShown && !roadtripMode - const tripOverview = useTripRouteOverview(tripId, days, assignments, reservations, tripAccommodations, routeProfile, overviewActive, places) + const overviewVias = useMemo(() => ({ storedAssignments, viasByDay: roadtripVias.byDay }), [storedAssignments, roadtripVias.byDay]) + const tripOverview = useTripRouteOverview(tripId, days, assignments, reservations, tripAccommodations, routeProfile, overviewActive, places, overviewVias) // Called here so the road trip's effects keep their place, right after the overview's. const { - roadtripActive, roadtripFeedActive, roadtripRoutes, roadtripVias, roadtripCorridor, refuel, followTrack, + roadtripActive, roadtripFeedActive, roadtripRoutes, roadtripCorridor, refuel, followTrack, roadtripPreferencesState, dailyTimesActive, dayBoundaries, resetDayBoundaries, dayBoundaryControls, saveRoadtripLimit, roadtripStopsOf, viaLiesBefore, viasAfterInsert, roadtripConnections, roadtripViaCounts, collapsedRoadtripDays, toggleRoadtripDay, roadtripMapLines, roadtripMapPlaces, roadtripLineColors, dawarichHiddenDates, } = useRoadtripFeed({ - tripId, trip, can, toast, t, enabledAddons, roadtripMode, roadtripSettings, isMobile, activeTab, + tripId, trip, can, toast, t, enabledAddons, roadtripMode, roadtripSettings, roadtripVias, isMobile, activeTab, days, reservations, storedAssignments, assignments, tripAccommodations, places, mapPlaces, expandedDayIds, routeProfile, visibleConnections, })