diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index 8cac042be..4eb009a13 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -2571,6 +2571,37 @@ describe('DayPlanSidebar', () => { expect(vi.mocked(reservationsApi.updatePositions).mock.calls[0][1]).toEqual([{ id: 69, day_plan_position: 0.5 }]) }) + it('FE-PLANNER-DAYPLAN-221: the slot is worked out over the hotel the list hides, so it does not land behind it (#2461)', async () => { + // The hotel a booking put on the day sits across the water and is no row of the list. + // Worked out over Amsterdam alone, the clock closed the day at 1.5: behind the hotel + // for the road trip, which then drove to Newcastle overland before the crossing. + const { reservationsApi } = await import('../../api/client') + const day = buildDay({ id: 10, date: '2026-10-06', title: 'Day 2' }) + const amsterdam = buildPlace({ id: 1, name: 'Amsterdam', lat: 52.3731, lng: 4.8926 }) + const hotel = buildPlace({ id: 3, name: 'Hotel Newcastle', lat: 54.975, lng: -1.61 }) + const ferry = buildReservation({ + id: 69, type: 'ferry', title: 'IJmuiden to Newcastle', day_id: 10, + reservation_time: '2026-10-06T17:30', reservation_end_time: '2026-10-06T23:00', + endpoints: [ + { role: 'from', sequence: 0, name: 'IJmuiden', code: null, lat: 52.4581, lng: 4.5879, timezone: null, local_date: null, local_time: null }, + { role: 'to', sequence: 1, name: 'Port of Tyne', code: null, lat: 54.9925, lng: -1.4522, timezone: null, local_date: null, local_time: null }, + ], + }) + seedStore(useTripStore, { reservations: [ferry] }) + render() + await waitFor(() => expect(vi.mocked(reservationsApi.updatePositions)).toHaveBeenCalled()) + expect(vi.mocked(reservationsApi.updatePositions).mock.calls[0][1]).toEqual([{ id: 69, day_plan_position: 0.5 }]) + }) + it('FE-PLANNER-DAYPLAN-204: a rejected slot write puts the bookings back where the server has them', async () => { const { reservationsApi } = await import('../../api/client') vi.mocked(reservationsApi.updatePositions).mockRejectedValueOnce(new Error('offline')) diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 899393b29..e98d3cf98 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -35,7 +35,7 @@ import { isDayInAccommodationRange, getAccommodationAnchors, getDayBookendHotels import { TRANSPORT_TYPES, parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getDisplayTimeForDay, getTransportRouteEndpoints, getTransportForDay as _getTransportForDay, getMergedItems as _getMergedItems, isCarrierTransport, hasCarrierEndpointOnDay, - getAssignmentReservations, timedSlot, rideSeatKey, + getAssignmentReservations, timedSlot, storedRideSlot, type MergedItem, } from '../../utils/dayMerge' import { withinDriveRange } from '../../utils/geo' @@ -438,14 +438,15 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { .slice().sort((a, b) => a.order_index - b.order_index) // Compute initial day_plan_position for a transport based on time - const computeTransportPosition = (r, da) => { + const computeTransportPosition = (r, da, dayId) => { // A ride that lands today goes where it adds the least road between the same clocks, - // by the rule the list and the road trip seat it with (`rideSeatKey`). This slot is - // stored, so seated by the clock alone a ferry between two untimed stops stayed at - // the end of the day, and the drive went overland before the crossing (#2461). - const seat = rideSeatKey(r, da.map(a => ({ type: 'place' as const, sortKey: a.order_index, data: a }))) - if (seat === -Infinity) return Math.min(...da.map(a => a.order_index)) - 0.5 - if (seat !== null) return seat + 0.5 + // by the rule the road trip seats it with. This slot is stored, so seated by the + // clock alone a ferry between two untimed stops stayed at the end of the day, and the + // drive went overland before the crossing (#2461). Worked out over every stored row of + // the day rather than `da`: the drive stops at the hotel and the service stops the + // list hides, and the slot is read against their order indexes too. + const ride = storedRideSlot(r, assignments[String(dayId)] || [], accommodations, dayId) + if (ride !== null) return ride const minutes = parseTimeToMinutes(r.reservation_time) ?? 0 // Find the last place with time <= transport time let afterIdx = -1 @@ -469,7 +470,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { ) const positions = sorted.map((r, idx) => ({ id: r.id, - day_plan_position: computeTransportPosition(r, da) + idx * 0.01, + day_plan_position: computeTransportPosition(r, da, dayId) + idx * 0.01, })) // Mark as initialized immediately to prevent re-entry for (const p of positions) initedTransportIds.current.add(p.id) diff --git a/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx b/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx index 6e414e608..4bba1ca18 100644 --- a/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx +++ b/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx @@ -8,7 +8,7 @@ import type { LegAlternatives } from './useRouteAlternatives' import { openLeg } from '../../../tests/helpers/legAlternatives' /** - * FE-ALTBAR-001..012: the ways of driving one leg, offered over the map. + * FE-ALTBAR-001..014: the ways of driving one leg, offered over the map. * * Every value shown comes from the same overlay the map draws. Working them out * here separately is what once made the list label the driven route "Fastest" @@ -27,6 +27,7 @@ const overlay = (over: Partial = {}): AlternativeOverlay => distance: 290_000, slowerThanQuickest: 0, otherEngine: false, + engine: 'osrm', labelBg: '#0a84ff', at: { lat: 53, lng: 11 }, ...over, @@ -143,19 +144,20 @@ describe('RoadtripAlternativesBar', () => { expect(onClose).toHaveBeenCalled() }) - it('FE-ALTBAR-010: an offer the other engine timed says so, and the rest do not', () => { + it('FE-ALTBAR-010: an offer the other engine timed says so, naming that engine, and the rest do not', () => { render( , ) expect(screen.getAllByLabelText(/avoidance router/i)).toHaveLength(1) + expect(screen.queryByLabelText(/main router/i)).toBeNull() }) it('FE-ALTBAR-011: while a choice is checked and saved the list stands still, and the one in hand says so', () => { @@ -171,7 +173,7 @@ describe('RoadtripAlternativesBar', () => { />, ) const [, current, checked, other] = screen.getAllByRole('button') - expect([current, checked, other].every(chip => (chip as HTMLButtonElement).disabled)).toBe(true) + expect([current, checked, other].every(chip => chip.getAttribute('aria-disabled') === 'true')).toBe(true) expect(checked).toHaveAttribute('aria-busy', 'true') expect(other).toHaveAttribute('aria-busy', 'false') expect(checked.parentElement).toHaveAttribute('aria-busy', 'true') @@ -186,7 +188,44 @@ describe('RoadtripAlternativesBar', () => { , ) const chips = screen.getAllByRole('button').slice(1) - expect(chips.every(chip => !(chip as HTMLButtonElement).disabled)).toBe(true) + expect(chips.every(chip => chip.getAttribute('aria-disabled') === 'false')).toBe(true) expect(chips[0].parentElement).toHaveAttribute('aria-busy', 'false') }) + + it('FE-ALTBAR-013: a keyboard keeps its place on the chip it chose while the check runs', () => { + // A disabled button hands its focus to the page, so Enter on a chip left the keyboard + // nowhere in the bar that was still open for another choice. + const onChoose = vi.fn() + const overlays = [overlay({ index: 0, note: 'Current' }), overlay({ index: 1 })] + const { rerender } = render( + , + ) + const chip = screen.getAllByRole('button')[2] + chip.focus() + fireEvent.click(chip) + expect(onChoose).toHaveBeenCalledWith(1) + rerender() + expect(document.activeElement).toBe(chip) + rerender() + expect(document.activeElement).toBe(chip) + }) + + it('FE-ALTBAR-014: the bar says a check is running and why the last one saved nothing, in a live line', () => { + const overlays = [overlay({ index: 0, note: 'Current' }), overlay({ index: 1 })] + const { rerender } = render( + , + ) + // There before it has anything to say, so a screen reader is listening when it does. + const status = screen.getByRole('status') + expect(status).toBeEmptyDOMElement() + expect(status).toHaveClass('sr-only') + + rerender() + expect(screen.getByRole('status')).toHaveTextContent(/checking this way/i) + + const notice = 'The road trip’s router won’t follow this way, so it was not saved.' + rerender() + expect(screen.getByRole('status')).toHaveTextContent(notice) + expect(screen.getByRole('status')).not.toHaveClass('sr-only') + }) }) diff --git a/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx b/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx index 9de8a62ac..83f70c518 100644 --- a/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx +++ b/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx @@ -5,7 +5,7 @@ import { Tooltip } from '../shared/Tooltip' import { useSettingsStore } from '../../store/settingsStore' import { formatDistance } from '../../utils/units' import type { LegAlternatives } from './useRouteAlternatives' -import { alternativesBusy, alternativesPhase, alternativeSubline, type AlternativeOverlay } from './alternativeOverlays' +import { alternativesBusy, alternativesPhase, alternativeSubline, otherEngineNote, type AlternativeOverlay } from './alternativeOverlays' interface RoadtripAlternativesBarProps { open: LegAlternatives | null @@ -38,6 +38,10 @@ export default function RoadtripAlternativesBar({ // While a choice is checked against the router and written, the list stands still: a // second click would race the first, and its answer could land on the leg after it. const busy = alternativesBusy(open) + // What the check is doing, and why the last one saved nothing. A toast says the latter + // too, but it is announced to nobody and gone in seconds, while this line sits beside + // the chips it is about. + const status = busy ? t('roadtrip.alt.checking') : (open.notice ?? '') return (
@@ -68,53 +72,60 @@ export default function RoadtripAlternativesBar({

{t('roadtrip.alt.onlyOne')}

) : (
- {overlays.map(alt => ( - - ))} + {/* Only where the drive time on the map came from the other engine. The + road and the length are the same question either way; the time is not, + and two engines' times sitting next to each other on the map invite a + subtraction that means nothing. Saying so here, where the choice is + actually made, is cheaper than a second request per offer to restate + the whole list in one engine's terms. */} + {note ? ( + + + + ) : null} + + ) + })}
)} + {/* Kept in the tree while empty, so a screen reader is listening before it speaks. */} +

