diff --git a/src/__tests__/sharing-surface-guard.test.ts b/src/__tests__/sharing-surface-guard.test.ts index d6ec5452e..9b5d0704a 100644 --- a/src/__tests__/sharing-surface-guard.test.ts +++ b/src/__tests__/sharing-surface-guard.test.ts @@ -2072,6 +2072,10 @@ const ACTOR_ROUTES: Record = { "The app shell reads it on every boot, including while a switch is on: the switcher, the banner naming whose record is open, and the route back out all bind this payload. Every field it returns is the caller's own — their preferences, their modules, their identity — and the one field about the switch says only which records they may open and which they are inside. It reads no row of the owner's, and its `accountAccess` block grants nothing: the resolver re-checks the grant on every delegated request regardless of what this payload said a moment ago.", "app/api/auth/me/locale/route.ts": "The UI language belongs to the person reading the screen, never to the record on it. Under a record scope this would transplant a delegate's choice onto the owner's row and send the owner's cron mail in a language they may not read. It has to keep working rather than merely refuse safely: the switcher's mount-time backfill fires on every page load, including the shared dashboard.", + "app/api/settings/module-list-layout/route.ts": + "How the person's own labs, illness and checkups lists are laid out: card or list view, manual order, sort. It reads and writes the caller's own row only. Inside another record the ids of a saved order simply match nothing and the list keeps the server order, so serving the person's own choice there reveals nothing of the owner's and grants nothing; refusing it under a switch broke the list controls in exactly the context they are used in.", + "app/api/settings/preview-features/route.ts": + "The person's own opt-in to preview features. A preview entry is chrome of the person reading the screen, never a property of the record on it; it reads and writes the caller's own row and nothing of the owner's.", "app/api/feature-flags/route.ts": "Reads the `AppSettings` singleton and no user row at all, so there is no record for a switch to substitute. It answers about the deployment, which for this purpose is the caller's side of the request; the shell gates the Coach launcher and the assistant surfaces on it, so a 403 here is a piece of chrome deciding it does not exist.", }; @@ -2171,8 +2175,11 @@ const ACTOR_ROUTES: Record = { * * v1.44 -- a guardian connects and disconnects a managed profile's OAuth * providers: two guardian-only entries. 266 -> 268. + * + * v1.44 -- the module list layout and the preview-feature switch are the + * person's own presentation and join the actor list. 268 -> 270. */ -const FROZEN_ENTRY_COUNT = 268; +const FROZEN_ENTRY_COUNT = 270; /** * The two surfaces that authenticate a Bearer token outside `requireAuth` — diff --git a/src/app/api/settings/module-list-layout/route.ts b/src/app/api/settings/module-list-layout/route.ts index 7bd93d3b8..481e38867 100644 --- a/src/app/api/settings/module-list-layout/route.ts +++ b/src/app/api/settings/module-list-layout/route.ts @@ -8,11 +8,17 @@ * keeps for its fields. The blob lives on its own `User` column * (`module_list_layout_json`). * - * Both halves are the caller's own presentation. Inside somebody else's - * record the ids in a saved order simply do not match and the list keeps the - * server order, so no record gate is needed here. + * Both halves are the caller's own presentation, so this is an actor surface + * (`requireActorAuth`): a browser acting for another record still reads and + * saves the person's own layout instead of being refused. Inside somebody + * else's record the ids in a saved order simply do not match and the list + * keeps the server order, so no record gate is needed here. + * + * The PUT reads and writes the blob under a row lock in one transaction, so + * two saves for different lists (two tabs, a quick view switch and a reorder) + * cannot each start from the same stored value and drop the other's change. */ -import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { apiHandler, requireActorAuth } from "@/lib/api-handler"; import { apiSuccess, returnAllZodIssues, safeJson } from "@/lib/api-response"; import { prisma, toJson } from "@/lib/db"; import { annotate } from "@/lib/logging/context"; @@ -25,8 +31,11 @@ import { moduleListLayoutPutSchema } from "@/lib/validations/settings-prefs"; export const dynamic = "force-dynamic"; -async function readStored(userId: string): Promise { - const row = await prisma.user.findUnique({ +async function readStored( + userId: string, + db: Pick = prisma, +): Promise { + const row = await db.user.findUnique({ where: { id: userId }, select: { moduleListLayoutJson: true }, }); @@ -34,7 +43,7 @@ async function readStored(userId: string): Promise { } export const GET = apiHandler(async () => { - const { user } = await requireAuth(); + const { user } = await requireActorAuth(); const layout = resolveModuleListLayout(await readStored(user.id)); annotate({ action: { name: "settings.module-list-layout.read" }, @@ -46,7 +55,7 @@ export const GET = apiHandler(async () => { }); export const PUT = apiHandler(async (request: Request) => { - const { user } = await requireAuth(); + const { user } = await requireActorAuth(); // 500 ids of up to 64 characters plus the envelope. const { data: body, error: jsonError } = await safeJson(request, { @@ -66,18 +75,21 @@ export const PUT = apiHandler(async (request: Request) => { // Preserve-when-absent: one stored read fills whichever field the body // left out, and every other list stays as stored. Only fields a person has // chosen are written, so a field never picked keeps following the default. - const stored = storedModuleListLayout(await readStored(user.id)); - const nextChoices = { - ...stored[list], - ...(view ? { view } : {}), - ...(order ? { order: [...new Set(order)] } : {}), - ...(sortDir ? { sortDir } : {}), - }; - const nextStored = { ...stored, [list]: nextChoices }; - - await prisma.user.update({ - where: { id: user.id }, - data: { moduleListLayoutJson: toJson({ version: 1, ...nextStored }) }, + const nextStored = await prisma.$transaction(async (tx) => { + await tx.$queryRaw`SELECT 1 FROM "users" WHERE "id" = ${user.id} FOR UPDATE`; + const stored = storedModuleListLayout(await readStored(user.id, tx)); + const nextChoices = { + ...stored[list], + ...(view ? { view } : {}), + ...(order ? { order: [...new Set(order)] } : {}), + ...(sortDir ? { sortDir } : {}), + }; + const next = { ...stored, [list]: nextChoices }; + await tx.user.update({ + where: { id: user.id }, + data: { moduleListLayoutJson: toJson({ version: 1, ...next }) }, + }); + return next; }); const layout = resolveModuleListLayout(nextStored); diff --git a/src/app/api/settings/preview-features/route.ts b/src/app/api/settings/preview-features/route.ts index 1b48c75fa..408b40a2f 100644 --- a/src/app/api/settings/preview-features/route.ts +++ b/src/app/api/settings/preview-features/route.ts @@ -5,8 +5,13 @@ * the flags the body names and keeps the rest; unknown flags are refused * with the multi-issue 422 so a typo cannot silently do nothing. The blob * lives on its own `User` column (`preview_features_json`). + * + * The opt-ins are the caller's own, so this is an actor surface + * (`requireActorAuth`): a browser acting for another record keeps reaching + * the person's own switch. The PATCH reads and writes under a row lock, so two + * flags switched at once both stay switched. */ -import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { apiHandler, requireActorAuth } from "@/lib/api-handler"; import { apiSuccess, returnAllZodIssues, safeJson } from "@/lib/api-response"; import { prisma, toJson } from "@/lib/db"; import { annotate } from "@/lib/logging/context"; @@ -19,8 +24,11 @@ import { previewFeaturesPatchSchema } from "@/lib/validations/settings-prefs"; export const dynamic = "force-dynamic"; -async function readStored(userId: string): Promise { - const row = await prisma.user.findUnique({ +async function readStored( + userId: string, + db: Pick = prisma, +): Promise { + const row = await db.user.findUnique({ where: { id: userId }, select: { previewFeaturesJson: true }, }); @@ -32,7 +40,7 @@ function enabledFlags(features: PreviewFeatures): string[] { } export const GET = apiHandler(async () => { - const { user } = await requireAuth(); + const { user } = await requireActorAuth(); const features = resolvePreviewFeatures(await readStored(user.id)); annotate({ action: { name: "settings.preview-features.read" }, @@ -42,7 +50,7 @@ export const GET = apiHandler(async () => { }); export const PATCH = apiHandler(async (request: Request) => { - const { user } = await requireAuth(); + const { user } = await requireActorAuth(); const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 1024, @@ -57,14 +65,17 @@ export const PATCH = apiHandler(async (request: Request) => { return returnAllZodIssues(parsed.error, 422); } - const current = resolvePreviewFeatures(await readStored(user.id)); - const next: PreviewFeatures = { - topics: parsed.data.topics ?? current.topics, - }; - - await prisma.user.update({ - where: { id: user.id }, - data: { previewFeaturesJson: toJson(next) }, + const next = await prisma.$transaction(async (tx) => { + await tx.$queryRaw`SELECT 1 FROM "users" WHERE "id" = ${user.id} FOR UPDATE`; + const current = resolvePreviewFeatures(await readStored(user.id, tx)); + const merged: PreviewFeatures = { + topics: parsed.data.topics ?? current.topics, + }; + await tx.user.update({ + where: { id: user.id }, + data: { previewFeaturesJson: toJson(merged) }, + }); + return merged; }); annotate({ diff --git a/tests/integration/settings-prefs-routes.test.ts b/tests/integration/settings-prefs-routes.test.ts index 5dcc4d7c5..61aaa5a00 100644 --- a/tests/integration/settings-prefs-routes.test.ts +++ b/tests/integration/settings-prefs-routes.test.ts @@ -15,7 +15,7 @@ process.env.ENCRYPTION_KEY ??= "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; import { cookieJar, headerJar } from "./mock-next-headers"; -import { getPrismaClient, truncateAllTables } from "./setup"; +import { getPrismaClient, switchSessionTo, truncateAllTables } from "./setup"; vi.mock("next/headers", async () => { const { cookieJar, headerJar } = await import("./mock-next-headers"); @@ -214,3 +214,111 @@ describe("/api/settings/preview-features — integration", () => { expect(empty.status).toBe(422); }); }); + +describe("settings presentation follows the person, also while acting for a record", () => { + const LAYOUT = "http://localhost/api/settings/module-list-layout"; + const PREVIEW = "http://localhost/api/settings/preview-features"; + + async function actingForAnotherRecord() { + const prisma = getPrismaClient(); + const owner = await prisma.user.create({ + data: { + username: "prefs-record-owner", + email: "prefs-owner@example.test", + role: "USER", + }, + }); + const actor = await prisma.user.create({ + data: { + username: "prefs-delegate", + email: "prefs-delegate@example.test", + role: "USER", + }, + }); + await prisma.accountGrant.create({ + data: { + grantorId: owner.id, + granteeId: actor.id, + access: "WRITE", + acceptedAt: new Date(), + }, + }); + const session = await prisma.session.create({ + data: { userId: actor.id, expiresAt: new Date(Date.now() + 60_000) }, + }); + cookieJar.set("healthlog_session", session.id); + await switchSessionTo(session.id, owner.id); + return { ownerId: owner.id, actorId: actor.id }; + } + + it("reads and saves the actor's own layout and preview switch", async () => { + const { ownerId, actorId } = await actingForAnotherRecord(); + const layout = await import("@/app/api/settings/module-list-layout/route"); + const preview = await import("@/app/api/settings/preview-features/route"); + + expect((await (layout.GET as Handler)(new Request(LAYOUT))).status).toBe( + 200, + ); + const put = await (layout.PUT as Handler)( + json("PUT", LAYOUT, { version: 1, list: "labs", view: "cards" }), + ); + expect(put.status).toBe(200); + expect((await (preview.GET as Handler)(new Request(PREVIEW))).status).toBe( + 200, + ); + const patch = await (preview.PATCH as Handler)( + json("PATCH", PREVIEW, { topics: true }), + ); + expect(patch.status).toBe(200); + + const rows = await getPrismaClient().user.findMany({ + where: { id: { in: [ownerId, actorId] } }, + select: { + id: true, + moduleListLayoutJson: true, + previewFeaturesJson: true, + }, + }); + const actor = rows.find((r) => r.id === actorId)!; + const owner = rows.find((r) => r.id === ownerId)!; + expect(actor.moduleListLayoutJson).toMatchObject({ + labs: { view: "cards" }, + }); + expect(actor.previewFeaturesJson).toEqual({ topics: true }); + expect(owner.moduleListLayoutJson).toBeNull(); + expect(owner.previewFeaturesJson).toBeNull(); + }); +}); + +describe("layout saves for different lists do not overwrite each other", () => { + const URL = "http://localhost/api/settings/module-list-layout"; + + it("keeps every list a burst of concurrent saves touched", async () => { + const { userId } = await seedUser(); + const { PUT } = await import("@/app/api/settings/module-list-layout/route"); + const lists = ["labs", "illness", "vorsorge"] as const; + await Promise.all( + Array.from({ length: 9 }, (_, i) => + (PUT as Handler)( + json("PUT", URL, { + version: 1, + list: lists[i % 3], + ...(i < 3 ? { view: "cards" } : { order: [`item-${i}`] }), + }), + ), + ), + ); + const row = await getPrismaClient().user.findUniqueOrThrow({ + where: { id: userId }, + select: { moduleListLayoutJson: true }, + }); + const stored = row.moduleListLayoutJson as Record< + string, + { view?: string; order?: string[] } + >; + for (const list of lists) { + expect(stored[list]?.view, list).toBe("cards"); + expect(stored[list]?.order, list).toBeDefined(); + } + }); +});