From e6daed9403d232d772620e045e765a5e4d56f675 Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 23 Sep 2026 09:42:22 +0200 Subject: [PATCH] fix(roadtrip): the drive between two days can take another way too The drive from where one day ended to where the next begins was the one stretch of a connected road trip that offered no other ways. The band above a card's first stop had no button on the desk, and the phone did not show that drive at all. A via behind a day's last stop, where the map already files a point dropped on that drive, was deleted when the day was reordered under Days, and was left on no stop when that last stop was removed or dragged up the day. Switching the avoided road classes or the day's profile kept the old road and minutes on it until a reload. The drive in now opens the same picker as any leg, on the desk band and on a new row at the head of the phone's chain that names where it leaves. A choice is proven with the seam's own router and written behind the last stop of the day before, also when a day with a single stop lies between. The rule for that via lives once in shared (carriedSeam): it stays with the last stop while that stop stays last, and is read like any other once it is not, for the planner's drags and removals as for the server's sorts and seated nights. The cached answer for a seam now records its mode and avoided classes, so changing either asks for it again. --- .../Roadtrip/RoadtripAlternativesBar.test.tsx | 2 +- .../Roadtrip/RoadtripSidebar.test.tsx | 57 +++++ .../components/Roadtrip/RoadtripSidebar.tsx | 23 +- .../Roadtrip/alternativePins.test.ts | 37 ++- .../components/Roadtrip/alternativePins.ts | 40 +++- .../components/Roadtrip/reanchorVias.test.ts | 56 +++++ .../Roadtrip/roadtripRowModel.test.ts | 53 ++++- .../components/Roadtrip/roadtripRowModel.ts | 45 +++- .../Roadtrip/useRoadtripRoutes.test.ts | 25 ++ .../components/Roadtrip/useRoadtripRoutes.ts | 39 +-- .../Roadtrip/useRouteAlternatives.test.ts | 32 ++- .../Roadtrip/useRouteAlternatives.ts | 36 ++- .../screens/trip/roadtrip/MRoadtripRows.tsx | 26 +- .../screens/trip/roadtrip/MRoadtripTab.tsx | 19 +- .../trip/roadtrip/useMRtAlternatives.ts | 15 +- .../useTripPlanner.roadtrip.test.tsx | 225 +++++++++++++++++- .../src/pages/tripPlanner/useTripPlanner.ts | 36 ++- client/tests/helpers/legAlternatives.ts | 4 +- .../tests/unit/mobile/trip/MMapArea.test.tsx | 2 +- .../unit/mobile/trip/MRoadtripRows.test.tsx | 16 +- .../unit/mobile/trip/MRoadtripTab.test.tsx | 63 ++++- .../mobile/trip/MRtAlternativesBar.test.tsx | 2 +- .../mobile/trip/useMRtAlternatives.test.ts | 31 ++- server/src/nest/accommodations/night-seat.ts | 12 +- .../nest/assignments/assignments.service.ts | 9 +- .../unit/nest/assignments.service.test.ts | 18 ++ shared/src/i18n/ar/roadtrip.ts | 1 + shared/src/i18n/br/roadtrip.ts | 1 + shared/src/i18n/ca/roadtrip.ts | 1 + shared/src/i18n/cs/roadtrip.ts | 1 + shared/src/i18n/de/roadtrip.ts | 1 + shared/src/i18n/en/roadtrip.ts | 1 + shared/src/i18n/es/roadtrip.ts | 1 + shared/src/i18n/fr/roadtrip.ts | 1 + shared/src/i18n/gr/roadtrip.ts | 1 + shared/src/i18n/hu/roadtrip.ts | 1 + shared/src/i18n/id/roadtrip.ts | 1 + shared/src/i18n/it/roadtrip.ts | 1 + shared/src/i18n/ja/roadtrip.ts | 1 + shared/src/i18n/ko/roadtrip.ts | 1 + shared/src/i18n/nl/roadtrip.ts | 1 + shared/src/i18n/pl/roadtrip.ts | 1 + shared/src/i18n/ru/roadtrip.ts | 1 + shared/src/i18n/sv/roadtrip.ts | 1 + shared/src/i18n/tr/roadtrip.ts | 1 + shared/src/i18n/uk/roadtrip.ts | 1 + shared/src/i18n/vi/roadtrip.ts | 1 + shared/src/i18n/zh-TW/roadtrip.ts | 1 + shared/src/i18n/zh/roadtrip.ts | 1 + shared/src/roadtrip/roadtripModel.spec.ts | 29 +++ shared/src/roadtrip/roadtripModel.ts | 51 +++- 51 files changed, 901 insertions(+), 125 deletions(-) diff --git a/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx b/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx index 3bdc18cbd..6e414e608 100644 --- a/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx +++ b/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx @@ -32,7 +32,7 @@ const overlay = (over: Partial = {}): AlternativeOverlay => ...over, }) -const leg = (over: Partial = {}): LegAlternatives => openLeg({ dayId: 4, index: 1, ...over }) +const leg = (over: Partial = {}): LegAlternatives => openLeg({ dayId: 4, drive: { kind: 'leg', index: 1 }, ...over }) beforeEach(() => { useSettingsStore.setState({ settings: { distance_unit: 'metric' } as never }) diff --git a/client/src/components/Roadtrip/RoadtripSidebar.test.tsx b/client/src/components/Roadtrip/RoadtripSidebar.test.tsx index d82741a1c..7b5ecef9a 100644 --- a/client/src/components/Roadtrip/RoadtripSidebar.test.tsx +++ b/client/src/components/Roadtrip/RoadtripSidebar.test.tsx @@ -1341,3 +1341,60 @@ describe('RoadtripSidebar with a hire car and the day\'s bookings (#2428)', () = expect(onOpenBooking).toHaveBeenCalledWith(12) }) }) + +describe('RoadtripSidebar with the drive in from the day before (#2461)', () => { + /** Where day 1 ended, three stops in, and a card joined to it by the road from there. */ + const yesterday = stop({ assignmentId: 9, name: 'Lüneburg', ownerDayId: 1, ownerIndex: 2 }) + const joined = (over: Partial = {}) => day({ + dayId: 2, + dayNumber: 2, + arrivingLeg: leg({ distance: 120000, duration: 5400, distanceText: '120 km', durationText: '1 h 30 min' }), + arrivingFrom: yesterday, + arrivingLine: [[53.2, 10.4], [52.5, 13.4]], + ...over, + }) + const pressed = () => screen.getAllByLabelText('Other ways').map(icon => icon.closest('button')?.getAttribute('aria-pressed')) + + it('FE-ROADTRIP-SIDEBAR-060: the drive in offers other ways as the drive it is, and shows pressed while they are open', () => { + // #2461: the band above the first stop was the one drive on the rail without the + // control, so the road between two days could not be changed at all. + const onAskAlternatives = vi.fn() + const first = wrap() + + // The drive in, then the card's own leg. + expect(pressed()).toEqual(['false', 'false']) + fireEvent.click(screen.getAllByLabelText('Other ways')[0]) + expect(onAskAlternatives).toHaveBeenCalledWith(2, { kind: 'arriving' }) + first.unmount() + + const open = wrap( + , + ) + expect(pressed()).toEqual(['true', 'false']) + open.unmount() + + // The card's first leg is another drive, even at the index the drive in would borrow. + wrap() + expect(pressed()).toEqual(['false', 'true']) + }) + + it('FE-ROADTRIP-SIDEBAR-061: no control for a reader, for a drive from a terminal, or for one the rail has no line for', () => { + const reader = wrap() + expect(screen.queryAllByLabelText('Other ways')).toHaveLength(0) + // The band is still drawn, only not as a control. + expect(screen.getByText('120 km in 1 h 30 min')).toBeInTheDocument() + reader.unmount() + + const landed = stop({ + assignmentId: -3000000141, + name: 'Munich Airport', + carrier: { reservationId: 70, type: 'flight', role: 'arrival', title: 'LH 2020', code: 'MUC', at: '14:30' }, + }) + const fromTerminal = wrap() + expect(screen.getAllByLabelText('Other ways')).toHaveLength(1) + fromTerminal.unmount() + + wrap() + expect(screen.getAllByLabelText('Other ways')).toHaveLength(1) + }) +}) diff --git a/client/src/components/Roadtrip/RoadtripSidebar.tsx b/client/src/components/Roadtrip/RoadtripSidebar.tsx index 0e24dc91f..9cfa5934f 100644 --- a/client/src/components/Roadtrip/RoadtripSidebar.tsx +++ b/client/src/components/Roadtrip/RoadtripSidebar.tsx @@ -17,7 +17,7 @@ import { formatDistance } from '../../utils/units' import { formatDate, formatClockTime } from '../../utils/formatters' import { formatDurationShort, isServiceStopType, serviceColor, type ScheduleEntry, type ScheduleWarning, refuelsRange } from './roadtripModel' import { STOP_KIND_BY_KEY } from './stopKinds' -import { destinationCount, isHop, legReroutable } from './roadtripRowModel' +import { arrivingReroutable, destinationCount, isHop, legReroutable } from './roadtripRowModel' import { spurWorthLabelling } from './accessSpur' import StopKindPicker from './StopKindPicker' import StopFillPicker from './StopFillPicker' @@ -26,7 +26,7 @@ import { MAX_TRIP_DAYS, type RoadtripStopType } from '@trek/shared' import { isCarrierMode, type CarrierTerminal } from '@trek/shared/roadtrip' import type { QuietDay, RoadtripDay, RoadtripRoutes, RoadtripStop } from './useRoadtripRoutes' import type { SpillMark } from './nightSpill' -import type { RailDrive } from './useRouteAlternatives' +import { ARRIVING_DRIVE, openOn, type LegAlternatives, type RailDrive } from './useRouteAlternatives' import { dayColor } from './dayColors' import type { RouteVia } from '../../types' import { FS } from './typeScale' @@ -63,8 +63,8 @@ interface RoadtripSidebarProps { refuel?: RefuelSearch onAskRefuel?: (dayId: number, dry: DryPoint & { lat: number; lng: number }) => void onAcceptRefuel?: (dayId: number, poi: RefuelCandidate, dry: DryPoint & { lat: number; lng: number }) => void - /** Which leg's alternatives are on show, so the rail can mark it. */ - openAlternatives?: { dayId: number; index: number } | null + /** Which drive's alternatives are on show, so the rail can mark it. */ + openAlternatives?: Pick | null /** * Opens the dialog for how long a stop takes. Absent leaves every stay read-only — * which is also what a viewer sees. @@ -1734,7 +1734,7 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onOpenBooking, ca onAskAlternatives(day.dayId, { kind: 'leg', index: i }) : undefined} - alternativesOpen={openAlternatives?.dayId === day.dayId && openAlternatives.index === i} + alternativesOpen={openOn(openAlternatives, day.dayId, { kind: 'leg', index: i })} /> ) : null} {refuelBandsFor(i)} @@ -1906,8 +1906,17 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onOpenBooking, ca 08:00 looks like eleven minutes went missing. A day whose first stop crossed over instead gets that road under its own block, so this is left out there. */} {/* A day that opens on an arrival terminal is joined by the ride, and the - terminal carries the booking the band names. */} - {day.arrivingLeg ? : null} + terminal carries the booking the band names. A drive by road is offered other + ways like any leg (`arrivingReroutable`); a choice is filed behind the stop it + leaves, on the day before, where the map files a point dropped on it. */} + {day.arrivingLeg ? ( + onAskAlternatives(day.dayId, ARRIVING_DRIVE) : undefined} + alternativesOpen={openOn(openAlternatives, day.dayId, ARRIVING_DRIVE)} + /> + ) : null} {runs.map(run => ( run.spill ? ( diff --git a/client/src/components/Roadtrip/alternativePins.test.ts b/client/src/components/Roadtrip/alternativePins.test.ts index 00216a5a6..fc25b131b 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, railLegAt } from './alternativePins' +import { MAX_PINS, pinAlternative, railDriveOn, railLegAt } from './alternativePins' import type { RailLegRoute, RailLegRouter, RoadtripDay, RoadtripStop } from './useRoadtripRoutes' import type { OfferedRoute } from './useRouteAlternatives' /** - * FE-ALTPIN-001..010: proving a choice with the rail's own router before it is saved. + * FE-ALTPIN-001..011: 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 @@ -149,3 +149,36 @@ describe('railLegAt', () => { expect(railLegAt(days, { dayId: 3, afterIndex: 0 })).toBeNull() }) }) + +describe('railDriveOn', () => { + const stop = (assignmentId: number, ownerDayId: number, ownerIndex: number) => + ({ assignmentId, ownerDayId, ownerIndex, lat: 50, lng: 10 }) as RoadtripStop + const seg = (distance: number) => ({ distance, duration: distance / 20 }) as RoadtripDay['legs'][number] + const LEG_LINE: [number, number][] = [[50, 10], [50.2, 10.4]] + const DRIVE_IN: [number, number][] = [[51, 9], [50, 10]] + const card = (over: Partial = {}) => ({ + dayId: 2, + stops: [stop(20, 2, 0), stop(21, 2, 1)], + legs: [seg(40_000)], + legLines: [LEG_LINE], + arrivingFrom: stop(12, 1, 2), + arrivingLeg: seg(90_000), + arrivingLine: DRIVE_IN, + ...over, + }) as RoadtripDay + + it('FE-ALTPIN-011: a leg is read off its index, the drive in off the stop the day before ended on', () => { + expect(railDriveOn(card(), { kind: 'leg', index: 0 })).toMatchObject({ + from: { assignmentId: 20 }, to: { assignmentId: 21 }, seg: { distance: 40_000 }, line: LEG_LINE, + }) + expect(railDriveOn(card(), { kind: 'arriving' })).toMatchObject({ + from: { assignmentId: 12, ownerDayId: 1, ownerIndex: 2 }, to: { assignmentId: 20 }, seg: { distance: 90_000 }, line: DRIVE_IN, + }) + // Nothing past the last stop, no drive in on a card nothing joins, and no road yet. + expect(railDriveOn(card(), { kind: 'leg', index: 1 })).toBeNull() + expect(railDriveOn(card({ arrivingFrom: undefined, arrivingLeg: undefined }), { kind: 'arriving' })).toBeNull() + expect(railDriveOn(card({ legs: [] }), { kind: 'leg', index: 0 })).toBeNull() + // A line the card does not have yet is no reason to refuse the drive. + expect(railDriveOn(card({ legLines: undefined }), { kind: 'leg', index: 0 })?.line).toBeUndefined() + }) +}) diff --git a/client/src/components/Roadtrip/alternativePins.ts b/client/src/components/Roadtrip/alternativePins.ts index 8558e3b9e..c4d886f1c 100644 --- a/client/src/components/Roadtrip/alternativePins.ts +++ b/client/src/components/Roadtrip/alternativePins.ts @@ -1,6 +1,7 @@ import { furthestFrom, sameRoad, type RoadLine } from '../Map/RouteCalculator' +import type { RouteSegment } from '@trek/shared/roadtrip' import type { RailLegRoute, RailLegRouter, RoadtripDay, RoadtripStop } from './useRoadtripRoutes' -import type { OfferedRoute, ViaAnchor } from './useRouteAlternatives' +import type { OfferedRoute, RailDrive, ViaAnchor } from './useRouteAlternatives' /** * The most points one choice may pin to the road. @@ -87,14 +88,41 @@ export async function pinAlternative({ * The leg the rail drives from the stop filed at `anchor` now, or null when no stop sits * there any more. * - * Read across every card in order, so the stop after it is found whether the leg runs - * inside one card or across to the next. The markers an automatic night puts on the - * chain stand on or between stops without being one, and a terminal borrows the index of - * a stored stop to be seated by, so neither can answer for the anchor. + * Read across every day in order, so the stop after it is found whether the leg runs + * inside one card or across to the next. The caller hands in the days with a single stop + * too: they draw no card, but the drive into the day after one leaves from its stop. The + * markers an automatic night puts on the chain stand on or between stops without being + * one, and a terminal borrows the index of a stored stop to be seated by, so neither can + * answer for the anchor. */ -export function railLegAt(days: readonly RoadtripDay[], anchor: ViaAnchor): { from: RoadtripStop; to: RoadtripStop } | null { +export function railLegAt(days: readonly Pick[], anchor: ViaAnchor): { from: RoadtripStop; to: RoadtripStop } | null { const chain = days.flatMap(day => day.stops).filter(stop => !stop.automaticNight) const at = chain.findIndex(stop => !stop.carrier && stop.ownerDayId === anchor.dayId && stop.ownerIndex === anchor.afterIndex) const to = at >= 0 ? chain[at + 1] : undefined return to ? { from: chain[at], to } : null } + +/** One drive on a card as the rail has it: its two stops, its road and the line it is drawn on. */ +export interface RailDriveOnCard { + from: RoadtripStop + to: RoadtripStop + seg: RouteSegment + line: [number, number][] | undefined +} + +/** + * The drive `drive` names on `day`, or null while the card has no such drive or no road + * for it yet. + * + * A leg runs from stop `index` to the next one. The drive arriving at the head of a + * connected card has no index on it: it leaves the last stop of the day before, which the + * card keeps as `arrivingFrom`, and ends at the card's first stop. Read here once, so the + * picker is handed the same two stops, figures and line whichever of the two it opens on. + */ +export function railDriveOn(day: RoadtripDay, drive: RailDrive): RailDriveOnCard | null { + const found = drive.kind === 'arriving' + ? { from: day.arrivingFrom, to: day.stops[0], seg: day.arrivingLeg, line: day.arrivingLine } + : { from: day.stops[drive.index], to: day.stops[drive.index + 1], seg: day.legs[drive.index], line: day.legLines?.[drive.index] } + const { from, to, seg, line } = found + return from && to && seg ? { from, to, seg, line } : null +} diff --git a/client/src/components/Roadtrip/reanchorVias.test.ts b/client/src/components/Roadtrip/reanchorVias.test.ts index e4133b36b..8fe6345b8 100644 --- a/client/src/components/Roadtrip/reanchorVias.test.ts +++ b/client/src/components/Roadtrip/reanchorVias.test.ts @@ -175,3 +175,59 @@ describe('reanchorAfterReorder', () => { expect(plan.vias).toEqual([]) }) }) + +/** + * A via behind a day's last stop shapes the drive into the next day, on a trip with + * connected days or a night drive: the map files a point dropped on that drive there, and + * a way chosen for it is written there. It is no leg of its own day, so the arithmetic + * above has to be told what it is (`carriedSeam`) or it takes it for one leading nowhere. + */ +describe('the drive into the next day (#2461)', () => { + it('FE-REANCHOR-023: a wholesale reorder that leaves the last stop last keeps the drive behind it', () => { + // Stops 10, 20, 30, 40; the via behind 40 is the join. Swapping the first two used to + // delete it, because a last stop has no leg. + const plan = reanchorByStopOrder([via(1, 0), via(9, 3)], [10, 20, 30, 40], [20, 10, 30, 40]) + expect(anchors(plan.vias)).toEqual({ 1: 1 }) + expect(plan.remove).toEqual([]) + expect(isEmptyReanchoring(reanchorByStopOrder([via(9, 3)], [10, 20, 30, 40], [10, 20, 30, 40]))).toBe(true) + }) + + it('FE-REANCHOR-024: taking out the last stop takes the drive out of it along, instead of leaving it on no stop', () => { + const plan = reanchorAfterRemove([via(2, 2), via(9, 3)], 3, 4) + expect(plan.remove).toEqual([2, 9]) + expect(plan.vias).toEqual([]) + }) + + it('FE-REANCHOR-025: taking out any other stop leaves it behind the same last stop, one number down', () => { + expect(anchors(reanchorAfterRemove([via(9, 3)], 1, 4).vias)).toEqual({ 9: 2 }) + expect(anchors(reanchorAfterRemove([via(9, 3)], 0, 4).vias)).toEqual({ 9: 2 }) + // A day left with one stop has no leg, but still the drive out of it. + const single = reanchorAfterRemove([via(1, 0), via(9, 1)], 0, 2) + expect(anchors(single.vias)).toEqual({ 9: 0 }) + 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', () => { + // 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 }) + // Anything moved in front of the last stop leaves it where it was. + expect(isEmptyReanchoring(reanchorAfterReorder([via(9, 3)], 0, 2, 4))).toBe(true) + }) + + it('FE-REANCHOR-027: a stop appended behind the last one keeps the point on the road to it', () => { + expect(isEmptyReanchoring(reanchorAfterInsert([via(9, 3)], 4, () => true))).toBe(true) + }) + + it('FE-REANCHOR-028: two stops swapped keep their leg, and the drive out of the one that is first now goes', () => { + for (const [from, to] of [[0, 1], [1, 0]] as const) { + const plan = reanchorAfterReorder([via(1, 0), via(9, 1)], from, to, 2) + expect(plan.remove, `${from}->${to}`).toEqual([9]) + expect(plan.vias, `${from}->${to}`).toEqual([]) + } + }) +}) diff --git a/client/src/components/Roadtrip/roadtripRowModel.test.ts b/client/src/components/Roadtrip/roadtripRowModel.test.ts index a6d0a6176..e60046742 100644 --- a/client/src/components/Roadtrip/roadtripRowModel.test.ts +++ b/client/src/components/Roadtrip/roadtripRowModel.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { + arrivingReroutable, destinationCount, firstStopOfPlace, legReroutable, @@ -15,7 +16,7 @@ import { import type { ScheduleEntry, ScheduleWarning } from './roadtripModel' import type { RoadtripDay, RoadtripStop, RouteSegment } from '@trek/shared/roadtrip' -// FE-RTROW-001 to FE-RTROW-055 +// FE-RTROW-001 to FE-RTROW-058 function stop(name: string, over: Partial = {}): RoadtripStop { return { @@ -649,3 +650,53 @@ describe('roadtripRows with a ride (#2428)', () => { expect(stageEnd(roadtripRows(opensOnRide()))?.stop.name).toBe('Hotel') }) }) + +describe('the drive in from the day before (#2461)', () => { + /** Where yesterday ended: stored on day 6, its third stop, and drawn on no stop of this card. */ + const yesterday = stop('Lüneburg', { ownerDayId: 6, ownerIndex: 2, legMode: null }) + const drive: RouteSegment = { ...seg(4), distance: 120_000, duration: 5_400 } + /** A card joined to the one before it, with the drive and the line it is drawn on. */ + const joined = (over: Partial = {}) => day([stop('Berlin', { incomingLegMode: 'driving' }), stop('Potsdam')], { + arrivingLeg: drive, + arrivingFrom: yesterday, + arrivingLine: [[53.2, 10.4], [52.5, 13.4]], + ...over, + }) + + it('FE-RTROW-056: heads the card with a row of its own that names where it leaves, and counts no stop for it', () => { + const rows = roadtripRows(joined()) + expect(rows.map(r => r.kind)).toEqual(['arriving', 'stop', 'leg', 'stop']) + expect(rows[0]).toMatchObject({ kind: 'arriving', seg: drive, from: { name: 'Lüneburg' }, mode: 'driving' }) + // Still two stops, numbered from one: the stop it leaves is not on this card. + expect(stopRows(rows).map(r => r.number)).toEqual([1, 2]) + expect(destinationCount(joined())).toBe(2) + expect(stageClocks(rows)).toEqual(stageClocks(roadtripRows(joined({ arrivingLeg: undefined })))) + expect(stageEnd(rows)?.stop.name).toBe('Potsdam') + }) + + it('FE-RTROW-057: no row for a hop, a ride, or a drive whose start the card was not told', () => { + const hop: RouteSegment = { ...seg(0), distance: 80, duration: 60 } + const ride: RouteSegment = { ...seg(0), distance: 0, duration: 9 * 3600, mode: 'flight' } + for (const over of [{ arrivingLeg: hop }, { arrivingLeg: ride }, { arrivingFrom: undefined }, { arrivingLeg: undefined }]) { + expect(roadtripRows(joined(over)).map(r => r.kind)).toEqual(['stop', 'leg', 'stop']) + } + }) + + it('FE-RTROW-058: other ways are offered on it exactly where a leg would be offered them', () => { + expect(arrivingReroutable(joined())).toBe(true) + // Nothing to offer against: no drive, no line, no stop it leaves or reaches. + expect(arrivingReroutable(joined({ arrivingLeg: undefined }))).toBe(false) + expect(arrivingReroutable(joined({ arrivingLine: undefined }))).toBe(false) + expect(arrivingReroutable(joined({ arrivingFrom: undefined }))).toBe(false) + expect(arrivingReroutable(joined({ stops: [] }))).toBe(false) + // A ride has no other way, and a hop is nothing to weigh. + expect(arrivingReroutable(joined({ arrivingLeg: { ...drive, mode: 'train' } }))).toBe(false) + expect(arrivingReroutable(joined({ arrivingLeg: { ...drive, distance: 80, duration: 60 } }))).toBe(false) + // A terminal leaves nothing a via can be filed behind. + const terminal = stop('Hamburg Airport', { carrier: { reservationId: 7, type: 'flight', role: 'arrival', title: 'LH 2020', code: null, at: '07:00' } }) + expect(arrivingReroutable(joined({ arrivingFrom: terminal }))).toBe(false) + // An automatic night at either end is a marker on the road, not a stop anybody chose. + expect(arrivingReroutable(joined({ arrivingFrom: night() }))).toBe(false) + expect(arrivingReroutable(joined({ stops: [night('start'), stop('Potsdam')] }))).toBe(false) + }) +}) diff --git a/client/src/components/Roadtrip/roadtripRowModel.ts b/client/src/components/Roadtrip/roadtripRowModel.ts index 25d5a3bb2..0a585d1ba 100644 --- a/client/src/components/Roadtrip/roadtripRowModel.ts +++ b/client/src/components/Roadtrip/roadtripRowModel.ts @@ -1,5 +1,5 @@ import { isServiceStopType, type ScheduleEntry, type ScheduleWarning } from './roadtripModel' -import type { CarrierTerminal, RoadtripDay, RoadtripStop, RouteSegment } from '@trek/shared/roadtrip' +import { isCarrierMode, type CarrierTerminal, type RoadtripDay, type RoadtripStop, type RouteSegment } from '@trek/shared/roadtrip' import { readStay } from './stayReading' /** @@ -21,6 +21,12 @@ export type RoadtripRow = | { kind: 'spill'; fromDayNumber: number; departs: string | null; stops: StopRow[] } | StopRow | { kind: 'leg'; index: number; seg: RouteSegment | undefined; mode: string | null } + /** + * The drive in from where the day before ended, above the card's first stop, on a trip + * with connected days. `from` is that last stop of the day before, which the card does + * not draw, so the row names it. + */ + | { kind: 'arriving'; seg: RouteSegment; from: RoadtripStop; mode: string | null } /** * A ride that leaves and lands on the day (#2428), as one row: the booking with both * its terminals inside it, in place of two stop rows and the leg between them. The @@ -156,6 +162,20 @@ export function roadtripRows(day: RoadtripDay): RoadtripRow[] { const rows: RoadtripRow[] = [] const spills = day.spills ?? [] + // The drive in from yesterday, where the desktop rail draws its band: above the first + // stop, because that is where it happens. A hop is left out like any other, and so is + // a ride: the terminal row it lands on already carries the booking, and a pill would + // read "No route" under a flight that has no road. + const arriving = day.arrivingLeg + if (arriving && day.arrivingFrom && !isHop(arriving) && !isCarrierMode(arriving.mode)) { + rows.push({ + kind: 'arriving', + seg: arriving, + from: day.arrivingFrom, + mode: day.stops[0]?.incomingLegMode ?? day.arrivingFrom.legMode ?? null, + }) + } + let number = 0 day.stops.forEach((stop, i) => { const spill = spills.find(s => s.at === i) @@ -250,6 +270,29 @@ export function legReroutable(day: RoadtripDay, index: number): boolean { && !day.stops[index].carrier } +/** + * Whether the drive arriving at the head of a connected card can be offered other ways. + * + * The same rule as `legReroutable`, read for the one drive a card holds without a stop of + * its own at its start: it leaves the last stop of the day before (`arrivingFrom`), and a + * choice is filed behind that stop, on that day, where the map already files a point + * dropped on this drive. It needs its road and the line it is drawn on, since that is + * what the picker offers as the current way. A ride has no other way, a hop is nothing to + * weigh, a terminal leaves nothing a via can be filed behind, and an automatic night at + * either end is a marker on the road rather than a stop anybody chose. + */ +export function arrivingReroutable(day: RoadtripDay): boolean { + const seg = day.arrivingLeg + const from = day.arrivingFrom + const to = day.stops[0] + return !!seg && !!from && !!to && !!day.arrivingLine + && !isCarrierMode(seg.mode) + && !isHop(seg) + && !from.carrier + && !from.automaticNight + && !to.automaticNight +} + /** Stops that carry a number, for a count that agrees with the numbering above. */ export function destinationCount(day: RoadtripDay): number { return day.stops.filter(s => !s.automaticNight && !s.carrier && !isServiceStopType(s.stopType)).length diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.test.ts b/client/src/components/Roadtrip/useRoadtripRoutes.test.ts index 89e105f58..732894ab1 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.test.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.test.ts @@ -776,6 +776,31 @@ describe('useRoadtripRoutes', () => { expect(inboundDry.lat).toBeGreaterThan(BERLIN[0]) expect(inboundDry.lat).toBeLessThan(LUENEBURG[0]) }) + + it('FE-ROADTRIP-ROUTES-049: switching what the trip avoids asks for the join again, with the new classes', async () => { + // The seam round skipped every join it already had an answer for, whatever that + // answer had been asked with, so the drive between two days kept its old road and + // its old minutes until the page was loaded again. + // + // The clock the requests are spaced by is held still and moved by hand, so neither + // round waits for real time between its requests. + let now = 10_000 + const clock = vi.spyOn(performance, 'now').mockImplementation(() => now) + try { + setting({ roadtrip_connect_days: true }) + const { days: daysList, assignments } = twoDays() + renderHook(() => useRoadtripRoutes(7, daysList, assignments)) + await waitFor(() => expect(askedFor(LUENEBURG, BERLIN)).toHaveLength(1)) + expect(askedFor(LUENEBURG, BERLIN)[0][1]).toMatchObject({ avoid: [] }) + + now += 10_000 + setting({ roadtrip_avoid: 'toll' }) + await waitFor(() => expect(askedFor(LUENEBURG, BERLIN)).toHaveLength(2)) + expect(askedFor(LUENEBURG, BERLIN)[1][1]).toMatchObject({ profile: 'driving', avoid: ['toll'] }) + } finally { + clock.mockRestore() + } + }) }) }) diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.ts b/client/src/components/Roadtrip/useRoadtripRoutes.ts index 188a8e339..b193e67bc 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.ts @@ -53,16 +53,25 @@ const stopKey = (s: RoadtripStop): string => `${s.lat.toFixed(5)},${s.lng.toFixed(5)},${s.legMode ?? ''},${s.incomingLegMode ?? ''}` /** - * The via points that shape the drive leaving a stop, as one comparable string. + * Everything a seam's answer depends on besides its two stops, as one comparable string: + * the mode it is driven in, the classes the trip avoids, and the via points that shape + * the drive leaving `from`. * * Read in two places that must agree: when deciding whether a seam still matches the * answer already in hand, and when recording what an answer was fetched for. One function - * so the two cannot drift and quietly stop refetching. + * so the two cannot drift and quietly stop refetching. The mode and the classes are in it + * because the vias alone were: switching motorways off, or the day's profile, re-ran the + * 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): string => - viasLeaving(from, viasByDay[from.ownerDayId] ?? []) - .map(v => `${v.lat.toFixed(5)},${v.lng.toFixed(5)}`) - .join('|') +const seamShape = (from: RoadtripStop, viasByDay: Record, mode: string, avoidKey: string): string => + [ + mode, + avoidKey, + viasLeaving(from, viasByDay[from.ownerDayId] ?? []) + .map(v => `${v.lat.toFixed(5)},${v.lng.toFixed(5)}`) + .join('|'), + ].join('#') /** 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)}` @@ -558,7 +567,7 @@ export function useRoadtripRoutes( * day in two pieces with a gap across the middle. */ const seams = useMemo(() => { - const out: { from: RoadtripStop; to: RoadtripStop; dayId: number }[] = [] + const out: { from: RoadtripStop; to: RoadtripStop; dayId: number; mode: string; shape: string }[] = [] 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 @@ -569,9 +578,11 @@ export function useRoadtripRoutes( // now. Skipping it whenever any answer existed is what made a via on a seam do // 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 have = seamLegs[legKey(from, to)] - if (have && have.shape === seamShape(from, viasByDay)) return - out.push({ from, to, dayId }) + if (have && have.shape === shape) return + out.push({ from, to, dayId, mode, shape }) } const routingChains = window ? [...plan, ...quietDays].sort((a, b) => a.dayNumber - b.dayNumber).filter(d => d.stops.length) @@ -595,8 +606,8 @@ export function useRoadtripRoutes( } } return out - }, [chains, plan, quietDays, window, legsByDay, seamLegs, rideLegs, viasByDay, connectDays]) - const seamKey = seams.map(s => `${legKey(s.from, s.to)}#${seamShape(s.from, viasByDay)}`).join(';') + }, [chains, plan, quietDays, window, legsByDay, seamLegs, rideLegs, viasByDay, connectDays, legModeOf, avoidKey]) + 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 * effect below: a seam, or a leg asked for again on demand (see `legRouter`). One clock @@ -616,7 +627,7 @@ export function useRoadtripRoutes( void (async () => { for (const seam of seams) { if (controller.signal.aborted) return - const mode = legModeOf(seam.from, seam.to, seam.dayId) + const { mode, shape } = seam // No router knows a ride. A booked ride's own pair has its leg in `rideLegs` and // never reaches here; what is left is a terminal seamed to an ordinary stop on // another card, which is a join of no minutes rather than a road. Filed here, @@ -625,7 +636,7 @@ export function useRoadtripRoutes( const key = legKey(seam.from, seam.to) const legs = carrierLegsFor([seam.from, seam.to], mode, (from, to) => dayNumberOf(to.ownerDayId) - dayNumberOf(from.ownerDayId), legKey) const leg = legs?.[key] - if (leg) setSeamLegs(prev => ({ ...prev, [key]: { ...leg, shape: seamShape(seam.from, viasByDay) } })) + if (leg) setSeamLegs(prev => ({ ...prev, [key]: { ...leg, shape } })) continue } // Paced before the request, against a clock that outlives this effect. @@ -670,7 +681,7 @@ export function useRoadtripRoutes( seg: { ...merged, mode }, line: r.coordinates, vias: [], - shape: seamShape(seam.from, viasByDay), + shape, }, })) } catch { diff --git a/client/src/components/Roadtrip/useRouteAlternatives.test.ts b/client/src/components/Roadtrip/useRouteAlternatives.test.ts index 6cb05934e..4cbd6a795 100644 --- a/client/src/components/Roadtrip/useRouteAlternatives.test.ts +++ b/client/src/components/Roadtrip/useRouteAlternatives.test.ts @@ -9,11 +9,11 @@ vi.mock('../Map/RouteCalculator', async importOriginal => ({ calculateAlternatives, })) -import { useRouteAlternatives, type AlternativesRequest } from './useRouteAlternatives' +import { ARRIVING_DRIVE, openOn, sameDrive, useRouteAlternatives, type AlternativesRequest } from './useRouteAlternatives' import type { RailLegRouter } from './useRoadtripRoutes' /** - * FE-ALTHOOK-001..012: asking for other ways of driving one leg. + * FE-ALTHOOK-001..013: 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. @@ -40,7 +40,7 @@ const route = vi.fn() function request(router: Partial = {}, over: Partial = {}): AlternativesRequest { return { dayId: 4, - index: 1, + drive: { kind: 'leg', index: 1 }, from: { lat: 53.55, lng: 9.99 }, to: { lat: 52.52, lng: 13.4 }, driven: { coordinates: RAIL, distance: 290_000, duration: 10_800 }, @@ -67,7 +67,7 @@ describe('useRouteAlternatives', () => { act(() => { result.current.ask(request()) }) expect(result.current.open).toMatchObject({ - dayId: 4, index: 1, loading: true, routes: [], proving: null, engine: 'osrm', + dayId: 4, drive: { kind: 'leg', index: 1 }, loading: true, routes: [], proving: null, engine: 'osrm', anchor: { dayId: 3, afterIndex: 2 }, ends: { from: 11, to: 12 }, }) @@ -173,11 +173,11 @@ describe('useRouteAlternatives', () => { const { result } = renderHook(() => useRouteAlternatives()) act(() => { result.current.ask(request()) }) - act(() => { result.current.ask(request({}, { index: 2 })) }) + act(() => { result.current.ask(request({}, { drive: { kind: 'leg', index: 2 } })) }) expect(signals[0].aborted).toBe(true) expect(signals[1].aborted).toBe(false) - expect(result.current.open?.index).toBe(2) + expect(result.current.open?.drive).toEqual({ kind: 'leg', index: 2 }) act(() => { result.current.close() }) expect(signals[1].aborted).toBe(true) @@ -242,9 +242,9 @@ describe('useRouteAlternatives', () => { expect(first?.aborted).toBe(true) expect(second?.aborted).toBe(false) - act(() => { result.current.ask(request({}, { index: 0 })) }) + act(() => { result.current.ask(request({}, { drive: { kind: 'leg', index: 0 } })) }) expect(second?.aborted).toBe(true) - expect(result.current.open).toMatchObject({ index: 0, proving: null }) + expect(result.current.open).toMatchObject({ drive: { kind: 'leg', index: 0 }, proving: null }) // Nothing open, nothing to mark. act(() => { result.current.close() }) @@ -253,3 +253,19 @@ describe('useRouteAlternatives', () => { expect(result.current.open).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) + expect(sameDrive({ kind: 'leg', index: 0 }, { kind: 'leg', index: 0 })).toBe(true) + expect(sameDrive({ kind: 'leg', index: 0 }, { kind: 'leg', index: 1 })).toBe(false) + expect(sameDrive(ARRIVING_DRIVE, { kind: 'leg', index: 0 })).toBe(false) + expect(sameDrive({ kind: 'leg', index: 0 }, ARRIVING_DRIVE)).toBe(false) + + const open = { dayId: 2, drive: ARRIVING_DRIVE } + expect(openOn(open, 2, { kind: 'arriving' })).toBe(true) + expect(openOn(open, 3, { kind: 'arriving' })).toBe(false) + expect(openOn(open, 2, { kind: 'leg', index: 0 })).toBe(false) + expect(openOn(null, 2, ARRIVING_DRIVE)).toBe(false) + }) +}) diff --git a/client/src/components/Roadtrip/useRouteAlternatives.ts b/client/src/components/Roadtrip/useRouteAlternatives.ts index 61f76dd44..09d3afde2 100644 --- a/client/src/components/Roadtrip/useRouteAlternatives.ts +++ b/client/src/components/Roadtrip/useRouteAlternatives.ts @@ -11,13 +11,30 @@ export interface OfferedRoute extends RouteAlternative { } /** - * Which drive on a card a picker is asked about: the leg from stop `index` to the next one. + * Which drive on a card a picker is asked about: the leg from stop `index` to the next + * one, or the drive `arriving` at the card's first stop from where the day before ended. * * A tagged union rather than a bare index, so the drive arriving at the head of a - * connected card can join it as a target of its own instead of borrowing an index no - * stop has. + * connected card is a target of its own instead of borrowing an index no stop has. */ -export type RailDrive = { kind: 'leg'; index: number } +export type RailDrive = { kind: 'leg'; index: number } | { kind: 'arriving' } + +/** The drive into a card from the day before. A card has one at most, so it needs no index. */ +export const ARRIVING_DRIVE: RailDrive = { kind: 'arriving' } + +/** Whether two drives are the same one of their card. */ +export function sameDrive(a: RailDrive, b: RailDrive): boolean { + if (a.kind === 'arriving' || b.kind === 'arriving') return a.kind === b.kind + return a.index === b.index +} + +/** + * Whether the picker is open on this drive of this card. The one test the rail, the + * planner and the phone ask, so a drive shows pressed exactly where the picker is. + */ +export function openOn(open: Pick | null | undefined, dayId: number, drive: RailDrive): boolean { + return !!open && open.dayId === dayId && sameDrive(open.drive, drive) +} /** Where the vias that shape a leg are stored: after stop `afterIndex` of day `dayId`. */ export interface ViaAnchor { @@ -27,9 +44,10 @@ export interface ViaAnchor { /** Which leg is being reconsidered, and what was offered for it. */ export interface LegAlternatives { + /** The card the drive is drawn on. */ dayId: number - /** Index of the leg within the day: the drive from stop `index` to `index + 1`. */ - index: number + /** Which drive of that card: one of its legs, or the drive arriving at its head. */ + drive: RailDrive /** Where a choice is written. Read off the stop the leg leaves, when it was asked about. */ anchor: ViaAnchor /** @@ -52,7 +70,7 @@ export interface LegAlternatives { /** Everything a picker needs to know about the leg it is opened on. */ export interface AlternativesRequest { dayId: number - index: number + drive: RailDrive from: { lat: number; lng: number } to: { lat: number; lng: number } /** The leg as the rail drives it now: its line, its figures, and the engine behind them. */ @@ -115,8 +133,8 @@ export function useRouteAlternatives(): RouteAlternativesState { stopProving() const controller = new AbortController() abortRef.current = controller - const { dayId, index, from, to, driven, anchor, ends, router } = request - const leg = { dayId, index, anchor, ends, engine: router.engine, route: router.route, proving: null } + const { dayId, drive, from, to, driven, anchor, ends, router } = request + const leg = { dayId, drive, anchor, ends, engine: router.engine, route: router.route, proving: null } setOpen({ ...leg, routes: [], loading: true, error: false }) const current: OfferedRoute = { diff --git a/client/src/mobile/screens/trip/roadtrip/MRoadtripRows.tsx b/client/src/mobile/screens/trip/roadtrip/MRoadtripRows.tsx index 63a4c1273..c5900643a 100644 --- a/client/src/mobile/screens/trip/roadtrip/MRoadtripRows.tsx +++ b/client/src/mobile/screens/trip/roadtrip/MRoadtripRows.tsx @@ -329,10 +329,16 @@ function legText(seg: RouteSegment | undefined, { t, unit }: RowChrome): string * reads as a button that was pressed and stuck. Offline it keeps its place but dims, * because a choice is saved as a via and vias are online only. Without `onAlternatives` * (no permission, or a leg that cannot be rerouted) the column stays empty. + * + * With `origin` it is the drive in from where the day before ended, at the head of the + * card. The stop it leaves is not on this card, so its name goes above the pill; without + * it the chain would open on a drive from nowhere. */ -export function RtLegRow({ seg, mode, chrome, onAlternatives, alternativesOpen = false, alternativesDisabled = false }: { +export function RtLegRow({ seg, mode, origin, chrome, onAlternatives, alternativesOpen = false, alternativesDisabled = false }: { seg: RouteSegment | undefined mode: string | null + /** The place the drive leaves from, named when it is not the stop drawn above it. */ + origin?: string chrome: RowChrome /** Asks for other ways of driving this leg. Absent means the leg offers none. */ onAlternatives?: () => void @@ -343,6 +349,12 @@ export function RtLegRow({ seg, mode, chrome, onAlternatives, alternativesOpen = const { t } = chrome const Icon = mode && LEG_ICONS[mode] ? LEG_ICONS[mode] : mode?.startsWith('plugin:') ? Zap : CarFront const text = legText(seg, chrome) + const pill = ( + + + ) return (
{/* flex-col, not flex: in a row the dashes would stretch sideways and read as @@ -351,10 +363,14 @@ export function RtLegRow({ seg, mode, chrome, onAlternatives, alternativesOpen =