{status}

) } diff --git a/client/src/components/Roadtrip/RoadtripSidebar.test.tsx b/client/src/components/Roadtrip/RoadtripSidebar.test.tsx index d3acb2862..4c06c9185 100644 --- a/client/src/components/Roadtrip/RoadtripSidebar.test.tsx +++ b/client/src/components/Roadtrip/RoadtripSidebar.test.tsx @@ -1418,7 +1418,7 @@ describe('RoadtripSidebar with the drive in from the day before (#2461)', () => wrap() const notice = screen.getByRole('status') - expect(within(notice).getByText('IJmuiden to Newcastle has no date, so the drive does not use it.')).toBeInTheDocument() + expect(within(notice).getByText('IJmuiden to Newcastle is on none of this trip’s days, so the drive does not use it.')).toBeInTheDocument() expect(within(notice).getAllByRole('listitem')).toHaveLength(1) fireEvent.click(within(notice).getByRole('button', { name: 'Open booking' })) expect(onOpenBooking).toHaveBeenCalledWith(70) @@ -1426,7 +1426,7 @@ describe('RoadtripSidebar with the drive in from the day before (#2461)', () => it('FE-ROADTRIP-SIDEBAR-063: says so without a button where bookings cannot be opened, and says nothing when every ride has a day', () => { const reader = wrap() - expect(screen.getByText('IJmuiden to Newcastle has no date, so the drive does not use it.')).toBeInTheDocument() + expect(screen.getByText('IJmuiden to Newcastle is on none of this trip’s days, so the drive does not use it.')).toBeInTheDocument() expect(screen.queryByRole('button', { name: 'Open booking' })).toBeNull() reader.unmount() diff --git a/client/src/components/Roadtrip/alternativeOverlays.test.ts b/client/src/components/Roadtrip/alternativeOverlays.test.ts index f8cdc6d9d..a2088b50c 100644 --- a/client/src/components/Roadtrip/alternativeOverlays.test.ts +++ b/client/src/components/Roadtrip/alternativeOverlays.test.ts @@ -1,11 +1,11 @@ import { describe, it, expect } from 'vitest' -import { alternativeSubline, alternativesBusy, alternativesPhase, buildAlternativeOverlays, type AlternativeOverlay } from './alternativeOverlays' +import { alternativeSubline, alternativesBusy, alternativesPhase, buildAlternativeOverlays, otherEngineNote, type AlternativeOverlay } from './alternativeOverlays' import { formatDurationShort } from './roadtripModel' import { ALT_PRIMARY, ALT_SECONDARY, ALT_LABEL_PRIMARY_BG, ALT_LABEL_SECONDARY_BG } from './alternativeColors' import type { RouteAlternative } from '../Map/RouteCalculator' /** - * FE-ALTOVL-001..021: turning the router's answers into something drawable. + * FE-ALTOVL-001..023: turning the router's answers into something drawable. * * Two decisions live here and neither is cosmetic: which of the offered roads is * drawn as the one you are on, and where each label hangs. A label anchored on a @@ -309,6 +309,47 @@ describe('alternativeSubline', () => { }) }) +describe('otherEngineNote', () => { + it('FE-ALTOVL-022: a route another engine timed is named after that engine, one the rail timed not at all', () => { + // On a plugin leg the offers are OSRM's. They used to be called the avoidance + // router's, an engine that had nothing to do with them. + const plugin = buildAlternativeOverlays( + [ + alt({ coordinates: line(20, 0), current: true, engine: 'plugin' }), + alt({ coordinates: line(20, 1) }), + alt({ coordinates: line(20, 2), engine: 'valhalla', avoids: 'motorway' }), + ], + LABELS, + 'plugin', + ) + expect(plugin.map(o => o.engine)).toEqual(['plugin', 'osrm', 'valhalla']) + expect(plugin.map(otherEngineNote)).toEqual([null, 'roadtrip.alt.otherEngineStandard', 'roadtrip.alt.otherEngine']) + // On a trip that avoids nothing, the avoidance router's offer is the other one. + const plain = buildAlternativeOverlays( + [alt({ coordinates: line(20, 0), current: true }), alt({ coordinates: line(20, 1), engine: 'valhalla' })], + LABELS, + ) + expect(plain.map(otherEngineNote)).toEqual([null, 'roadtrip.alt.otherEngine']) + }) + + it('FE-ALTOVL-023: a line OSRM drew in the avoidance router’s place heads the list as the other engine’s', () => { + const out = buildAlternativeOverlays( + [ + alt({ coordinates: line(20, 0), current: true, engine: 'osrm' }), + alt({ coordinates: line(20, 1), engine: 'valhalla' }), + ], + LABELS, + 'valhalla', + ) + expect(out.map(otherEngineNote)).toEqual(['roadtrip.alt.otherEngineStandard', null]) + // Nothing is measured against it: the offer is the quickest the rail's engine timed. + expect(out[1].note).toBe('Fastest') + expect(out.every(o => o.slowerThanQuickest === 0)).toBe(true) + // An engine no offer comes from has nothing to say. + expect(otherEngineNote({ otherEngine: true, engine: 'plugin' })).toBeNull() + }) +}) + describe('alternativesBusy', () => { it('FE-ALTOVL-021: a picker is busy exactly while one of its offers is being checked', () => { expect(alternativesBusy(null)).toBe(false) diff --git a/client/src/components/Roadtrip/alternativeOverlays.ts b/client/src/components/Roadtrip/alternativeOverlays.ts index aa59b5143..293d58478 100644 --- a/client/src/components/Roadtrip/alternativeOverlays.ts +++ b/client/src/components/Roadtrip/alternativeOverlays.ts @@ -24,6 +24,8 @@ export interface AlternativeOverlay { * `slowerThanQuickest` stays zero. */ otherEngine: boolean + /** The engine that priced this route, which names the other engine when it is one. */ + engine: RouteEngine labelBg: string /** Where to hang the label — a point on this route and on no other. */ at: { lat: number; lng: number } @@ -157,6 +159,7 @@ export function buildAlternativeOverlays( ? 0 : Math.max(0, Math.round(route.duration - routes[quickest].duration)), otherEngine, + engine: engineOf(route), labelBg: primary ? ALT_LABEL_PRIMARY_BG : ALT_LABEL_SECONDARY_BG, at: at ?? { lat: 0, lng: 0 }, } @@ -203,6 +206,22 @@ export function alternativeSubline(alt: AlternativeOverlay, slower: (time: strin return slower(formatDurationShort(alt.slowerThanQuickest)) } +/** + * What a route another engine timed says about it, as a translation key: which engine it + * was. Null for a route the rail's own engine timed. + * + * Both bars used to call every such route the avoidance router's. A leg a route provider + * plugin drives is offered OSRM's ways, and a leg OSRM drew while the avoidance router did + * not answer heads its list with OSRM's line, so the note named an engine that had nothing + * to do with either. One reading for the desk and the phone. + */ +export function otherEngineNote(alt: Pick): string | null { + if (!alt.otherEngine) return null + if (alt.engine === 'valhalla') return 'roadtrip.alt.otherEngine' + // No offer comes from a plugin: only the rail's own line on a plugin leg is one's. + return alt.engine === 'osrm' ? 'roadtrip.alt.otherEngineStandard' : null +} + /** * Whether a choice is being checked and saved, so neither bar takes another one meanwhile. * One reading for the desk and the phone, which both stand still while it runs. diff --git a/client/src/components/Roadtrip/alternativePins.test.ts b/client/src/components/Roadtrip/alternativePins.test.ts index fc25b131b..8145b587c 100644 --- a/client/src/components/Roadtrip/alternativePins.test.ts +++ b/client/src/components/Roadtrip/alternativePins.test.ts @@ -1,10 +1,10 @@ import { describe, it, expect, vi } from 'vitest' -import { MAX_PINS, pinAlternative, railDriveOn, railLegAt } from './alternativePins' +import { MAX_PINS, pinAlternative, railDriveOn, railLegAt, refusalHint } from './alternativePins' import type { RailLegRoute, RailLegRouter, RoadtripDay, RoadtripStop } from './useRoadtripRoutes' import type { OfferedRoute } from './useRouteAlternatives' /** - * FE-ALTPIN-001..011: proving a choice with the rail's own router before it is saved. + * FE-ALTPIN-001..012: proving a choice with the rail's own router before it is saved. * * A choice used to be one via at the point where the offer strayed furthest, written * without asking whether the router then drove the offer. On a ferry it did not (OSRM @@ -182,3 +182,24 @@ describe('railDriveOn', () => { expect(railDriveOn(card({ legLines: undefined }), { kind: 'leg', index: 0 })?.line).toBeUndefined() }) }) + +describe('refusalHint', () => { + /** Echoes the key with its values, so a case reads which sentence was picked and with what. */ + const t = (key: string, params?: Record) => + params ? `${key}(${Object.entries(params).map(([k, v]) => `${k}=${v}`).join(',')})` : key + + it('FE-ALTPIN-012: a refused way says what would get it driven: a ferry booking, or the class left out avoided', () => { + // The ferry the router would not board wins: the crossing is a booking, whatever else + // the way leaves out. + expect(refusalHint({ hasFerry: true, avoids: 'motorway' }, { hasFerry: false }, t)).toBe('roadtrip.alt.ferryNotHeld') + // A way without the motorway on a trip that does not avoid it is held for a few pins + // at most; ticking the class is what drives it. + expect(refusalHint({ avoids: 'motorway' }, { hasFerry: false }, t)) + .toBe('roadtrip.alt.avoidNotHeld(class=roadtrip.avoid.motorway,setting=roadtrip.avoid.section)') + expect(refusalHint({ avoids: 'toll' }, {}, t)) + .toBe('roadtrip.alt.avoidNotHeld(class=roadtrip.avoid.toll,setting=roadtrip.avoid.section)') + // A plain way, or a ferry the router did board, has no way out to offer. + expect(refusalHint({}, {}, t)).toBeNull() + expect(refusalHint({ hasFerry: true }, { hasFerry: true }, t)).toBeNull() + }) +}) diff --git a/client/src/components/Roadtrip/alternativePins.ts b/client/src/components/Roadtrip/alternativePins.ts index c4d886f1c..55b9dc4d9 100644 --- a/client/src/components/Roadtrip/alternativePins.ts +++ b/client/src/components/Roadtrip/alternativePins.ts @@ -84,6 +84,25 @@ export async function pinAlternative({ } } +/** + * The way out of a refusal, where there is one, worded by `t`; null where there is none. + * + * A crossing by ferry is a booking, and the drive follows a booked ferry from terminal to + * terminal. A way offered because it leaves a class of road out is driven once the trip + * avoids that class: without it the router holds such a way only as far as a few pins + * reach, and on a long leg takes the class back between them, so the choice was refused + * after seconds of checking with no word on what would work. + */ +export function refusalHint( + offer: Pick, + last: Pick, + t: (key: string, params?: Record) => string, +): string | null { + if (offer.hasFerry && !last.hasFerry) return t('roadtrip.alt.ferryNotHeld') + if (!offer.avoids) return null + return t('roadtrip.alt.avoidNotHeld', { class: t(`roadtrip.avoid.${offer.avoids}`), setting: t('roadtrip.avoid.section') }) +} + /** * The leg the rail drives from the stop filed at `anchor` now, or null when no stop sits * there any more. diff --git a/client/src/components/Roadtrip/reanchorVias.test.ts b/client/src/components/Roadtrip/reanchorVias.test.ts index 8fe6345b8..3430551ee 100644 --- a/client/src/components/Roadtrip/reanchorVias.test.ts +++ b/client/src/components/Roadtrip/reanchorVias.test.ts @@ -207,14 +207,16 @@ describe('the drive into the next day (#2461)', () => { expect(single.remove).toEqual([1]) }) - it('FE-REANCHOR-026: a last stop dragged up the day takes its drive along; a stop dragged behind it splits that drive', () => { + it('FE-REANCHOR-026: a last stop dragged up the day takes its drive along, and so does a stop dragged behind it', () => { // Left alone, the via sat at an index past the end of the day. const up = reanchorAfterReorder([via(1, 0), via(9, 3)], 3, 1, 4) expect(up.remove).toEqual([9]) expect(anchors(up.vias)).toEqual({}) - // Behind the old last stop now comes the moved one, and the point stays on the road - // leaving the old last stop, the way an insert into any leg keeps its first half. - expect(anchors(reanchorAfterReorder([via(9, 3)], 1, 3, 4).vias)).toEqual({ 9: 2 }) + // Behind the old last stop now comes the moved one. The point lay on the road into the + // next day, not on the way to the stop that took its place, so it goes as well. + const behind = reanchorAfterReorder([via(9, 3)], 1, 3, 4) + expect(behind.remove).toEqual([9]) + expect(behind.vias).toEqual([]) // Anything moved in front of the last stop leaves it where it was. expect(isEmptyReanchoring(reanchorAfterReorder([via(9, 3)], 0, 2, 4))).toBe(true) }) @@ -230,4 +232,16 @@ describe('the drive into the next day (#2461)', () => { expect(plan.vias, `${from}->${to}`).toEqual([]) } }) + + it('FE-REANCHOR-029: under Days and in the server’s sort, a last stop moved up the day takes its drive along too', () => { + // It followed its stop instead, and the drive from 40 to 20 inside the day went + // through a point on the road to tomorrow, for everybody on the trip. + const up = reanchorByStopOrder([via(1, 0), via(9, 3)], [10, 20, 30, 40], [10, 40, 20, 30]) + expect(up.remove).toEqual([9]) + expect(anchors(up.vias)).toEqual({}) + const front = reanchorByStopOrder([via(9, 2)], [10, 20, 30], [30, 10, 20]) + expect(front).toEqual({ vias: [], remove: [9] }) + // Two stops swapped: the drive out of the one that is first now goes here as well. + expect(reanchorByStopOrder([via(9, 1)], [10, 20], [20, 10])).toEqual({ vias: [], remove: [9] }) + }) }) diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.test.ts b/client/src/components/Roadtrip/useRoadtripRoutes.test.ts index 732894ab1..0d75878e9 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.test.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.test.ts @@ -801,6 +801,41 @@ describe('useRoadtripRoutes', () => { clock.mockRestore() } }) + + it('FE-ROADTRIP-ROUTES-051: a join OSRM answered in the engine’s place is drawn, flagged, and asked for again on request', async () => { + // Filed like any answer, it stood in for the weighed road until the page was loaded + // again, and a picker on it read the stand-in line as the road already taken. + let now = 10_000 + const clock = vi.spyOn(performance, 'now').mockImplementation(() => now) + try { + setting({ roadtrip_connect_days: true, roadtrip_avoid: 'toll' }) + const standIn = { ...routed(1), avoidance: { asked: ['toll'], achieved: [], fellBack: true } } + calculateRouteWithLegs.mockImplementation((wp: { lat: number }[]) => + Promise.resolve(wp[0].lat === LUENEBURG[0] ? standIn : routed(1))) + const { days: daysList, assignments } = twoDays() + const { result } = renderHook(() => useRoadtripRoutes(7, daysList, assignments)) + await waitFor(() => expect(result.current.lines).toHaveLength(3)) + await waitFor(() => expect(result.current.loading).toBe(false)) + const lueneburg = result.current.days[0].stops[1] + const berlin = result.current.days[1].stops[0] + expect(result.current.legRouter!(lueneburg, berlin, 2).standIn).toBe(true) + // The day runs came from the engine asked. + expect(result.current.legRouter!(result.current.days[0].stops[0], lueneburg, 1).standIn).toBe(false) + // Flagged on the card the join arrives on, the way a day's own run is. + expect(result.current.days[1].avoidMissed).toEqual(['toll']) + expect(result.current.days[0].avoidMissed ?? []).toEqual([]) + + const asked = askedFor(LUENEBURG, BERLIN).length + now += 10_000 + calculateRouteWithLegs.mockImplementation(() => Promise.resolve(routed(1))) + act(() => { result.current.reroute!() }) + await waitFor(() => expect(askedFor(LUENEBURG, BERLIN).length).toBeGreaterThan(asked)) + await waitFor(() => expect(result.current.legRouter!(lueneburg, berlin, 2).standIn).toBe(false)) + expect(result.current.days[1].avoidMissed ?? []).toEqual([]) + } finally { + clock.mockRestore() + } + }) }) }) @@ -1109,4 +1144,22 @@ describe('a leg asked for again on demand', () => { expect(result.current.days[0].avoidMissed).toEqual(['ferry', 'toll']) }) + + it('FE-ROADTRIP-ROUTES-050: a leg the stand-in drew says so to whoever asks, and rerouting asks the engine again', async () => { + // The rail keeps a day run until its stops or vias change, so a choice of the leg's + // own road, which writes nothing, left the stand-in line on the map. + act(() => { useSettingsStore.setState(s => ({ settings: { ...s.settings, roadtrip_avoid: 'toll' } })) }) + calculateRouteWithLegs.mockResolvedValue({ ...routed(1), avoidance: { asked: ['toll'], achieved: [], fellBack: true } }) + const { result } = renderHook(() => useRoadtripRoutes(7, [day(1, 1)], map(1, [{ id: 1, at: HAMBURG }, { id: 2, at: BERLIN }]))) + await waitFor(() => expect(result.current.loading).toBe(false)) + const [from, to] = result.current.days[0].stops + expect(result.current.legRouter!(from, to, 1)).toMatchObject({ engine: 'valhalla', standIn: true }) + expect(calculateRouteWithLegs).toHaveBeenCalledTimes(1) + + calculateRouteWithLegs.mockResolvedValue({ ...routed(1), avoidance: { asked: ['toll'], achieved: ['toll'], fellBack: false } }) + act(() => { result.current.reroute!() }) + await waitFor(() => expect(calculateRouteWithLegs).toHaveBeenCalledTimes(2)) + await waitFor(() => expect(result.current.legRouter!(from, to, 1).standIn).toBe(false)) + expect(result.current.days[0].avoidMissed ?? []).toEqual([]) + }) }) diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.ts b/client/src/components/Roadtrip/useRoadtripRoutes.ts index b193e67bc..881e3c823 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.ts @@ -11,6 +11,7 @@ import type { Assignment, AssignmentsMap, Accommodation, Day, Reservation, Route import type { RoadtripVia, RoadtripDayBoundary } from '@trek/shared' import { dayWindow } from './dayWindow' import { useTranslation } from '../../i18n/TranslationContext' +import { stayStartingOn } from '../../utils/dayMerge' export type { RoadtripStop, RoadtripDay, RoadtripRoutes, QuietDay, AccessSpur, RoutedLeg, SnappedPoint } from '@trek/shared/roadtrip' @@ -73,11 +74,19 @@ const seamShape = (from: RoadtripStop, viasByDay: Record, .join('|'), ].join('#') +/** + * A drive between two days as it was answered: the leg, the shape it was asked for + * (`seamShape`), whether OSRM answered in place of the engine asked (`standIn`), and the + * classes the answer could not avoid. + */ +type SeamLeg = RoutedLeg & { shape: string; standIn?: boolean; missed?: RouteAvoidClass[] } + /** The drive from one stop to the next, identified the same way `planKey` identifies them. */ const legKey = (from: RoadtripStop, to: RoadtripStop): string => `${stopKey(from)}>${stopKey(to)}` const EMPTY_ACCOMMODATIONS: Accommodation[] = [] const EMPTY_RESERVATIONS: Reservation[] = [] +const NO_STAND_INS: ReadonlySet = new Set() /** One leg as the rail's own router answered it, for a surface checking a choice against it. */ export interface RailLegRoute { @@ -107,6 +116,12 @@ export interface RailLegRouter { avoid: RouteAvoidClass[] /** Which engine prices this leg on the rail. */ engine: RouteEngine + /** + * True when the line the rail has for this leg came from OSRM standing in for `engine`, + * which did not answer when the leg was routed. The leg's own road is then not what the + * map shows, whatever its vias say, and only asking again (`reroute`) puts it there. + */ + standIn: boolean /** The leg from its first stop through `pins` to its last, asked exactly as the rail asks it. */ route: (pins: { lat: number; lng: number }[], signal: AbortSignal) => Promise } @@ -124,6 +139,15 @@ export type RoadtripRoutesView = RoadtripRoutes & { * different days are a seam, asked for under the card it arrives on. */ legRouter?: (from: RoadtripStop, to: RoadtripStop, cardDayId: number) => RailLegRouter + /** + * Asks the router again for every leg OSRM drew while the engine above it did not + * answer: the days whose run fell back, and the joins between days that did. + * + * Nothing else would. A day run is asked again only when its stops or vias change, and + * a choice of the leg's own road writes nothing, so the stand-in line stayed on the map + * under a picker that said the road had been taken. + */ + reroute?: () => void } const asStop = (a: Assignment, ownerDayId: number, ownerIndex: number, accommodations: Accommodation[]): RoadtripStop | null => { @@ -132,7 +156,7 @@ const asStop = (a: Assignment, ownerDayId: number, ownerIndex: number, accommoda // Check-in only. A check-out is the LATEST the room has to be handed back, not the // earliest anybody may leave, so it says nothing about when the drive sets off and // has no business in the chain. It stays a booking detail, shown under Days. - const stay = accommodations.find(stay => stay.place_id === a.place_id && stay.start_day_id === ownerDayId) + const stay = stayStartingOn(accommodations, a.place_id, ownerDayId) return { assignmentId: a.id, ownerDayId, @@ -215,7 +239,11 @@ export function useRoadtripRoutes( * it draws two disconnected runs on the map. Filed by leg key like every other leg, so * everything downstream reads them without knowing where they came from. */ - const [seamLegs, setSeamLegs] = useState>({}) + const [seamLegs, setSeamLegs] = useState>({}) + /** The legs of the day runs that OSRM drew while the engine asked did not answer. */ + const [standInLegs, setStandInLegs] = useState>(NO_STAND_INS) + /** Bumped to route everything again (`reroute`): every answer already cached comes back at once. */ + const [rerouteRound, setRerouteRound] = useState(0) const [loading, setLoading] = useState(false) const abortRef = useRef(null) @@ -363,6 +391,7 @@ export function useRoadtripRoutes( setLegsByDay({}) setSnapByDay({}) setMissedByDay({}) + setStandInLegs(NO_STAND_INS) setLoading(false) return } @@ -373,6 +402,7 @@ export function useRoadtripRoutes( const collected: Record> = {} const collectedSnaps: Record> = {} const collectedMisses: Record = {} + const collectedStandIns = new Set() const tasks: (() => Promise)[] = [] /** @@ -462,7 +492,11 @@ export function useRoadtripRoutes( collectedMisses[day.dayId] = [...new Set([...(collectedMisses[day.dayId] ?? []), ...missed])] } } - Object.assign(dayLegs, foldRouteRun(run, stopAt, r, mode)) + const folded = foldRouteRun(run, stopAt, r, mode) + // 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) + Object.assign(dayLegs, folded) return } catch (err) { if (controller.signal.aborted) return @@ -505,6 +539,7 @@ export function useRoadtripRoutes( setLegsByDay({ ...collected }) setSnapByDay({ ...collectedSnaps }) setMissedByDay({ ...collectedMisses }) + setStandInLegs(new Set(collectedStandIns)) } void (async () => { for (let i = 0; i < tasks.length; i++) { @@ -530,7 +565,7 @@ export function useRoadtripRoutes( // planKey stands in for `plan`: same geometry, same legs. avoidKey, not `avoid`: // a fresh array every render would re-route on every render. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [planKey, routeProfile, distanceUnit, tripId, avoidKey]) + }, [planKey, routeProfile, distanceUnit, tripId, avoidKey, rerouteRound]) /** * Every leg known so far, by the two stops it connects. @@ -566,8 +601,9 @@ export function useRoadtripRoutes( * run. Left unrouted the rail shows a chain that stops halfway and the map draws the * day in two pieces with a gap across the middle. */ - const seams = useMemo(() => { + const { seams, seamMisses } = useMemo(() => { const out: { from: RoadtripStop; to: RoadtripStop; dayId: number; mode: string; shape: string }[] = [] + const misses: Record = {} const want = (from: RoadtripStop, to: RoadtripStop, dayId: number): void => { // The two ends of one ride: the leg between them is the booking's, never a road. if (rideLegs[legKey(from, to)]) return @@ -581,7 +617,14 @@ export function useRoadtripRoutes( const mode = legModeOf(from, to, dayId) const shape = seamShape(from, viasByDay, mode, avoidKey) const have = seamLegs[legKey(from, to)] - if (have && have.shape === shape) return + const current = have?.shape === shape + // What the drawn join could not avoid is flagged on the card it arrives on, the way + // a day's own run is. A join OSRM drove in the engine's place said nothing at all. + if (current && have.missed?.length) misses[dayId] = [...new Set([...(misses[dayId] ?? []), ...have.missed])] + // An answer OSRM gave in the engine's place is drawn but not settled: it is asked + // for again on the next seam round, instead of standing in for the weighed road + // until the page is loaded again. + if (current && !have.standIn) return out.push({ from, to, dayId, mode, shape }) } const routingChains = window @@ -605,8 +648,16 @@ export function useRoadtripRoutes( if (from && to) want(from, to, routingChains[d + 1].dayId) } } - return out + return { seams: out, seamMisses: misses } }, [chains, plan, quietDays, window, legsByDay, seamLegs, rideLegs, viasByDay, connectDays, legModeOf, avoidKey]) + /** 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 } + for (const [dayId, classes] of Object.entries(seamMisses)) { + out[Number(dayId)] = [...new Set([...(out[Number(dayId)] ?? []), ...classes])] + } + return out + }, [missedByDay, seamMisses]) const seamKey = seams.map(s => `${legKey(s.from, s.to)}#${s.shape}`).join(';') /** * When the last request outside the routing round went out, across every run of the @@ -682,6 +733,8 @@ export function useRoadtripRoutes( line: r.coordinates, vias: [], shape, + standIn: r.avoidance?.fellBack === true, + missed: r.avoidance ? r.avoidance.asked.filter(cls => !r.avoidance!.achieved.includes(cls)) : [], }, })) } catch { @@ -696,7 +749,7 @@ export function useRoadtripRoutes( // seamKey stands in for `seams`: same pairs, same requests. avoidKey, not `avoid`: // a fresh array every render would re-ask for every seam on every render. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [seamKey, routeProfile, tripId, avoidKey]) + }, [seamKey, routeProfile, tripId, avoidKey, rerouteRound]) const legRouter = useCallback((from: RoadtripStop, to: RoadtripStop, cardDayId: number): RailLegRouter => { // The day the routing round asked this pair under: its own stored day for a pair @@ -704,10 +757,14 @@ export function useRoadtripRoutes( // is what a route provider plugin is told. const dayId = from.ownerDayId === to.ownerDayId ? from.ownerDayId : cardDayId const mode = legModeOf(from, to, dayId) + // Whose line the rail shows, read the way `allLegs` picks it: a day run's over a seam's. + const key = legKey(from, to) + const byRun = plan.some(day => legsByDay[day.dayId]?.[key] !== undefined) return { mode, avoid: avoidedClasses(mode, avoid), engine: routeEngineFor(mode, avoid), + standIn: byRun ? standInLegs.has(key) : seamLegs[key]?.standIn === true, route: async (pins, signal) => { await spacedRequest(signal) const r = await calculateRouteWithLegs( @@ -723,10 +780,12 @@ export function useRoadtripRoutes( } }, } - }, [legModeOf, avoid, spacedRequest, tripId]) + }, [legModeOf, avoid, spacedRequest, tripId, plan, legsByDay, standInLegs, seamLegs]) - const assembled = useMemo(() => assembleRoadtrip({ plan, quietDays, window, distanceUnit, allLegs, snapByDay, missedByDay, loading, limits, vehicleKind, connectDays, boundaries, + const reroute = useCallback(() => setRerouteRound(round => round + 1), []) + + const assembled = useMemo(() => assembleRoadtrip({ plan, quietDays, window, distanceUnit, allLegs, snapByDay, missedByDay: missedOnCards, loading, limits, vehicleKind, connectDays, boundaries, labels: { start: t('roadtrip.window.resume'), end: t('roadtrip.window.stop') }, - }), [plan, quietDays, window, distanceUnit, t, allLegs, snapByDay, missedByDay, loading, limits, vehicleKind, connectDays, boundaries]) - return useMemo(() => ({ ...assembled, legRouter }), [assembled, legRouter]) + }), [plan, quietDays, window, distanceUnit, t, allLegs, snapByDay, missedOnCards, loading, limits, vehicleKind, connectDays, boundaries]) + return useMemo(() => ({ ...assembled, legRouter, reroute }), [assembled, legRouter, reroute]) } diff --git a/client/src/components/Roadtrip/useRouteAlternatives.test.ts b/client/src/components/Roadtrip/useRouteAlternatives.test.ts index 4cbd6a795..054d628a1 100644 --- a/client/src/components/Roadtrip/useRouteAlternatives.test.ts +++ b/client/src/components/Roadtrip/useRouteAlternatives.test.ts @@ -13,7 +13,7 @@ import { ARRIVING_DRIVE, openOn, sameDrive, useRouteAlternatives, type Alternati import type { RailLegRouter } from './useRoadtripRoutes' /** - * FE-ALTHOOK-001..013: asking for other ways of driving one leg. + * FE-ALTHOOK-001..015: asking for other ways of driving one leg. * * The case that carries the feature: the road the rail drives is always the first entry, * taken from the rail, and the offers come from the engine the rail drives the leg with. @@ -46,7 +46,7 @@ function request(router: Partial = {}, over: Partial { }) }) +describe('a leg OSRM drew while the rail’s engine did not answer', () => { + it('FE-ALTHOOK-014: Current is marked as OSRM’s line, and the picker knows the leg was drawn by the stand-in', async () => { + // Marked as the rail's engine, the stand-in line was set against the second engine's + // offers as if one speed model had timed them all. + const { result } = renderHook(() => useRouteAlternatives()) + + act(() => { result.current.ask(request({ avoid: ['toll'], engine: 'valhalla', standIn: true })) }) + await waitFor(() => expect(result.current.open?.loading).toBe(false)) + + expect(result.current.open).toMatchObject({ engine: 'valhalla', standIn: true }) + expect(result.current.open?.routes[0]).toMatchObject({ current: true, engine: 'osrm' }) + }) + + it('FE-ALTHOOK-015: a check that saved nothing leaves its reason on the picker until the next one starts', async () => { + const { result } = renderHook(() => useRouteAlternatives()) + act(() => { result.current.ask(request()) }) + await waitFor(() => expect(result.current.open?.loading).toBe(false)) + expect(result.current.open?.notice).toBeNull() + + act(() => { result.current.prove(1) }) + act(() => { result.current.settle('Not saved.') }) + expect(result.current.open).toMatchObject({ proving: null, notice: 'Not saved.' }) + + act(() => { result.current.prove(2) }) + expect(result.current.open).toMatchObject({ proving: 2, notice: null }) + act(() => { result.current.settle() }) + expect(result.current.open?.notice).toBeNull() + + act(() => { result.current.settle('Not saved.') }) + act(() => { result.current.ask(request({}, { drive: { kind: 'leg', index: 0 } })) }) + expect(result.current.open?.notice).toBeNull() + }) +}) + describe('which drive a picker is open on', () => { it('FE-ALTHOOK-013: the drive in from the day before is its own drive, never a leg at some index', () => { expect(sameDrive(ARRIVING_DRIVE, { kind: 'arriving' })).toBe(true) diff --git a/client/src/components/Roadtrip/useRouteAlternatives.ts b/client/src/components/Roadtrip/useRouteAlternatives.ts index 09d3afde2..d664634f7 100644 --- a/client/src/components/Roadtrip/useRouteAlternatives.ts +++ b/client/src/components/Roadtrip/useRouteAlternatives.ts @@ -58,6 +58,11 @@ export interface LegAlternatives { ends: { from: number; to: number } /** The engine the rail drives this leg with. Offers from another cannot be read against it. */ engine: RouteEngine + /** + * True when the line the rail has for this leg came from OSRM standing in for `engine`, + * which did not answer. Its own road is then not on the map, whatever the leg's vias say. + */ + standIn: boolean /** Routes the leg through pins exactly as the rail does, to check a choice before saving it. */ route: RailLegRouter['route'] routes: OfferedRoute[] @@ -65,6 +70,8 @@ export interface LegAlternatives { error: boolean /** The offer being checked against the rail's router and saved, or null while none is. */ proving: number | null + /** Why the last check saved nothing, for the bar to say beside the offers. Null otherwise. */ + notice: string | null } /** Everything a picker needs to know about the leg it is opened on. */ @@ -91,8 +98,11 @@ export interface RouteAlternativesState { * closes, moves to another leg, or another check starts. */ prove: (index: number) => AbortSignal - /** Ends a check that led to no save, leaving the picker open for another choice. */ - settle: () => void + /** + * Ends a check that led to no save, leaving the picker open for another choice. `notice` + * says why, for the bar to announce. + */ + settle: (notice?: string) => void } /** @@ -134,15 +144,18 @@ export function useRouteAlternatives(): RouteAlternativesState { const controller = new AbortController() abortRef.current = controller const { dayId, drive, from, to, driven, anchor, ends, router } = request - const leg = { dayId, drive, anchor, ends, engine: router.engine, route: router.route, proving: null } + const leg = { dayId, drive, anchor, ends, engine: router.engine, standIn: router.standIn, route: router.route, proving: null, notice: null } setOpen({ ...leg, routes: [], loading: true, error: false }) + // Marked with the engine that drew it. A line OSRM drew while the rail's engine did not + // answer was marked as that engine's, so the list set it against the offers as if one + // speed model had timed them all. const current: OfferedRoute = { coordinates: driven.coordinates, distance: driven.distance, duration: driven.duration, divergence: null, - engine: router.engine, + engine: router.standIn ? 'osrm' : router.engine, current: true, } // A plugin's mode means nothing to the routers asked here, so its leg is offered the @@ -176,13 +189,13 @@ export function useRouteAlternatives(): RouteAlternativesState { stopProving() const controller = new AbortController() provingRef.current = controller - setOpen(o => (o ? { ...o, proving: index } : o)) + setOpen(o => (o ? { ...o, proving: index, notice: null } : o)) return controller.signal }, [stopProving]) - const settle = useCallback(() => { + const settle = useCallback((notice?: string) => { provingRef.current = null - setOpen(o => (o ? { ...o, proving: null } : o)) + setOpen(o => (o ? { ...o, proving: null, notice: notice ?? null } : o)) }, []) // One object for as long as nothing in it changes. The planner keys its close gate and diff --git a/client/src/mobile/screens/trip/roadtrip/MRtAlternativesBar.tsx b/client/src/mobile/screens/trip/roadtrip/MRtAlternativesBar.tsx index dbdbef8b3..fbfaf0fae 100644 --- a/client/src/mobile/screens/trip/roadtrip/MRtAlternativesBar.tsx +++ b/client/src/mobile/screens/trip/roadtrip/MRtAlternativesBar.tsx @@ -1,7 +1,7 @@ import { AlertTriangle, Check, Loader2, Shuffle, X } from 'lucide-react' import MBadge from '../../../components/MBadge' import MIconBtn from '../../../components/MIconBtn' -import { alternativeSubline, type AlternativeOverlay } from '../../../../components/Roadtrip/alternativeOverlays' +import { alternativeSubline, otherEngineNote, type AlternativeOverlay } from '../../../../components/Roadtrip/alternativeOverlays' import { useSettingsStore } from '../../../../store/settingsStore' import { formatDistance } from '../../../../utils/units' import { badgeLabel } from './stageBadges' @@ -31,6 +31,7 @@ export interface MRtAlternativesBarProps { export default function MRtAlternativesBar({ planner, alts }: MRtAlternativesBarProps) { const { t } = planner const title = t('roadtrip.alt.title') + const note = alts.picked ? otherEngineNote(alts.picked) : null return (
{/* The desk hides this sentence in a tooltip on the chip. There is no hover here, so it stands beside the button whose choice it qualifies. */} - {alts.picked?.otherEngine && ( + {note && ( - {t('roadtrip.alt.otherEngine')} + {t(note)} )}