fix(timeline): cap lab markers instead of refusing the request

A values list naming more than 24 lab markers answered 422. The list is a
device preference, so once a 25th marker was ticked every later visit
failed: the page rendered its error card, and the values menu that could
untick it only renders on success.

The server now keeps the first 24 markers and leaves the rest out, as it
already does for a marker the record no longer has; a malformed key still
answers 422. The page applies the same cap before it asks, and the values
menu holds further unchosen markers back at 24 with a line saying why,
while a chosen one can always be removed.
This commit is contained in:
Marc-André Bombeck committed 2026-10-29 21:06:52 +01:00
1 parent 00d129ad42
commit 21f6f5796b
18 files changed
+164 -25

No files matched your search

+10 -8
View File
@@ -26190,17 +26190,19 @@ paths:
- in: query
name: values
schema:
description: Comma-separated value series to include, as `MeasurementType` names, `MOOD`, or a lab marker as
description: "Comma-separated value series to include, as `MeasurementType` names, `MOOD`, or a lab marker as
`lab:<biomarkerId>` (see `availableLabs`). A name given twice counts once, so the list is bounded by the
names that exist; an unknown measurement name answers 422, and so do more than 24 lab markers (ids, unlike
names, are not a closed set). A lab marker that is not in the record (deleted, or another record's) is
left out of `labSeries` without an error, so a stored choice never breaks the page.
names that exist; an unknown measurement name or a malformed marker key answers 422. At most 24 lab
markers are drawn (ids, unlike names, are not a closed set): the first 24 are kept and any further ones
left out without an error. A lab marker that is not in the record (deleted, or another record's) is
likewise left out of `labSeries`, so a stored choice never breaks the page."
type: string
description: Comma-separated value series to include, as `MeasurementType` names, `MOOD`, or a lab marker as
description: "Comma-separated value series to include, as `MeasurementType` names, `MOOD`, or a lab marker as
`lab:<biomarkerId>` (see `availableLabs`). A name given twice counts once, so the list is bounded by the
names that exist; an unknown measurement name answers 422, and so do more than 24 lab markers (ids, unlike
names, are not a closed set). A lab marker that is not in the record (deleted, or another record's) is left
out of `labSeries` without an error, so a stored choice never breaks the page.
names that exist; an unknown measurement name or a malformed marker key answers 422. At most 24 lab markers
are drawn (ids, unlike names, are not a closed set): the first 24 are kept and any further ones left out
without an error. A lab marker that is not in the record (deleted, or another record's) is likewise left out
of `labSeries`, so a stored choice never breaks the page."
responses:
"200":
description: The timeline for the requested zoom and window.
+1
View File
@@ -13733,6 +13733,7 @@
"labs": {
"menuTitle": "Laborwerte",
"otherPanel": "Weitere",
"full": "Bis zu {max} Laborwerte gleichzeitig. Entferne einen, um einen anderen hinzuzufügen.",
"above": "über dem Referenzbereich",
"below": "unter dem Referenzbereich",
"reference": "Referenz {range}",
+1
View File
@@ -13733,6 +13733,7 @@
"labs": {
"menuTitle": "Lab values",
"otherPanel": "Other",
"full": "Up to {max} lab values at once. Remove one to add another.",
"above": "above range",
"below": "below range",
"reference": "reference {range}",
+1
View File
@@ -13733,6 +13733,7 @@
"labs": {
"menuTitle": "Valores de laboratorio",
"otherPanel": "Otros",
"full": "Hasta {max} valores de laboratorio a la vez. Quita uno para añadir otro.",
"above": "por encima del intervalo",
"below": "por debajo del intervalo",
"reference": "referencia {range}",
+1
View File
@@ -13733,6 +13733,7 @@
"labs": {
"menuTitle": "Valeurs de laboratoire",
"otherPanel": "Autres",
"full": "Jusqu’à {max} valeurs de laboratoire à la fois. Retirez-en une pour en ajouter une autre.",
"above": "au-dessus de l’intervalle",
"below": "en dessous de l’intervalle",
"reference": "référence {range}",
+1
View File
@@ -13733,6 +13733,7 @@
"labs": {
"menuTitle": "Valori di laboratorio",
"otherPanel": "Altri",
"full": "Fino a {max} valori di laboratorio alla volta. Rimuovine uno per aggiungerne un altro.",
"above": "sopra l’intervallo",
"below": "sotto l’intervallo",
"reference": "riferimento {range}",
+1
View File
@@ -13733,6 +13733,7 @@
"labs": {
"menuTitle": "검사 수치",
"otherPanel": "기타",
"full": "검사 수치는 한 번에 최대 {max}개까지 표시할 수 있습니다. 다른 수치를 추가하려면 하나를 제거하세요.",
"above": "범위 초과",
"below": "범위 미만",
"reference": "참고 범위 {range}",
+1
View File
@@ -13733,6 +13733,7 @@
"labs": {
"menuTitle": "Labwaarden",
"otherPanel": "Overig",
"full": "Maximaal {max} labwaarden tegelijk. Verwijder er een om een andere toe te voegen.",
"above": "boven het bereik",
"below": "onder het bereik",
"reference": "referentie {range}",
+1
View File
@@ -13733,6 +13733,7 @@
"labs": {
"menuTitle": "Wyniki badań",
"otherPanel": "Inne",
"full": "Naraz można pokazać do {max} wyników badań. Usuń jeden, aby dodać inny.",
"above": "powyżej zakresu",
"below": "poniżej zakresu",
"reference": "zakres {range}",
+1
View File
@@ -13733,6 +13733,7 @@
"labs": {
"menuTitle": "Valores de exames",
"otherPanel": "Outros",
"full": "Até {max} valores de exames ao mesmo tempo. Remova um para adicionar outro.",
"above": "acima da faixa",
"below": "abaixo da faixa",
"reference": "referência {range}",
@@ -295,3 +295,69 @@ describe("lab lines on the timeline page", () => {
expect(mobile.scrollWidth).toBeLessThanOrEqual(mobile.clientWidth);
});
});
describe("the lab-marker cap on the timeline page", () => {
const id = (i: number) => `m${String(i).padStart(2, "0")}`;
const marker = (i: number) => ({
key: `lab:${id(i)}`,
biomarkerId: id(i),
name: `Marker ${String(i).padStart(2, "0")}`,
panel: null,
unit: "U/L",
});
/** A record with 26 markers, a stored choice naming 30 (four deleted). */
async function mountMany() {
const stored = Array.from({ length: 30 }, (_, i) => `lab:${id(i)}`);
window.localStorage.setItem(VALUES_KEY, JSON.stringify(stored));
setSearch("?zoom=all");
const net = stubFetch()
.on("GET", "/api/timeline", () => ({
data: {
...timeline([]),
availableLabs: Array.from({ length: 26 }, (_, i) => marker(i)),
},
}))
.on("GET", "/api/timeline/readiness", {
data: { verdict: "carries", since: "2024-01-01", lanes: [] },
})
.on("GET", "/api/life-events", { data: { events: [] } });
const { screen } = await renderWithApp(<TimelineView />, {
user: {
timezone: "Europe/Berlin",
modules: { timeline: true, labs: true },
},
});
return { net, screen };
}
it("asks for no more markers than the server takes, and holds further ones back until one is removed", async () => {
await page.viewport(1280, 900);
const { net, screen } = await mountMany();
const first24 = Array.from({ length: 24 }, (_, i) => `lab:${id(i)}`);
await expect.poll(() => lastTimelineValues(net)).toEqual(first24);
await screen.getByRole("button", { name: /Values \(24\)/ }).click();
const extra = screen.getByRole("menuitemcheckbox", { name: "Marker 24" });
await expect.element(extra).toHaveAttribute("aria-checked", "false");
await expect.element(extra).toHaveAttribute("aria-disabled", "true");
await expect
.element(
screen.getByText("Up to 24 lab values at once", { exact: false }),
)
.toBeVisible();
// A chosen marker can always be removed.
const chosen = screen.getByRole("menuitemcheckbox", { name: "Marker 00" });
expect(chosen.element().getAttribute("aria-disabled")).not.toBe("true");
await chosen.click();
await expect.element(chosen).toHaveAttribute("aria-checked", "false");
expect(JSON.parse(window.localStorage.getItem(VALUES_KEY)!)).toEqual(
first24.slice(1),
);
await expect.element(extra).not.toHaveAttribute("aria-disabled", "true");
expect(
document.querySelector('[data-slot="timeline-values-labs-full"]'),
).toBeNull();
});
});
+16 -1
View File
@@ -9,7 +9,9 @@
* not listed again beside it: each one is a named row under the lanes, and
* the menu marks it with its colour. Below the measurements it offers the
* lab markers the record has values for (#1299), under the groups the labs
* list sorts them into; a lab line is chosen and stored like any other.
* list sorts them into; a lab line is chosen and stored like any other, up
* to `TIMELINE_LAB_SERIES_MAX` at once: past it the unchosen markers are
* held back with a line that says why, and a chosen one can always go.
*/
import { ChartLine, ChevronDown, Layers } from "lucide-react";
@@ -25,6 +27,7 @@ import {
DropdownMenuTrigger,
} from "@/components/ui/dropdown-menu";
import {
TIMELINE_LAB_SERIES_MAX,
timelineLabMarkerId,
type TimelineLaneKey,
type TimelineResponse,
@@ -61,12 +64,16 @@ export function ValueSeriesMenu({
iconOnly?: boolean;
}) {
const { t } = useTranslations();
const labsFull =
selected.filter((key) => timelineLabMarkerId(key) !== null).length >=
TIMELINE_LAB_SERIES_MAX;
const option = (key: string) => {
const checked = selected.includes(key);
return (
<DropdownMenuCheckboxItem
key={key}
checked={checked}
disabled={labsFull && !checked && timelineLabMarkerId(key) !== null}
data-slot="timeline-values-option"
data-key={key}
onSelect={(event) => event.preventDefault()}
@@ -121,6 +128,14 @@ export function ValueSeriesMenu({
<DropdownMenuLabel data-slot="timeline-values-labs">
{t("timeline.labs.menuTitle")}
</DropdownMenuLabel>
{labsFull && (
<p
data-slot="timeline-values-labs-full"
className="text-muted-foreground px-2 pb-1 text-xs"
>
{t("timeline.labs.full", { max: TIMELINE_LAB_SERIES_MAX })}
</p>
)}
{labGroups.map((group) => (
<DropdownMenuGroup
key={group.panel ?? ""}
+4 -1
View File
@@ -37,6 +37,7 @@ import { Skeleton } from "@/components/ui/skeleton";
import { useAuth } from "@/hooks/use-auth";
import { useRecordCapabilities } from "@/hooks/use-record-capabilities";
import {
capTimelineLabKeys,
DAY_QUERY_PARAM,
TIMELINE_ZOOMS,
timelineLabMarkerId,
@@ -184,8 +185,10 @@ function useValueLines(
},
[modules],
);
// Lab markers past the cap are not asked for (the server would leave
// them out); the menu holds further ones back.
const values = useMemo(
() => storedValues.filter(allowed),
() => capTimelineLabKeys(storedValues.filter(allowed)),
[storedValues, allowed],
);
// One colour per chosen line, shared by the chart, the selection bar, the
+16 -1
View File
@@ -437,7 +437,7 @@ export type TimelineLabMarker = TimelineResponse["availableLabs"][number];
export const TIMELINE_LAB_KEY_PREFIX = "lab:";
/**
* The most lab markers one request names. Measurement names are a closed
* The most lab markers one request draws. Measurement names are a closed
* set and bound themselves; marker ids are not.
*/
export const TIMELINE_LAB_SERIES_MAX = 24;
@@ -449,6 +449,21 @@ export function timelineLabMarkerId(key: string): string | null {
return /^[A-Za-z0-9_-]{1,64}$/.test(id) ? id : null;
}
/**
* `keys` with every lab marker past the first `TIMELINE_LAB_SERIES_MAX` left
* out, everything else in its place. The server applies it to what it is
* asked for and the page to what it asks, so a stored list longer than the
* cap (another device, an older build) loads its first markers instead of
* failing the page.
*/
export function capTimelineLabKeys(keys: readonly string[]): string[] {
let labs = 0;
return keys.filter(
(key) =>
timelineLabMarkerId(key) === null || ++labs <= TIMELINE_LAB_SERIES_MAX,
);
}
/** The `values` entry for a lab marker. */
export function timelineLabKey(biomarkerId: string): string {
return `${TIMELINE_LAB_KEY_PREFIX}${biomarkerId}`;
+1 -1
View File
@@ -370,7 +370,7 @@ export const timelineQuerySchema = z
.string()
.optional()
.describe(
"Comma-separated value series to include, as `MeasurementType` names, `MOOD`, or a lab marker as `lab:<biomarkerId>` (see `availableLabs`). A name given twice counts once, so the list is bounded by the names that exist; an unknown measurement name answers 422, and so do more than 24 lab markers (ids, unlike names, are not a closed set). A lab marker that is not in the record (deleted, or another record's) is left out of `labSeries` without an error, so a stored choice never breaks the page.",
"Comma-separated value series to include, as `MeasurementType` names, `MOOD`, or a lab marker as `lab:<biomarkerId>` (see `availableLabs`). A name given twice counts once, so the list is bounded by the names that exist; an unknown measurement name or a malformed marker key answers 422. At most 24 lab markers are drawn (ids, unlike names, are not a closed set): the first 24 are kept and any further ones left out without an error. A lab marker that is not in the record (deleted, or another record's) is likewise left out of `labSeries`, so a stored choice never breaks the page.",
),
})
.refine((q) => q.zoom !== "range" || (q.from && q.to), {
+13 -2
View File
@@ -272,12 +272,23 @@ describe("a lab marker in `values`", () => {
expect(parseSeriesKeys("lab:../etc")).toBeNull();
});
it("is capped, because marker ids are not a closed set", () => {
it("keeps the first markers up to the cap and leaves the rest out, so a long stored list still loads", () => {
const many = (n: number) =>
Array.from({ length: n }, (_, i) => `lab:m${i}`).join(",");
expect(parseSeriesKeys(many(TIMELINE_LAB_SERIES_MAX))).toHaveLength(
TIMELINE_LAB_SERIES_MAX,
);
expect(parseSeriesKeys(many(TIMELINE_LAB_SERIES_MAX + 1))).toBeNull();
const capped = parseSeriesKeys(
`WEIGHT,${many(TIMELINE_LAB_SERIES_MAX + 6)},MOOD`,
);
expect(capped).toEqual([
"WEIGHT",
...Array.from({ length: TIMELINE_LAB_SERIES_MAX }, (_, i) => `lab:m${i}`),
"MOOD",
]);
// A malformed key past the cap still refuses: the cap is not a filter.
expect(
parseSeriesKeys(`${many(TIMELINE_LAB_SERIES_MAX)},lab:../x`),
).toBeNull();
});
});
+7 -8
View File
@@ -17,8 +17,8 @@ import { MeasurementType as MeasurementTypes } from "@/generated/prisma/enums";
import type { MeasurementType } from "@/generated/prisma/enums";
import {
capTimelineLabKeys,
DAY_NOTABLE_MAX_SPAN_DAYS,
TIMELINE_LAB_SERIES_MAX,
TIMELINE_LANE_KEYS,
timelineLabMarkerId,
type TimelineBucket,
@@ -118,7 +118,10 @@ const MEASUREMENT_TYPE_SET: ReadonlySet<string> = new Set(
* lanes. A lab marker (`lab:<biomarkerId>`, #1299) is checked for its shape
* only: whether the record has it is the loader's question, and the answer
* to a stale one is no line rather than an error. Marker ids are not a
* closed set, so their count is capped.
* closed set, so their count is capped: the first
* `TIMELINE_LAB_SERIES_MAX` are kept and the rest left out. A refusal would
* be stored with the choice and fail the page on every visit, with the menu
* that could undo it never drawn.
*/
export function parseSeriesKeys(raw: string | undefined): string[] | null {
if (raw === undefined) return [...DEFAULT_TIMELINE_SERIES];
@@ -130,15 +133,11 @@ export function parseSeriesKeys(raw: string | undefined): string[] | null {
.filter((k) => k.length > 0),
),
];
let labs = 0;
for (const key of keys) {
if (timelineLabMarkerId(key) !== null) {
labs += 1;
continue;
}
if (timelineLabMarkerId(key) !== null) continue;
if (key !== MOOD_SERIES_KEY && !MEASUREMENT_TYPE_SET.has(key)) return null;
}
return labs > TIMELINE_LAB_SERIES_MAX ? null : keys;
return capTimelineLabKeys(keys);
}
/**
+22 -3
View File
@@ -299,12 +299,31 @@ describe("lab lines on the timeline", () => {
expect(timeline.labSeries).toEqual([]);
expect(timeline.availableLabs).toEqual([]);
// A list of ids is capped, and a malformed one is refused.
const many = Array.from({ length: 25 }, (_, i) => `lab:m${i}`).join(",");
expect((await getTimeline(`values=${many}`)).status).toBe(422);
// A malformed id is refused.
expect((await getTimeline(`values=lab:..%2Fx`)).status).toBe(422);
});
it("draws the first 24 markers of a longer list and leaves the rest out, rather than refusing", async () => {
const owner = await makeUser("owner");
const { marker } = await seedFerritin(owner.id);
await signIn(owner.id);
const filler = Array.from({ length: 24 }, (_, i) => `lab:m${i}`);
// Past the cap: the record's own marker is the 25th, so it is not drawn.
const past = await getTimeline(
`zoom=all&values=WEIGHT,${[...filler, `lab:${marker.id}`].join(",")}`,
);
expect(past.status).toBe(200);
expect((await data(past)).labSeries).toEqual([]);
// Within the cap it is.
const within = await data(
await getTimeline(
`zoom=all&values=${[...filler.slice(1), `lab:${marker.id}`].join(",")}`,
),
);
expect(within.labSeries.map((s) => s.key)).toEqual([`lab:${marker.id}`]);
});
it("refuses a grant scoped to some sections, and shares the lines on a whole-record grant", async () => {
const owner = await makeUser("owner");
const { marker } = await seedFerritin(owner.id);