From 8f3e4b63873e3b1316079a4924854e841b6b1578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 8 Aug 2026 16:06:57 +0200 Subject: [PATCH] fix(insights): cap the ECG teaser, never the ECG page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The five-strip cap belonged on the overview teaser — a glance on /insights — not on the surface a user opens to read their history. It was living in the shared EcgSection, which the dedicated /insights/ecg page reuses, so an account with many recordings could only ever reach the newest five there and the older strips became unreachable with no pagination behind them. The cap is a prop now. The teaser passes ECG_OVERVIEW_LIMIT; the page passes nothing and lists every recording. The constant lives in the React-free classification module so the overview page can pass it without pulling the dynamically-loaded ECG component into its bundle. --- src/app/insights/ecg/__tests__/page.test.tsx | 20 ++++++++ src/app/insights/page-client.tsx | 5 +- .../insights/__tests__/ecg-section.test.tsx | 47 ++++++++++++++----- src/components/insights/ecg-section.tsx | 28 +++++++---- src/lib/insights/ecg-classification.ts | 9 ++++ 5 files changed, 87 insertions(+), 22 deletions(-) diff --git a/src/app/insights/ecg/__tests__/page.test.tsx b/src/app/insights/ecg/__tests__/page.test.tsx index fec827e45..d516d91f6 100644 --- a/src/app/insights/ecg/__tests__/page.test.tsx +++ b/src/app/insights/ecg/__tests__/page.test.tsx @@ -76,6 +76,26 @@ describe("/insights/ecg page (H1)", () => { expect(html).not.toContain(" { + // The teaser on `/insights` caps at five; this dedicated page must not. + // An account with many strips reaches all of them here, or the older ones + // become unreachable with no pagination behind them. Eight in, eight out. + const recordings = Array.from({ length: 8 }, (_, index) => ({ + ...seededRecording(), + id: `rec-${index}`, + recordedAt: new Date(Date.UTC(2026, 6, 10 - index, 8, 0)).toISOString(), + })); + const html = render({ recordings, hasRecordings: true }); + + const rows = (html.match(/data-slot="ecg-row"/g) ?? []).length; + expect(rows).toBe(8); + for (const rec of recordings) { + expect(html, `${rec.id} must be reachable on the full page`).toContain( + `/insights/ecg/${rec.id}`, + ); + } + }); + it("carries no disclaimer paragraph above the list", () => { // Two muted lines used to lead this page — one from the shell's own // explainer slot and one hand-rolled beneath it, both saying HealthLog diff --git a/src/app/insights/page-client.tsx b/src/app/insights/page-client.tsx index 7d7d1dc9e..1f7f54db6 100644 --- a/src/app/insights/page-client.tsx +++ b/src/app/insights/page-client.tsx @@ -9,6 +9,9 @@ import { SlidersHorizontal, TrendingUp } from "lucide-react"; import { useAuth } from "@/hooks/use-auth"; import { queryKeys } from "@/lib/query-keys"; import { apiGet } from "@/lib/api/api-fetch"; +// Pure constant — importing it here does NOT pull the dynamically-loaded ECG +// component into this bundle, which is exactly why it lives in the lib module. +import { ECG_OVERVIEW_LIMIT } from "@/lib/insights/ecg-classification"; import { useFeatureFlags } from "@/hooks/use-feature-flags"; import { useInsightsLayoutQuery } from "@/hooks/use-insights-layout"; import { usePullToRefresh } from "@/hooks/use-pull-to-refresh"; @@ -527,7 +530,7 @@ export default function InsightsPageClient() { "health-status": , breathing: , "labs-changes": , - ecg: , + ecg: , }; const orderedSectionIds = orderedVisibleSectionIds(layout); diff --git a/src/components/insights/__tests__/ecg-section.test.tsx b/src/components/insights/__tests__/ecg-section.test.tsx index eb663f15d..8dff451da 100644 --- a/src/components/insights/__tests__/ecg-section.test.tsx +++ b/src/components/insights/__tests__/ecg-section.test.tsx @@ -28,7 +28,7 @@ vi.mock("@tanstack/react-query", () => ({ useQuery: (opts: unknown) => useQueryMock(opts), })); -const { EcgSection, ECG_LIST_LIMIT } = await import("../ecg-section"); +const { EcgSection, ECG_OVERVIEW_LIMIT } = await import("../ecg-section"); type EcgRecordingListItem = import("../ecg-section").EcgRecordingListItem; const IRREGULAR_REC: EcgRecordingListItem = { @@ -53,11 +53,12 @@ const NORMAL_REC: EcgRecordingListItem = { function renderSection( data: { recordings: EcgRecordingListItem[]; hasRecordings: boolean } | undefined, + props: { limit?: number } = {}, ) { useQueryMock.mockReturnValue({ data, isLoading: false }); return renderToStaticMarkup( - + , ); } @@ -99,30 +100,52 @@ describe("", () => { ); }); - it("caps the list at the most recent handful", () => { - // Eight strips in, five out — and the five are the ones the route - // handed back first, which it orders `recordedAt desc`. - const recordings = Array.from({ length: 8 }, (_, index) => ({ + const eightRecordings = () => + Array.from({ length: 8 }, (_, index) => ({ ...NORMAL_REC, id: `ecg_${index}`, recordedAt: new Date(Date.UTC(2026, 5, 20 - index, 9, 15)).toISOString(), })); - const html = renderSection({ recordings, hasRecordings: true }); - expect(ECG_LIST_LIMIT).toBe(5); - expect(countRows(html)).toBe(ECG_LIST_LIMIT); - for (const kept of recordings.slice(0, ECG_LIST_LIMIT)) { + it("caps the OVERVIEW teaser at the most recent handful", () => { + // The teaser passes `limit`. Eight strips in, five out — and the five are + // the ones the route handed back first, which it orders `recordedAt desc`. + const recordings = eightRecordings(); + const html = renderSection( + { recordings, hasRecordings: true }, + { limit: ECG_OVERVIEW_LIMIT }, + ); + + expect(ECG_OVERVIEW_LIMIT).toBe(5); + expect(countRows(html)).toBe(ECG_OVERVIEW_LIMIT); + for (const kept of recordings.slice(0, ECG_OVERVIEW_LIMIT)) { expect(html, `${kept.id} should be listed`).toContain( `/insights/ecg/${kept.id}`, ); } - for (const dropped of recordings.slice(ECG_LIST_LIMIT)) { - expect(html, `${dropped.id} is past the cap`).not.toContain( + for (const dropped of recordings.slice(ECG_OVERVIEW_LIMIT)) { + expect(html, `${dropped.id} is past the teaser cap`).not.toContain( `/insights/ecg/${dropped.id}`, ); } }); + it("renders every recording when NO limit is given (the full page)", () => { + // The dedicated `/insights/ecg` page passes no `limit`, so an account with + // many strips reaches all of them — the cap is a teaser affordance, never + // a wall on the full surface. Eight seeded, eight rendered, every one + // reachable by its own address. + const recordings = eightRecordings(); + const html = renderSection({ recordings, hasRecordings: true }); + + expect(countRows(html)).toBe(8); + for (const rec of recordings) { + expect(html, `${rec.id} must be reachable on the full page`).toContain( + `/insights/ecg/${rec.id}`, + ); + } + }); + it("opens the recording's own address when it has a waveform", () => { const html = renderSection({ recordings: [IRREGULAR_REC], diff --git a/src/components/insights/ecg-section.tsx b/src/components/insights/ecg-section.tsx index ab6ed1669..eee82e990 100644 --- a/src/components/insights/ecg-section.tsx +++ b/src/components/insights/ecg-section.tsx @@ -12,10 +12,16 @@ import { cn } from "@/lib/utils"; import { SectionHeading } from "@/components/ui/section-heading"; import { ecgResultLabel, + ECG_OVERVIEW_LIMIT, type EcgClassification, } from "@/lib/insights/ecg-classification"; import { InsightSectionCard } from "./insight-section-card"; +// Re-exported so the tests (and any teaser call site) can keep importing the +// overview cap from the component they are exercising; the value itself lives +// in the React-free lib module. +export { ECG_OVERVIEW_LIMIT }; + /** * v1.28.50 — ECG recording list. * @@ -38,12 +44,6 @@ import { InsightSectionCard } from "./insight-section-card"; * when the user has no recordings — never an empty / alarming card. */ -/** - * How many strips the list paints. The API already caps its own read at 200; - * this is the reading limit, not the fetch limit. - */ -export const ECG_LIST_LIMIT = 5; - export interface EcgRecordingListItem { id: string; recordedAt: string; @@ -72,12 +72,20 @@ interface EcgSectionProps { * teaser keeps its own heading unchanged. */ hideHeading?: boolean; + /** + * Cap the number of strips shown. The overview teaser passes + * `ECG_OVERVIEW_LIMIT`; the dedicated `/insights/ecg` page passes nothing, + * so it renders the full list. Omitted = no cap: a full surface must never + * silently hide a recording behind a limit with no way to reach it. + */ + limit?: number; } export function EcgSection({ enabled = true, className, hideHeading = false, + limit, }: EcgSectionProps) { const { isAuthenticated } = useAuth(); const { t } = useTranslations(); @@ -98,9 +106,11 @@ export function EcgSection({ // Data-availability gate — never paint an empty card. if (isLoading || !data || !data.hasRecordings) return null; - // The route hands them back most-recent-first; the slice is the reading - // limit on top of that, so the newest strips are the ones on screen. - const recordings = data.recordings.slice(0, ECG_LIST_LIMIT); + // The route hands them back most-recent-first. A `limit` (the overview + // teaser) keeps the newest few; no limit (the dedicated page) shows them + // all, so an older strip is never made unreachable. + const recordings = + limit != null ? data.recordings.slice(0, limit) : data.recordings; return (
= { IRREGULAR: "insights.ecg.result.irregular",