From 34c1dc79b5905b852a8b77e3639d659a8606c40a Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 23 Sep 2026 12:03:00 +0200 Subject: [PATCH] fix(roadtrip): a drive bent through a via prints its whole length, and a refusal is said once A via cuts a drive into one routed leg per waypoint pair. Folding them back onto the stop pair summed the metres and seconds but kept the first piece's printed texts and end point, and the phone reads the texts first: a drive into the next day of 3.9 mi in 14 min showed 1.9 mi in 6 min. Every choice under "Other ways" writes vias, so every choice ran into it. The pieces are now merged by one shared helper that writes the texts from the sums in the reader's unit, for a day's own run, for the drive between two days and in the server's plan; the drive between two days is asked for again when the unit changes, as a day's run already is. On the desk a refused way was said in the bar's status line and again in a toast laid over that line. The desk keeps the status line; the phone, whose bar has no room for the sentence, keeps the toast. --- .../components/Roadtrip/useRoadtripRoutes.ts | 21 +++--- .../useTripPlanner.roadtrip.test.tsx | 44 +++++++---- .../src/pages/tripPlanner/useTripPlanner.ts | 12 ++- .../nest/roadtrip/roadtrip-plan.service.ts | 2 +- shared/src/roadtrip/routeRun.spec.ts | 74 +++++++++++++++++++ shared/src/roadtrip/routeRun.ts | 47 +++++++++--- 6 files changed, 161 insertions(+), 39 deletions(-) create mode 100644 shared/src/roadtrip/routeRun.spec.ts diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.ts b/client/src/components/Roadtrip/useRoadtripRoutes.ts index 881e3c823..45fabf069 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.ts @@ -1,5 +1,5 @@ import { useRoadtripSettings } from '../../hooks/useRoadtripSettings' -import { assembleRoadtrip, carrierLegsFor, carrierSeam, foldRouteRun, isCarrierMode, seatCarrierStops, standsAsDay, terminalAssignmentId, viasLeaving, type CarrierSeam, type RoadtripStop, type RoadtripRoutes, type PlanDay, type QuietDay, type RoutedLeg } from '@trek/shared/roadtrip' +import { assembleRoadtrip, carrierLegsFor, carrierSeam, foldRouteRun, isCarrierMode, mergeRouteSegments, seatCarrierStops, standsAsDay, terminalAssignmentId, viasLeaving, type CarrierSeam, type RoadtripStop, type RoadtripRoutes, type PlanDay, type QuietDay, type RoutedLeg } from '@trek/shared/roadtrip' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { avoidedClasses, calculateRouteWithLegs, routeEngineFor, RoutingRefusedError, type RouteEngine } from '../Map/RouteCalculator' import { resolveLegMode } from '../Planner/legMode' @@ -65,10 +65,13 @@ const stopKey = (s: RoadtripStop): string => * seam round and found every seam already answered, so the drive between two days kept * its old road and its old minutes until the page was loaded again. */ -const seamShape = (from: RoadtripStop, viasByDay: Record, mode: string, avoidKey: string): string => +const seamShape = (from: RoadtripStop, viasByDay: Record, mode: string, avoidKey: string, unit: string): string => [ mode, avoidKey, + // The seam's printed texts are in this unit, so a switch asks for it again, the way a + // day's own run is asked for again. + unit, viasLeaving(from, viasByDay[from.ownerDayId] ?? []) .map(v => `${v.lat.toFixed(5)},${v.lng.toFixed(5)}`) .join('|'), @@ -492,7 +495,7 @@ export function useRoadtripRoutes( collectedMisses[day.dayId] = [...new Set([...(collectedMisses[day.dayId] ?? []), ...missed])] } } - const folded = foldRouteRun(run, stopAt, r, mode) + const folded = foldRouteRun(run, stopAt, r, mode, distanceUnit) // Remembered per leg, so a picker opened on one of them knows the line on the // map is not its engine's and does not read it as the road already taken. if (r.avoidance?.fellBack) for (const key of Object.keys(folded)) collectedStandIns.add(key) @@ -615,7 +618,7 @@ export function useRoadtripRoutes( // nothing at all: the first answer was cached under the pair, and dragging the // point changed the request nobody was going to send again. const mode = legModeOf(from, to, dayId) - const shape = seamShape(from, viasByDay, mode, avoidKey) + const shape = seamShape(from, viasByDay, mode, avoidKey, distanceUnit) const have = seamLegs[legKey(from, to)] const current = have?.shape === shape // What the drawn join could not avoid is flagged on the card it arrives on, the way @@ -649,7 +652,7 @@ export function useRoadtripRoutes( } } return { seams: out, seamMisses: misses } - }, [chains, plan, quietDays, window, legsByDay, seamLegs, rideLegs, viasByDay, connectDays, legModeOf, avoidKey]) + }, [chains, plan, quietDays, window, legsByDay, seamLegs, rideLegs, viasByDay, connectDays, legModeOf, avoidKey, distanceUnit]) /** Each card's unavoided classes, its own run's and those of the join it is reached by. */ const missedOnCards = useMemo(() => { const out: Record = { ...missedByDay } @@ -718,14 +721,10 @@ export function useRoadtripRoutes( ) if (controller.signal.aborted) return // One leg per waypoint PAIR, so a shaped seam comes back in pieces and the rail - // wants the whole drive: summed here, exactly as a day run folds its via legs + // wants the whole drive: merged here, exactly as a day run folds its via legs // back onto the stop pair they belong to. if (!r.legs.length) continue - const merged = r.legs.length === 1 ? r.legs[0] : { - ...r.legs[0], - distance: r.legs.reduce((sum, l) => sum + (l.distance ?? 0), 0), - duration: r.legs.reduce((sum, l) => sum + (l.duration ?? 0), 0), - } + const merged = mergeRouteSegments(r.legs, distanceUnit) setSeamLegs(prev => ({ ...prev, [legKey(seam.from, seam.to)]: { diff --git a/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx b/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx index 008c70aeb..9821b16d5 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx +++ b/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx @@ -1121,10 +1121,10 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => { expect(rt.vias.addMany).not.toHaveBeenCalled() expect(rt.alt.close).not.toHaveBeenCalled() // Said to the bar as well, which keeps it beside the offers and announces it. + // Exactly this sentence: nothing about a ferry the offer never took. expect(rt.alt.settle).toHaveBeenCalledWith('The road trip’s router won’t follow this way, so it was not saved.') - expect(toasts).toContainEqual({ message: 'The road trip’s router won’t follow this way, so it was not saved.', type: 'error' }) - // Nothing about a ferry the offer never took. - expect(toasts).not.toContainEqual(expect.objectContaining({ message: expect.stringContaining('ferry') })) + // The desk's bar says it in its own status line, and a toast on top covered that line. + expect(toasts.filter(x => /router|ferry|ticked/.test(x.message))).toEqual([]) }) it('FE-TP-ROAD-118: a ferry the router would not board says how to get it driven', async () => { @@ -1138,10 +1138,9 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => { expect(rt.vias.addMany).not.toHaveBeenCalled() const ferryHint = 'This way crosses by ferry. Add the ferry as a transport booking and the drive follows it. If it lands the next day, put the stops across the water on that day.' - expect(toasts).toContainEqual({ message: ferryHint, type: 'info' }) // The ferry is the way out here, not the motorway the offer also leaves out. - expect(toasts).not.toContainEqual(expect.objectContaining({ message: expect.stringContaining('ticked') })) expect(rt.alt.settle).toHaveBeenCalledWith(`The road trip’s router won’t follow this way, so it was not saved. ${ferryHint}`) + expect(toasts.filter(x => /router|ferry|ticked/.test(x.message))).toEqual([]) }) it('FE-TP-ROAD-135: a way that leaves a class out says the trip has to avoid it to be driven', async () => { @@ -1155,10 +1154,29 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => { await act(async () => { await result.current.chooseRouteAlternative(1) }) expect(rt.vias.addMany).not.toHaveBeenCalled() - expect(toasts).toContainEqual({ - message: 'The road trip only drives this way with “Motorways” ticked under “Avoid where possible” in its settings.', - type: 'info', - }) + expect(rt.alt.settle).toHaveBeenCalledWith( + 'The road trip’s router won’t follow this way, so it was not saved. The road trip only drives this way with “Motorways” ticked under “Avoid where possible” in its settings.', + ) + expect(toasts.filter(x => /router|ferry|ticked/.test(x.message))).toEqual([]) + }) + + it('FE-TP-ROAD-139: on the phone a refused way is said as a toast too, its bar has no room for the sentence', async () => { + const desktopWidth = window.innerWidth + Object.defineProperty(window, 'innerWidth', { configurable: true, writable: true, value: 390 }) + try { + routedDay() + openWith([detour({ hasFerry: true, avoids: 'motorway', engine: 'valhalla' })]) + rt.legRoute.mockResolvedValue(answer(RAIL_LINE, { hasFerry: false })) + const { result } = await renderRoadtrip() + + await act(async () => { await result.current.chooseRouteAlternative(1) }) + + expect(rt.vias.addMany).not.toHaveBeenCalled() + expect(toasts).toContainEqual({ message: 'The road trip’s router won’t follow this way, so it was not saved.', type: 'error' }) + expect(toasts).toContainEqual(expect.objectContaining({ message: expect.stringContaining('crosses by ferry'), type: 'info' })) + } finally { + Object.defineProperty(window, 'innerWidth', { configurable: true, writable: true, value: desktopWidth }) + } }) it('FE-TP-ROAD-136: the router own road on a leg OSRM drew in its place asks the rail for the leg again, and says so', async () => { @@ -1279,8 +1297,8 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => { await act(async () => { await result.current.chooseRouteAlternative(1) }) expect(rt.vias.addMany).not.toHaveBeenCalled() - expect(rt.alt.settle).toHaveBeenCalled() - expect(toasts).toContainEqual({ message: 'The router is not answering right now.', type: 'error' }) + expect(rt.alt.settle).toHaveBeenCalledWith('The router is not answering right now.') + expect(toasts.filter(x => /router|ferry|ticked/.test(x.message))).toEqual([]) }) it('FE-TP-ROAD-124: a router that fails while checking says so and writes nothing', async () => { @@ -1292,8 +1310,8 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => { await act(async () => { await result.current.chooseRouteAlternative(1) }) expect(rt.vias.addMany).not.toHaveBeenCalled() - expect(rt.alt.settle).toHaveBeenCalled() - expect(toasts).toContainEqual({ message: 'The router is not answering right now.', type: 'error' }) + expect(rt.alt.settle).toHaveBeenCalledWith('The router is not answering right now.') + expect(toasts.filter(x => /router|ferry|ticked/.test(x.message))).toEqual([]) }) it('FE-TP-ROAD-125: a check abandoned by closing the picker ends in silence', async () => { diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 0aa4001ba..dc50ea6ce 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -1671,7 +1671,9 @@ export function useTripPlanner() { } catch { if (signal.aborted) return routeAlternatives.settle(t('roadtrip.alt.failed')) - toast.error(t('roadtrip.alt.failed')) + // The desk's bar says it in its own status line; a toast on top covered that line + // and printed the same sentence twice. The phone's bar has no room for a sentence. + if (isMobile) toast.error(t('roadtrip.alt.failed')) return } if (signal.aborted) return @@ -1679,8 +1681,10 @@ export function useTripPlanner() { const refusal = t(proof.fellBack ? 'roadtrip.alt.failed' : 'roadtrip.alt.notHeld') const hint = proof.fellBack ? null : refusalHint(offer, proof.last, t) routeAlternatives.settle(hint ? `${refusal} ${hint}` : refusal) - toast.error(refusal, 6000) - if (hint) toast.info(hint, 8000) + if (isMobile) { + toast.error(refusal, 6000) + if (hint) toast.info(hint, 8000) + } return } @@ -1718,7 +1722,7 @@ export function useTripPlanner() { if (open.standIn) toast.info(t('roadtrip.alt.standIn'), 8000) // A picker opened on another leg while this was written belongs to that leg now. if (!signal.aborted) routeAlternatives.close() - }, [routeAlternatives, roadtripVias, roadtripRoutes, toast, t]) + }, [routeAlternatives, roadtripVias, roadtripRoutes, toast, t, isMobile]) /** * A click on the drawn route puts a via there, and the drive is redrawn through it. diff --git a/server/src/nest/roadtrip/roadtrip-plan.service.ts b/server/src/nest/roadtrip/roadtrip-plan.service.ts index 9414e733f..898099d4a 100644 --- a/server/src/nest/roadtrip/roadtrip-plan.service.ts +++ b/server/src/nest/roadtrip/roadtrip-plan.service.ts @@ -299,7 +299,7 @@ export class RoadtripPlanService { }); Object.assign( allLegs, - foldRouteRun(stops, stopAt, { coordinates: routed.leg.line, legs, vias: routed.leg.vias }, profile), + foldRouteRun(stops, stopAt, { coordinates: routed.leg.line, legs, vias: routed.leg.vias }, profile, distanceUnit), ); missedByDay[dayId] = [...new Set([...(missedByDay[dayId] ?? []), ...routed.avoidMissed])]; stops.forEach((stop, index) => { diff --git a/shared/src/roadtrip/routeRun.spec.ts b/shared/src/roadtrip/routeRun.spec.ts new file mode 100644 index 000000000..f4214949a --- /dev/null +++ b/shared/src/roadtrip/routeRun.spec.ts @@ -0,0 +1,74 @@ +/** + * ROADTRIP-ROUTERUN-001..003: a drive the router answered in pieces reads as one drive. + * + * A via cuts a stop-to-stop drive into one leg per waypoint pair. The numbers were summed + * back together, but the printed texts stayed those of the first piece, and the phone reads + * a segment's text before its numbers: a 3.9 mi drive into the next day said "1.9 mi in + * 6 min" (#2461). + */ +import type { RoadtripStop, RouteSegment } from './planning-types'; +import { formatDurationShort } from './roadtripModel'; +import { foldRouteRun, mergeRouteSegments, roadtripLegKey } from './routeRun'; +import { formatDistance } from './units'; + +import { describe, expect, it } from 'vitest'; + +const piece = (from: [number, number], to: [number, number], distance: number, duration: number): RouteSegment => ({ + mid: [(from[0] + to[0]) / 2, (from[1] + to[1]) / 2], + from, + to, + distance, + duration, + distanceText: formatDistance(distance / 1000, 'imperial'), + durationText: formatDurationShort(duration), + drivingText: formatDurationShort(duration), + walkingText: '', +}); + +const A: [number, number] = [52.37, 4.89]; +const VIA: [number, number] = [52.36, 4.9]; +const B: [number, number] = [52.35, 4.87]; + +describe('mergeRouteSegments', () => { + it('ROADTRIP-ROUTERUN-001: the pieces of one drive add up, and their texts are written from the sums', () => { + const first = piece(A, VIA, 2996, 417); + const second = piece(VIA, B, 3280, 423); + + const merged = mergeRouteSegments([first, second], 'imperial'); + + expect(merged.distance).toBe(6276); + expect(merged.duration).toBe(840); + expect(merged.distanceText).toBe(formatDistance(6.276, 'imperial')); + expect(merged.distanceText).not.toBe(first.distanceText); + expect(merged.durationText).toBe('14 min'); + expect(merged.drivingText).toBe('14 min'); + // The drive ends where its last piece does, not at the via. + expect(merged.from).toEqual(A); + expect(merged.to).toEqual(B); + }); + + it('ROADTRIP-ROUTERUN-002: a drive the router answered in one piece is handed on as it came', () => { + const only = piece(A, B, 3000, 600); + + expect(mergeRouteSegments([only], 'metric')).toBe(only); + }); +}); + +describe('foldRouteRun', () => { + it('ROADTRIP-ROUTERUN-003: a stop pair bent through a via gets the whole drive and its texts, in the reader unit', () => { + const stops = [ + { lat: A[0], lng: A[1] }, + { lat: B[0], lng: B[1] }, + ] as unknown as RoadtripStop[]; + const legs = [piece(A, VIA, 2996, 417), piece(VIA, B, 3280, 423)]; + + const folded = foldRouteRun(stops, [0, 2], { coordinates: [A, VIA, B], legs }, 'driving', 'metric'); + const seg = folded[roadtripLegKey(stops[0]!, stops[1]!)]!.seg; + + expect(seg.distance).toBe(6276); + expect(seg.duration).toBe(840); + expect(seg.distanceText).toBe(formatDistance(6.276, 'metric')); + expect(seg.durationText).toBe('14 min'); + expect(seg.mode).toBe('driving'); + }); +}); diff --git a/shared/src/roadtrip/routeRun.ts b/shared/src/roadtrip/routeRun.ts index 854fe7aef..09db903ca 100644 --- a/shared/src/roadtrip/routeRun.ts +++ b/shared/src/roadtrip/routeRun.ts @@ -1,17 +1,51 @@ import { lineMetres, sliceAtMeters, projectOntoRoute } from './corridor'; -import type { RoadtripStop, RouteSegment, RouteVia, RoutedLeg } from './planning-types'; -import { legIndexForAlong } from './roadtripModel'; +import type { DistanceUnit, RoadtripStop, RouteSegment, RouteVia, RoutedLeg } from './planning-types'; +import { formatDurationShort, legIndexForAlong } from './roadtripModel'; +import { formatDistance } from './units'; export const roadtripStopKey = (stop: RoadtripStop): string => `${stop.lat.toFixed(5)},${stop.lng.toFixed(5)},${stop.legMode ?? ''},${stop.incomingLegMode ?? ''}`; export const roadtripLegKey = (from: RoadtripStop, to: RoadtripStop): string => `${roadtripStopKey(from)}>${roadtripStopKey(to)}`; +/** The walking pace a segment's walking time is printed at, in metres per second. */ +const WALKING_METRES_PER_SECOND = 5000 / 3600; + +/** + * The pieces a via cuts one stop-to-stop drive into, read as that one drive again. + * + * The router answers one leg per pair of waypoints, so a drive bent through a via comes + * back in pieces. The numbers add up. The printed texts do not: carried over from the first + * piece, they had the phone print "1.9 mi in 6 min" under a 3.9 mi drive, because it reads + * a segment's text before its numbers. So the texts are written again from the sums, in the + * reader's unit, and the drive ends where its last piece does rather than at the first via. + */ +export function mergeRouteSegments(parts: RouteSegment[], unit: DistanceUnit): RouteSegment { + const first = parts[0]!; + if (parts.length === 1) return first; + const last = parts[parts.length - 1]!; + const distance = parts.reduce((sum, leg) => sum + (leg.distance ?? 0), 0); + const duration = parts.reduce((sum, leg) => sum + (leg.duration ?? 0), 0); + const durationText = formatDurationShort(duration); + return { + ...first, + to: last.to, + mid: [(first.from[0] + last.to[0]) / 2, (first.from[1] + last.to[1]) / 2], + distance, + duration, + distanceText: formatDistance(distance / 1000, unit), + durationText, + drivingText: durationText, + walkingText: formatDurationShort(distance / WALKING_METRES_PER_SECOND), + }; +} + export function foldRouteRun( stops: RoadtripStop[], stopAt: number[], route: { coordinates: [number, number][]; legs: RouteSegment[]; vias?: RouteVia[] }, mode: string, + unit: DistanceUnit, ): Record { const spine = route.coordinates.map(([lat, lng]) => ({ lat, lng })); const cuts: { key: string; from: number; to: number; seg: RouteSegment }[] = []; @@ -19,14 +53,7 @@ export function foldRouteRun( for (let index = 0; index < stops.length - 1; index++) { const parts = route.legs.slice(stopAt[index], stopAt[index + 1]); if (!parts.length) continue; - const merged = - parts.length === 1 - ? parts[0]! - : { - ...parts[0]!, - distance: parts.reduce((sum, leg) => sum + leg.distance, 0), - duration: parts.reduce((sum, leg) => sum + leg.duration, 0), - }; + const merged = mergeRouteSegments(parts, unit); const from = travelled; travelled += merged.distance; cuts.push({ key: roadtripLegKey(stops[index]!, stops[index + 1]!), from, to: travelled, seg: { ...merged, mode } });