From 7001308636fa4d73a897b4b1b57f39e5576e4878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 19:56:59 +0100 Subject: [PATCH] fix(measurements): mark an implausible edited value on its field Typing 720 instead of 72.0 in the measurement edit sheet showed the English banner "Validation failed" in every locale, with no field marked. The route refuses a changed value outside the type's band with a 422 whose issue list names the field, and the client dropped that list. The sheet now checks the same band before sending and says, under the value input and in the unit it shows, which range is accepted. ApiError carries the field paths of a refusal, so a value the server still refuses lands on the field too; any other refusal goes through localizedApiError instead of printing the server's prose. --- messages/de.json | 3 +- messages/en.json | 3 +- messages/es.json | 3 +- messages/fr.json | 3 +- messages/it.json | 3 +- messages/ko.json | 3 +- messages/nl.json | 3 +- messages/pl.json | 3 +- messages/pt-BR.json | 3 +- messages/ru.json | 3 +- .../__tests__/measurement-edit-range.test.ts | 74 ++++++++++++ .../list/measurement-edit-sheet.tsx | 10 +- .../measurements/list/use-measurement-edit.ts | 110 ++++++++++++++---- src/lib/api/__tests__/api-fetch.test.ts | 32 +++++ src/lib/api/api-fetch.ts | 23 +++- 15 files changed, 243 insertions(+), 36 deletions(-) create mode 100644 src/components/measurements/__tests__/measurement-edit-range.test.ts diff --git a/messages/de.json b/messages/de.json index c35f312d1..38a6915f3 100644 --- a/messages/de.json +++ b/messages/de.json @@ -986,7 +986,8 @@ "typeStressLevel": "Stresslevel", "typeTrainingReadiness": "Trainingsbereitschaft", "typeAcuteTrainingLoad": "Akute Trainingsbelastung", - "sourceGarmin": "Garmin" + "sourceGarmin": "Garmin", + "valueOutOfRange": "Gib einen Wert zwischen {min} und {max} {unit} ein." }, "metricCatalog": { "title": "Was HealthLog erfassen kann", diff --git a/messages/en.json b/messages/en.json index 29d5ddf0b..f237a82cf 100644 --- a/messages/en.json +++ b/messages/en.json @@ -986,7 +986,8 @@ "typeStressLevel": "Stress level", "typeTrainingReadiness": "Training readiness", "typeAcuteTrainingLoad": "Acute training load", - "sourceGarmin": "Garmin" + "sourceGarmin": "Garmin", + "valueOutOfRange": "Enter a value between {min} and {max} {unit}." }, "metricCatalog": { "title": "What HealthLog can track", diff --git a/messages/es.json b/messages/es.json index 71ee4da7e..78b9b5535 100644 --- a/messages/es.json +++ b/messages/es.json @@ -986,7 +986,8 @@ "typeStressLevel": "Nivel de estrés", "typeTrainingReadiness": "Preparación para entrenar", "typeAcuteTrainingLoad": "Carga de entrenamiento aguda", - "sourceGarmin": "Garmin" + "sourceGarmin": "Garmin", + "valueOutOfRange": "Introduce un valor entre {min} y {max} {unit}." }, "metricCatalog": { "title": "Lo que HealthLog puede registrar", diff --git a/messages/fr.json b/messages/fr.json index 8d7b7a40c..ca9d45fbf 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -986,7 +986,8 @@ "typeStressLevel": "Niveau de stress", "typeTrainingReadiness": "Disponibilité à l'entraînement", "typeAcuteTrainingLoad": "Charge d'entraînement aiguë", - "sourceGarmin": "Garmin" + "sourceGarmin": "Garmin", + "valueOutOfRange": "Saisissez une valeur entre {min} et {max} {unit}." }, "metricCatalog": { "title": "Ce que HealthLog peut suivre", diff --git a/messages/it.json b/messages/it.json index 02478f1e1..50bc0e36f 100644 --- a/messages/it.json +++ b/messages/it.json @@ -986,7 +986,8 @@ "typeStressLevel": "Livello di stress", "typeTrainingReadiness": "Prontezza all'allenamento", "typeAcuteTrainingLoad": "Carico di allenamento acuto", - "sourceGarmin": "Garmin" + "sourceGarmin": "Garmin", + "valueOutOfRange": "Inserisci un valore tra {min} e {max} {unit}." }, "metricCatalog": { "title": "Cosa può monitorare HealthLog", diff --git a/messages/ko.json b/messages/ko.json index fd4860a74..7fc9fdf61 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -986,7 +986,8 @@ "typeStressLevel": "스트레스 수준", "typeTrainingReadiness": "트레이닝 준비도", "typeAcuteTrainingLoad": "급성 훈련 부하", - "sourceGarmin": "Garmin" + "sourceGarmin": "Garmin", + "valueOutOfRange": "{min}에서 {max} {unit} 사이의 값을 입력해 주세요." }, "metricCatalog": { "title": "HealthLog가 기록할 수 있는 것", diff --git a/messages/nl.json b/messages/nl.json index 6680b0928..0e5b745b5 100644 --- a/messages/nl.json +++ b/messages/nl.json @@ -986,7 +986,8 @@ "typeStressLevel": "Stressniveau", "typeTrainingReadiness": "Trainingsgereedheid", "typeAcuteTrainingLoad": "Acute trainingsbelasting", - "sourceGarmin": "Garmin" + "sourceGarmin": "Garmin", + "valueOutOfRange": "Vul een waarde in tussen {min} en {max} {unit}." }, "metricCatalog": { "title": "Wat HealthLog kan bijhouden", diff --git a/messages/pl.json b/messages/pl.json index 02ca079b0..c46749950 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -986,7 +986,8 @@ "typeStressLevel": "Poziom stresu", "typeTrainingReadiness": "Gotowość do treningu", "typeAcuteTrainingLoad": "Ostre obciążenie treningowe", - "sourceGarmin": "Garmin" + "sourceGarmin": "Garmin", + "valueOutOfRange": "Wpisz wartość od {min} do {max} {unit}." }, "metricCatalog": { "title": "Co HealthLog potrafi śledzić", diff --git a/messages/pt-BR.json b/messages/pt-BR.json index 18df65df1..cbe230a62 100644 --- a/messages/pt-BR.json +++ b/messages/pt-BR.json @@ -986,7 +986,8 @@ "typeStressLevel": "Nível de estresse", "typeTrainingReadiness": "Prontidão para treinar", "typeAcuteTrainingLoad": "Carga de treino aguda", - "sourceGarmin": "Garmin" + "sourceGarmin": "Garmin", + "valueOutOfRange": "Informe um valor entre {min} e {max} {unit}." }, "metricCatalog": { "title": "O que o HealthLog pode acompanhar", diff --git a/messages/ru.json b/messages/ru.json index 4cf579664..4fabe3cfa 100644 --- a/messages/ru.json +++ b/messages/ru.json @@ -986,7 +986,8 @@ "typeStressLevel": "Уровень стресса", "typeTrainingReadiness": "Готовность к тренировке", "typeAcuteTrainingLoad": "Острая тренировочная нагрузка", - "sourceGarmin": "Garmin" + "sourceGarmin": "Garmin", + "valueOutOfRange": "Введите значение от {min} до {max} {unit}." }, "metricCatalog": { "title": "Что умеет отслеживать HealthLog", diff --git a/src/components/measurements/__tests__/measurement-edit-range.test.ts b/src/components/measurements/__tests__/measurement-edit-range.test.ts new file mode 100644 index 000000000..fa62aef1d --- /dev/null +++ b/src/components/measurements/__tests__/measurement-edit-range.test.ts @@ -0,0 +1,74 @@ +/** + * The edit sheet refuses an implausible value at the value field, before the + * request, in the unit the input shows. The server checks the same band and + * used to be the only one that did: its 422 reached the sheet as the English + * banner "Validation failed", in every locale, with no field marked. + */ +import { describe, expect, it } from "vitest"; + +import type { UnitDisplay } from "@/hooks/use-unit-display"; +import { getUnitForType } from "@/lib/measurements/unit-map"; +import { editValueRange } from "../list/use-measurement-edit"; +import { rowDisplayFor } from "../list/use-measurement-row-display"; + +const KG_PER_LB = 0.45359237; + +function display( + preference: "metric" | "imperial", + glucose: "mg/dL" | "mmol/L", +) { + const imperial = preference === "imperial"; + const unitDisplay = { + preference, + isTransformed: (type: string) => imperial && type === "WEIGHT", + toDisplay: (type: string, v: number) => + imperial && type === "WEIGHT" ? v / KG_PER_LB : v, + fromDisplay: (type: string, v: number) => + imperial && type === "WEIGHT" ? v * KG_PER_LB : v, + unitFor: (type: string) => + imperial && type === "WEIGHT" ? "lb" : getUnitForType(type), + } as unknown as UnitDisplay; + return rowDisplayFor(glucose, unitDisplay); +} + +const row = (type: string, unit: string) => ({ + id: "m1", + type, + value: 72, + unit, + source: "MANUAL", + measuredAt: "2026-11-10T08:00:00.000Z", + notes: null, +}); + +describe("editValueRange", () => { + it("names the band in the stored unit for a metric reader", () => { + expect( + editValueRange(display("metric", "mg/dL"), row("WEIGHT", "kg")), + ).toEqual({ min: 1, max: 500, unit: "kg" }); + }); + + it("names it in pounds for an imperial reader, who types pounds", () => { + const range = editValueRange( + display("imperial", "mg/dL"), + row("WEIGHT", "kg"), + ); + expect(range?.unit).toBe("lb"); + expect(range?.max).toBeCloseTo(1102.3, 1); + }); + + it("names it in mmol/L for a glucose edit in mmol/L", () => { + const range = editValueRange( + display("metric", "mmol/L"), + row("BLOOD_GLUCOSE", "mg/dL"), + ); + expect(range?.unit).toBe("mmol/L"); + expect(range!.max).toBeLessThan(100); + }); + + it("has nothing to say for a type without a band", () => { + expect( + editValueRange(display("metric", "mg/dL"), row("NO_SUCH_TYPE", "x")), + ).toBeNull(); + }); +}); diff --git a/src/components/measurements/list/measurement-edit-sheet.tsx b/src/components/measurements/list/measurement-edit-sheet.tsx index 8a8200370..f0576b6e6 100644 --- a/src/components/measurements/list/measurement-edit-sheet.tsx +++ b/src/components/measurements/list/measurement-edit-sheet.tsx @@ -3,6 +3,7 @@ import { createPortal } from "react-dom"; import { Loader2, MoreHorizontal, Trash2 } from "lucide-react"; +import { FieldError } from "@/components/forms/field-error"; import { SeededFormDiscardDialog } from "@/components/forms/use-seeded-form-dismissal"; import { AlertDialog, @@ -57,6 +58,8 @@ export function MeasurementEditSheet({ setEditValue, editError, editErrorDescriptor, + editValueError, + editValueErrorId, editMeasuredAt, setEditMeasuredAt, editNotes, @@ -103,9 +106,12 @@ export function MeasurementEditSheet({ onChange={(e) => setEditValue(e.target.value)} required aria-required="true" - aria-invalid={!!editError || undefined} - aria-describedby={editErrorDescriptor} + aria-invalid={!!(editValueError || editError) || undefined} + aria-describedby={ + editValueError ? editValueErrorId : editErrorDescriptor + } /> +
diff --git a/src/components/measurements/list/use-measurement-edit.ts b/src/components/measurements/list/use-measurement-edit.ts index f4213ba97..8a524db5c 100644 --- a/src/components/measurements/list/use-measurement-edit.ts +++ b/src/components/measurements/list/use-measurement-edit.ts @@ -5,9 +5,14 @@ import { useMutation, useQueryClient } from "@tanstack/react-query"; import { useSeededFormDismissal } from "@/components/forms/use-seeded-form-dismissal"; import { ApiError, apiPut } from "@/lib/api/api-fetch"; +import { localizedApiError } from "@/lib/api/localized-error"; import { toCanonicalMgdl } from "@/lib/glucose"; -import { useTranslations } from "@/lib/i18n/context"; +import { useFormatters, useTranslations } from "@/lib/i18n/context"; import { getUnitForType } from "@/lib/measurements/unit-map"; +import { + VALUE_RANGES, + validateMeasurementRange, +} from "@/lib/measurements/value-ranges"; import { invalidateKeys, measurementDependentKeys, @@ -78,6 +83,58 @@ export function storedValueFor( return parsedValue; } +/** + * The stored value's plausible band for the row's type, in the unit the edit + * input shows, or null for a type without one. The same table the server + * checks the edit against. + */ +export function editValueRange( + { rowDisplay }: Pick, + measurement: Measurement, +): { min: number; max: number; unit: string } | null { + const range = VALUE_RANGES[measurement.type]; + if (!range) return null; + const low = rowDisplay({ ...measurement, value: range.min }); + const high = rowDisplay({ ...measurement, value: range.max }); + const round = (n: number) => Math.round(n * 10) / 10; + return { min: round(low.value), max: round(high.value), unit: low.unit }; +} + +type Translate = ReturnType["t"]; + +/** The sentence for a value outside its band, in the unit the input shows. */ +function outOfRangeMessage( + display: Pick, + measurement: Measurement, + t: Translate, + formatNumber: (value: number) => string, +): string { + const range = editValueRange(display, measurement); + return range + ? t("measurements.valueOutOfRange", { + min: formatNumber(range.min), + max: formatNumber(range.max), + unit: range.unit, + }) + : t("measurements.invalidValue"); +} + +/** + * The banner for a refused save. A 409 names its reason in `meta.errorCode`: + * the edit collides with another reading's (type, time, source, stage), or + * the reading belongs to a server-owned source (connector, import, computed). + */ +function saveRefusal(err: unknown, t: Translate): string { + const code = err instanceof ApiError ? err.meta?.errorCode : undefined; + if (code === "measurement.duplicate_timestamp") { + return t("measurements.duplicateTimestamp"); + } + if (code === "measurement.update.server_owned_source") { + return t("measurements.serverOwnedSource"); + } + return localizedApiError(err, t, "measurements.saveError"); +} + /** * The edit sheet's state: the reading being edited, the draft seeded in the * person's display unit, the save (converted back to the stored unit) and @@ -94,6 +151,7 @@ export function useMeasurementEdit({ deleteMutation: MeasurementListData["deleteMutation"]; }) { const { t } = useTranslations(); + const fmt = useFormatters(); const queryClient = useQueryClient(); const [editing, setEditing] = useState(null); const [editValue, setEditValue] = useState(""); @@ -105,10 +163,13 @@ export function useMeasurementEdit({ notes: "", }); const [editError, setEditError] = useState(null); + // A refused value is said under the value input, not in the banner. + const [editValueError, setEditValueError] = useState(null); const [editDeleteDialogOpen, setEditDeleteDialogOpen] = useState(false); // The error banner is linked to the inputs through `aria-describedby`, so a // screen reader announces why a submit was refused. const editErrorId = useId(); + const editValueErrorId = useId(); // The sheet's footer is a slot the form portals its buttons into. const editFormId = useId(); const [editFooterEl, setEditFooterEl] = useState(null); @@ -138,28 +199,19 @@ export function useMeasurementEdit({ void refetchInactiveDailyReads(queryClient); }, onError: (err) => { - // A 409 names its reason in `meta.errorCode`: the edit collides with - // another reading's (type, time, source, stage), or the reading belongs - // to a server-owned source (connector, import, computed). - if ( - err instanceof ApiError && - err.meta?.errorCode === "measurement.duplicate_timestamp" - ) { - setEditError(t("measurements.duplicateTimestamp")); - return; + if (err instanceof ApiError && err.issuePaths.includes("value")) { + setEditValueError(valueRefusal()); + } else { + setEditError(saveRefusal(err, t)); } - if ( - err instanceof ApiError && - err.meta?.errorCode === "measurement.update.server_owned_source" - ) { - setEditError(t("measurements.serverOwnedSource")); - return; - } - setEditError( - err instanceof Error ? err.message : t("measurements.saveError"), - ); }, }); + + function valueRefusal(): string { + return editing + ? outOfRangeMessage(display, editing, t, fmt.number) + : t("measurements.invalidValue"); + } const editDismissal = useSeededFormDismissal({ seed: editSeed, value: { @@ -188,11 +240,13 @@ export function useMeasurementEdit({ setEditNotes(seed.notes); setEditSeed(seed); setEditError(null); + setEditValueError(null); } function dismissEdit() { setEditing(null); setEditError(null); + setEditValueError(null); setEditDeleteDialogOpen(false); } @@ -216,9 +270,11 @@ export function useMeasurementEdit({ e.preventDefault(); if (!editing) return; + setEditError(null); + setEditValueError(null); const parsedValue = Number(editValue); if (!Number.isFinite(parsedValue)) { - setEditError(t("measurements.invalidValue")); + setEditValueError(t("measurements.invalidValue")); return; } @@ -229,8 +285,16 @@ export function useMeasurementEdit({ } const canonicalValue = storedValueFor(display, editing, parsedValue); + // The server refuses a changed value outside the type's band; say so at + // the field before sending, in the unit the input shows. + if ( + editValue !== editSeed.value && + validateMeasurementRange(editing.type, canonicalValue) !== null + ) { + setEditValueError(valueRefusal()); + return; + } - setEditError(null); updateMutation.mutate({ id: editing.id, value: canonicalValue, @@ -250,6 +314,8 @@ export function useMeasurementEdit({ editError, editErrorId, editErrorDescriptor: editError ? editErrorId : undefined, + editValueError, + editValueErrorId, editFormId, editFooterEl, setEditFooterEl, diff --git a/src/lib/api/__tests__/api-fetch.test.ts b/src/lib/api/__tests__/api-fetch.test.ts index e526e6f12..e876a0ee2 100644 --- a/src/lib/api/__tests__/api-fetch.test.ts +++ b/src/lib/api/__tests__/api-fetch.test.ts @@ -190,6 +190,38 @@ describe("apiFetchEnvelope", () => { }); }); +describe("ApiError.issuePaths", () => { + it("names the fields a 422's issue list refused", async () => { + fetchMock.mockResolvedValueOnce( + jsonResponse( + { + data: null, + error: "Validation failed", + details: { + issues: [ + { path: "value", code: "custom", message: "out of range" }, + ], + }, + }, + 422, + ), + ); + const error = await apiPut("/api/measurements/m1", { value: 720 }).catch( + (e: unknown) => e, + ); + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).issuePaths).toEqual(["value"]); + }); + + it("is empty when the refusal named no field", async () => { + fetchMock.mockResolvedValueOnce( + jsonResponse({ data: null, error: "Nope" }, 409), + ); + const error = await apiGet("/api/x").catch((e: unknown) => e); + expect((error as ApiError).issuePaths).toEqual([]); + }); +}); + describe("apiFetchRaw", () => { it("returns the raw Response without ok-check or unwrap", async () => { const res = new Response("plain", { status: 500 }); diff --git a/src/lib/api/api-fetch.ts b/src/lib/api/api-fetch.ts index 6f2648c1d..2a6adcd8b 100644 --- a/src/lib/api/api-fetch.ts +++ b/src/lib/api/api-fetch.ts @@ -17,7 +17,8 @@ * through `readError` (server envelope `error` string, falling back * to `Request failed ()`), `status` carrying the HTTP code, * `meta` carrying the error envelope's `meta` object when present - * (e.g. `meta.errorCode`). + * (e.g. `meta.errorCode`), `issuePaths` the fields a 422's + * `details.issues` names. * - Network failures reject with the native `TypeError` from `fetch`, * exactly as before — only HTTP-level failures become `ApiError`. * - 401 / 403 / 429 are NOT special-cased: the wrapper throws the same @@ -70,17 +71,24 @@ export class ApiError extends Error { * server). Its first characters are the error id a person reads out. */ readonly requestId: string | null; + /** + * The fields a refusal's `details.issues` names (`"value"`, `"measuredAt"`), + * so a form can mark the one the server refused. Empty when it named none. + */ + readonly issuePaths: readonly string[]; constructor( message: string, status: number, meta?: Record, requestId?: string | null, + issuePaths: readonly string[] = [], ) { super(message); this.name = "ApiError"; this.status = status; this.meta = meta; + this.issuePaths = issuePaths; const fromMeta = typeof meta?.requestId === "string" && meta.requestId.length > 0 ? meta.requestId @@ -97,11 +105,21 @@ interface Envelope> { async function throwApiError(res: Response): Promise { let meta: Record | undefined; + let issuePaths: string[] = []; try { - const json = (await res.clone().json()) as { meta?: unknown }; + const json = (await res.clone().json()) as { + meta?: unknown; + details?: { issues?: unknown }; + }; if (json && typeof json.meta === "object" && json.meta !== null) { meta = json.meta as Record; } + const issues = json?.details?.issues; + if (Array.isArray(issues)) { + issuePaths = issues.flatMap((issue: { path?: unknown }) => + typeof issue?.path === "string" ? [issue.path] : [], + ); + } } catch { /* non-JSON body — readError supplies the fallback message */ } @@ -110,6 +128,7 @@ async function throwApiError(res: Response): Promise { res.status, meta, res.headers.get("x-request-id"), + issuePaths, ); // A server-side failure is worth an id in the session's support info, // whether or not the call site ends up showing it.