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 9da920c9e..5b2a43239 100644 --- a/src/components/layout/__tests__/nav-model-shared-record.test.ts +++ b/src/components/layout/__tests__/nav-model-shared-record.test.ts @@ -14,9 +14,24 @@ * Mutation check, run: flip the default in `isNavDestinationVisible` so an * unflagged destination survives a switch → "hides everything that has not * been classified" goes red naming Insights and the Coach. + * + * Mutation checks for the utility tail, run against `visibleUtilityDestinations`: + * - restore the old empty tail for every shared record → both "offers + * Settings" cases and the parity case go red (3); + * - point the switched entry at the fixed `/settings/account` → the adult + * MANAGE case and the parity case go red (2); + * - keep Notifications in the switched tail beside the remapped Settings + * entry → "never offers Notifications under a switch" goes red, with the + * two "offers Settings" cases and the parity case (4). */ import { describe, expect, it } from "vitest"; +import { SETTINGS_SECTIONS } from "@/components/settings/settings-shell"; +import { + isSettingsDestinationListedForRecord, + type SettingsRecordContext, +} from "@/lib/record-settings/classification"; + import { BOTTOM_NAV_PRIMARY_SLOT_HREFS, NAV_DESTINATIONS, @@ -116,11 +131,93 @@ describe("a module the RECORD does not track", () => { }); describe("the utility tail under a switch", () => { - it("disappears entirely", () => { - // Settings holds credentials, integrations, notification channels and - // grant management; Notifications is the delegate's own device business. - expect(visibleUtilityDestinations({ sharedRecord: true })).toEqual([]); - expect(visibleUtilityDestinations().length).toBeGreaterThan(0); + const MANAGED_AT_MANAGE: SettingsRecordContext = { + recordKind: "managed", + level: "manage", + }; + const SHARED_AT_MANAGE: SettingsRecordContext = { + recordKind: "shared", + level: "manage", + }; + + const hrefs = (record: SettingsRecordContext | null) => + visibleUtilityDestinations({ record }).map((d) => d.href); + + it("keeps both utilities in one's own record", () => { + expect(hrefs(null)).toEqual(["/settings/account", "/notifications"]); + expect(visibleUtilityDestinations().map((d) => d.href)).toEqual( + hrefs(null), + ); + }); + + /** + * #939 — a self-hoster looking after a managed profile was told to open + * Settings → Modules inside it and could not find Settings anywhere: the + * tail used to be empty for every shared record, while the Settings shell + * lists the profile's own configuration there. The entry is offered again, + * and it opens on a page the shell admits. + */ + it("offers Settings inside a managed profile, because the shell has pages for it there", () => { + const tail = visibleUtilityDestinations({ record: MANAGED_AT_MANAGE }); + expect(tail.map((d) => d.tKey)).toEqual(["nav.settings"]); + // The profile card: `account` is classified managed-guardian and is the + // first section the shell lists for a managed record. + expect(tail[0].href).toBe("/settings/account"); + }); + + it("offers Settings to an adult MANAGE share, landing on the only page it opens", () => { + // `/settings/account` is refused in an adult share; the one destination + // listed there is the record content, so the entry must not point at the + // account page the bar uses in one's own record. + expect(hrefs(SHARED_AT_MANAGE)).toEqual(["/settings/anamnesis"]); + }); + + it("offers nothing to a READ or WRITE share, where the shell lists nothing", () => { + for (const recordKind of ["shared", "managed"] as const) { + for (const level of ["read", "write"] as const) { + expect(hrefs({ recordKind, level })).toEqual([]); + } + } + }); + + it("offers nothing to a refused or pending switch, which carries no level", () => { + // `resolveRecordCapabilities` answers `recordKind: "shared", level: null` + // for both; a Settings entry there would lead into a context nobody has + // proven. + expect(hrefs({ recordKind: "shared", level: null })).toEqual([]); + }); + + it("never offers Notifications under a switch", () => { + // Notifications is the delegate's own device business; its routes refuse + // under a switch at every level and for every record kind. + for (const recordKind of ["shared", "managed"] as const) { + for (const level of ["read", "write", "manage", null] as const) { + expect(hrefs({ recordKind, level })).not.toContain("/notifications"); + } + } + }); + + it("lands on the first section the Settings shell lists, for every record kind", () => { + // The parity net. The navigation walks the slug registry, the shell walks + // its own ordered section list; both filter with the same predicate, and + // this holds the first answers equal so a reorder on either side cannot + // land somebody on a page the shell does not open with. + const contexts: SettingsRecordContext[] = [ + MANAGED_AT_MANAGE, + SHARED_AT_MANAGE, + ]; + for (const record of contexts) { + const firstListed = SETTINGS_SECTIONS.find((section) => + isSettingsDestinationListedForRecord(section.slug, record), + ); + // Non-zero: a predicate that listed nothing would make both sides + // agree on "no entry" and prove nothing about the landing. + expect(firstListed, record.recordKind).toBeDefined(); + // A module-gated landing could vanish from the shell when its module is + // off, leaving the entry pointing at a section the list no longer shows. + expect(firstListed?.moduleGate, record.recordKind).toBeUndefined(); + expect(hrefs(record)).toEqual([`/settings/${firstListed?.slug}`]); + } }); }); diff --git a/src/components/layout/__tests__/sidebar-nav.test.tsx b/src/components/layout/__tests__/sidebar-nav.test.tsx index f31a5088a..ea1c5d49c 100644 --- a/src/components/layout/__tests__/sidebar-nav.test.tsx +++ b/src/components/layout/__tests__/sidebar-nav.test.tsx @@ -1,4 +1,6 @@ import { describe, expect, it, vi } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; import { renderToStaticMarkup } from "react-dom/server"; // `` reads the active route from `usePathname()`. Stub @@ -243,3 +245,95 @@ describe(" admin entry mirrors Settings (no sub-item expansion)", () expect(html).not.toMatch(/aria-label="Admin sections"/); }); }); + +describe(" inside somebody else's record (#939)", () => { + /** + * A self-hoster looking after a managed profile could not find Settings + * anywhere after switching into it, although the Settings shell lists the + * profile's own configuration there. The footer entry is back wherever the + * shell has pages for the record, and points at the first of them; the + * identity block stays a plain block, because it names the person at the + * keyboard and must not open somebody else's settings under that name. + * + * Mutation checks, run: + * - `FooterIdentity` always rendering the link → all three switched cases + * go red: the managed case on the account-settings label, the adult + * MANAGE and READ cases on the identity link's `/settings/account` href; + * - the empty tail restored for shared records in + * `visibleUtilityDestinations` → both MANAGE cases go red on the missing + * Settings link, while the READ case stays green. + */ + const accountSettingsLabel = ( + JSON.parse( + readFileSync(join(process.cwd(), "messages/en.json"), "utf8"), + ) as { nav: { accountSettings: string } } + ).nav.accountSettings; + + function renderInRecord( + active: { + recordKind: "managed" | "shared"; + level: "read" | "write" | "manage"; + } | null, + ) { + const user = mockUserRef.value as typeof mockUserRef.value & { + accountAccess?: unknown; + }; + user.accountAccess = active + ? { + accounts: [], + canSwitch: true, + active: { + accountId: "record-1", + username: "record", + displayName: null, + fullName: null, + recordKind: active.recordKind, + level: active.level, + accessLevel: active.level === "read" ? "read" : "write", + canWrite: active.level !== "read", + sections: null, + writableDomains: [], + manageableDomains: + active.level === "manage" ? ["measurements"] : [], + }, + } + : undefined; + try { + return render(); + } finally { + delete user.accountAccess; + } + } + + const settingsLinks = (html: string) => + [...html.matchAll(/]*data-slot="nav-settings-link"[^>]*>/g)].map( + (match) => /href="([^"]+)"/.exec(match[0])?.[1], + ); + + it("keeps the account settings door in one's own record", () => { + const html = renderInRecord(null); + expect(settingsLinks(html)).toEqual(["/settings/account"]); + expect(html).toContain(`aria-label="${accountSettingsLabel}"`); + }); + + it("offers Settings inside a managed profile and keeps the identity block a plain block", () => { + const html = renderInRecord({ recordKind: "managed", level: "manage" }); + expect(settingsLinks(html)).toEqual(["/settings/account"]); + expect(html).not.toContain(`aria-label="${accountSettingsLabel}"`); + expect(html).not.toContain('href="/notifications"'); + }); + + it("lands an adult MANAGE share on the one Settings page it opens", () => { + const html = renderInRecord({ recordKind: "shared", level: "manage" }); + expect(settingsLinks(html)).toEqual(["/settings/anamnesis"]); + expect(html).not.toContain('href="/settings/account"'); + }); + + it("offers no Settings entry to a READ share", () => { + const html = renderInRecord({ recordKind: "shared", level: "read" }); + // Non-zero proof that the sidebar rendered at all. + expect(html).toContain('href="/measurements"'); + expect(settingsLinks(html)).toEqual([]); + expect(html).not.toContain('href="/settings/'); + }); +}); diff --git a/src/components/layout/nav-model.ts b/src/components/layout/nav-model.ts index 47f9c9fbd..9b1e2934b 100644 --- a/src/components/layout/nav-model.ts +++ b/src/components/layout/nav-model.ts @@ -20,6 +20,12 @@ import { } from "lucide-react"; import type { ModuleKey } from "@/lib/modules/registry"; +// The leaf module, not the `@/lib/record-settings` barrel, for the reason +// `auth-shell.tsx` gives: the barrel drags server-side schemas into the chrome. +import { + recordSettingsLandingDestination, + type SettingsRecordContext, +} from "@/lib/record-settings/classification"; import { isSharedRecordPathPresentable } from "@/lib/navigation/shared-record"; import type { ShareDomain } from "@/lib/sharing/scope"; @@ -277,19 +283,18 @@ export const NAV_DESTINATIONS: ReadonlyArray = [ /** * v1.17.1 (F-1 residue) — the shared UTILITY tail. * - * Settings and Notifications are reachable on both bars but are not feature - * destinations: on desktop they live in the sidebar footer + avatar menu, on - * mobile at the tail of the "More" hub. They used to be a second hand-curated - * list on each bar — the exact drift the one-model contract above set out to - * kill, just pushed down a level. This list is the single source both bars - * consume for the tail, so the two surfaces can no longer disagree on which - * utility links exist or in which order. + * Settings and Notifications are account utilities, not feature destinations: + * on desktop they live in the sidebar footer + avatar menu, on mobile in the + * top-bar user menu (UI-STANDARDS §10; the More hub carries features only). + * They used to be a second hand-curated list on each bar — the exact drift the + * one-model contract above set out to kill, just pushed down a level. This + * list is the single source both bars consume, so the two surfaces can no + * longer disagree on which utility links exist or in which order. * - * Order is the footer/hub order: Settings → Notifications. + * Order is the footer/menu order: Settings → Notifications. * - * Admin is intentionally NOT here: it is a role-gated, desktop-sidebar-only - * surface (the mobile bar never exposes it), so it is not a shared tail - * destination and stays local to the sidebar. + * Admin is intentionally NOT here: it is a role-gated surface that never + * appears under a switch, so it stays local to each bar. */ export interface NavUtilityDestination { href: string; @@ -303,21 +308,41 @@ export const NAV_UTILITY_DESTINATIONS: ReadonlyArray = [ { href: "/notifications", tKey: "nav.notifications", icon: Bell }, ]; +/** Is this utility entry the Settings door, wherever it lands. */ +export function isSettingsUtilityDestination( + d: Pick, +): boolean { + return d.href.startsWith("/settings/"); +} + /** - * The utility tail visible to this account. Both bars consume this for their - * tail (the sidebar footer + avatar menu, the bottom-nav More hub) so the two - * surfaces share one definition of the utility links. + * The utility tail visible to this session. Both bars consume it, so the + * sidebar footer and the mobile user menu share one definition. + * + * `record` is the shared record the browser is acting on, as its + * server-resolved kind and grant level; absent or null is one's own record. + * + * Under a switch, Notifications is never offered: it is the delegate's own + * device business, and its routes refuse. Settings is offered exactly when + * the Settings shell would list at least one section for that record, and + * it lands on the first section the shell lists — a managed profile at MANAGE + * opens on its profile card, an adult share at MANAGE on the anamnesis page. + * Both answers come from `recordSettingsLandingDestination`, which reads the + * predicate the shell filters its own list with, so the entry cannot lead to + * a page the shell refuses and cannot be missing where the shell has pages. + * Every other shared context — READ, WRITE, a refused or pending switch — + * gets no utility at all. */ export function visibleUtilityDestinations( - opts: { sharedRecord?: boolean } = {}, + opts: { record?: SettingsRecordContext | null } = {}, ): NavUtilityDestination[] { - // v1.36.0 — the utility tail is account configuration by definition: - // Settings holds credentials, integrations, notification channels and grant - // management, and Notifications is the delegate's own device business. Every - // route behind both refuses under a switch; the tail simply stops being - // offered so nobody walks into the refusal. - if (opts.sharedRecord) return []; - return [...NAV_UTILITY_DESTINATIONS]; + const record = opts.record ?? null; + if (record === null) return [...NAV_UTILITY_DESTINATIONS]; + const landing = recordSettingsLandingDestination(record); + if (landing === null) return []; + return NAV_UTILITY_DESTINATIONS.filter(isSettingsUtilityDestination).map( + (d) => ({ ...d, href: `/settings/${landing}` }), + ); } /** diff --git a/src/components/layout/sidebar-nav.tsx b/src/components/layout/sidebar-nav.tsx index 3d8588db9..90edb5afd 100644 --- a/src/components/layout/sidebar-nav.tsx +++ b/src/components/layout/sidebar-nav.tsx @@ -20,6 +20,7 @@ import { AccountSwitcherMenuItems } from "@/components/layout/account-switcher-m import { medicationsPrefetchIntentProps } from "@/lib/queries/prefetch-medications"; import { isNavDestinationActive, + isSettingsUtilityDestination, visibleNavDestinations, visibleUtilityDestinations, } from "@/components/layout/nav-model"; @@ -64,10 +65,12 @@ function getInitials(name: string): string { * The sidebar footer's name-and-avatar block. * * A link into the account settings normally, and a plain block while this - * browser is acting on somebody else's record — the settings routes refuse - * under a switch, so the link would lead only to an explanation of why it did - * not work. It keeps naming the person at the keyboard either way, which is - * what a switched session needs it to do. + * browser is acting on somebody else's record. The block names the person at + * the keyboard, and `/settings/account` inside a record is either refused (an + * adult share) or the RECORD's profile card (a managed profile) — a door on + * one's own name into somebody else's settings would mislabel both. A record + * that has Settings pages reaches them through the footer Settings entry, + * which is labelled as Settings and lands where the shell opens. */ function FooterIdentity({ sharedRecord, @@ -307,7 +310,12 @@ export function SidebarNav() { // v1.36.0 — while this browser is acting on somebody else's record, the nav // shows only what sharing covers. The server refuses the rest regardless; // dropping the entries spares a delegate a click that ends in a 403. - const { inSharedRecord: sharedRecord, sections } = useRecordCapabilities(); + const { + inSharedRecord: sharedRecord, + sections, + recordKind, + level, + } = useRecordCapabilities(); const visibleNavItems = useMemo( () => visibleNavDestinations(user?.modules, mounted, sharedRecord, sections), @@ -347,22 +355,26 @@ export function SidebarNav() { } // v1.17.1 (F-1 residue) — the sidebar footer utility links derive from - // the SAME shared list the mobile More-hub tail consumes, so the two - // surfaces can no longer drift into two hand-curated utility lists. - // Notifications is surfaced in the avatar menu (not the footer), so the - // footer takes every utility entry except `/notifications`; Admin is the - // role-gated, sidebar-only surface and is inserted separately below. + // the SAME shared list the mobile user menu consumes, so the two surfaces + // can no longer drift into two hand-curated utility lists. Notifications is + // surfaced in the avatar menu (not the footer), so the footer takes every + // utility entry except `/notifications`; Admin is the role-gated, + // sidebar-only surface and is inserted separately below. + // + // Under a switch the list answers for the record on screen: a Settings + // entry exactly when the Settings shell lists a section for it, pointing at + // the first one (see `visibleUtilityDestinations`). const footerUtilityItems = useMemo( () => - visibleUtilityDestinations({ sharedRecord }).filter( - (d) => d.href !== "/notifications", - ), - [sharedRecord], + visibleUtilityDestinations({ + record: sharedRecord ? { recordKind, level } : null, + }).filter((d) => d.href !== "/notifications"), + [sharedRecord, recordKind, level], ); function isUtilityActive(href: string) { // Settings matches the whole `/settings/*` shell; the rest match exact. - return href === "/settings/account" + return isSettingsUtilityDestination({ href }) ? pathname.startsWith("/settings") : pathname === href; } @@ -374,8 +386,10 @@ export function SidebarNav() { }) { const Icon = item.icon; const isActive = isUtilityActive(item.href); - const tourId = - item.href === "/settings/account" ? "nav-settings" : undefined; + const isSettings = isSettingsUtilityDestination(item); + const tourId = isSettingsUtilityDestination(item) + ? "nav-settings" + : undefined; if (collapsed) { return ( @@ -384,6 +398,7 @@ export function SidebarNav() { href={item.href} aria-current={isActive ? "page" : undefined} data-tour-id={tourId} + data-slot={isSettings ? "nav-settings-link" : undefined} className={cn( "flex items-center justify-center rounded-lg p-2.5 transition-colors", isActive @@ -406,6 +421,7 @@ export function SidebarNav() { href={item.href} aria-current={isActive ? "page" : undefined} data-tour-id={tourId} + data-slot={isSettings ? "nav-settings-link" : undefined} className={cn( "flex items-center gap-3 rounded-lg px-3 py-2.5 text-sm font-medium transition-colors", isActive @@ -607,11 +623,11 @@ export function SidebarNav() { role-gated Admin entry inserted before Settings. */}
{footerUtilityItems - .filter((item) => item.href !== "/settings/account") + .filter((item) => !isSettingsUtilityDestination(item)) .map((item) => renderUtilityLink(item))} {renderAdminLink()} {footerUtilityItems - .filter((item) => item.href === "/settings/account") + .filter((item) => isSettingsUtilityDestination(item)) .map((item) => renderUtilityLink(item))}
diff --git a/src/components/layout/top-bar.tsx b/src/components/layout/top-bar.tsx index 2a64baa23..fccdb8be1 100644 --- a/src/components/layout/top-bar.tsx +++ b/src/components/layout/top-bar.tsx @@ -1,17 +1,19 @@ "use client"; import { - Bell, ChevronDown, LogIn, LogOut, Monitor, Moon, - Settings, Shield, Sun, } from "lucide-react"; import { AccountSwitcherMenuItems } from "@/components/layout/account-switcher-menu"; +import { + isSettingsUtilityDestination, + visibleUtilityDestinations, +} from "@/components/layout/nav-model"; import { SHELL_HEADER_BAND } from "@/components/layout/shell-metrics"; import { cn } from "@/lib/utils"; import { Logo } from "@/components/ui/logo"; @@ -19,6 +21,7 @@ import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; import { Skeleton } from "@/components/ui/skeleton"; import Link from "next/link"; import { useAuth, useLogout } from "@/hooks/use-auth"; +import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; import { useTheme } from "@/components/providers"; import { useTranslations } from "@/lib/i18n/context"; import { @@ -42,8 +45,17 @@ function getInitials(name: string): string { export function TopBar() { const { user, isLoading } = useAuth(); - // v1.36.0 — acting on somebody else's record; see the menu below. - const sharedRecord = user?.accountAccess?.active != null; + // v1.36.0 — acting on somebody else's record; see the menu below. Read from + // the capability hook, as the sidebar does, so a refused or pending switch + // counts as switched here too and both bars offer the same utilities. + const { + inSharedRecord: sharedRecord, + recordKind, + level, + } = useRecordCapabilities(); + const utilityItems = visibleUtilityDestinations({ + record: sharedRecord ? { recordKind, level } : null, + }); const logout = useLogout(); const { theme, setTheme } = useTheme(); const { t } = useTranslations(); @@ -115,34 +127,38 @@ export function TopBar() { - {/* v1.36.0 — the account utilities drop out while this browser is - acting on somebody else's record. Every route behind them - refuses under a switch, and a control that leads only to an - explanation of why it does not work is worse than no control. - The switcher below is how the person gets back. */} - {!sharedRecord && ( - <> - - - - {t("nav.settings")} + {/* The account utilities come from the list the desktop sidebar + reads. Inside somebody else's record Notifications and Admin + drop out — every route behind them refuses under a switch — + and Settings stays only where the record has Settings pages, + pointing at the first one the shell opens. The switcher + below is how the person gets back. */} + {utilityItems.map((item) => { + const Icon = item.icon; + return ( + + + + {t(item.tKey)} - - - - {t("nav.notifications")} - - - {isAdmin && ( - - - - {t("nav.admin")} - - - )} - + ); + })} + {!sharedRecord && isAdmin && ( + + + + {t("nav.admin")} + + )} {/* v1.4.36 W4e — About moved into the Admin Console (`/admin/about`). The dropdown entry was redundant