feat(roadtrip): remember which track a day follows

The vias already made the day follow the road; nothing said which road. After a
reload the badge could only say "shaped by hand", and re-fitting after the stops
changed meant finding the track again by eye.

One row per day, written in the same transaction as the chain it describes: a
day claiming to follow a road whose vias never landed would be worse than a day
claiming nothing. Three states rather than two, which is why the field is an
object and not an id — absent leaves whatever the day followed alone, null says
it follows nothing, an object records the road it now takes. A bare nullable id
could not tell "do not touch this" apart from "forget it", and a hand-dragged
via on the same day must not wipe the label.

The foreign key is the point: delete the imported track and the label cascades
with it, so no day is left claiming to follow a line nobody can see. The vias
stay, because they are the drive.

Both write paths check that the place is a track of this trip before it becomes
a label — permission alone would let another trip's place id, or an ordinary
place, become a name that can never be drawn.

Also clears three reliability findings the scanner raised on this branch: two
default sorts given an explicit comparator, and a trailing-slash regex the
scanner is right to call backtracking, walked instead.
This commit is contained in:
Maurice
2026-09-05 00:49:59 +02:00
committed by Maurice
parent 6fca238440
commit 676c82a308
40 changed files with 356 additions and 31 deletions
+14 -1
View File
@@ -31,10 +31,23 @@ const OSRM_PROFILE_PATH: Record<'driving' | 'walking' | 'cycling', string> = {
* A configured base is expected to serve the standard OSRM layout,
* `<base>/route/v1/<profile>/…`, which is what `osrm-routed` does out of the box.
*/
/**
* A configured base URL without its trailing slashes.
*
* Walked rather than matched with `/\/+$/`: that pattern backtracks over a run of
* slashes, and the value comes from an instance setting somebody types. Linear either
* way in practice, but the regex is the shape a scanner is right to flag.
*/
function withoutTrailingSlashes(value: string): string {
let end = value.length
while (end > 0 && value[end - 1] === '/') end -= 1
return value.slice(0, end)
}
function routeBaseFor(profile: 'driving' | 'walking' | 'cycling'): string {
const configured = useSettingsStore.getState().settings.routing_base_url?.trim()
if (!configured) return OSRM_PROFILE_BASE[profile]
return `${configured.replace(/\/+$/, '')}/route/v1/${OSRM_PROFILE_PATH[profile]}`
return `${withoutTrailingSlashes(configured)}/route/v1/${OSRM_PROFILE_PATH[profile]}`
}
/**
@@ -236,7 +236,9 @@ export default function RoadtripCorridorPanel({ corridor, routes, onAddPoi }: Ro
const socketsFound = useMemo(() => {
const seen = new Set<string>()
for (const p of search.results) for (const s of p.charging?.sockets ?? []) seen.add(s.type)
return [...seen].sort()
// Compared explicitly: the default sort is by UTF-16 code unit, which is not the
// order anybody reads a list of plug names in.
return [...seen].sort((a, b) => a.localeCompare(b))
}, [search.results])
const hasCharging = socketsFound.length > 0
@@ -60,6 +60,8 @@ interface RoadtripSidebarProps {
* what somebody reading the day needs to know.
*/
viaCounts?: Record<number, number>
/** The name of the track each day follows, so the badge can say which road it is. */
trackNames?: Record<number, string>
}
const MODE_ICON: Record<string, LucideIcon> = {
@@ -759,7 +761,7 @@ function Stop({ stop, number, entry, late, driveFindings, selected, continues, s
* (`dayOrderMap` numbers the selected day's assignments from 1). A rail counting across
* the trip would put "17" beside a pin the map calls "3".
*/
function DaySection({ day, selectedAssignmentId, onSelectStop, onReorderStop, onMoveStopToDay, drag, onAskAlternatives, openAlternatives, onEditStay, onSetStopKind, onFollowTrack, viaCount }: {
function DaySection({ day, selectedAssignmentId, onSelectStop, onReorderStop, onMoveStopToDay, drag, onAskAlternatives, openAlternatives, onEditStay, onSetStopKind, onFollowTrack, viaCount, trackName }: {
day: RoadtripDay
selectedAssignmentId?: number | null
onSelectStop?: (placeId: number, assignmentId: number) => void
@@ -773,6 +775,7 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onReorderStop, on
onSetStopKind?: RoadtripSidebarProps['onSetStopKind']
onFollowTrack?: RoadtripSidebarProps['onFollowTrack']
viaCount?: number
trackName?: string
}): React.ReactElement {
const { from, setFrom, dropAt, setDropAt } = drag
const dragging = from?.dayId === day.dayId ? from.index : null
@@ -837,7 +840,7 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onReorderStop, on
none of them, so this badge is the only place a drive shaped by hand differs
from one the router picked on its own. */}
{onFollowTrack ? (
<Tooltip label={t('roadtrip.track.hint')}>
<Tooltip label={trackName ? t('roadtrip.track.current', { name: trackName }) : t('roadtrip.track.hint')}>
<button
type="button"
onClick={() => onFollowTrack(day.dayId)}
@@ -1029,7 +1032,7 @@ function QuietDaySection({ day, onMoveStopToDay, drag }: {
*/
export default function RoadtripSidebar({
routes, selectedAssignmentId, onSelectStop, onReorderStop, onMoveStopToDay, onAskAlternatives, openAlternatives, onEditStay,
onSetStopKind, onFollowTrack, viaCounts,
onSetStopKind, onFollowTrack, viaCounts, trackNames,
}: RoadtripSidebarProps): React.ReactElement {
const { t } = useTranslation()
// One drag state for the whole rail rather than one per day: a stop that cannot leave
@@ -1074,6 +1077,7 @@ export default function RoadtripSidebar({
onSetStopKind={onSetStopKind}
onFollowTrack={onFollowTrack}
viaCount={viaCounts?.[day.dayId] ?? 0}
trackName={trackNames?.[day.dayId]}
/>
))}
{routes.quietDays.map(day => (
@@ -29,6 +29,8 @@ function follow(over: Partial<FollowTrack> = {}): FollowTrack {
clear: vi.fn().mockResolvedValue(undefined),
viaCount: 0,
available: true,
current: null,
namesByDay: {},
...over,
}
}
@@ -75,7 +75,7 @@ export default function RoadtripTrackModal({ follow, dayNumber }: {
const distanceUnit = useSettingsStore(s => s.settings.distance_unit)
const [picked, setPicked] = useState<number | null>(null)
const { dayId, busy, round, error, outcome, tracks, viaCount } = follow
const { dayId, busy, round, error, outcome, tracks, viaCount, current } = follow
// A track picked on one day means nothing on the next.
useEffect(() => { setPicked(null) }, [dayId])
@@ -97,6 +97,14 @@ export default function RoadtripTrackModal({ follow, dayNumber }: {
}
>
<div className="flex flex-col gap-3">
{/* What the day follows today, before anything is picked. Read back from the day
rather than remembered from the run that applied it, which is the whole reason
it is stored: it has to survive a reload. */}
{current ? (
<p className="rounded-xl bg-surface-secondary px-3 py-2 text-caption text-content-secondary">
{t('roadtrip.track.current', { name: current.name })}
</p>
) : null}
{tracks.length === 0 ? (
<div className="flex flex-col items-center gap-2 py-2">
<EmptyState scene="transport" size={96} compact title={t('roadtrip.track.empty')} />
@@ -47,6 +47,7 @@ const routes = (days: RoadtripDay[]): RoadtripRoutes => ({
function viasStub(over: Partial<RoadtripVias> = {}): RoadtripVias {
return {
byDay: {},
trackByDay: {},
add: vi.fn(),
addMany: vi.fn().mockResolvedValue(undefined),
move: vi.fn(),
@@ -88,6 +89,26 @@ describe('useFollowTrack', () => {
expect(result.current.available).toBe(true)
})
it('FE-FOLLOWHOOK-011: the track a day follows is read back from the day, not remembered', () => {
// The whole point of storing it: a field that only ever held what this session did
// would look right until somebody refreshed the page.
const vias = viasStub({ trackByDay: { 7: { day_id: 7, place_id: 3, stray_km: 0.8 } } });
const { result } = renderHook(() => useFollowTrack(1, [TRACK], routes([day()]), vias))
expect(result.current.namesByDay).toEqual({ 7: 'Atlantic Road' })
act(() => { result.current.open(7) })
expect(result.current.current).toEqual({ name: 'Atlantic Road', strayKm: 0.8 })
})
it('FE-FOLLOWHOOK-012: a stored track whose place is gone names nothing', () => {
// The row cascades with the place, so this is belt and braces — but a label that
// outlived its line would be a day claiming to follow something invisible.
const vias = viasStub({ trackByDay: { 7: { day_id: 7, place_id: 999, stray_km: null } } })
const { result } = renderHook(() => useFollowTrack(1, [TRACK], routes([day()]), vias))
act(() => { result.current.open(7) })
expect(result.current.current).toBeNull()
})
it('FE-FOLLOWHOOK-010: a trip that imported nothing offers no way in', () => {
const { result } = renderHook(() => useFollowTrack(1, [PLAIN], routes([day()]), viasStub()))
expect(result.current.available).toBe(false)
@@ -112,9 +133,12 @@ describe('useFollowTrack', () => {
await act(async () => { await result.current.apply(3) })
expect(vias.addMany).toHaveBeenCalledTimes(1)
const [dayId, written, legs] = (vias.addMany as ReturnType<typeof vi.fn>).mock.calls[0]
const [dayId, written, legs, track] = (vias.addMany as ReturnType<typeof vi.fn>).mock.calls[0]
expect(dayId).toBe(7)
expect(legs).toEqual([0])
// The chain and the name it came from land in one write: a day claiming to follow a
// road whose vias never arrived would be worse than a day claiming nothing.
expect(track).toMatchObject({ place_id: 3 })
expect(written.length).toBeGreaterThan(0)
// Every via hangs behind the first stop, because the day has exactly one leg.
expect(written.every((v: { after_order_index: number }) => v.after_order_index === 0)).toBe(true)
@@ -131,8 +155,9 @@ describe('useFollowTrack', () => {
await act(async () => { await result.current.apply(3) })
expect(result.current.outcome).toEqual({ vias: 0, strayKm: 0, capped: false })
// Still a write: the legs are cleared, so applying a second track does not stack.
expect(vias.addMany).toHaveBeenCalledWith(7, [], [0])
// Still a write: the legs are cleared so a second track does not stack, and the name
// is recorded even though the drive needed no bending to follow it.
expect(vias.addMany).toHaveBeenCalledWith(7, [], [0], { place_id: 3, stray_km: 0 })
})
it('FE-FOLLOWHOOK-005: a routing outage is reported rather than written as an empty plan', async () => {
@@ -168,7 +193,8 @@ describe('useFollowTrack', () => {
expect(result.current.viaCount).toBe(2)
await act(async () => { await result.current.clear() })
expect(vias.addMany).toHaveBeenCalledWith(7, [], [0])
// null, not nothing: dropping the points is also dropping the claim.
expect(vias.addMany).toHaveBeenCalledWith(7, [], [0], null)
expect(result.current.outcome).toEqual({ vias: 0, strayKm: 0, capped: false })
})
@@ -50,6 +50,16 @@ export interface FollowTrack {
clear: () => Promise<void>
/** How many vias the open day carries — what makes clearing worth offering. */
viaCount: number
/**
* The track the open day follows, if it follows one.
*
* Read back from the day rather than remembered from the run that applied it: the point
* of storing it at all is that it survives a reload, and a field that only ever held
* what this session did would look right until somebody refreshed.
*/
current: { name: string; strayKm: number | null } | null
/** The name of the track each day follows, for the rail's badge. */
namesByDay: Record<number, string>
/**
* Whether the trip holds a track at all.
*
@@ -185,7 +195,9 @@ export function useFollowTrack(
if (controller.signal.aborted) { setBusy(false); return }
try {
await vias.addMany(dayId, plan.vias, plan.legs)
// The chain and the name it came from, in one write: a day claiming to follow a
// road whose vias never landed would be worse than a day claiming nothing.
await vias.addMany(dayId, plan.vias, plan.legs, { place_id: trackId, stray_km: plan.strayKm })
setOutcome({ vias: plan.vias.length, strayKm: plan.strayKm, capped: plan.capped })
} catch {
setError('save')
@@ -195,6 +207,29 @@ export function useFollowTrack(
}, [tracks, dayId, stops, busy, tripId, vias])
const viaCount = dayId === null ? 0 : (vias.byDay[dayId]?.length ?? 0)
/**
* The name behind each day's stored track id.
*
* A lookup over `places` rather than a parse: naming a line costs a row, and the rail
* asks for this on every render.
*/
const namesByDay = useMemo(() => {
const names: Record<number, string> = {}
const byId = new Map(places.map(p => [p.id, p]))
for (const [day, track] of Object.entries(vias.trackByDay)) {
const place = byId.get(track.place_id)
if (place?.name) names[Number(day)] = place.name
}
return names
}, [places, vias.trackByDay])
const current = useMemo(() => {
if (dayId === null) return null
const track = vias.trackByDay[dayId]
const name = namesByDay[dayId]
return track && name ? { name, strayKm: track.stray_km } : null
}, [dayId, vias.trackByDay, namesByDay])
// A cheap look at the column, not a parse: this runs on every planner render, and the
// question is only whether anything at all was ever imported.
const available = useMemo(() => places.some(p => (p.route_geometry?.length ?? 0) > 2), [places])
@@ -206,7 +241,8 @@ export function useFollowTrack(
try {
// Every leg of the day, so nothing is left behind on a leg the track never reached.
const legs = Array.from({ length: Math.max(0, stops.length - 1) }, (_, i) => i)
await vias.addMany(dayId, [], legs)
// null rather than nothing: dropping the points is also dropping the claim.
await vias.addMany(dayId, [], legs, null)
setOutcome({ vias: 0, strayKm: 0, capped: false })
} catch {
setError('save')
@@ -215,5 +251,5 @@ export function useFollowTrack(
}
}, [dayId, busy, stops.length, vias])
return { dayId, open, close, tracks, busy, round, error, outcome, apply, clear, viaCount, available }
return { dayId, open, close, tracks, busy, round, error, outcome, apply, clear, viaCount, available, current, namesByDay }
}
@@ -310,7 +310,9 @@ export function useRoadtripRoutes(
const viaKey = useMemo(
() => Object.entries(viasByDay)
.map(([dayId, vias]) => `${dayId}:${vias.map(v => `${v.after_order_index}@${v.lat.toFixed(5)},${v.lng.toFixed(5)}`).join('|')}`)
.sort()
// Only the order has to be stable — this is a cache key, not a list anybody reads
// — but it has to be stable on purpose rather than by default.
.sort((a, b) => a.localeCompare(b))
.join(';'),
[viasByDay],
)
@@ -1,11 +1,19 @@
import { useCallback, useEffect, useState } from 'react'
import { roadtripApi } from '../../api/client'
import { isEmptyReanchoring, type Reanchoring } from './roadtripModel'
import type { RoadtripVia } from '@trek/shared'
import type { RoadtripDayTrack, RoadtripVia } from '@trek/shared'
export interface RoadtripVias {
/** Every via of the trip, keyed by day. */
byDay: Record<number, RoadtripVia[]>
/**
* Which imported track each day was fitted to, keyed by day.
*
* The vias are what make the day follow it; this is what lets the day say so. Empty for
* a day shaped by hand, and gone by itself when the track is deleted — the row cascades
* with the place, so nothing here can name a line that no longer exists.
*/
trackByDay: Record<number, RoadtripDayTrack>
add: (dayId: number, afterOrderIndex: number, lat: number, lng: number) => Promise<void>
/**
* Lay a chain of vias on one day, optionally clearing the legs it fills first.
@@ -14,7 +22,13 @@ export interface RoadtripVias {
* trip re-route between each one, spaced by the routing host's rate limit, so a
* twenty-anchor track would spend half a minute drawing routes nobody asked to see.
*/
addMany: (dayId: number, vias: { after_order_index: number; lat: number; lng: number }[], replaceLegs?: number[]) => Promise<void>
addMany: (
dayId: number,
vias: { after_order_index: number; lat: number; lng: number }[],
replaceLegs?: number[],
/** Absent leaves the day's track alone, null clears it, an object records a new one. */
track?: { place_id: number; stray_km?: number | null } | null,
) => Promise<void>
move: (dayId: number, id: number, lat: number, lng: number) => Promise<void>
remove: (dayId: number, id: number) => Promise<void>
/**
@@ -28,6 +42,7 @@ export interface RoadtripVias {
}
const EMPTY: Record<number, RoadtripVia[]> = {}
const EMPTY_TRACKS: Record<number, RoadtripDayTrack> = {}
/**
* The points this trip's drives are routed through.
@@ -42,6 +57,7 @@ const EMPTY: Record<number, RoadtripVia[]> = {}
*/
export function useRoadtripVias(tripId: number | string | null, active: boolean): RoadtripVias {
const [byDay, setByDay] = useState<Record<number, RoadtripVia[]>>(EMPTY)
const [trackByDay, setTrackByDay] = useState<Record<number, RoadtripDayTrack>>(EMPTY_TRACKS)
const group = useCallback((vias: RoadtripVia[]) => {
const next: Record<number, RoadtripVia[]> = {}
@@ -50,14 +66,18 @@ export function useRoadtripVias(tripId: number | string | null, active: boolean)
}, [])
const reload = useCallback(async () => {
if (!tripId || !active) { setByDay(EMPTY); return }
if (!tripId || !active) { setByDay(EMPTY); setTrackByDay(EMPTY_TRACKS); return }
try {
const { vias } = await roadtripApi.listVias(tripId)
const { vias, tracks } = await roadtripApi.listVias(tripId)
setByDay(group(vias))
const byId: Record<number, RoadtripDayTrack> = {}
for (const track of tracks ?? []) byId[track.day_id] = track
setTrackByDay(byId)
} catch {
// An instance with the addon off answers 404 here, which is not an error worth
// reporting — it just means there are no vias to draw.
setByDay(EMPTY)
setTrackByDay(EMPTY_TRACKS)
}
}, [tripId, active, group])
@@ -73,12 +93,18 @@ export function useRoadtripVias(tripId: number | string | null, active: boolean)
dayId: number,
vias: { after_order_index: number; lat: number; lng: number }[],
replaceLegs?: number[],
track?: { place_id: number; stray_km?: number | null } | null,
) => {
if (!tripId) return
// An empty chain with nothing to clear is not a write. It is a real call though, from
// a track that thinned down to nothing on a very short day.
if (!vias.length && !replaceLegs?.length) return
await roadtripApi.addVias(tripId, dayId, { vias, replace_legs: replaceLegs })
// a track that thinned down to nothing on a very short day, and from a track that the
// drive already followed — which still has a name to record.
if (!vias.length && !replaceLegs?.length && track === undefined) return
await roadtripApi.addVias(tripId, dayId, {
vias,
replace_legs: replaceLegs,
...(track === undefined ? {} : { track }),
})
await reload()
}, [tripId, reload])
@@ -100,5 +126,5 @@ export function useRoadtripVias(tripId: number | string | null, active: boolean)
await reload()
}, [tripId, reload])
return { byDay, add, addMany, move, remove, reanchor }
return { byDay, trackByDay, add, addMany, move, remove, reanchor }
}