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={