diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index e02690ce0..29cc4ea93 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -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:` (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:` (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. diff --git a/messages/de.json b/messages/de.json index 08080a48e..aaffa5bcc 100644 --- a/messages/de.json +++ b/messages/de.json @@ -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}", diff --git a/messages/en.json b/messages/en.json index 300570a96..e64c16a4b 100644 --- a/messages/en.json +++ b/messages/en.json @@ -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}", diff --git a/messages/es.json b/messages/es.json index 69768f6be..498dc6b0b 100644 --- a/messages/es.json +++ b/messages/es.json @@ -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}", diff --git a/messages/fr.json b/messages/fr.json index e383d3264..27a2fb692 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -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}", diff --git a/messages/it.json b/messages/it.json index 91d8ce687..e292975f0 100644 --- a/messages/it.json +++ b/messages/it.json @@ -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}", diff --git a/messages/ko.json b/messages/ko.json index ea29acfee..2772754cb 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -13733,6 +13733,7 @@ "labs": { "menuTitle": "검사 수치", "otherPanel": "기타", + "full": "검사 수치는 한 번에 최대 {max}개까지 표시할 수 있습니다. 다른 수치를 추가하려면 하나를 제거하세요.", "above": "범위 초과", "below": "범위 미만", "reference": "참고 범위 {range}", diff --git a/messages/nl.json b/messages/nl.json index 00b4e2b9e..137b3f252 100644 --- a/messages/nl.json +++ b/messages/nl.json @@ -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}", diff --git a/messages/pl.json b/messages/pl.json index 3246df662..bec5c7352 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -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}", diff --git a/messages/pt-BR.json b/messages/pt-BR.json index 27f894ae0..3747bf718 100644 --- a/messages/pt-BR.json +++ b/messages/pt-BR.json @@ -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}", diff --git a/src/components/timeline/__tests__/timeline-labs.browser.test.tsx b/src/components/timeline/__tests__/timeline-labs.browser.test.tsx index 1be1f5a7f..1860f3470 100644 --- a/src/components/timeline/__tests__/timeline-labs.browser.test.tsx +++ b/src/components/timeline/__tests__/timeline-labs.browser.test.tsx @@ -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(, { + 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(); + }); +}); diff --git a/src/components/timeline/timeline-menus.tsx b/src/components/timeline/timeline-menus.tsx index a20abefac..62faa0dfe 100644 --- a/src/components/timeline/timeline-menus.tsx +++ b/src/components/timeline/timeline-menus.tsx @@ -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 ( event.preventDefault()} @@ -121,6 +128,14 @@ export function ValueSeriesMenu({ {t("timeline.labs.menuTitle")} + {labsFull && ( +

+ {t("timeline.labs.full", { max: TIMELINE_LAB_SERIES_MAX })} +

+ )} {labGroups.map((group) => ( storedValues.filter(allowed), + () => capTimelineLabKeys(storedValues.filter(allowed)), [storedValues, allowed], ); // One colour per chosen line, shared by the chart, the selection bar, the diff --git a/src/lib/day/contract.ts b/src/lib/day/contract.ts index 97651415e..e87001167 100644 --- a/src/lib/day/contract.ts +++ b/src/lib/day/contract.ts @@ -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}`; diff --git a/src/lib/day/wire-schemas.ts b/src/lib/day/wire-schemas.ts index 4a5c892c3..eba8ef32c 100644 --- a/src/lib/day/wire-schemas.ts +++ b/src/lib/day/wire-schemas.ts @@ -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:` (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:` (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), { diff --git a/src/lib/timeline/__tests__/lab-series.test.ts b/src/lib/timeline/__tests__/lab-series.test.ts index 1a29fdc5f..311fcda46 100644 --- a/src/lib/timeline/__tests__/lab-series.test.ts +++ b/src/lib/timeline/__tests__/lab-series.test.ts @@ -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(); }); }); diff --git a/src/lib/timeline/load-timeline.ts b/src/lib/timeline/load-timeline.ts index 4e07ee67b..8de5bc5c1 100644 --- a/src/lib/timeline/load-timeline.ts +++ b/src/lib/timeline/load-timeline.ts @@ -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 = new Set( * lanes. A lab marker (`lab:`, #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); } /** diff --git a/tests/integration/timeline-lab-series.test.ts b/tests/integration/timeline-lab-series.test.ts index 9b6f54278..5fc0767c6 100644 --- a/tests/integration/timeline-lab-series.test.ts +++ b/tests/integration/timeline-lab-series.test.ts @@ -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);