Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 52 additions & 19 deletions e2e/delegated-writes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,22 @@
*
* ## Why it gates itself instead of seeding a WRITE grant directly
*
* The grant level is chosen in the invitation form, which is another chunk's
* work. Rather than mint a WRITE row behind the UI's back — which would prove
* the journey works for a grant no person can create — the journey looks for
* the level control and stands down when it is not there yet.
* The grant level is chosen in the invitation form. Rather than mint a WRITE
* row behind the UI's back — which would prove the journey works for a grant
* no person can create — the journey looks for the level control and stands
* down when it is not there yet.
*
* **To enable this once the invite form ships its level control:** if it lands
* under a different `data-slot` than the constant below, change that one
* string. Nothing else in this file assumes anything about the control except
* that picking WRITE and submitting mints a WRITE grant.
* That guard was written against a `data-slot` the form never shipped under
* (`grant-invite-level`; the control landed as `grant-invite-access-option`),
* so from the day the control arrived until 2026-08-03 every test in this file
* skipped and the whole delegated-write journey ran nowhere. The file said out
* loud what to change and nobody changed it, which is the standing lesson
* about a check that cannot fail: the skip is quiet, and a quiet skip and a
* passing suite look identical in a CI summary.
*
* The skip is deliberately loud rather than silent: it names the missing
* control, so a run where the control exists and the journey still does not
* execute reads as a bug in this file rather than as an absence upstream.
* The constant is the real one now. If it moves again, change that one string:
* nothing else here assumes anything about the control except that choosing
* WRITE and submitting mints a WRITE grant.
*
* ## What this spec cannot cover
*
Expand All @@ -45,10 +48,10 @@ import {
* The invitation form's grant-level control. The journey runs when this is on
* the page and stands down when it is not. One string, one place.
*/
const GRANT_LEVEL_SLOT = "grant-invite-level";
const GRANT_LEVEL_SLOT = "grant-invite-access-option";

/** The value the level control carries for a grant that may add entries. */
const WRITE_LEVEL_VALUE = "write";
const WRITE_LEVEL_VALUE = "WRITE";

test.describe("delegated writes", () => {
// One journey in order, like the read-only sibling: each step is the next
Expand Down Expand Up @@ -110,9 +113,11 @@ test.describe("delegated writes", () => {
await expect(submit).toBeEnabled({ timeout: 1000 });
}).toPass({ timeout: 15_000 });

await ownerPage
.locator(`[data-slot="${GRANT_LEVEL_SLOT}"]`)
.selectOption(WRITE_LEVEL_VALUE);
const writeOption = ownerPage.locator(
`[data-slot="${GRANT_LEVEL_SLOT}"][data-access="${WRITE_LEVEL_VALUE}"]`,
);
await writeOption.click();
await expect(writeOption).toHaveAttribute("data-selected", "true");

// Read the posted body: a level control that renders and sends a hardcoded
// level would pass every render assertion and ship a read-only grant.
Expand All @@ -125,7 +130,7 @@ test.describe("delegated writes", () => {
access?: string;
};
expect(
posted.access?.toLowerCase(),
posted.access,
"the invitation must carry the level the owner chose",
).toBe(WRITE_LEVEL_VALUE);
});
Expand Down Expand Up @@ -162,8 +167,14 @@ test.describe("delegated writes", () => {
res.request().method() === "POST" &&
res.url().endsWith("/api/measurements"),
);
await page.getByRole("button", { name: /add measurement/i }).click();
await page.locator('input[name="value"], #value').first().fill("71.5");
// Stable attributes and the form's real fields, neither of which this
// step had. It looked for a button named "Add measurement" (the header
// reads "Add") and then for a `value` input (the form opens on blood
// pressure, which has three). Both were wrong from the day they were
// written and nobody found out, because the whole file was skipping.
await page.locator('[data-slot="measurement-add"]').click();
await page.locator("#sys").fill("124");
await page.locator("#dia").fill("78");
await page.getByRole("button", { name: /^save$/i }).click();
expect((await post).status(), "the write must be accepted").toBeLessThan(
300,
Expand All @@ -180,6 +191,28 @@ test.describe("delegated writes", () => {
);
});

test("a deep link opens exactly what the level admits", async ({ page }) => {
// The gate binds to the level the server resolved, not to a blanket
// "somebody else's record" flag. Both halves matter and only a browser
// can show either: the SSR suite holds a component's paint, never a URL.
//
// Admitted: entering a reading, so `?add=` opens the same sheet the
// header button opens.
await page.goto("/measurements?add=WEIGHT");
await expect(
page.locator('[data-slot="shared-record-banner"]'),
).toBeVisible();
await expect(
page.locator('[data-slot="responsive-sheet-content"]').first(),
).toBeVisible();

// Also admitted: adding a medication with its schedule.
await page.goto("/medications?new=1");
await expect(
page.locator('[data-slot="medication-wizard-dialog"]'),
).toBeVisible();
});

test("the owner sees that somebody else was in their record", async () => {
await ownerPage.goto("/settings/access");
const rows = ownerPage.locator('[data-slot="record-activity-row"]');
Expand Down
21 changes: 14 additions & 7 deletions src/__tests__/delegable-surface-guard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,11 +428,11 @@ const DELEGABLE_ROUTES: Record<string, string> = {
"app/api/biomarkers/[id]/route.ts":
"One biomarker of the record, fetch-then-guard against the resolved user.",
"app/api/allergies/route.ts":
"The record's allergy list — the single most useful thing a caregiver can read, and a plain list of the owner's own rows.",
"The record's allergy list — the single most useful thing a caregiver can read, and a plain list of the owner's own rows. The POST beside it is NOT delegable and is deliberately absent from the write literal below: the only surface that posts to it lives under `/settings`, which a switch closes, so admitting the write would freeze a permission ahead of any caller for it. The route comment carries the argument.",
"app/api/allergies/[id]/route.ts":
"One allergy of the record, fetch-then-guard against the resolved user.",
"app/api/family-history/route.ts":
"The record's family history. The payload describes the owner's relatives, so it is the one admitted read where third-party health information is present by design rather than by accident; a caregiver reading it is the use the feature exists for, and the row is stored as the owner's.",
"The record's family history. The payload describes the owner's relatives, so it is the one admitted read where third-party health information is present by design rather than by accident; a caregiver reading it is the use the feature exists for, and the row is stored as the owner's. Its POST is not delegable, for the reason its allergy sibling gives plus one of its own — see the route comment.",
"app/api/family-history/[id]/route.ts":
"One family-history entry of the record, fetch-then-guard against the resolved user.",
"app/api/mental-health/assessments/route.ts":
Expand Down Expand Up @@ -512,6 +512,17 @@ const DELEGABLE_ROUTES: Record<string, string> = {
* thirty-one read-only delegable modules do not do, and it is the difference
* the identifier matcher above cannot see.
*
* v1.36.x — `POST /api/allergies` and `POST /api/family-history` left this
* list, and the removal is worth reading before either is proposed again. The
* argument for admitting them was never wrong; what they lacked was a caller.
* The only surface in the product that posts to either lives in Settings →
* Anamnese, and a switch closes `/settings` — so a delegate could not reach
* the form at any grant level, and the two entries were a permission frozen
* ahead of the surface that would exercise it. Both delegable READ arms stay:
* a caregiver reading the allergy list is what the feature is for. Whoever
* builds a caregiver-reachable medical-history surface adds them back in the
* same diff, which is the two-ended change this list is meant to hold to.
*
* Every member also has to call `auditLog`, asserted below. That is not a
* stylistic preference. The decision not to add a `writtenBy` column to eleven
* tables rests entirely on the audit trail carrying the actor, and `auditLog`
Expand All @@ -528,10 +539,6 @@ const DELEGABLE_WRITE_ROUTES: Record<string, string> = {
"Entering a lab result. The free-text path may mint a biomarker, and mints it into the RECORD's catalogue — a result added to somebody's record that left the marker on the helper's account would be worse than useless to the owner.",
"app/api/biomarkers/route.ts":
"Adding an analyte to the record's catalogue. A name the record already tracks is the ordinary 409 from the same `(userId, name)` uniqueness the owner would hit themselves.",
"app/api/allergies/route.ts":
"Adding an allergy. The cleanest admission in the set: a plain statement about the record's own body, and the single most useful thing a caregiver can contribute.",
"app/api/family-history/route.ts":
"Adding a family-history entry. Third-party health data by design — the row describes the record's relatives — and it is stored as the record's own, which is exactly how the delegable read arm already serves it.",
"app/api/illness/episodes/route.ts":
"Opening an illness episode. The module gate runs against the RECORD before the write, so a delegate cannot create an episode inside a record whose owner switched the module off.",
"app/api/custom-metrics/[id]/entries/route.ts":
Expand Down Expand Up @@ -569,7 +576,7 @@ const ACTOR_ROUTES: Record<string, string> = {
* stay a formality by accident: every addition has to be counted here as well
* as listed above, which is one more place a careless admission has to pass.
*/
const FROZEN_ENTRY_COUNT = 57;
const FROZEN_ENTRY_COUNT = 55;

/**
* The two surfaces that authenticate a Bearer token outside `requireAuth` —
Expand Down
4 changes: 3 additions & 1 deletion src/__tests__/success-affordance-guard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,9 @@ const PINNED_AFFORDANCES: Record<
"toast.success": 2,
},
"src/components/medications/take-all-due.ts": { "toast.success": 1 },
"src/components/medications/use-medication-intake.ts": { "toast.success": 5 },
// v1.36.x — one fewer: the log-intake path's three-armed toast collapsed
// into the shared `intakeToastOptions` decision plus a single call pair.
"src/components/medications/use-medication-intake.ts": { "toast.success": 4 },
"src/components/medications/wizard/medication-wizard-dialog.tsx": {
"toast.success": 1,
},
Expand Down
37 changes: 32 additions & 5 deletions src/app/api/allergies/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import { NextRequest } from "next/server";

import { prisma } from "@/lib/db";
import { apiHandler, requireRecordAuth } from "@/lib/api-handler";
import { apiHandler, requireAuth, requireRecordAuth } from "@/lib/api-handler";
import { annotate } from "@/lib/logging/context";
import { auditLog } from "@/lib/auth/audit";
import {
Expand Down Expand Up @@ -67,10 +67,37 @@ export const GET = apiHandler(async (request: NextRequest) => {
export const POST = apiHandler(withIdempotency<[NextRequest]>(postAllergy));

async function postAllergy(request: NextRequest): Promise<Response> {
// v1.36.x — a delegated write, and the cleanest of them: the row is a plain
// statement about the record's own body, and the caller appears in the audit
// trail rather than in the row.
const { user } = await requireRecordAuth("write");
// v1.36.x — the GET above is delegable and this is not, which is the
// opposite of where the classification landed and worth the paragraph.
//
// Nothing about the row changed: it is still a plain statement about the
// record's own body, still the single most useful thing a caregiver could
// contribute, and the argument for admitting it still holds. What it does
// not have is a caller. The only place in the product that posts here is the
// allergy manager in Settings → Anamnese, and `/settings/*` is not a
// shared-record destination — the shell shows the "not part of what was
// shared" panel there, so no delegate can reach the form at any level.
//
// An admitted write with no reachable surface is the one-ended change this
// repository keeps rediscovering (CLAUDE.md, "A two-ended change carries
// both ends"): the permission ships, the consumer is the follow-up, and
// nothing in the gate notices because every other check proves the other
// end. The frozen list is built the other way round on purpose — its own
// actor-surface note says the rest "arrive as their own diffs; naming them
// before they exist would freeze a guess."
//
// So this arm waits for the surface that would exercise it, and the two
// land together. Choosing that surface is design work, not a fix: allergies
// and family history have exactly one home today, that home is a personal
// account surface a switch rightly closes, and bolting a second copy onto a
// shared page would split one concept across two places. Re-admitting is
// one line here plus one entry in `delegable-surface-guard.test.ts` plus the
// paragraph that argues it — which is exactly the reviewed diff that guard
// exists to force.
//
// The half that was always the point is untouched: a caregiver can still
// READ the allergy list inside the record.
const { user } = await requireAuth();

const { data: rawBody, error: jsonError } = await safeJson(request, {
maxBytes: 16 * 1024,
Expand Down
23 changes: 17 additions & 6 deletions src/app/api/family-history/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import { NextRequest } from "next/server";

import { prisma } from "@/lib/db";
import { apiHandler, requireRecordAuth } from "@/lib/api-handler";
import { apiHandler, requireAuth, requireRecordAuth } from "@/lib/api-handler";
import { annotate } from "@/lib/logging/context";
import { auditLog } from "@/lib/auth/audit";
import {
Expand Down Expand Up @@ -62,11 +62,22 @@ export const POST = apiHandler(
);

async function postFamilyHistory(request: NextRequest): Promise<Response> {
// v1.36.x — a delegated write, and the one where third-party health data is
// present by design: the row describes the record's relatives. It is stored
// as the record's own, exactly as the delegable READ arm above already
// serves it, and the audit trail names who entered it.
const { user } = await requireRecordAuth("write");
// v1.36.x — not a delegated write, though the GET above is delegable. The
// reasoning is written out at the sibling arm in `api/allergies/route.ts`
// and is the same here: the only surface that posts to either route is the
// manager in Settings → Anamnese, and `/settings/*` is closed inside a
// shared record, so no delegate can reach the form at any level. An
// admitted write with no reachable caller is a permission frozen ahead of
// the surface that would exercise it.
//
// This arm carried an extra reason to wait. The row describes the record's
// RELATIVES, so it is the one admitted write where third-party health
// information is present by design — and frequently the delegate is that
// relative, asserting a condition about themselves into somebody else's
// record. The classification admitted it and named the discomfort. Landing
// it together with the surface that offers it means the copy on that surface
// can say whose statement the row is, which no route comment can.
const { user } = await requireAuth();

const { data: rawBody, error: jsonError } = await safeJson(request, {
maxBytes: 16 * 1024,
Expand Down
11 changes: 10 additions & 1 deletion src/app/measurements/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export default function MeasurementsPage() {
actions={
canAdd ? (
<Button
data-slot="measurement-add"
className="min-h-11 sm:min-h-9"
onClick={() => {
setReturnTo(null);
Expand All @@ -170,8 +171,16 @@ export default function MeasurementsPage() {
}
/>

{/* v1.36.x — the sheet answers the same question the header button
answers. `?add=<TYPE>` 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. */}
<ResponsiveSheet
open={dialogOpen}
open={dialogOpen && canAdd}
onOpenChange={(open) => {
setDialogOpen(open);
if (!open) {
Expand Down
5 changes: 4 additions & 1 deletion src/app/medications/page-client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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. */}
<MedicationWizardDialog
open={dialogOpen}
open={dialogOpen && canAdd}
onOpenChange={setDialogOpen}
mode="create"
onSuccess={closeDialog}
Expand Down
Loading
Loading