feat(places): make the kind of stop editable, and enforce it on the way in

Three holes, one seam. Leaving the corridor popup by "more details" dropped the
kind and the dwell it had just worked out, so a fuel stop silently became a
numbered destination that counted in every total, in the badge and in the PDF.
Once set, the kind could never be changed back through the web UI at all. And
the REST route did not validate the field: both request schemas are open
objects, so stop_type: 'banana' was written straight to the column, where
isServiceStopType read it as false and the stop counted as a destination again.

The form now carries the field, so the popup's answer survives the trip through
it, an ordinary place can become a service stop, and a wrong kind can be taken
off. Editing keeps stop_type and deliberately leaves duration_minutes out: how
long a stay takes belongs to the rail's own dialog, and sending it from here
would overwrite what was set there.

The schemas gain the one field rather than closing: roughly 190 call sites post
whatever their form holds, and a closed object would reject all of them at once.
The doc comment claiming the list was already enforced is corrected — it was
describing an intention.

The two driving findings move into the stop's badge row in warning colours,
beside the stay and the walk from the road. All three say what this stop costs,
and one row reads as one answer rather than as notes stacked underneath.
This commit is contained in:
Maurice
2026-09-04 22:29:47 +02:00
committed by Maurice
parent e79a2591a1
commit ea9de65550
30 changed files with 205 additions and 40 deletions
@@ -1,3 +1,5 @@
import type { RoadtripStopType } from '@trek/shared'
export interface PlaceFormData {
name: string
description: string
@@ -24,6 +26,14 @@ export interface PlaceFormData {
// submit payload when unchanged, and useTripPlanner strips it off the place
// update and PUTs it per assignment instead.
assignment_notes?: string
// What kind of stop on a drive this is, if any (#1797). Optional and absent from
// DEFAULT_FORM for the same reason as image_url: a missing key writes nothing, and a
// place created outside a road trip has no opinion about fuel stops.
stop_type?: RoadtripStopType | null
// Offered only while creating, and only alongside a stop_type: the corridor popup
// suggests how long that kind of pause usually takes, and the full form should not
// throw the suggestion away on the way through. Editing a stay is the rail's job.
duration_minutes?: number
}
export function isGoogleMapsUrl(input: string): boolean {
@@ -1,14 +1,16 @@
import { useState, useEffect, useRef, useMemo, useCallback } from 'react'
import Modal from '../shared/Modal'
import type { RoadtripStopType } from '@trek/shared'
import { STOP_KINDS } from '../Roadtrip/stopKinds'
import CustomSelect from '../shared/CustomSelect'
import NoteFormatToolbar from '../shared/NoteFormatToolbar'
import { mapsApi } from '../../api/client'
import { recordPlacePick } from '../../api/placeShadow'
import { useAuthStore } from '../../store/authStore'
import { useAddonStore } from '../../store/addonStore'
import { useCanDo } from '../../store/permissionsStore'
import { useTripStore } from '../../store/tripStore'
import { useSettingsStore } from '../../store/settingsStore'
import { useAddonStore } from '../../store/addonStore'
import CollectionPicker from '../Collections/CollectionPicker'
import PlaceDetailsColumn, { type PlaceDetailsSelection } from './PlaceDetailsColumn'
import { useToast } from '../shared/Toast'
@@ -38,7 +40,7 @@ interface PlaceFormModalProps {
onClose: () => void
onSave: (data: PlaceSubmitData, files?: File[]) => Promise<{ id: number } | void> | void
place: Place | null
prefillCoords?: { lat: number; lng: number; name?: string; address?: string; website?: string; phone?: string; osm_id?: string } | null
prefillCoords?: { lat: number; lng: number; name?: string; address?: string; website?: string; phone?: string; osm_id?: string; stop_type?: RoadtripStopType | null; duration_minutes?: number } | null
tripId: number
categories: Category[]
onCategoryCreated: (category: { name: string; color?: string; icon?: string }) => Promise<Category> | undefined
@@ -88,6 +90,9 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
onCategoryCreated, assignmentId, dayAssignments = [], isMobile = false,
onOpenExpense,
} = props
// Hidden while the addon is off, because the kinds only mean anything to the road trip
// rail: on an instance without it they would be six labels that change nothing.
const roadtripEnabled = useAddonStore(s => s.isEnabled('roadtrip'))
const [form, setForm] = useState(DEFAULT_FORM)
const [mapsSearch, setMapsSearch] = useState('')
const [mapsResults, setMapsResults] = useState([])
@@ -153,6 +158,11 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
notes: place.notes || '',
transport_mode: place.transport_mode || 'walking',
website: place.website || '',
// Carried through every edit. Without it, opening a fuel stop to fix a typo
// submits an empty kind and turns it back into a numbered destination.
// duration_minutes deliberately stays out: how long a stay takes belongs to the
// rail's own dialog, and sending it from here would overwrite what was set there.
stop_type: place.stop_type ?? null,
// The day-specific note rides only with an assignment in context (#2163);
// otherwise the key stays absent so submit never sends a notes write.
...(assignment ? { assignment_notes: assignment.notes || '' } : {}),
@@ -167,6 +177,8 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
website: prefillCoords.website || '',
phone: prefillCoords.phone || '',
osm_id: prefillCoords.osm_id,
stop_type: prefillCoords.stop_type ?? null,
duration_minutes: prefillCoords.duration_minutes,
})
} else {
setForm(DEFAULT_FORM)
@@ -500,6 +512,15 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
lat: form.lat ? Number.parseFloat(form.lat) : null,
lng: form.lng ? Number.parseFloat(form.lng) : null,
category_id: form.category_id || null,
// An explicit null is how a stop stops being a fuel stop; the service reads it
// that way rather than as "leave alone", which is what a missing key means.
stop_type: form.stop_type || null,
// Only on the way in, and only with a kind: it is the popup's suggestion for how
// long that kind of pause takes. On an edit it is left out entirely, because the
// stay belongs to the rail's dialog and sending it here would overwrite it.
...(!place && form.stop_type && form.duration_minutes
? { duration_minutes: form.duration_minutes }
: {}),
_pendingFiles: pendingFiles.length > 0 ? pendingFiles : undefined,
}
// #2163: the per-assignment note only travels when an assignment is in
@@ -548,6 +569,7 @@ function usePlaceFormModal(props: PlaceFormModalProps) {
dayAssignments,
isMobile,
collectionsEnabled,
roadtripEnabled,
form,
setForm,
mapsSearch,
@@ -620,6 +642,7 @@ export default function PlaceFormModal(props: PlaceFormModalProps) {
dayAssignments,
isMobile,
collectionsEnabled,
roadtripEnabled,
form,
setForm,
mapsSearch,
@@ -891,6 +914,30 @@ export default function PlaceFormModal(props: PlaceFormModalProps) {
</div>
</div>
{/* What kind of stop this is on a drive.
Under the category rather than beside it, because the two answer different
questions: a category is the traveller's own label, editable and shared across
the whole instance, while this is a fact about the place. Refuelling is not a
taste. Shown always, not only in road trip mode: a fuel stop added on a Tuesday
is still a fuel stop when the mode is switched on later, and this is the only
place in the web UI where the kind can be taken off again. */}
{roadtripEnabled ? (
<div>
<label className="block text-sm font-medium text-content-secondary mb-1">{t('roadtrip.stop.kind')}</label>
<CustomSelect
value={form.stop_type ?? ''}
onChange={value => handleChange('stop_type', String(value) as RoadtripStopType | '')}
placeholder={t('roadtrip.stop.none')}
options={[
{ value: '', label: t('roadtrip.stop.none') },
...STOP_KINDS.map(k => ({ value: k.key, label: t(k.labelKey) })),
]}
size="sm"
/>
<p className="mt-1 text-caption text-content-faint">{t('roadtrip.stop.kindHelp')}</p>
</div>
) : null}
{/* Category */}
<div>
<label className="block text-sm font-medium text-content-secondary mb-1">{t('places.formCategory')}</label>
@@ -1,7 +1,7 @@
import React from 'react'
import {
MapPin, CarFront, Footprints, Bike, Zap, AlertTriangle, Moon,
ParkingSquare, Shuffle,
ParkingSquare, Shuffle, Fuel, Clock,
type LucideIcon,
} from 'lucide-react'
import { useTranslation } from '../../i18n/TranslationContext'
@@ -454,15 +454,19 @@ function RouteViaStop({ via }: { via: RouteVia }): React.ReactElement {
}
/**
* A finding about the drive rather than about a stop: too long at the wheel in one go, or
* the tank running out before anywhere to fill it.
* A finding about the drive leaving this stop: too long at the wheel in one go, or the
* tank running out before anywhere to fill it.
*
* Sits in the chain between the two stops the drive runs between, because that is where
* it happens. Neither finding names a time of day: with no stop pinned to a clock the
* cascade produces no times at all, so both are durations and distances, which exist as
* soon as the leg has routed.
* Wears the same two-part shell as the stay and the walk, in warning colours, and sits in
* the same row: all of them say what this stop costs, and one row of badges reads as one
* answer rather than as notes stacked under each other. The index is the stop the leg
* LEAVES, so the badge means "the drive from here".
*
* Neither finding names a time of day. With no stop pinned to a clock the cascade
* produces no times at all, so both are durations and distances, which exist as soon as
* the leg has routed.
*/
function DriveFinding({ warning }: { warning: ScheduleWarning }): React.ReactElement | null {
function DriveFindingBadge({ warning }: { warning: ScheduleWarning }): React.ReactElement | null {
const { t } = useTranslation()
const distanceUnit = useSettingsStore(s => s.settings.distance_unit)
const text = warning.code === 'leg'
@@ -471,21 +475,24 @@ function DriveFinding({ warning }: { warning: ScheduleWarning }): React.ReactEle
? t('roadtrip.limit.range', { distance: formatDistance(warning.sinceKm ?? 0, distanceUnit) })
: null
if (!text) return null
const Icon = warning.code === 'range' ? Fuel : Clock
return (
<div className="grid items-stretch" style={RAIL_GRID}>
<span className="relative z-[1] flex flex-col items-center">
<span className="flex-1" style={RAIL_DASH} aria-hidden />
<span
className="inline-flex h-[16px] items-stretch self-start overflow-hidden rounded border border-warning-soft"
title={text}
>
<span className="flex items-center bg-warning-soft px-1 text-warning" style={{ fontSize: FS.micro }}>
<Icon size={9} aria-hidden />
</span>
<span className="px-1.5 pb-1">
<span
className="inline-flex w-fit items-center gap-1 rounded-full bg-warning-soft px-1.5 py-0.5 font-semibold text-warning"
style={{ fontSize: FS.label }}
>
<AlertTriangle size={10} className="shrink-0" aria-hidden />
{text}
</span>
<span
className="flex items-center border-s border-warning-soft bg-surface-card px-1.5 font-semibold tabular-nums text-warning"
style={{ fontSize: FS.label }}
>
{warning.code === 'range'
? formatDistance(warning.sinceKm ?? 0, distanceUnit)
: `+${formatDurationShort((warning.overMinutes ?? 0) * 60)}`}
</span>
</div>
</span>
)
}
@@ -556,12 +563,14 @@ function Arrival({ entry }: { entry: ScheduleEntry }): React.ReactElement {
)
}
function Stop({ stop, number, entry, late, selected, continues, onSelect, onMove, canMove, onEditStay }: {
function Stop({ stop, number, entry, late, driveFindings, selected, continues, onSelect, onMove, canMove, onEditStay }: {
stop: RoadtripStop
/** Position within the day — the same count the map badges its markers with. */
number: number
entry: ScheduleEntry | undefined
late: ScheduleWarning | undefined
/** Findings about the drive LEAVING this stop, when the limits are set and it goes over. */
driveFindings?: ScheduleWarning[]
selected: boolean
/** Whether the chain goes on below, so the marker keeps hold of the line. */
continues: boolean
@@ -629,6 +638,7 @@ function Stop({ stop, number, entry, late, selected, continues, onSelect, onMove
<span className="flex flex-wrap items-center gap-1">
<StayBadge minutes={stop.dwellMinutes} onEdit={onEditStay} />
{spurWorthLabelling(stop.offRoadMeters) ? <OffRoadBadge meters={stop.offRoadMeters ?? 0} /> : null}
{(driveFindings ?? []).map(w => <DriveFindingBadge key={w.code} warning={w} />)}
</span>
{lateText ? (
<span
@@ -784,6 +794,7 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onReorderStop, on
number={counted}
entry={day.schedule.entries[i]}
late={marks.find(w => w.code === 'late')}
driveFindings={day.driveWarnings.filter(w => w.index === i)}
selected={selectedAssignmentId === stop.assignmentId}
continues={i < last}
onSelect={onSelectStop ? () => onSelectStop(stop.placeId, stop.assignmentId) : undefined}
@@ -799,12 +810,6 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onReorderStop, on
alternativesOpen={openAlternatives?.dayId === day.dayId && openAlternatives.index === i}
/>
) : null}
{/* Between the two stops, which is where "somewhere on this drive" actually
is. The range finding is the only one with its remedy right beside it:
fuel and charging are the corridor search's first two categories. */}
{i < last ? day.driveWarnings.filter(w => w.index === i).map(w => (
<DriveFinding key={w.code} warning={w} />
)) : null}
{/* After the band, because a plugin halt happens on the drive it describes
rather than before setting off. */}
{i < last ? (day.legVias[i] ?? []).map((via, vi) => (
@@ -39,7 +39,8 @@ interface RoadtripStopPopupProps {
onClose: () => void
onSave: (input: { stopType: RoadtripStopType | null; dwellMinutes: number }) => Promise<void> | void
/** Opens the full place form instead, carrying what has been filled in so far. */
onMoreDetails: () => void
/** Hands over what has been picked here, so the full form opens on the same answer. */
onMoreDetails: (stop: { stopType: RoadtripStopType | null; dwellMinutes: number }) => void
}
/**
@@ -161,7 +162,7 @@ export default function RoadtripStopPopup({
<div className="flex items-center gap-2 border-t border-edge-faint pt-3">
<button
type="button"
onClick={onMoreDetails}
onClick={() => onMoreDetails({ stopType, dwellMinutes: dwell })}
className="rounded-lg px-2.5 py-1.5 text-caption text-content-muted transition-colors hover:bg-surface-hover hover:text-content"
>
{t('roadtrip.stop.moreDetails')}