From 45363edb7779273da1a1b0a197646c31589473a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 3 Aug 2026 18:33:48 +0200 Subject: [PATCH] Withhold the controls a shared record refuses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nine surfaces offered a delegate an action the server declines, and the switch had never been asked about any of them. The Today rail's dismiss and the coach check-in's keep / let-go write through routes that resolve the caller, so both refuse under a switch; they now ask canManage and are absent at either grant level. A priority action with no href and no handler is no longer rendered as an inert button at all — the type carries that rule now. The Vorsorge dashboard card offered the mark-done that /checkups already withholds, and the checkups list branch inlined its own ungated copy of the button the cards branch gates, so which view a browser last chose decided whether the action appeared. Both bind canManage. The chart overlay cog persists through a route that resolves the caller and stores a preference belonging to the person rather than the record. Gated once in the control so the three chart wrappers cannot drift. Three query parameters opened sheets past a gated button: ?add= on measurements, ?new=1 on medications, and ?edit=1 on a medication detail, the last of which opens a wizard refused at both levels. A deep link is the same affordance as the control that produces it. The efficacy retarget dial rewrites a setting the owner chose. The episode documents card offered a link and an upload the vault gates on the same endpoint throughout. And the dose ledger built its own success toast, missing both the Undo suppression and the "saved to" receipt its two siblings already carry — the one a delegate met on every dose. The Coach drawer is not mounted inside a shared record, so every button calling askCoach() opened nothing. The launch provider now publishes no value there and each entry point's existing null check does the rest; the documents sheet, the one that rendered without checking, now checks. --- src/app/measurements/page.tsx | 10 ++- src/app/medications/page-client.tsx | 5 +- .../__tests__/chart-overlay-controls.test.tsx | 69 ++++++++++++++- .../charts/chart-overlay-controls.tsx | 13 ++- .../daily/__tests__/priority-card.test.tsx | 26 ++++++ src/components/daily/priority-card.tsx | 74 +++++++++++------ src/components/daily/today-hero.tsx | 17 +++- .../documents/document-detail-sheet.tsx | 9 +- .../documents/episode-documents-card.tsx | 55 +++++++----- .../vorsorge-dashboard-card.tsx | 55 +++++++----- .../vorsorge-section.tsx | 37 +++++---- .../detail/efficacy/efficacy-tab.tsx | 8 +- .../detail/medication-detail-tabs.tsx | 6 +- .../medications/dose-history-ledger.tsx | 51 ++++++++---- .../__tests__/coach-launch-context.test.tsx | 83 +++++++++++++++++-- src/lib/insights/coach-launch-context.tsx | 60 +++++++++++--- 16 files changed, 452 insertions(+), 126 deletions(-) diff --git a/src/app/measurements/page.tsx b/src/app/measurements/page.tsx index f6517480a..821d564c6 100644 --- a/src/app/measurements/page.tsx +++ b/src/app/measurements/page.tsx @@ -170,8 +170,16 @@ export default function MeasurementsPage() { } /> + {/* v1.36.x — the sheet answers the same question the header button + answers. `?add=` opens it without passing the button, and a + deep link is the same affordance as the control that produces it, so + it gets the same gate. Gating the open rather than only the param + also covers the first-paint window: `canAdd` reads true until + `/api/auth/me` settles, and a sheet opened in that frame withdraws + when the answer lands instead of standing on a form the server + refuses. */} { setDialogOpen(open); if (!open) { diff --git a/src/app/medications/page-client.tsx b/src/app/medications/page-client.tsx index 544d2a86c..5b4f4dda1 100644 --- a/src/app/medications/page-client.tsx +++ b/src/app/medications/page-client.tsx @@ -617,8 +617,11 @@ export default function MedicationsPageClient() { "Vollständig bearbeiten" reopens the wizard in edit mode from there); the list page wizard only ever creates. The wizard owns its own ResponsiveSheet shell with the sticky footer. */} + {/* v1.36.x — `?new=1` (the retired `/medications/new` route redirects + here) opens the create wizard without passing the gated Add control, + so the wizard asks the same `canAdd` the control asks. */} ({ + useAuth: () => ({ + user: { + id: "delegate", + username: "delegate", + email: null, + role: "USER", + avatarUrl: null, + modules: {}, + accountAccess: mockAccessRef.value, + }, + isAuthenticated: true, + isLoading: false, + refetch: vi.fn(), + }), +})); + +const { ChartOverlayControls, ChartOverlayControlsBody, DEFAULT_CHART_OVERLAY_PREFS, - type ChartOverlayPrefs, -} from "../chart-overlay-controls"; +} = await import("../chart-overlay-controls"); +type ChartOverlayPrefs = import("../chart-overlay-controls").ChartOverlayPrefs; /** * v1.4.18 — per-chart overlay-controls popover. @@ -48,6 +83,34 @@ describe("", () => { expect(html).toContain('aria-label="Chart overlay settings"'); }); + it("is absent inside somebody else's record, at both levels", () => { + // v1.36.x — every toggle persists through + // `PUT /api/dashboard/chart-overlay-prefs`, which resolves the caller and + // refuses under a switch, and the preference belongs to the person rather + // than to the record. Absent, not disabled: a greyed cog would still + // claim the chart is the delegate's to configure. + // + // Mutation check, run: dropping the `if (!canManage) return null` bail + // from `` → both legs go red on the trigger slot. + for (const access of ["read", "write"] as const) { + mockAccessRef.value = { + accounts: [OWNER], + active: { ...OWNER, access, canWrite: access === "write" }, + canSwitch: true, + }; + const html = renderToStaticMarkup( + withProvider( + {}} + />, + ), + ); + expect(html, `grant level: ${access}`).toBe(""); + } + mockAccessRef.value = OWN_RECORD; + }); + it("default prefs match the clean-line baseline (every toggle OFF)", () => { expect(DEFAULT_CHART_OVERLAY_PREFS).toEqual({ showTrendIndicator: false, diff --git a/src/components/charts/chart-overlay-controls.tsx b/src/components/charts/chart-overlay-controls.tsx index 93e8c19d4..d6ba6429d 100644 --- a/src/components/charts/chart-overlay-controls.tsx +++ b/src/components/charts/chart-overlay-controls.tsx @@ -13,6 +13,7 @@ import { import { Switch } from "@/components/ui/switch"; import { Label } from "@/components/ui/label"; import { Button } from "@/components/ui/button"; +import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; import { useTranslations } from "@/lib/i18n/context"; import { COMPARISON_BASELINES, @@ -89,8 +90,18 @@ export function ChartOverlayControls({ onChange, triggerClassName, hasComparisonData = true, -}: ChartOverlayControlsProps): ReactElement { +}: ChartOverlayControlsProps): ReactElement | null { const { t } = useTranslations(); + // The cog is gated here rather than at each of the eight charts that mount + // it, so the three chart wrappers cannot drift apart on the same question. + // Every toggle persists through `PUT /api/dashboard/chart-overlay-prefs`, + // which resolves the caller and refuses under a switch; and the preference + // belongs to the person reading the chart, not to the record — the same + // reasoning that keeps the mood tag layout off the delegable list. So a + // delegate reads the owner's chart in its stored shape and is not offered a + // dial that would silently fail. + const { canManage } = useRecordCapabilities(); + if (!canManage) return null; return ( diff --git a/src/components/daily/__tests__/priority-card.test.tsx b/src/components/daily/__tests__/priority-card.test.tsx index d588ae69f..c3a6a5df0 100644 --- a/src/components/daily/__tests__/priority-card.test.tsx +++ b/src/components/daily/__tests__/priority-card.test.tsx @@ -111,12 +111,38 @@ describe("", () => { kind: "coach_checkin", actions: [{ labelKey: "daily.action.logDose", intent: "dose.log" }], })} + onAction={() => {}} />, ); expect(html).toContain(" { + // v1.36.x — the caller withholds `onAction` inside somebody else's + // record. The mutating half of the rail must then be absent, not an + // inert button: `TodayHero` passes no handler at either grant level, and + // a rendered control that calls nothing is worse than one that 403s. + const html = render( + , + ); + expect(html).not.toContain("Log dose"); + // Navigation is untouched — reading is never what a delegation withholds. + expect(html).toContain('href="/checkups"'); + }); + it("resolves action label keys through the active locale", () => { const html = render( void; /** @@ -129,7 +134,13 @@ export function PriorityCard({ }: PriorityCardProps) { const { t } = useTranslations(); const Icon = KIND_ICON[item.kind]; - const actions = item.actions.slice(0, 3); + // Navigation always survives; a mutating action survives only when somebody + // is there to run it. That is what removes the whole non-navigation half of + // this card inside a record the caller may not change, without this file + // learning anything about grants. + const actions = item.actions + .filter((action) => action.href || onAction) + .slice(0, 3); const hue = KIND_HUE[item.kind]; // S12 — the quiet "reached" moment: the milestone card swaps the generic // fade-in for the `.milestone-reached` treatment (a soft `--success` halo @@ -187,15 +198,23 @@ export function PriorityCard({ ) : null} {actions.length > 0 ? (
- {actions.map((action) => ( - - ))} + {actions.map((action) => + action.href ? ( + + ) : onAction ? ( + + ) : null, + )}
) : null} @@ -203,6 +222,26 @@ export function PriorityCard({ ); } +/** A navigation action. Always offered — reading is never delegated away. */ +function ActionLink({ href, label }: { href: string; label: string }) { + return ( + + {label} + + ); +} + +/** + * A mutating action. `onAction` is required rather than optional so the type + * carries the rule: this button cannot be rendered without the handler that + * makes it do something. + */ function ActionButton({ action, label, @@ -211,22 +250,9 @@ function ActionButton({ }: { action: PriorityItemAction; label: string; - onAction?: (intent: string) => void; + onAction: (intent: string) => void; pending?: boolean; }) { - if (action.href) { - return ( - - {label} - - ); - } return ( - + {canManage ? ( + <> + + {/* The upload affordance is a deep link into the vault with + this episode pre-filtered. The vault's own upload control + is `canManage`-gated, so an ungated link here would send a + delegate to a page with nothing to press. */} + + + ) : null} {hasMore ? ( + {canManage ? ( + + ) : null} ); diff --git a/src/components/measurement-reminders/vorsorge-section.tsx b/src/components/measurement-reminders/vorsorge-section.tsx index 3f0fa6413..c5412c6da 100644 --- a/src/components/measurement-reminders/vorsorge-section.tsx +++ b/src/components/measurement-reminders/vorsorge-section.tsx @@ -983,21 +983,28 @@ function VorsorgeCard({
- + {/* The same gate the cards branch applies through + `primaryButton`. The list branch used to inline its own + ungated copy, and which branch a person sees is a per-browser + preference that survives the switch — so the identical action + was offered or withheld depending on a view toggle. */} + {canManage ? ( + + ) : null} {headerActions}
diff --git a/src/components/medications/detail/efficacy/efficacy-tab.tsx b/src/components/medications/detail/efficacy/efficacy-tab.tsx index 8d0ac882b..bba726adc 100644 --- a/src/components/medications/detail/efficacy/efficacy-tab.tsx +++ b/src/components/medications/detail/efficacy/efficacy-tab.tsx @@ -36,6 +36,7 @@ import { queryKeys } from "@/lib/query-keys"; import { apiGet, apiPut } from "@/lib/api/api-fetch"; import { useTranslations, useFormatters } from "@/lib/i18n/context"; import { useAuth } from "@/hooks/use-auth"; +import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; import { DEFAULT_TIMEZONE } from "@/lib/tz/format"; import type { MedicationEfficacyDTO, @@ -388,6 +389,11 @@ function RetargetControl({ onChanged: () => void; }) { const { t } = useTranslations(); + // Repointing what a medication is judged against rewrites a setting the + // owner chose — not an admitted create, and `PUT .../efficacy/target` + // resolves the caller, so it is refused at both grant levels. The Wirkung + // tab itself stays readable; only the dial goes. + const { canManage } = useRecordCapabilities(); const [value, setValue] = useState(""); const [busy, setBusy] = useState(false); @@ -437,7 +443,7 @@ function RetargetControl({ } }; - if (items.length === 0) return null; + if (!canManage || items.length === 0) return null; return (
- void runUndoIntake({ - medication: { id: medicationId, name: medicationName }, - eventId, - t, - queryClient, - }), - }, + description: t("recordSharing.toast.savedTo", { + name: recordName, + }), } - : undefined, + : eventId + ? { + action: { + label: t("medications.intakeUndo"), + onClick: () => + void runUndoIntake({ + medication: { id: medicationId, name: medicationName }, + eventId, + t, + queryClient, + }), + }, + } + : undefined, ); await invalidateKeys(queryClient, [ ...medicationDependentKeys, @@ -314,7 +329,15 @@ export function DoseHistoryLedger({ setMarking(null); } }, - [marking, medicationId, medicationName, queryClient, queryKey, t], + [ + marking, + medicationId, + medicationName, + queryClient, + queryKey, + recordName, + t, + ], ); /** diff --git a/src/lib/insights/__tests__/coach-launch-context.test.tsx b/src/lib/insights/__tests__/coach-launch-context.test.tsx index 0fbe78781..a4054c979 100644 --- a/src/lib/insights/__tests__/coach-launch-context.test.tsx +++ b/src/lib/insights/__tests__/coach-launch-context.test.tsx @@ -1,11 +1,7 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; -import { - CoachLaunchProvider, - resolveLaunchState, - useCoachLaunch, -} from "../coach-launch-context"; +import type { AccountAccess } from "@/lib/sharing/account-access-view"; /** * v1.4.27 R3d MB4 — Coach launch context smoke contract. @@ -19,8 +15,48 @@ import { * 3. The hook returns `null` when called outside the provider — * consumers degrade gracefully (e.g. the launch button renders * nothing rather than crashing). + * 4. v1.36.x — the provider publishes nothing inside somebody else's + * record, so (3) also covers every consumer under a switch. The shell + * mounts no drawer there, and a launch call that opened nothing was a + * button that silently did nothing on every page that offers one. + * + * Mutation check, run: dropping the `inSharedRecord` arm from the provider's + * value memo → "publishes nothing inside somebody else's record" goes red + * with the full context shape in the diff. */ +const OWNER = { + accountId: "acct-owner", + username: "owner", + displayName: "Margarethe", + access: "write" as const, + canWrite: true, +}; + +const mockAccessRef: { value: AccountAccess } = { + value: { accounts: [OWNER], active: null, canSwitch: true }, +}; + +vi.mock("@/hooks/use-auth", () => ({ + useAuth: () => ({ + user: { + id: "delegate", + username: "delegate", + email: null, + role: "USER", + avatarUrl: null, + modules: {}, + accountAccess: mockAccessRef.value, + }, + isAuthenticated: true, + isLoading: false, + refetch: vi.fn(), + }), +})); + +const { CoachLaunchProvider, resolveLaunchState, useCoachLaunch } = + await import("../coach-launch-context"); + function Probe({ output }: { output: string[] }) { const launch = useCoachLaunch(); if (!launch) { @@ -72,6 +108,41 @@ describe("CoachLaunchProvider", () => { expect(html).toContain('data-slot="child-mount"'); expect(html).toContain("child"); }); + + it("publishes nothing inside somebody else's record, at both levels", () => { + for (const access of ["read", "write"] as const) { + mockAccessRef.value = { + accounts: [OWNER], + active: { ...OWNER, access, canWrite: access === "write" }, + canSwitch: true, + }; + const output: string[] = []; + renderToStaticMarkup( + + + , + ); + // The whole recorded value, not a boolean: a failure prints the shape + // that leaked rather than `false !== true`. + expect(output, `grant level: ${access}`).toEqual(["null"]); + } + mockAccessRef.value = { accounts: [OWNER], active: null, canSwitch: true }; + }); + + it("still renders its children there — only the launch value is withheld", () => { + mockAccessRef.value = { + accounts: [OWNER], + active: OWNER, + canSwitch: true, + }; + const html = renderToStaticMarkup( + +
child
+ , + ); + mockAccessRef.value = { accounts: [OWNER], active: null, canSwitch: true }; + expect(html).toContain('data-slot="child-mount"'); + }); }); /** diff --git a/src/lib/insights/coach-launch-context.tsx b/src/lib/insights/coach-launch-context.tsx index c8f1cbd12..fc5689729 100644 --- a/src/lib/insights/coach-launch-context.tsx +++ b/src/lib/insights/coach-launch-context.tsx @@ -10,6 +10,7 @@ import { type ReactNode, } from "react"; +import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; import type { CoachScopeSource, CoachScopeWindow } from "@/lib/ai/coach/types"; /** @@ -191,7 +192,30 @@ export interface CoachLaunchProviderProps { children: ReactNode; } +/** + * Owns the drawer's launch state — and publishes it only where a drawer + * exists to receive it. + * + * v1.36.x — the shell stopped mounting `` inside somebody + * else's record, and this provider kept answering: `askCoach()` set an open + * flag that nothing was reading, so every per-page Coach entry point became a + * button that did nothing at all. A control that errors tells a person where + * they stand; one that silently does nothing tells them the product is + * broken. + * + * The gate lives here, on the publisher, rather than in `useCoachLaunch()`. + * Six components read this context and five already treat `null` as "no Coach + * here", so withholding the value fixes all of them at once and a seventh + * inherits the rule. Putting it in the hook instead would drag the account + * query into every one of those components for an answer that is the same in + * all of them. + * + * Nothing is taken away by this: `/insights` and `/coach` are not + * shared-record destinations, so the Coach is outside what sharing covers to + * begin with. + */ export function CoachLaunchProvider({ children }: CoachLaunchProviderProps) { + const { inSharedRecord } = useRecordCapabilities(); const [open, setOpen] = useState(false); const [closeIntent, setCloseIntent] = useState(null); const [prefill, setPrefill] = useState(null); @@ -277,20 +301,24 @@ export function CoachLaunchProvider({ children }: CoachLaunchProviderProps) { [], ); - const value = useMemo( - () => ({ - open, - closeIntent, - prefill, - autoSend, - scope, - documentId, - workoutId, - askCoach, - registerScope, - setOpen: handleSetOpen, - }), + const value = useMemo( + () => + inSharedRecord + ? null + : { + open, + closeIntent, + prefill, + autoSend, + scope, + documentId, + workoutId, + askCoach, + registerScope, + setOpen: handleSetOpen, + }, [ + inSharedRecord, closeIntent, open, prefill, @@ -316,6 +344,12 @@ export function CoachLaunchProvider({ children }: CoachLaunchProviderProps) { * `` so consumer components can degrade gracefully * (e.g. the hero strip's "Ask the coach" action stays disabled until * the provider mounts). + * + * v1.36.x — also `null` inside somebody else's record, and by the same + * mechanism: the provider publishes nothing there (see its own docblock), so + * every consumer's existing `if (!launch) return null` becomes right without + * being told. Kept as a plain context read on purpose — the answer is decided + * once, in the provider, rather than by a hook that six components call. */ export function useCoachLaunch(): CoachLaunchValue | null { return useContext(CoachLaunchContext);