From 39ae034b8c0eade4d7c934c8825e2491b7c83ac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 24 Sep 2026 09:48:12 +0200 Subject: [PATCH] fix(modules): route nav entries and module pages through the surface map Navigation destinations, the bottom bar's fixed slots and the tour stops no longer carry their own module keys; each asks the surface map by its href. Insights has no owner there, so switching AI analysis off leaves the Insights entry, slot and tour stop in place. The shell's page gate now resolves Insights sub-pages by slug as well as top-level pages, so /insights/mood, /insights/medications and the recovery metric pages show the module notice when their module is off, worded from the access reason, with the Settings link only for the record's own switch. The per-page redirect guard, the achievements redirect and the nutrients page's own notice are gone; nothing redirects any more. --- src/app/achievements/page.tsx | 25 +---- src/app/insights/blood-glucose/page.tsx | 13 +-- src/app/insights/nutrients/page.tsx | 57 ++-------- src/app/insights/recovery/page.tsx | 13 +-- src/app/insights/sleep/page.tsx | 14 +-- src/app/insights/workouts/[id]/page.tsx | 13 --- src/app/insights/workouts/page-client.tsx | 13 +-- src/app/insights/workouts/page.tsx | 8 +- .../__tests__/bottom-nav-module-gate.test.tsx | 13 ++- .../__tests__/module-page-gate.test.tsx | 101 ++++++++++++++++++ .../__tests__/nav-model-shared-record.test.ts | 9 +- .../layout/__tests__/nav-model.test.ts | 13 ++- .../layout/__tests__/sidebar-nav.test.tsx | 6 +- src/components/layout/bottom-nav.tsx | 61 ++++------- .../layout/module-disabled-notice.tsx | 6 +- src/components/layout/module-page-gate.tsx | 62 +++++++---- src/components/layout/nav-model.ts | 70 +++++------- src/components/onboarding/tour.tsx | 2 +- .../__tests__/use-module-page-guard.test.tsx | 86 --------------- src/hooks/use-module-page-guard.ts | 45 -------- .../onboarding/__tests__/tour-state.test.ts | 13 +++ src/lib/onboarding/tour-state.ts | 21 +--- 22 files changed, 263 insertions(+), 401 deletions(-) create mode 100644 src/components/layout/__tests__/module-page-gate.test.tsx delete mode 100644 src/hooks/__tests__/use-module-page-guard.test.tsx delete mode 100644 src/hooks/use-module-page-guard.ts diff --git a/src/app/achievements/page.tsx b/src/app/achievements/page.tsx index abee3ed10..57d1d4f8a 100644 --- a/src/app/achievements/page.tsx +++ b/src/app/achievements/page.tsx @@ -34,8 +34,6 @@ import { EmptyState } from "@/components/ui/empty-state"; import { PageHeader } from "@/components/ui/page-header"; import { PageAuthGate } from "@/components/ui/page-auth-gate"; import { QueryErrorCard } from "@/components/ui/query-error-card"; -import { useEffect } from "react"; -import { useRouter } from "next/navigation"; import { useAuth } from "@/hooks/use-auth"; import { useTranslations } from "@/lib/i18n/context"; import { formatDate } from "@/lib/format"; @@ -262,7 +260,6 @@ function AchievementCard({ achievement, t }: AchievementCardProps) { export default function AchievementsPage() { const { user, isAuthenticated, isLoading: authLoading } = useAuth(); const { t } = useTranslations(); - const router = useRouter(); // v1.18.0 — the achievements module gate. When the account has the // module turned off the whole page disappears: an empty state stands in @@ -279,29 +276,13 @@ export default function AchievementsPage() { enabled: isAuthenticated && achievementsEnabled, }); - useEffect(() => { - if (!authLoading && isAuthenticated && !achievementsEnabled) { - router.push("/insights"); - } - }, [authLoading, isAuthenticated, achievementsEnabled, router]); - if (authLoading || (achievementsEnabled && isLoading)) { return ; } - // v1.18.0 — module off ⇒ the surface disappears entirely. The nav entry is - // hidden by the same gate, so this only catches a direct URL hit. It used to - // render `null` and stop there, which left a blank page with no explanation - // and no route anywhere — a dead end rather than a gate. It now bounces to - // /insights, matching every other module-gated page (`useModulePageGuard`); - // the brief `null` is the pre-redirect frame, not the destination. - // - // The shared guard is not used wholesale here because it also redirects an - // unauthenticated visitor to /auth/login, and this page deliberately answers - // that case with a "sign in to see your achievements" header instead. - if (isAuthenticated && !achievementsEnabled) { - return null; - } + // Module off: the shell answers this URL with the module notice + // (`ModulePageGate`) before the page renders, so there is no branch for it + // here. `achievementsEnabled` above only keeps the query from firing. if (!isAuthenticated) { return ( diff --git a/src/app/insights/blood-glucose/page.tsx b/src/app/insights/blood-glucose/page.tsx index d8dcdddb2..0007fef34 100644 --- a/src/app/insights/blood-glucose/page.tsx +++ b/src/app/insights/blood-glucose/page.tsx @@ -1,11 +1,10 @@ "use client"; -import { Droplet, Loader2 } from "lucide-react"; +import { Droplet } from "lucide-react"; import { HealthKitMetricPage } from "@/components/insights/healthkit-metric-page"; import { GlucoseClinicalPanel } from "@/components/insights/glucose/glucose-clinical-panel"; import { useAuth } from "@/hooks/use-auth"; -import { useModulePageGuard } from "@/hooks/use-module-page-guard"; import { useTranslations } from "@/lib/i18n/context"; import { MGDL_PER_MMOL, resolveGlucoseUnit } from "@/lib/glucose"; @@ -28,19 +27,9 @@ import { MGDL_PER_MMOL, resolveGlucoseUnit } from "@/lib/glucose"; export default function InsightsBlutzuckerPage() { const { user } = useAuth(); const { t } = useTranslations(); - const { ready } = useModulePageGuard("glucose"); const glucoseUnit = resolveGlucoseUnit(user?.glucoseUnit ?? null); const isMmol = glucoseUnit === "mmol/L"; - // v1.18.0 B1 — bounce a direct URL hit on a disabled-glucose account. - if (!ready) { - return ( -
- -
- ); - } - return ( apiPatch("/api/auth/me/modules", { nutrients: true }), - onSuccess: () => { - void queryClient.invalidateQueries({ queryKey: queryKeys.authMe() }); - }, - onError: () => toast.error(t("nutrients.page.moduleEnableError")), - }); - - if (!nutrientsEnabled) { - return ( - - {/* - The shared notice rather than this page's own copy: "nutrients is - off" was three situations wearing one sentence, and only one of them - is fixed by the button below. A delegate whose grant does not reach - the measurements section, and an account on an instance where the - operator removed the module, both used to be handed a switch that - would refuse them. The notice reads the reason off `moduleAccess` - and drops the action for the two cases it cannot help. - */} - } - action={ - - } - /> - - ); - } - if (overview.isError) { return ( - - - ); - } return ( - - - ); - } - if (isEmpty) { return ( - - - ); - } - return ( (undefined); const { workouts, @@ -76,15 +74,6 @@ export default function InsightsWorkoutsPageClient() { return label === key ? sport : label; }; - // v1.18.0 B1 — bounce a direct URL hit on a disabled-workouts account. - if (!ready) { - return ( -
- -
- ); - } - return ( mounted module gate", () => { expect(html).toContain('href="/insights"'); mockUserRef.value = { id: "u1", modules: {} }; }); + + it("keeps the Insights slot with AI analysis off", () => { + mockUserRef.value = { id: "u1", modules: { insights: false } }; + const html = render(); + expect(html).toContain('href="/insights"'); + mockUserRef.value = { id: "u1", modules: {} }; + }); }); diff --git a/src/components/layout/__tests__/module-page-gate.test.tsx b/src/components/layout/__tests__/module-page-gate.test.tsx new file mode 100644 index 000000000..37828815d --- /dev/null +++ b/src/components/layout/__tests__/module-page-gate.test.tsx @@ -0,0 +1,101 @@ +/** + * The one direct-URL gate for module pages: top-level pages and Insights + * sub-pages answer through the surface map, render the inline notice worded + * from `moduleAccess`, and never redirect. + */ +import { describe, expect, it, vi } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; + +const pathRef = { value: "/" }; +vi.mock("next/navigation", () => ({ + usePathname: () => pathRef.value, + useRouter: () => ({ push: vi.fn(), replace: vi.fn() }), +})); + +type MockUser = { + modules: Record; + moduleAccess?: Record; +}; +const userRef: { value: MockUser | null } = { value: { modules: {} } }; +vi.mock("@/hooks/use-auth", () => ({ + useAuth: () => ({ user: userRef.value, isAuthenticated: true }), +})); + +import { I18nProvider } from "@/lib/i18n/context"; +import { ModulePageGate, moduleOwningPath } from "../module-page-gate"; + +function render(pathname: string, user: MockUser) { + pathRef.value = pathname; + userRef.value = user; + return renderToStaticMarkup( + + +

page

+ + , + ); +} + +describe("moduleOwningPath", () => { + it.each([ + ["/mood", "mood"], + ["/labs/123", "labs"], + ["/insights/mood", "mood"], + ["/insights/workouts/abc", "workouts"], + ["/insights/blood-glucose", "glucose"], + ["/insights/medications", "medications"], + ["/insights/recovery", "recovery"], + ["/insights/six-minute-walk", "recovery"], + ["/insights/breathing-disturbances", "sleep"], + ])("%s belongs to %s", (path, owner) => { + expect(moduleOwningPath(path)).toBe(owner); + }); + + it.each(["/", "/measurements", "/insights", "/insights/weight", "/checkups"])( + "%s belongs to no module", + (path) => { + expect(moduleOwningPath(path)).toBeUndefined(); + }, + ); +}); + +describe("", () => { + it("renders the page while its module is on", () => { + const html = render("/insights/mood", { modules: { mood: true } }); + expect(html).toContain('data-testid="page"'); + expect(html).not.toContain("module-disabled-notice"); + }); + + it("keeps the Insights area with AI analysis off", () => { + const html = render("/insights", { modules: { insights: false } }); + expect(html).toContain('data-testid="page"'); + }); + + it("replaces an Insights sub-page with the notice, Settings link included, when the record switched it off", () => { + const html = render("/insights/mood", { + modules: { mood: false }, + moduleAccess: { mood: "disabled" }, + }); + expect(html).not.toContain('data-testid="page"'); + expect(html).toContain('data-slot="module-disabled-notice"'); + expect(html).toContain('data-module="mood"'); + expect(html).toContain('data-module-access="disabled"'); + expect(html).toContain('data-slot="module-off-open-settings"'); + }); + + it("offers no Settings link when the operator switched the module off", () => { + const html = render("/insights/sleep", { + modules: { sleep: false }, + moduleAccess: { sleep: "unavailable" }, + }); + expect(html).toContain('data-module-access="unavailable"'); + expect(html).not.toContain("module-off-open-settings"); + }); + + it("covers a sub-page no nav entry names (medications)", () => { + const html = render("/insights/medications", { + modules: { medications: false }, + }); + expect(html).toContain('data-module="medications"'); + }); +}); diff --git a/src/components/layout/__tests__/nav-model-shared-record.test.ts b/src/components/layout/__tests__/nav-model-shared-record.test.ts index f9c6005f9..92e080398 100644 --- a/src/components/layout/__tests__/nav-model-shared-record.test.ts +++ b/src/components/layout/__tests__/nav-model-shared-record.test.ts @@ -48,16 +48,17 @@ import { NAV_DESTINATIONS, isDestinationInSharedRecord, mobileMoreHubDestinations, + navDestinationModule, visibleNavDestinations, visibleUtilityDestinations, } from "../nav-model"; /** Everything enabled, so the module gate never masks the sharing gate. */ const ALL_MODULES = Object.fromEntries( - NAV_DESTINATIONS.filter((d) => d.requiresModule).map((d) => [ - d.requiresModule as string, - true, - ]), + NAV_DESTINATIONS.flatMap((d) => { + const owner = navDestinationModule(d); + return owner ? [[owner, true]] : []; + }), ); function hrefsInSharedRecord(): string[] { diff --git a/src/components/layout/__tests__/nav-model.test.ts b/src/components/layout/__tests__/nav-model.test.ts index e37cbd032..99541122c 100644 --- a/src/components/layout/__tests__/nav-model.test.ts +++ b/src/components/layout/__tests__/nav-model.test.ts @@ -102,15 +102,12 @@ describe("visibleNavDestinations module gate", () => { expect(on.indexOf("/cycle")).toBeLessThan(on.indexOf("/medications")); }); - it("drops a module-gated entry (mood / labs / coach / achievements / insights / medications) when its module is disabled", () => { + it("drops a module-gated entry (mood / labs / coach / achievements / medications) when its module is disabled", () => { const disabled = visibleNavDestinations({ mood: false, labs: false, coach: false, achievements: false, - // v1.18.0 — Insights is now `requiresModule: "insights"`, so the - // top-level /insights entry drops when the module is off. - insights: false, // v1.18.1 (D3) — medications graduated to a toggleable module. medications: false, }).map((d) => d.href); @@ -118,10 +115,16 @@ describe("visibleNavDestinations module gate", () => { expect(disabled).not.toContain("/labs"); expect(disabled).not.toContain("/coach"); expect(disabled).not.toContain("/achievements"); - expect(disabled).not.toContain("/insights"); expect(disabled).not.toContain("/medications"); }); + it("keeps Insights with AI analysis off: the insights key owns no destination", () => { + const hrefs = visibleNavDestinations({ insights: false }).map( + (d) => d.href, + ); + expect(hrefs).toContain("/insights"); + }); + it("keeps every module-gated entry when its module is enabled", () => { const enabled = visibleNavDestinations({ mood: true, diff --git a/src/components/layout/__tests__/sidebar-nav.test.tsx b/src/components/layout/__tests__/sidebar-nav.test.tsx index 176fdb67a..7effc6cb9 100644 --- a/src/components/layout/__tests__/sidebar-nav.test.tsx +++ b/src/components/layout/__tests__/sidebar-nav.test.tsx @@ -86,8 +86,9 @@ function render({ // pure `visibleNavDestinations(modules, true)` contract verified in // `nav-model.test.ts`; the mounted render is exercised in e2e. -// CORE destinations carry no `requiresModule` and always render. -const CORE_HREFS = ["/", "/measurements", "/checkups"] as const; +// CORE destinations have no owner in the surface map and always render. +// `/insights` is one of them: the `insights` key is AI analysis only. +const CORE_HREFS = ["/", "/measurements", "/checkups", "/insights"] as const; // A representative slice of the module-gated entries that must stay out of // the first-paint markup whatever the module map says. const GATED_HREFS = [ @@ -96,7 +97,6 @@ const GATED_HREFS = [ "/medications", "/labs", "/illness", - "/insights", "/coach", "/achievements", ] as const; diff --git a/src/components/layout/bottom-nav.tsx b/src/components/layout/bottom-nav.tsx index 8990eb73d..6200f8be4 100644 --- a/src/components/layout/bottom-nav.tsx +++ b/src/components/layout/bottom-nav.tsx @@ -17,7 +17,7 @@ import { isDestinationInSharedRecord, mobileMoreHubDestinations, } from "@/components/layout/nav-model"; -import type { ModuleKey } from "@/lib/modules/registry"; +import { isSurfaceVisible } from "@/lib/modules/surface"; import { useCallback, useMemo, useState } from "react"; import { useAuth } from "@/hooks/use-auth"; import { useMounted } from "@/hooks/use-mounted"; @@ -60,34 +60,19 @@ interface NavLink { // surfaces tell one story instead of two hand-curated ones that drift, // and the headline invariant is a tested model function rather than // inline bar logic. -// The Meds slot is module-gated exactly like the Insights slot below: -// `/medications` renders nothing when the medications module is off, so a -// pinned tab would be a visible control over a blank page. -const PRIMARY_LEFT: ReadonlyArray = [ +// The Meds slot follows the medications module through the surface map +// (`nav:/medications`): a pinned tab over a module that is off would be a +// visible control over a notice. +const PRIMARY_LEFT: ReadonlyArray = [ { href: "/", tKey: "nav.dashboard", icon: Home }, - { - href: "/medications", - tKey: "nav.medications", - icon: Pill, - requiresModule: "medications", - }, + { href: "/medications", tKey: "nav.medications", icon: Pill }, ]; -// v1.18.0 — the Insights primary slot is module-gated. Insights is now a -// toggleable module (`requiresModule: "insights"` in the shared nav model), -// so the fixed slot must respect the same per-user map the More hub already -// honours rather than pinning a destination the account turned off. When -// insights is disabled the slot is dropped (it is excluded from the More hub -// by `BOTTOM_NAV_PRIMARY_SLOT_HREFS`, so a disabled module is hidden -// everywhere, not relocated). Fail-open: a missing key / unloaded map keeps -// the slot, mirroring the gate's default-on contract. -const PRIMARY_RIGHT: ReadonlyArray = [ - { - href: "/insights", - tKey: "nav.insights", - icon: Lightbulb, - requiresModule: "insights", - }, +// The Insights slot belongs to no module (the `insights` key is AI analysis, +// not the area), so it stays whatever the module map says; it still drops +// inside a shared record, like any destination sharing does not cover. +const PRIMARY_RIGHT: ReadonlyArray = [ + { href: "/insights", tKey: "nav.insights", icon: Lightbulb }, ]; export function BottomNav() { @@ -115,7 +100,8 @@ export function BottomNav() { // The capture button opens the picker, so it shows exactly when the picker // would offer at least one kind — the same per-section rule, asked once. const canCapture = - visibleCaptureKinds(capabilities, CAPTURE_KIND_ORDER).length > 0; + visibleCaptureKinds(capabilities, CAPTURE_KIND_ORDER, user?.modules) + .length > 0; // v1.17.1 (F-1) — the More hub is the model-computed hub: every visible // feature destination that isn't a primary slot, plus the shared utility @@ -132,23 +118,22 @@ export function BottomNav() { [user?.modules, mounted, sharedRecord, sections], ); - // v1.18.0 — drop a module-gated primary slot (Insights) when the account - // has that module disabled. Fail-closed until mounted (so a disabled - // Insights never flickers in), then the real map applies: a missing key / - // unloaded map keeps the slot, mirroring the gate + More-hub default-on - // contract. + // Every slot but Home asks two things: does sharing cover it, and does its + // module (if the surface map names one) stay on. Fail-closed until mounted, + // so SSR and the first paint agree, then the real map applies: a missing + // key / unloaded map keeps the slot, the gate's default-on contract. const primarySlotVisible = useCallback( - (item: NavLink & { requiresModule?: ModuleKey }) => { - // An ungated slot (Home) is pinned unconditionally — the bar always - // has an anchor, whatever the module map or a record switch says. - if (!item.requiresModule) return true; + (item: NavLink) => { + // Home is pinned unconditionally — the bar always has an anchor, + // whatever the module map or a record switch says. + if (item.href === "/") return true; return ( // v1.36.0 — the fixed slot asks the shared destination model // whether sharing covers it, rather than carrying its own answer: - // Insights is an AI surface and drops out under a switch. + // Insights drops out under a switch. (!sharedRecord || isDestinationInSharedRecord(item.href, sections)) && mounted && - user?.modules?.[item.requiresModule] !== false + isSurfaceVisible(`nav:${item.href}`, user?.modules) ); }, [user?.modules, mounted, sharedRecord, sections], diff --git a/src/components/layout/module-disabled-notice.tsx b/src/components/layout/module-disabled-notice.tsx index 9cb047ee3..84e5a35aa 100644 --- a/src/components/layout/module-disabled-notice.tsx +++ b/src/components/layout/module-disabled-notice.tsx @@ -79,7 +79,11 @@ export function ModuleDisabledNotice({ const moduleName = t(MODULE_REGISTRY[moduleKey].labelKey); return ( -
+
`), so + * `/insights/mood` and `/insights/workouts/` follow their own module while + * `/insights` itself follows none. Every other page answers through the nav + * destination it sits under (`nav:`), most specific first. + */ +export function moduleOwningPath(pathname: string): ModuleKey | undefined { + if (pathname.startsWith(INSIGHTS_PREFIX)) { + const slug = pathname.slice(INSIGHTS_PREFIX.length).split("/")[0]; + const owner = surfaceModule(`insights-page:${slug}`); + if (owner !== undefined) return owner; + } + const destination = NAV_DESTINATIONS.find((d) => + isNavDestinationActive(d.href, pathname), + ); + return destination ? navDestinationModule(destination) : undefined; +} + +/** + * A page of a module that is switched off answers with an inline notice that + * names the reason, in one place for every module page: top-level pages + * (`/mood`, `/labs` …) and Insights sub-pages (`/insights/mood`, + * `/insights/sleep`, `/insights/medications` …) alike. + * + * A person who reaches such a page anyway (a bookmark, a link in a report, + * the tour) gets the notice rather than a redirect, which would drop where + * they were, and rather than the page's own error rows, which would say + * "could not be loaded" about a module that is not there. The notice reads + * the reason from `moduleAccess` and offers Settings only when the record's + * own switch is what is off. * * Paint, not enforcement: every route behind a module refuses on its own * through the server gate. Mounted inside the shell, which renders children @@ -38,19 +61,18 @@ export function ModulePageGate({ children }: { children: ReactNode }) { const { user } = useAuth(); const { t } = useTranslations(); - const destination = NAV_DESTINATIONS.find( - (d) => d.requiresModule && isNavDestinationActive(d.href, pathname), - ); - const moduleKey = destination?.requiresModule; + const moduleKey = moduleOwningPath(pathname); const off = moduleKey !== undefined && user?.modules?.[moduleKey] === false; + if (!off) return <>{children}; - if (!off || !destination || !moduleKey) return <>{children}; - - const Icon = destination.icon; + const destination = NAV_DESTINATIONS.find((d) => + isNavDestinationActive(d.href, pathname), + ); + const Icon = destination?.icon; return ( } + icon={Icon ? : undefined} action={