From bc43a9f0620944cab33e10e85dde9197924a5388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 3 Aug 2026 12:11:29 +0200 Subject: [PATCH] Drop the shared delete, bulk and capture affordances under a switch Three controls carry most of the app's mutation surface between them. The row delete is spelled once for measurements, lab results, biomarkers, allergies, family history, custom metrics and medication supply; the selection bar is the only bulk delete; the capture picker is the fastest path to four entry forms on a phone and bypasses every page header. Deleting belongs to the owner at every grant level, so the first two are absent inside somebody else's record rather than greyed out: a disabled bin still claims the row is the delegate's to remove. The picker keeps the two kinds a delegation admits and drops a mood entry and a glass of water, which the server refuses under a switch; when nothing survives, the bar's plus button goes with them instead of opening an empty sheet. --- .../delegated-write-affordances.test.tsx | 198 ++++++++++++++++++ src/components/data-list/delete-button.tsx | 12 ++ .../data-list/selection-action-bar.tsx | 9 +- .../__tests__/capture-picker-discard.test.tsx | 13 ++ .../layout/__tests__/capture-picker.test.tsx | 13 ++ src/components/layout/bottom-nav.tsx | 31 ++- src/components/layout/capture-picker.tsx | 92 ++++++-- 7 files changed, 334 insertions(+), 34 deletions(-) create mode 100644 src/components/__tests__/delegated-write-affordances.test.tsx diff --git a/src/components/__tests__/delegated-write-affordances.test.tsx b/src/components/__tests__/delegated-write-affordances.test.tsx new file mode 100644 index 000000000..ae1f4e03d --- /dev/null +++ b/src/components/__tests__/delegated-write-affordances.test.tsx @@ -0,0 +1,198 @@ +/** + * What a delegate is offered, rendered rather than described. + * + * v1.36.0 shipped sharing read-only and gated it only in the application + * chrome. Every feature surface still painted its add button, and a delegate + * who tapped one met a 403 from a server that was right to refuse. These + * render the real controls to markup and assert the paint, on the three + * capability states a session can be in. + * + * The suite is SSR-only (`@testing-library/react` is not a dependency here), + * which bounds what it can prove: it holds the RENDER, never the click. A + * control that is absent here cannot be tapped, and that is the whole property + * — but a control that is present here is not proven to work, and a submit + * path is not proven at all. That leg lives in `e2e/account-sharing.spec.ts`. + * + * Mutation checks, run: + * - `useRecordCapabilities` returning `canAdd: true` unconditionally → the + * read-only legs for the intake row and the card menu go red. + * - `DeleteButton` dropping its `canManage` bail → "no row delete inside + * somebody else's record" goes red. + */ +import { describe, expect, it, vi } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; + +import { I18nProvider } from "@/lib/i18n/context"; +import type { AccountAccess } from "@/lib/sharing/account-access-view"; + +const OWNER = { + accountId: "acct-owner", + username: "owner", + displayName: "Margarethe", + access: "read" as const, + canWrite: false, +}; + +const OWN_RECORD: AccountAccess = { + accounts: [OWNER], + active: null, + canSwitch: true, +}; +const READ_ONLY: AccountAccess = { + accounts: [OWNER], + active: OWNER, + canSwitch: true, +}; +const WRITABLE: AccountAccess = { + accounts: [OWNER], + active: { ...OWNER, access: "write", canWrite: true }, + canSwitch: true, +}; + +const mockAccessRef: { value: AccountAccess } = { value: OWN_RECORD }; + +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(), + }), +})); + +import { DeleteButton } from "@/components/data-list/delete-button"; +import { SelectionActionBar } from "@/components/data-list/selection-action-bar"; +import { MedicationCardMenu } from "@/components/medications/medication-card-menu"; +import { MedicationIntakeActions } from "@/components/medications/card-parts/medication-intake-actions"; +import { visibleCaptureKinds } from "@/components/layout/capture-picker"; + +function render(access: AccountAccess, node: React.ReactNode): string { + mockAccessRef.value = access; + return renderToStaticMarkup( + {node}, + ); +} + +describe("row delete", () => { + const node = ( + {}} title="Delete?" description="Gone." /> + ); + + it("renders in the caller's own record", () => { + expect(render(OWN_RECORD, node)).toContain(" { + // Not disabled. A greyed bin still claims the row is the delegate's to + // remove, and it is not — at either grant level, including for a row the + // delegate entered themselves. + expect(render(READ_ONLY, node)).toBe(""); + expect(render(WRITABLE, node)).toBe(""); + }); +}); + +describe("bulk selection bar", () => { + const node = ( + {}} + onConfirmDelete={() => {}} + isDeleting={false} + confirmTitle="Delete 3?" + confirmBody="Gone." + /> + ); + + it("renders in the caller's own record", () => { + expect(render(OWN_RECORD, node)).toContain( + 'data-slot="selection-action-bar"', + ); + }); + + it("is absent inside somebody else's record", () => { + expect(render(WRITABLE, node)).toBe(""); + }); +}); + +describe("marking a dose", () => { + const node = ( + {}} /> + ); + + it("is offered to a delegate who may write", () => { + // The verb the delegation was written for: somebody looking after a + // parent marks the morning dose taken. + expect(render(WRITABLE, node)).toContain(" { + expect(render(READ_ONLY, node)).toBe(""); + }); + + it("is offered in the caller's own record", () => { + expect(render(OWN_RECORD, node)).toContain(" { + const node = ( + {}} + onOpenHistory={() => {}} + onLogSideEffect={() => {}} + /> + ); + + it("renders its trigger in the caller's own record", () => { + expect(render(OWN_RECORD, node)).toContain(" { + // One item survives for them — noting a side effect — so the menu stays. + expect(render(WRITABLE, node)).toContain(" { + // Every item in it is owner work, so the trigger goes with them rather + // than opening onto an empty sheet. + expect(render(READ_ONLY, node)).toBe(""); + }); +}); + +describe("the capture picker's kinds", () => { + const ALL = ["measurement", "medication", "mood", "water"] as const; + + it("offers everything in the caller's own record", () => { + expect( + visibleCaptureKinds({ canAdd: true, canManage: true }, true, [...ALL]), + ).toEqual(["measurement", "medication", "mood", "water"]); + }); + + it("offers a delegate only what the delegation admits", () => { + // A reading and a dose are admitted verbs. A mood entry and a glass of + // water are not, and the server refuses them under a switch. + expect( + visibleCaptureKinds({ canAdd: true, canManage: false }, true, [...ALL]), + ).toEqual(["measurement", "medication"]); + }); + + it("offers a read-only delegate nothing", () => { + expect( + visibleCaptureKinds({ canAdd: false, canManage: false }, true, [...ALL]), + ).toEqual([]); + }); + + it("still honours the module gate for the owner", () => { + expect( + visibleCaptureKinds({ canAdd: true, canManage: true }, false, [...ALL]), + ).toEqual(["measurement", "medication", "mood"]); + }); +}); diff --git a/src/components/data-list/delete-button.tsx b/src/components/data-list/delete-button.tsx index c963e0026..9dc7d96ea 100644 --- a/src/components/data-list/delete-button.tsx +++ b/src/components/data-list/delete-button.tsx @@ -15,6 +15,7 @@ import { AlertDialogTrigger, } from "@/components/ui/alert-dialog"; import { useTranslations } from "@/lib/i18n/context"; +import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; /** * Shared single-row delete button + confirm dialog for the data- @@ -25,6 +26,15 @@ import { useTranslations } from "@/lib/i18n/context"; * so the title/description are passed in rather than reaching for a fixed * translation key. `cancelLabel` / `confirmLabel` default to the shared * `common.*` strings. + * + * v1.36.x — this is the one place a row delete is spelled across the app + * (measurements, lab results, biomarkers, allergies, family history, custom + * metrics, medication inventory), so the delegation rule is enforced here + * rather than at twelve call sites: deleting is the owner's alone, at every + * grant level, so inside somebody else's record the trigger is absent. Not + * disabled — a greyed bin still claims the row is the delegate's to remove. + * A caller that ever needs a delete a delegate MAY perform would have to say + * so explicitly, and today there is none. */ export function DeleteButton({ onConfirm, @@ -47,6 +57,8 @@ export function DeleteButton({ iconClassName?: string; }) { const { t } = useTranslations(); + const { canManage } = useRecordCapabilities(); + if (!canManage) return null; return ( diff --git a/src/components/data-list/selection-action-bar.tsx b/src/components/data-list/selection-action-bar.tsx index 027c1e9d9..866bc722d 100644 --- a/src/components/data-list/selection-action-bar.tsx +++ b/src/components/data-list/selection-action-bar.tsx @@ -15,6 +15,7 @@ import { AlertDialogTrigger, } from "@/components/ui/alert-dialog"; import { useTranslations } from "@/lib/i18n/context"; +import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; /** * Selection action bar for the data-management lists (measurements + @@ -29,6 +30,11 @@ import { useTranslations } from "@/lib/i18n/context"; * * The confirm copy is passed in because the title/body differ per surface * (measurements vs mood) and the count is interpolated by the caller. + * + * v1.36.x — bulk delete is the owner's alone at every grant level, so the bar + * is absent inside somebody else's record. The row checkboxes that feed it are + * dropped at their lists for the same reason; this guard is the backstop, so a + * list that forgets cannot leave a delete button floating over the page. */ export function SelectionActionBar({ count, @@ -46,8 +52,9 @@ export function SelectionActionBar({ confirmBody: string; }) { const { t } = useTranslations(); + const { canManage } = useRecordCapabilities(); - if (count <= 0) return null; + if (count <= 0 || !canManage) return null; return (
({ nutrientsEnabled: true, })); +// v1.36.x — the picker asks what the record allows before it offers a kind. +// These fixtures are the caller's own record, which is every kind; the +// delegation cases are covered in +// `src/components/__tests__/delegated-write-affordances.test.tsx`. +vi.mock("@/hooks/use-record-capabilities", () => ({ + useRecordCapabilities: () => ({ + inSharedRecord: false, + canWrite: false, + canAdd: true, + canManage: true, + }), +})); + vi.mock("react", async (importOriginal) => { const actual = await importOriginal(); return { diff --git a/src/components/layout/__tests__/capture-picker.test.tsx b/src/components/layout/__tests__/capture-picker.test.tsx index e5756bfde..16b464180 100644 --- a/src/components/layout/__tests__/capture-picker.test.tsx +++ b/src/components/layout/__tests__/capture-picker.test.tsx @@ -10,6 +10,19 @@ vi.mock("@/hooks/use-module-enabled", () => ({ moduleKey === "nutrients" ? moduleGate.nutrientsEnabled : true, })); +// v1.36.x — the picker asks what the record allows before it offers a kind. +// These fixtures are the caller's own record, which is every kind; the +// delegation cases are covered in +// `src/components/__tests__/delegated-write-affordances.test.tsx`. +vi.mock("@/hooks/use-record-capabilities", () => ({ + useRecordCapabilities: () => ({ + inSharedRecord: false, + canWrite: false, + canAdd: true, + canManage: true, + }), +})); + vi.mock("@/components/ui/responsive-sheet", () => ({ ResponsiveSheet: ({ open, diff --git a/src/components/layout/bottom-nav.tsx b/src/components/layout/bottom-nav.tsx index 3b67fa163..384bab972 100644 --- a/src/components/layout/bottom-nav.tsx +++ b/src/components/layout/bottom-nav.tsx @@ -21,6 +21,7 @@ import type { ModuleKey } from "@/lib/modules/registry"; import { useMemo, useState } from "react"; import { useAuth } from "@/hooks/use-auth"; import { useMounted } from "@/hooks/use-mounted"; +import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; import { Sheet, SheetContent, @@ -97,6 +98,8 @@ export function BottomNav() { // the same resolved payload, so the two bars cannot disagree about what a // delegate is offered. const sharedRecord = user?.accountAccess?.active != null; + const { canAdd, canManage } = useRecordCapabilities(); + const canCapture = canAdd || canManage; // 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 @@ -191,18 +194,24 @@ export function BottomNav() { collar (a `bg-card` ring that punches it out of the bar) plus a stronger shadow so it reads as the bar's primary CTA rather than a fifth flush tab. */} + {/* v1.36.x — a read-only delegate has nothing to capture: every + surface behind the picker is refused for them, so the bar's + primary CTA goes rather than opening onto an empty sheet. At + WRITE the picker itself drops the kinds the grant excludes. */}
- + {canCapture && ( + + )}
{primaryRight.map(renderPrimary)} diff --git a/src/components/layout/capture-picker.tsx b/src/components/layout/capture-picker.tsx index 79eebaaed..cd3630d4d 100644 --- a/src/components/layout/capture-picker.tsx +++ b/src/components/layout/capture-picker.tsx @@ -21,6 +21,10 @@ import { import { sheetBodyHasUnsavedInput } from "@/components/dashboard/quick-entry-sheets"; import { useTranslations } from "@/lib/i18n/context"; import { useModuleEnabled } from "@/hooks/use-module-enabled"; +import { + useRecordCapabilities, + type RecordCapabilities, +} from "@/hooks/use-record-capabilities"; /** * The center "Log" capture action (iOS parity — the bottom bar's @@ -48,6 +52,45 @@ import { useModuleEnabled } from "@/hooks/use-module-enabled"; type CaptureKind = "measurement" | "medication" | "mood" | "water"; +/** + * v1.36.x — which of the four capture surfaces a delegation admits. + * + * A WRITE grant covers entering a reading and marking a dose. It does not + * cover a mood entry or a glass of water: those verbs stay on the owner's own + * authentication and the server refuses them under a switch. The picker is the + * fastest path into all four on a phone, so it is the surface where offering + * an unadmitted one costs the most. + */ +const DELEGABLE_CAPTURE_KINDS: ReadonlySet = new Set([ + "measurement", + "medication", +]); + +/** The order the chooser lists them in. */ +const CAPTURE_KIND_ORDER: ReadonlyArray = [ + "measurement", + "medication", + "mood", + "water", +]; + +/** + * The capture kinds to offer, given what the record allows and whether the + * nutrients module is on. Exported pure so the delegation rule can be pinned + * without opening a sheet: an SSR test cannot tap the button that opens it. + */ +export function visibleCaptureKinds( + caps: Pick, + nutrientsEnabled: boolean, + kinds: ReadonlyArray, +): CaptureKind[] { + return kinds.filter((kind) => { + if (kind === "water" && !nutrientsEnabled) return false; + if (caps.canManage) return true; + return caps.canAdd && DELEGABLE_CAPTURE_KINDS.has(kind); + }); +} + interface CapturePickerProps { /** Whether the picker chooser sheet is open. */ open: boolean; @@ -58,6 +101,12 @@ interface CapturePickerProps { export function CapturePicker({ open, onOpenChange }: CapturePickerProps) { const { t } = useTranslations(); const nutrientsEnabled = useModuleEnabled("nutrients"); + const capabilities = useRecordCapabilities(); + const offered = visibleCaptureKinds( + capabilities, + nutrientsEnabled, + CAPTURE_KIND_ORDER, + ); const [kind, setKind] = useState(null); const [footerEl, setFooterEl] = useState(null); // v1.30.1 — mirrors `QuickEntrySheets`' `confirmDiscardOpen`: hold a @@ -66,7 +115,7 @@ export function CapturePicker({ open, onOpenChange }: CapturePickerProps) { const [confirmDiscardOpen, setConfirmDiscardOpen] = useState(false); function chooseKind(next: CaptureKind) { - if (next === "water" && !nutrientsEnabled) return; + if (!offered.includes(next)) return; // Close the chooser first, then open the form sheet so only one // bottom-sheet is mounted at a time (no stacked backdrops). onOpenChange(false); @@ -89,7 +138,7 @@ export function CapturePicker({ open, onOpenChange }: CapturePickerProps) { closeForm(); } - const options: ReadonlyArray<{ + const allOptions: ReadonlyArray<{ kind: CaptureKind; label: string; description: string; @@ -120,6 +169,7 @@ export function CapturePicker({ open, onOpenChange }: CapturePickerProps) { icon: GlassWater, }, ]; + const options = allOptions.filter((opt) => offered.includes(opt.kind)); const formTitleByKind: Record = { measurement: t("measurements.addMeasurement"), @@ -145,27 +195,25 @@ export function CapturePicker({ open, onOpenChange }: CapturePickerProps) { className="grid grid-cols-1 gap-2" data-testid="capture-picker-options" > - {options.map((opt) => - opt.kind !== "water" || nutrientsEnabled ? ( - - ) : null, - )} + + + ))}