From 9dcef3b0e297972e6e8b871f77f3872e60cec23f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 13 Sep 2026 14:03:16 +0200 Subject: [PATCH] fix(nav): offer Settings inside a record that has Settings pages A self-hoster looking after a managed profile could not find Settings after switching into it: the utility entries vanished for every shared record, while the Settings shell lists the profile's own configuration and the section gate admits it. The utility tail now answers for the record on screen. Settings is offered when the shell lists at least one section for it and links to the first one: the profile card for a managed profile, the anamnesis page for an adult share at manage. Read and write shares, and a refused or pending switch, get no utility. Notifications stays hidden under every switch. The desktop sidebar footer and the mobile user menu read the same list; the footer identity block stays a plain block. Refs #939 --- .../__tests__/nav-model-shared-record.test.ts | 107 +++++++++++++++++- .../layout/__tests__/sidebar-nav.test.tsx | 94 +++++++++++++++ src/components/layout/nav-model.ts | 69 +++++++---- src/components/layout/sidebar-nav.tsx | 54 +++++---- src/components/layout/top-bar.tsx | 76 ++++++++----- 5 files changed, 324 insertions(+), 76 deletions(-) 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