From 720e6c89b9dd1ab03797a18431a5dfdd31122e89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 10 May 2026 11:01:01 +0200 Subject: [PATCH] refactor(v1.4.18): apply simplify-review safe suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven mechanical cleanups, no behaviour changes: - Drop the duplicate ChartOverlayPrefs type + DEFAULT in chart-overlay-controls; re-export the canonical pair from dashboard-layout so a future fourth toggle can't drift between the UI and persistence layers. - useChartOverlayPrefs accepts null/undefined chartKey and short- circuits the dashboard-layout fetch when none is supplied. Drops the chartKey ?? "bp" fallback and the three discard-the-result ternaries in health-chart; mini-mode renders no longer subscribe to the BP chart's persisted prefs. - /api/analytics no longer pair-and-counts BP twice — the windowed helper already produces last30Days, so the headline reads it back instead of running computeBpInTargetPct on the same input. - Drop the redundant moodEntries.map shape in the achievements route; the prisma select already returns the matching shape. - Merge the five hidden-metric cases into the no-precondition block of isEarnable; the defensive comment admitted they couldn't fire. - Remove the deprecated getAchievementCategory wrapper (zero callers). - Strip six "v1.4.18 gradient removed" what-comments scattered across the chart wrappers; the polish tests pin the absence. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/api/analytics/route.ts | 25 ++++------------ .../api/gamification/achievements/route.ts | 6 +--- .../__tests__/chart-overlay-controls.test.tsx | 6 ++-- .../charts/chart-overlay-controls.tsx | 30 +++++++++---------- src/components/charts/health-chart.tsx | 17 ++++------- .../charts/medication-compliance-chart.tsx | 2 -- src/components/charts/mood-chart.tsx | 2 -- src/hooks/use-chart-overlay-prefs.ts | 13 +++++++- src/lib/gamification/achievements.ts | 25 ++++------------ 9 files changed, 47 insertions(+), 79 deletions(-) diff --git a/src/app/api/analytics/route.ts b/src/app/api/analytics/route.ts index 83e751641..3f212fe83 100644 --- a/src/app/api/analytics/route.ts +++ b/src/app/api/analytics/route.ts @@ -4,10 +4,7 @@ import { annotate } from "@/lib/logging/context"; import { apiSuccess } from "@/lib/api-response"; import { summarize, type DataPoint } from "@/lib/analytics/trends"; import { getBpTargets } from "@/lib/analytics/bp-targets"; -import { - computeBpInTargetPct, - computeBpInTargetWindows, -} from "@/lib/analytics/bp-in-target"; +import { computeBpInTargetWindows } from "@/lib/analytics/bp-in-target"; import type { MeasurementType } from "@/generated/prisma/client"; import { measurementTypeEnum } from "@/lib/validations/measurement"; @@ -82,27 +79,17 @@ export const GET = apiHandler(async () => { }), ]); - // Pure pair-and-count helper (see lib/analytics/bp-in-target.ts). - // Replaces an earlier inline implementation that divided by - // `sysData.length` and ignored a same-Berlin-day fallback for - // imports rounded past the 5-minute window — which made the tile - // read 0 % even when readings clearly sat inside target. - const result = computeBpInTargetPct(sysData, diaData, bpTargets); - bpInTargetPct = result?.pct ?? null; - - // v1.4.18 A1 — populate the dashboard tile's 7T / 30T sub-values. - // The headline (`bpInTargetPct`, also 30 days) is kept for backward - // compatibility with cached client bundles; the tile reads the - // explicit window fields below for the sub-row. `last30Days` here - // is the same calculation as the headline but kept independent so - // a future refactor (e.g. headline → "today's reading in target?") - // can move without breaking the sub-values. + // v1.4.18 A1 — windowed helper computes the 7d / 30d sub-values + // for the dashboard tile in a single pair-and-count pass. The + // headline (`bpInTargetPct`) reuses the 30-day result so we don't + // re-pair the same data twice. See lib/analytics/bp-in-target.ts. const windows = computeBpInTargetWindows( sysData, diaData, bpTargets, now, ); + bpInTargetPct = windows.last30Days?.pct ?? null; bpInTargetPct7d = windows.last7Days?.pct ?? null; bpInTargetPct30d = windows.last30Days?.pct ?? null; } diff --git a/src/app/api/gamification/achievements/route.ts b/src/app/api/gamification/achievements/route.ts index e171ac655..62d32c81f 100644 --- a/src/app/api/gamification/achievements/route.ts +++ b/src/app/api/gamification/achievements/route.ts @@ -604,11 +604,7 @@ export const GET = apiHandler(async (request: NextRequest) => { const expansionValues = buildExpansionMetricValues({ measurements, - moodEntries: moodEntries.map((m) => ({ - date: m.date, - score: m.score, - moodLoggedAt: m.moodLoggedAt, - })), + moodEntries, intakeEvents, auditEvents, }); diff --git a/src/components/charts/__tests__/chart-overlay-controls.test.tsx b/src/components/charts/__tests__/chart-overlay-controls.test.tsx index 06a77dff3..c8b2e0452 100644 --- a/src/components/charts/__tests__/chart-overlay-controls.test.tsx +++ b/src/components/charts/__tests__/chart-overlay-controls.test.tsx @@ -5,7 +5,7 @@ import { I18nProvider } from "@/lib/i18n/context"; import { ChartOverlayControls, DEFAULT_CHART_OVERLAY_PREFS, - type ChartOverlayPrefsValue, + type ChartOverlayPrefs, } from "../chart-overlay-controls"; /** @@ -61,7 +61,7 @@ describe("", () => { // mirrors what the radix-ui Switch will pass at runtime without // needing a DOM event-loop. const onChange = vi.fn(); - const start: ChartOverlayPrefsValue = DEFAULT_CHART_OVERLAY_PREFS; + const start: ChartOverlayPrefs = DEFAULT_CHART_OVERLAY_PREFS; // Render once — this binds the callback closure. renderToStaticMarkup( withProvider( @@ -71,7 +71,7 @@ describe("", () => { // Simulate the merge the component performs: setKey("showTargetRange", true) // would call onChange({ ...start, showTargetRange: true }). - const next: ChartOverlayPrefsValue = { + const next: ChartOverlayPrefs = { ...start, showTargetRange: true, }; diff --git a/src/components/charts/chart-overlay-controls.tsx b/src/components/charts/chart-overlay-controls.tsx index 3d2961012..bf3617af3 100644 --- a/src/components/charts/chart-overlay-controls.tsx +++ b/src/components/charts/chart-overlay-controls.tsx @@ -14,6 +14,7 @@ import { Switch } from "@/components/ui/switch"; import { Label } from "@/components/ui/label"; import { Button } from "@/components/ui/button"; import { useTranslations } from "@/lib/i18n/context"; +import { type ChartOverlayPrefs } from "@/lib/dashboard-layout"; /** * v1.4.18 — per-chart overlay-controls popover. @@ -46,18 +47,20 @@ import { useTranslations } from "@/lib/i18n/context"; * pure UI. */ -export interface ChartOverlayPrefsValue { - /** "7-day trend" line + numeric delta chip in chart header. */ - showTrendIndicator: boolean; - /** Linear-regression trend line + arrow direction + personal baseline. */ - showTrendArrow: boolean; - /** Target-zone shading (BP / BMI / medication thresholds). */ - showTargetRange: boolean; -} +/** + * Re-export the canonical type+default from `@/lib/dashboard-layout` + * so chart-side callers can keep importing them from this module. + * Single source of truth lives in dashboard-layout (the persistence + * layer); this module is just the UI. + */ +export { + DEFAULT_CHART_OVERLAY_PREFS, + type ChartOverlayPrefs, +} from "@/lib/dashboard-layout"; export interface ChartOverlayControlsProps { - prefs: ChartOverlayPrefsValue; - onChange: (next: ChartOverlayPrefsValue) => void; + prefs: ChartOverlayPrefs; + onChange: (next: ChartOverlayPrefs) => void; /** * Optional CSS class for the trigger button — letting the chart * wrapper align the cog with its title row. @@ -76,7 +79,7 @@ export function ChartOverlayControls({ const targetRangeId = useId(); const setKey = ( - key: keyof ChartOverlayPrefsValue, + key: keyof ChartOverlayPrefs, value: boolean, ): void => { onChange({ ...prefs, [key]: value }); @@ -161,8 +164,3 @@ export function ChartOverlayControls({ ); } -export const DEFAULT_CHART_OVERLAY_PREFS: ChartOverlayPrefsValue = { - showTrendIndicator: false, - showTrendArrow: false, - showTargetRange: false, -}; diff --git a/src/components/charts/health-chart.tsx b/src/components/charts/health-chart.tsx index 86f81853f..45d722820 100644 --- a/src/components/charts/health-chart.tsx +++ b/src/components/charts/health-chart.tsx @@ -316,12 +316,12 @@ export function HealthChart({ // v1.4.18 — three overlay toggles (showTrendIndicator / showTrendArrow // / showTargetRange) are persisted per chart via the new // `useChartOverlayPrefs` hook. When the chart isn't bound to a - // persistent key (mini mode / ad-hoc render) the prefs fall through - // to the clean-line default (every flag false). - const overlayPrefs = useChartOverlayPrefs(chartKey ?? "bp"); - const showMA = chartKey ? overlayPrefs.prefs.showTrendIndicator : false; - const showTrend = chartKey ? overlayPrefs.prefs.showTrendArrow : false; - const showBands = chartKey ? overlayPrefs.prefs.showTargetRange : false; + // persistent key (mini mode / ad-hoc render) the hook short-circuits + // to the clean-line default and skips its dashboard-layout fetch. + const overlayPrefs = useChartOverlayPrefs(chartKey); + const showMA = overlayPrefs.prefs.showTrendIndicator; + const showTrend = overlayPrefs.prefs.showTrendArrow; + const showBands = overlayPrefs.prefs.showTargetRange; const bmiDivisor = valueMode === "bmi" && user?.heightCm ? (user.heightCm / 100) ** 2 : null; @@ -968,9 +968,6 @@ export function HealthChart({ ) : null} - {/* v1.4.18 — gradient fill removed per Marc's feedback. - The clean line is the chart; no painted background under - it. */}
)} - {/* v1.4.18 — gradient Area removed; only the clean - line below paints the metric. */} {types.map((type, i) => ( )} - {/* v1.4.18 — gradient defs removed; clean line only. */} {isLoading ? (
@@ -422,7 +421,6 @@ export function MedicationComplianceChart({ /> ) : null} - {/* v1.4.18 — gradient Area removed; clean line only. */} - {/* v1.4.18 — gradient defs removed; clean line only. */} {isLoading ? (
@@ -779,7 +778,6 @@ export function MoodChart({ ); }} /> - {/* v1.4.18 — gradient Area removed; clean line only. */} void; isSaving: boolean; @@ -44,9 +46,14 @@ export function useChartOverlayPrefs(chartKey: ChartOverlayKey): { // Keep the cache warm — the dashboard page mounts a query against // the same key on first paint, so this hook just piggy-backs. staleTime: 60_000, + // Skip the fetch entirely when the caller doesn't pass a chartKey + // (mini-mode / ad-hoc chart usage). Saves a wasted GET on pages like + // /insights that mount charts without per-chart persistence. + enabled: Boolean(chartKey), }); const prefs = useMemo(() => { + if (!chartKey) return DEFAULT_CHART_OVERLAY_PREFS; return ( layout?.chartOverlayPrefs?.[chartKey] ?? DEFAULT_CHART_OVERLAY_PREFS ); @@ -54,6 +61,9 @@ export function useChartOverlayPrefs(chartKey: ChartOverlayKey): { const mutation = useMutation({ mutationFn: async (next: ChartOverlayPrefs): Promise => { + // No-op when no chartKey — the setter is wired up but the hook + // is in "ad-hoc render" mode where overlays don't persist. + if (!chartKey) return; const res = await fetch("/api/dashboard/chart-overlay-prefs", { method: "PUT", headers: { "content-type": "application/json" }, @@ -64,6 +74,7 @@ export function useChartOverlayPrefs(chartKey: ChartOverlayKey): { } }, onMutate: async (next) => { + if (!chartKey) return { previous: undefined }; await queryClient.cancelQueries({ queryKey: ["dashboard-layout"] }); const previous = queryClient.getQueryData([ "dashboard-layout", diff --git a/src/lib/gamification/achievements.ts b/src/lib/gamification/achievements.ts index 90b4582c3..d01ecd423 100644 --- a/src/lib/gamification/achievements.ts +++ b/src/lib/gamification/achievements.ts @@ -39,8 +39,7 @@ export type AchievementMetricKey = * Achievement categories — used by the /achievements UI to visually group * the badges. Pure presentation: the computation logic in this file does * not branch on category, and the metric → category mapping is derived in - * `getAchievementCategory` below so a metric is always in exactly one - * group. + * `categoryForMetric` below so a metric is always in exactly one group. * * v1.4.18 adds two new buckets: * - `mood` for the new mood-tracking achievements @@ -129,18 +128,6 @@ function categoryForMetric( } } -/** - * @deprecated kept only for legacy callers; new code should read - * `AchievementDefinition.category` directly. v1.4.18 made category a - * stored field on the definition so hidden Easter-eggs (which share - * metrics with no other badge) don't need a special case. - */ -export function getAchievementCategory( - metric: AchievementMetricKey, -): AchievementCategory { - return categoryForMetric(metric); -} - /** * Stable category render order on the /achievements page. Mood slots * between vitals and security so the most-used categories cluster at @@ -823,17 +810,15 @@ function isEarnable( case "consistentMonthCount": case "entryDayStreak": case "weekendStreakCount": - // No metric-data precondition — the user has an account, so the - // achievement is always discoverable. - return true; case "nightOwlCount": case "earlyBirdCount": case "leapDayCount": case "doctorPdfCount": case "localeFlipCount": - // Hidden achievements are filtered by category check above; this - // branch is only reached if a hidden definition's category got - // misconfigured. Stay defensive. + // No metric-data precondition — the user has an account, so the + // achievement is always discoverable. The five hidden-metric keys + // are filtered earlier in `applyDiscoveryFilter` by category, but + // we cover them here for exhaustiveness. return true; } }