From b2d9b25bb6611324b02ac68ec17aa3a497b7aa91 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 7 Sep 2026 17:23:55 +0200 Subject: [PATCH] fix(roadtrip): say which filter emptied the list, and stop typing a limit racing itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four small things, and one that was not this branch's fault but is now its neighbour. The driving limits saved on every keystroke. Typing "180" sent three settings writes carrying 1, 18 and 180, unordered and concurrent over HTTP/2, so whichever the server committed last won: the number that decides every over-budget warning could quietly end up a tenth of what was typed, and only show it after the next reload. Typed locally now, committed on blur or Enter, and a value that did not actually change writes nothing. The empty-result message always blamed the name box. Four controls narrow that list and only one of them is the name, so picking a section with no hits produced `Nothing on the way matches ""` — empty quotes naming a filter the reader never set. The count line above it was gated the same way, so the section case lost its "x of y" as well. Both follow whatever actually narrowed the list. The check-out dropdown printed a bare ISO date, the only place in the planner that did; it goes through the same helper as the rail beside it. Switching an addon off left open MCP sessions holding its write tools. A `when:` gate runs once per session, at attach time, so the tools stay registered and callable while the REST half answers 404 for the same user in the same moment — and the list that decides whether to tear those sessions down had drifted: airtrail and collections gate tools and were missing from it, which this addon would have inherited. The list now sits beside ADDON_IDS and a parity test reads the gates out of the source and holds it there. Read out of the source rather than out of a runtime registry on purpose: a runtime set only holds the gates whose modules the caller happened to import, so it would pass in a suite that imports none — which is the shape of test that hid this in the first place. RoadtripLimitsCard gets the test it never had, including the one case that matters: three keystrokes are not three writes. --- .../Roadtrip/RoadtripCorridorPanel.tsx | 14 +- .../Roadtrip/RoadtripLimitsCard.test.tsx | 130 ++++++++++++++++++ .../Roadtrip/RoadtripLimitsCard.tsx | 23 +++- .../components/Roadtrip/RoadtripStopPopup.tsx | 9 +- server/src/addons.ts | 31 +++++ server/src/nest/admin/admin.service.ts | 26 ++-- .../unit/nest/mcp-gated-addons.parity.test.ts | 82 +++++++++++ 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 + 30 files changed, 318 insertions(+), 20 deletions(-) create mode 100644 client/src/components/Roadtrip/RoadtripLimitsCard.test.tsx create mode 100644 server/tests/unit/nest/mcp-gated-addons.parity.test.ts diff --git a/client/src/components/Roadtrip/RoadtripCorridorPanel.tsx b/client/src/components/Roadtrip/RoadtripCorridorPanel.tsx index 3d1beffba..10428dcbd 100644 --- a/client/src/components/Roadtrip/RoadtripCorridorPanel.tsx +++ b/client/src/components/Roadtrip/RoadtripCorridorPanel.tsx @@ -515,13 +515,23 @@ export default function RoadtripCorridorPanel({ corridor, routes, onAddPoi }: Ro search.loading ? t('roadtrip.poi.searchingHint') : filteredToNothing - ? t('roadtrip.poi.noMatch', { name: corridor.nameFilter.trim() }) + // Four controls narrow this list and only one of them is the + // name box. Blaming it regardless produced `Nothing on the + // way matches ""` — empty quotes naming a filter the reader + // never set — when a section, a plug type or a minimum power + // was what emptied it. + ? corridor.nameFilter.trim() + ? t('roadtrip.poi.noMatch', { name: corridor.nameFilter.trim() }) + : t('roadtrip.poi.noneMatchFilters') : t('roadtrip.poi.empty') } /> ) : ( <> - {corridor.nameFilter.trim() ? ( + {/* Shown whenever anything narrowed the list, not only the name + box: a section or a plug filter hides hits just as much, and + the reader is owed the same "x of y" either way. */} + {corridor.visible.length !== search.results.length ? (

{t('roadtrip.poi.foundFiltered', { count: corridor.visible.length, total: search.results.length })}

