mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 04:32:42 +00:00
fix(insights): correct the active-energy assessment id and share the status card
The active-energy page passed `ACTIVE_ENERGY_BURNED` as `statusMetric`, but that is the DB MeasurementType, not the registry id the metric-status route accepts. The route's closed enum only knows `ACTIVE_ENERGY`, so every fetch 422'd and the assessment card silently fell back to its empty body. Point the page at the `ACTIVE_ENERGY` registry id and type `statusMetric` (plus the `useInsightMetricStatus` metric param) to the closed `MetricStatusMetricId` union instead of a bare string, so a future MeasurementType-vs-registry-id mismatch is a compile error rather than a silent 422. A typecheck pass over all 29 pages confirms active-energy was the only diverging one. Extract a shared `MetricStatusCard` that owns the `useInsightMetricStatus` hook and the assessment-card wiring, collapsing the duplicated mount block in the HealthKit metric scaffold and the bespoke sleep page to a single line each (only the icon differs). Behaviour is unchanged: the card mounts only on the data-bearing branch, so a source-less account never fires an assessment round-trip. Add a test asserting the active-energy page wires an accepted registry id. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 6e371aaeb9698f7b4251c22e1d8f381ddfa70a00)
This commit is contained in:
1 parent
5827ef58cb
commit
58b793ee4a
6 files changed
+112
-48
No files matched your search
@@ -17,7 +17,7 @@ export default function InsightsActiveEnergyPage() {
|
||||
return (
|
||||
<HealthKitMetricPage
|
||||
measurementType="ACTIVE_ENERGY_BURNED"
|
||||
statusMetric="ACTIVE_ENERGY_BURNED"
|
||||
statusMetric="ACTIVE_ENERGY"
|
||||
insightMetric="ACTIVE_ENERGY_BURNED"
|
||||
chartKey="activeEnergy"
|
||||
i18nPrefix="insights.activeEnergy"
|
||||
|
||||
@@ -3,11 +3,10 @@
|
||||
import Link from "next/link";
|
||||
import { Moon } from "lucide-react";
|
||||
|
||||
import { useInsightMetricStatus } from "@/hooks/use-insight-status";
|
||||
import { useInsightsAnalytics } from "@/hooks/use-insights-analytics";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { InsightStatusCard } from "@/components/insights/insight-status-card";
|
||||
import { MetricStatusCard } from "@/components/insights/metric-status-card";
|
||||
import { MetricEmptyState } from "@/components/insights/metric-empty-state";
|
||||
import { MetricTargetSummary } from "@/components/insights/metric-target-summary";
|
||||
import { SleepOverview } from "@/components/insights/sleep-overview";
|
||||
@@ -34,16 +33,6 @@ export default function InsightsSchlafPage() {
|
||||
|
||||
const { isEmpty } = useInsightsAnalytics("SLEEP_DURATION");
|
||||
|
||||
// v1.8.7.1 — the per-section sleep assessment now rides the generic
|
||||
// metric-status route, keyed by `SLEEP_DURATION`. The hook runs on every
|
||||
// render but only fetches once the page has data (the empty branch below
|
||||
// short-circuits before the card mounts), so a source-less account never
|
||||
// fires an assessment round-trip.
|
||||
const { data: status, isLoading: isStatusLoading } = useInsightMetricStatus(
|
||||
"SLEEP_DURATION",
|
||||
!isEmpty,
|
||||
);
|
||||
|
||||
if (isEmpty) {
|
||||
return (
|
||||
<SubPageShell
|
||||
@@ -82,18 +71,15 @@ export default function InsightsSchlafPage() {
|
||||
{/*
|
||||
v1.8.7.1 — the per-section sleep assessment lands via the generic
|
||||
metric-status route (`?metric=SLEEP_DURATION`), closing the slot
|
||||
the v1.4.28 deferral left open. The card consumes the same
|
||||
`InsightStatusData` envelope every sibling sub-page reads.
|
||||
the v1.4.28 deferral left open. The shared `<MetricStatusCard>`
|
||||
owns the hook + card; only the icon differs from the HealthKit
|
||||
scaffold (Moon vs Sparkles). The card mounts only on this
|
||||
data-bearing branch, so a source-less account never fetches.
|
||||
*/}
|
||||
<InsightStatusCard
|
||||
title={t("insights.assessmentTitle")}
|
||||
<MetricStatusCard
|
||||
metric="SLEEP_DURATION"
|
||||
icon={<Moon className="h-5 w-5" />}
|
||||
text={status?.text ?? null}
|
||||
hasProvider={status?.hasProvider ?? false}
|
||||
cached={status?.cached ?? false}
|
||||
updatedAt={status?.updatedAt ?? null}
|
||||
loading={isStatusLoading}
|
||||
preparing={status?.preparing ?? false}
|
||||
enabled={!isEmpty}
|
||||
/>
|
||||
</SubPageShell>
|
||||
);
|
||||
|
||||
@@ -50,6 +50,8 @@ vi.mock("@/hooks/use-insight-status", async (importOriginal) => {
|
||||
});
|
||||
|
||||
import { HealthKitMetricPage } from "../healthkit-metric-page";
|
||||
import { METRIC_STATUS_IDS } from "@/lib/insights/metric-status-registry";
|
||||
import InsightsActiveEnergyPage from "@/app/insights/active-energy/page";
|
||||
|
||||
function render(node: React.ReactNode, locale: "en" | "de" = "en") {
|
||||
const queryClient = new QueryClient({
|
||||
@@ -116,9 +118,10 @@ describe("<HealthKitMetricPage> status mount", () => {
|
||||
|
||||
const html = render(<HealthKitMetricPage {...baseProps} />);
|
||||
|
||||
// No assessment heading, and the generic fetch is disabled.
|
||||
// No assessment heading, and the card (which owns the hook) never
|
||||
// mounts, so the generic fetch is never wired.
|
||||
expect(html).not.toContain("Assessment");
|
||||
expect(metricStatusMock).toHaveBeenCalledWith("", false);
|
||||
expect(metricStatusMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("renders the insufficient-data empty state (no card) when the metric has no data", () => {
|
||||
@@ -132,9 +135,30 @@ describe("<HealthKitMetricPage> status mount", () => {
|
||||
/>,
|
||||
);
|
||||
|
||||
// Empty branch: no settled assessment text, and the status fetch is
|
||||
// disabled so a source-less account never fires the round-trip.
|
||||
// Empty branch: the page short-circuits to the empty state before the
|
||||
// card JSX, so no settled assessment text and the card (and its fetch)
|
||||
// never mounts — a source-less account never fires the round-trip.
|
||||
expect(html).not.toContain("Your HRV has trended upward");
|
||||
expect(metricStatusMock).toHaveBeenCalledWith("HEART_RATE_VARIABILITY", false);
|
||||
expect(metricStatusMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("wires the active-energy page to an accepted registry id (not the MeasurementType remap)", () => {
|
||||
// v1.8.7.1 (Design H1) — the active-energy page formerly passed
|
||||
// `ACTIVE_ENERGY_BURNED` (the DB MeasurementType), which the route's
|
||||
// closed enum rejects with 422; the registry id is `ACTIVE_ENERGY`.
|
||||
// Render the real page module and assert the hook receives a metric id
|
||||
// the route accepts.
|
||||
analyticsMock.mockReturnValue({
|
||||
data: { summaries: { ACTIVE_ENERGY_BURNED: { count: 30 } } },
|
||||
isEmpty: false,
|
||||
});
|
||||
metricStatusMock.mockReturnValue({ data: undefined, isLoading: false });
|
||||
|
||||
render(<InsightsActiveEnergyPage />);
|
||||
|
||||
expect(metricStatusMock).toHaveBeenCalled();
|
||||
const passedMetric = metricStatusMock.mock.calls[0][0] as string;
|
||||
expect(passedMetric).toBe("ACTIVE_ENERGY");
|
||||
expect(METRIC_STATUS_IDS).toContain(passedMetric);
|
||||
});
|
||||
});
|
||||
@@ -6,15 +6,15 @@ import type { ComponentProps, ReactNode } from "react";
|
||||
import { Sparkles } from "lucide-react";
|
||||
|
||||
import { useAuth } from "@/hooks/use-auth";
|
||||
import { useInsightMetricStatus } from "@/hooks/use-insight-status";
|
||||
import { useInsightsAnalytics } from "@/hooks/use-insights-analytics";
|
||||
import { useInsightsLayoutPrefs } from "@/hooks/use-insights-layout-prefs";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import type { InsightMetric } from "@/lib/insights/metric-availability";
|
||||
import type { MetricStatusMetricId } from "@/lib/insights/metric-status-registry";
|
||||
import type { ChartOverlayKey } from "@/lib/dashboard-layout";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { HealthChartDynamic } from "@/components/charts/health-chart-dynamic";
|
||||
import { InsightStatusCard } from "@/components/insights/insight-status-card";
|
||||
import { MetricStatusCard } from "@/components/insights/metric-status-card";
|
||||
import { MetricEmptyState } from "@/components/insights/metric-empty-state";
|
||||
import { MetricStatStrip } from "@/components/insights/metric-stat-strip";
|
||||
import { MeasurementDiversityNudge } from "@/components/insights/measurement-diversity-nudge";
|
||||
@@ -109,8 +109,14 @@ export interface HealthKitMetricPageProps {
|
||||
* data-bearing branch; the empty (insufficient-data) branch keeps the
|
||||
* existing `<MetricEmptyState>` note and never fires an assessment
|
||||
* fetch. Omit it for metrics that should not carry an assessment.
|
||||
*
|
||||
* Typed to the closed `MetricStatusMetricId` union (the registry-id
|
||||
* vocabulary the route's Zod enum accepts) rather than a bare string,
|
||||
* so a page that passes a MeasurementType remap (e.g.
|
||||
* `ACTIVE_ENERGY_BURNED` instead of the `ACTIVE_ENERGY` registry id)
|
||||
* is a compile error, not a silent 422.
|
||||
*/
|
||||
statusMetric?: string;
|
||||
statusMetric?: MetricStatusMetricId;
|
||||
}
|
||||
|
||||
export function HealthKitMetricPage({
|
||||
@@ -136,14 +142,6 @@ export function HealthKitMetricPage({
|
||||
|
||||
const { data: analytics, isEmpty } = useInsightsAnalytics(insightMetric);
|
||||
|
||||
// v1.8.7.1 — generic per-metric assessment. The hook must run on every
|
||||
// render (rules of hooks), so it is always called; `enabled` gates the
|
||||
// actual fetch on a configured `statusMetric` AND the presence of data
|
||||
// so an insufficient-data page never fires an assessment round-trip.
|
||||
const { data: status, isLoading: isStatusLoading } = useInsightMetricStatus(
|
||||
statusMetric ?? "",
|
||||
Boolean(statusMetric) && !isEmpty,
|
||||
);
|
||||
const rawSummary = analytics?.summaries?.[measurementType] ?? null;
|
||||
// The stat strip renders display-unit values. The summary holds stored
|
||||
// values, so when the page renders a scaled unit (e.g. WALKING_SPEED
|
||||
@@ -225,15 +223,10 @@ export function HealthKitMetricPage({
|
||||
<MetricTargetSummary slug={targetSummarySlug} />
|
||||
) : null}
|
||||
{statusMetric ? (
|
||||
<InsightStatusCard
|
||||
title={t("insights.assessmentTitle")}
|
||||
<MetricStatusCard
|
||||
metric={statusMetric}
|
||||
icon={<Sparkles className="h-5 w-5" />}
|
||||
text={status?.text ?? null}
|
||||
hasProvider={status?.hasProvider ?? false}
|
||||
cached={status?.cached ?? false}
|
||||
updatedAt={status?.updatedAt ?? null}
|
||||
loading={isStatusLoading}
|
||||
preparing={status?.preparing ?? false}
|
||||
enabled={!isEmpty}
|
||||
/>
|
||||
) : null}
|
||||
</SubPageShell>
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
"use client";
|
||||
|
||||
import type { ReactNode } from "react";
|
||||
|
||||
import { useInsightMetricStatus } from "@/hooks/use-insight-status";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import type { MetricStatusMetricId } from "@/lib/insights/metric-status-registry";
|
||||
import { InsightStatusCard } from "@/components/insights/insight-status-card";
|
||||
|
||||
interface MetricStatusCardProps {
|
||||
/** The registry metric id the generic assessment route keys on. */
|
||||
metric: MetricStatusMetricId;
|
||||
/** The leading glyph (Sparkles for the generic scaffold, Moon for sleep). */
|
||||
icon: ReactNode;
|
||||
/**
|
||||
* Gate the fetch on the page having data, so an insufficient-data page
|
||||
* never fires an assessment round-trip. The hook still runs on every
|
||||
* render (rules of hooks); `enabled` decides whether it talks to the
|
||||
* network.
|
||||
*/
|
||||
enabled: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* v1.8.7.1 — shared mount for the generic per-metric assessment card.
|
||||
*
|
||||
* Owns the `useInsightMetricStatus` hook + the eight-prop `<InsightStatusCard>`
|
||||
* wiring (`status?.x ?? default`) so the HealthKit metric scaffold and the
|
||||
* bespoke sleep page consume one seam instead of repeating the block. Only
|
||||
* the icon differs between the two sites; everything else is identical.
|
||||
*/
|
||||
export function MetricStatusCard({
|
||||
metric,
|
||||
icon,
|
||||
enabled,
|
||||
}: MetricStatusCardProps) {
|
||||
const { t } = useTranslations();
|
||||
const { data: status, isLoading } = useInsightMetricStatus(metric, enabled);
|
||||
|
||||
return (
|
||||
<InsightStatusCard
|
||||
title={t("insights.assessmentTitle")}
|
||||
icon={icon}
|
||||
text={status?.text ?? null}
|
||||
hasProvider={status?.hasProvider ?? false}
|
||||
cached={status?.cached ?? false}
|
||||
updatedAt={status?.updatedAt ?? null}
|
||||
loading={isLoading}
|
||||
preparing={status?.preparing ?? false}
|
||||
/>
|
||||
);
|
||||
}
|
||||
@@ -4,6 +4,7 @@ import { useQuery } from "@tanstack/react-query";
|
||||
|
||||
import { useAuth } from "@/hooks/use-auth";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import type { MetricStatusMetricId } from "@/lib/insights/metric-status-registry";
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
|
||||
/**
|
||||
@@ -183,7 +184,15 @@ export function useInsightStatus(metric: InsightStatusMetric) {
|
||||
* no data (the page renders the insufficient-data empty state instead of
|
||||
* the card), so a brand-new account never fires an assessment round-trip.
|
||||
*/
|
||||
export function useInsightMetricStatus(metric: string, enabled = true) {
|
||||
export function useInsightMetricStatus(
|
||||
// The closed registry-id union (the route's Zod enum vocabulary). `""`
|
||||
// is the disabled-fetch placeholder the `HealthKitMetricPage` passes when
|
||||
// no `statusMetric` is wired (the hook must still run per the rules of
|
||||
// hooks); `enabled` is false on that branch so the empty id never reaches
|
||||
// a round-trip. A typo'd metric id is therefore a compile error.
|
||||
metric: MetricStatusMetricId | "",
|
||||
enabled = true,
|
||||
) {
|
||||
const { isAuthenticated } = useAuth();
|
||||
const { locale } = useTranslations();
|
||||
|
||||
|
||||
Reference in new issue
Block a user