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
This commit is contained in:
Marc-André Bombeck committed 2026-09-13 14:03:16 +02:00
1 parent f5fc713783
commit 9dcef3b0e2
5 files changed
+324 -76

No files matched your search

@@ -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}`]);
}
});
});
@@ -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";
// `<SidebarNav>` reads the active route from `usePathname()`. Stub
@@ -243,3 +245,95 @@ describe("<SidebarNav> admin entry mirrors Settings (no sub-item expansion)", ()
expect(html).not.toMatch(/aria-label="Admin sections"/);
});
});
describe("<SidebarNav> 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(/<a\b[^>]*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/');
});
});
+47 -22
View File
@@ -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<NavDestination> = [
/**
* 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<NavUtilityDestination> = [
{ href: "/notifications", tKey: "nav.notifications", icon: Bell },
];
/** Is this utility entry the Settings door, wherever it lands. */
export function isSettingsUtilityDestination(
d: Pick<NavUtilityDestination, "href">,
): 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}` }),
);
}
/**
+35 -19
View File
@@ -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 (
<Tooltip key={item.href}>
@@ -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. */}
<div className={cn("space-y-1 pb-1", collapsed ? "px-1.5" : "px-3")}>
{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))}
</div>
+46 -30
View File
@@ -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() {
<ChevronDown className="h-3 w-3 opacity-60" />
</DropdownMenuTrigger>
<DropdownMenuContent align="end" className="w-60">
{/* 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 && (
<>
<DropdownMenuItem asChild>
<Link href="/settings/account" className="cursor-pointer">
<Settings className="mr-2 h-4 w-4" />
{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 (
<DropdownMenuItem key={item.href} asChild>
<Link
href={item.href}
className="cursor-pointer"
data-slot={
isSettingsUtilityDestination(item)
? "nav-settings-link"
: undefined
}
>
<Icon className="mr-2 h-4 w-4" />
{t(item.tKey)}
</Link>
</DropdownMenuItem>
<DropdownMenuItem asChild>
<Link href="/notifications" className="cursor-pointer">
<Bell className="mr-2 h-4 w-4" />
{t("nav.notifications")}
</Link>
</DropdownMenuItem>
{isAdmin && (
<DropdownMenuItem asChild>
<Link href="/admin" className="cursor-pointer">
<Shield className="mr-2 h-4 w-4" />
{t("nav.admin")}
</Link>
</DropdownMenuItem>
)}
</>
);
})}
{!sharedRecord && isAdmin && (
<DropdownMenuItem asChild>
<Link href="/admin" className="cursor-pointer">
<Shield className="mr-2 h-4 w-4" />
{t("nav.admin")}
</Link>
</DropdownMenuItem>
)}
{/* v1.4.36 W4e — About moved into the Admin Console
(`/admin/about`). The dropdown entry was redundant