diff --git a/client/src/components/Roadtrip/RoadtripLimitsCard.test.tsx b/client/src/components/Roadtrip/RoadtripLimitsCard.test.tsx new file mode 100644 index 000000000..8ab93f52a --- /dev/null +++ b/client/src/components/Roadtrip/RoadtripLimitsCard.test.tsx @@ -0,0 +1,130 @@ +import React from 'react' +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { fireEvent, render, screen } from '../../../tests/helpers/render' +import { useSettingsStore } from '../../store/settingsStore' +import RoadtripLimitsCard from './RoadtripLimitsCard' + +/** + * FE-ROADTRIP-LIMITS-001..008 — the three numbers that decide every warning. + * + * The one that matters is when a number is written. These fields had no local + * state, so every keystroke was a settings PUT: typing "180" sent three, and + * they raced. The value that decides whether a day is over budget could end up a + * tenth of what was typed and only say so after the next reload. + */ + +function open(onSave?: (key: string, value: number) => void) { + render() + fireEvent.click(screen.getByRole('button')) +} + +/** The three inputs, in the order the dialog lists them. */ +const inputs = () => screen.getAllByRole('spinbutton') as HTMLInputElement[] + +beforeEach(() => { + useSettingsStore.setState({ + settings: { + roadtrip_leg_minutes: 0, + roadtrip_day_minutes: 0, + roadtrip_range_km: 0, + distance_unit: 'metric', + } as never, + }) +}) + +describe('RoadtripLimitsCard', () => { + it('FE-ROADTRIP-LIMITS-001: typing a number writes nothing until the field is left', () => { + const onSave = vi.fn() + open(onSave) + + const [leg] = inputs() + fireEvent.change(leg, { target: { value: '1' } }) + fireEvent.change(leg, { target: { value: '18' } }) + fireEvent.change(leg, { target: { value: '180' } }) + + // Three keystrokes used to be three unordered PUTs carrying 1, 18 and 180. + expect(onSave).not.toHaveBeenCalled() + expect(leg.value).toBe('180') + }) + + it('FE-ROADTRIP-LIMITS-002: leaving the field saves once, with what was typed', () => { + const onSave = vi.fn() + open(onSave) + + const [leg] = inputs() + fireEvent.change(leg, { target: { value: '180' } }) + fireEvent.blur(leg) + + expect(onSave).toHaveBeenCalledTimes(1) + expect(onSave).toHaveBeenCalledWith('roadtrip_leg_minutes', 180) + }) + + it('FE-ROADTRIP-LIMITS-003: Enter commits without having to click away', () => { + const onSave = vi.fn() + open(onSave) + + const [, day] = inputs() + fireEvent.change(day, { target: { value: '540' } }) + fireEvent.keyDown(day, { key: 'Enter' }) + + expect(onSave).toHaveBeenCalledWith('roadtrip_day_minutes', 540) + }) + + it('FE-ROADTRIP-LIMITS-004: leaving a field nobody touched saves nothing', () => { + const onSave = vi.fn() + open(onSave) + + fireEvent.blur(inputs()[0]) + + expect(onSave).not.toHaveBeenCalled() + }) + + it('FE-ROADTRIP-LIMITS-005: retyping the value it already had is not a write', () => { + useSettingsStore.setState({ + settings: { roadtrip_leg_minutes: 180, roadtrip_day_minutes: 0, roadtrip_range_km: 0, distance_unit: 'metric' } as never, + }) + const onSave = vi.fn() + open(onSave) + + const [leg] = inputs() + fireEvent.change(leg, { target: { value: '180' } }) + fireEvent.blur(leg) + + expect(onSave).not.toHaveBeenCalled() + }) + + it('FE-ROADTRIP-LIMITS-006: clearing a field turns the limit off', () => { + useSettingsStore.setState({ + settings: { roadtrip_leg_minutes: 180, roadtrip_day_minutes: 0, roadtrip_range_km: 0, distance_unit: 'metric' } as never, + }) + const onSave = vi.fn() + open(onSave) + + const [leg] = inputs() + fireEvent.change(leg, { target: { value: '' } }) + fireEvent.blur(leg) + + expect(onSave).toHaveBeenCalledWith('roadtrip_leg_minutes', 0) + }) + + it('FE-ROADTRIP-LIMITS-007: the range field carries the reader own unit', () => { + useSettingsStore.setState({ + settings: { roadtrip_leg_minutes: 0, roadtrip_day_minutes: 0, roadtrip_range_km: 0, distance_unit: 'imperial' } as never, + }) + open(vi.fn()) + + expect(screen.getByText('mi')).toBeInTheDocument() + expect(screen.queryByText('km')).not.toBeInTheDocument() + }) + + it('FE-ROADTRIP-LIMITS-008: without a way to save, the dialog is read-only', () => { + // `onSave` absent is how the caller says the reader may look but not change. + open(undefined) + + const [leg] = inputs() + fireEvent.change(leg, { target: { value: '180' } }) + // Nothing to assert but the absence of a crash: the commit path has no + // handler to call, and must not assume one. + expect(() => fireEvent.blur(leg)).not.toThrow() + }) +}) diff --git a/client/src/components/Roadtrip/RoadtripLimitsCard.tsx b/client/src/components/Roadtrip/RoadtripLimitsCard.tsx index f9c771f24..f7c84b720 100644 --- a/client/src/components/Roadtrip/RoadtripLimitsCard.tsx +++ b/client/src/components/Roadtrip/RoadtripLimitsCard.tsx @@ -39,6 +39,20 @@ function LimitRow({ icon: Icon, label, suffix, value, placeholder, onChange }: { placeholder: string onChange: (next: number) => void }): React.ReactElement { + // Typed here, saved on blur or Enter. Writing on every keystroke sent one + // settings PUT per character — typing "180" produced three, carrying 1, 18 and + // 180, unordered and concurrent over HTTP/2. Whichever the server committed + // last won, so the limit that decides every over-budget warning could quietly + // end up a tenth of what was typed, and only show it after the next reload. + const [draft, setDraft] = useState(null) + const shown = draft ?? (value ? String(value) : '') + const commit = () => { + if (draft === null) return + const next = parseLimit(draft) + setDraft(null) + if (next !== (value ?? 0)) onChange(next) + } + return (