mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-07 04:12:04 +00:00
feat(api): publish the resolved notification preferences on the account payload
The medication detail's notification section decides between the server-side reminder switch and the client-managed chip on `notificationPrefs.medication.clientManaged`, read off `GET /api/auth/me`. That payload never carried the field — the preferences live on their own endpoint, which is not on the app-boot path — so the flag was `undefined` for every account: the chip never rendered, and a person whose phone owns the reminders was shown a switch that no longer decides anything. The write path and the dispatch gate were correct the whole time; only the web reflection was missing its end. The payload now carries the fully-resolved block from the same resolver the dedicated endpoint uses, so the two cannot disagree, and an account that never opted in reads `false` rather than absent. No extra query: the session already loads the row the column sits on. The pair guard over this payload gained the direction that could not see it. It asked that every published field has a reader; a reader with no field was invisible. It now also reads the response types clients declare for this endpoint and holds them against what the route publishes. A typed guard was tried first and does not work here — each consumer declares its own local interface, so the compiler checks every file against its own private claim and agrees with all of them.
This commit is contained in:
1 parent
f3321b8031
commit
fd6fa18913
5 files changed
+138
-21
No files matched your search
@@ -65898,11 +65898,26 @@ components:
|
||||
anyOf:
|
||||
- $ref: "#/components/schemas/RecordSessionState"
|
||||
- type: "null"
|
||||
notificationPrefs:
|
||||
type: object
|
||||
properties:
|
||||
medication:
|
||||
type: object
|
||||
properties:
|
||||
clientManaged:
|
||||
type: boolean
|
||||
required:
|
||||
- clientManaged
|
||||
additionalProperties: {}
|
||||
required:
|
||||
- medication
|
||||
additionalProperties: {}
|
||||
required:
|
||||
- id
|
||||
- username
|
||||
- accountAccess
|
||||
- recordSession
|
||||
- notificationPrefs
|
||||
additionalProperties: {}
|
||||
description: "The signed-in account: its identity, its preferences, and (since v1.36.0) what account sharing lets it do.
|
||||
Additional properties are the preference fields this spec does not yet enumerate. Under an active switch this
|
||||
|
||||
@@ -543,27 +543,18 @@ test.describe("notification preferences drive the dispatch decision", () => {
|
||||
expect(smtp.accepted()).toEqual([EMAIL_RECIPIENT]);
|
||||
});
|
||||
|
||||
/**
|
||||
* The web reflection of the opt-in, and the reason it is a test rather than a
|
||||
* screenshot: the section reads `notificationPrefs.medication.clientManaged`
|
||||
* off `/api/auth/me`, and that payload did not publish it until this branch.
|
||||
* The flag was `undefined` for every account, so the chip never rendered and
|
||||
* a person whose phone owned the reminders was shown a server-side switch
|
||||
* that decided nothing. Both halves are asserted — the chip appears AND the
|
||||
* switch row is gone — because either one alone passes on the wrong render.
|
||||
*/
|
||||
test("the client-managed opt-in is visible on the medication it silences", async ({
|
||||
page,
|
||||
}) => {
|
||||
test.skip(
|
||||
true,
|
||||
"The chip exists and is unreachable. " +
|
||||
"`src/components/medications/sections/notifications-section.tsx` " +
|
||||
"decides between the reminder switch and the client-managed chip on " +
|
||||
"`notificationPrefs.medication.clientManaged` read out of " +
|
||||
"`/api/auth/me` — a field that payload does not carry (the prefs live " +
|
||||
"on `/api/auth/me/notification-prefs`). So the flag reads as false for " +
|
||||
"every account, the chip never renders, and a person whose phone owns " +
|
||||
"the reminders is shown a server-side switch that no longer decides " +
|
||||
"anything. The write path and the dispatch decision are correct — the " +
|
||||
"two tests above prove both — only the web reflection is missing its " +
|
||||
"end. The account-payload guard cannot see this: it asks that every " +
|
||||
"field in the payload has a reader, and this is a reader with no " +
|
||||
"field. Kept at full strength rather than relaxed to match today's " +
|
||||
"render.",
|
||||
);
|
||||
|
||||
await configureChannels(page);
|
||||
await pinAccountClockToMidday(page);
|
||||
const medicationId = await createOverdueMedication(page);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
/**
|
||||
* v1.32.36 — pair guard over the account payload (`GET /api/auth/me`).
|
||||
* v1.32.36 — pair guard over the account payload (`GET /api/auth/me`), in both
|
||||
* directions.
|
||||
*
|
||||
* The recurring failure this exists to prevent: a two-ended feature whose ends
|
||||
* ship in different releases. The column lands with the schema change, the
|
||||
@@ -33,12 +34,39 @@
|
||||
* be allowlisted with that reason written down, and the reason is an
|
||||
* assumption about another repository until an audit crosses over.
|
||||
*
|
||||
* ## The other direction: a reader with no field
|
||||
*
|
||||
* Everything above asks that a published field has a reader. It cannot see the
|
||||
* inverse — a client that reads a field the payload never published — and that
|
||||
* blind spot cost a shipped feature: the medication surface decided between
|
||||
* the server-side reminder switch and the client-managed chip on
|
||||
* `notificationPrefs.medication.clientManaged`, read off this payload, which
|
||||
* did not carry it. The flag was `undefined` for every account, so the chip
|
||||
* never rendered and a person whose phone owned the reminders was shown a
|
||||
* switch that decided nothing. Every guard was green.
|
||||
*
|
||||
* The last case closes it, and the shape it takes is deliberate. A typed guard
|
||||
* — one `AccountPayload` interface, `tsc` refusing an absent property — was the
|
||||
* first idea and does not work here: the consumers declare their own local
|
||||
* response interfaces (`interface UserPrefsResponse { … }`), so `tsc` checks
|
||||
* each file against its own private claim about the payload and agrees with
|
||||
* every one of them. So the check reads those local claims instead, out of the
|
||||
* file that makes them, and holds them against what the route publishes. It
|
||||
* asserts a non-zero match count, because a textual guard that silently
|
||||
* matches nothing is the failure mode this repository already has on record.
|
||||
*
|
||||
* Its limit, written down: it sees the response type a `/api/auth/me` read
|
||||
* names, so a consumer that types the read as `unknown` and indexes into it, or
|
||||
* imports its response type from another module, is invisible to it.
|
||||
*
|
||||
* Mutation check: append a field to the response literal in
|
||||
* `src/app/api/auth/me/route.ts` and this goes red; delete the practice-name
|
||||
* read in `health-record-export-panel.tsx` and it goes red too.
|
||||
* read in `health-record-export-panel.tsx` and it goes red too; delete
|
||||
* `notificationPrefs` from the response literal and the last case goes red
|
||||
* naming the medication notification section.
|
||||
*/
|
||||
import { readFileSync, readdirSync, statSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { join, relative } from "node:path";
|
||||
|
||||
import { describe, it, expect } from "vitest";
|
||||
|
||||
@@ -189,6 +217,31 @@ function readsField(text: string, field: string): boolean {
|
||||
return re.test(text);
|
||||
}
|
||||
|
||||
/**
|
||||
* Top-level member names of a locally declared `interface Name { … }`, or null
|
||||
* when the file does not declare one under that name (the type is imported, or
|
||||
* is a shared transport type this file only references).
|
||||
*
|
||||
* Members are read at one indent level, the same way the route's response
|
||||
* literal is read above — the tree is prettier-formatted, so a nested object's
|
||||
* keys sit deeper and are not mistaken for top-level fields.
|
||||
*/
|
||||
function declaredMembers(text: string, name: string): string[] | null {
|
||||
const open = new RegExp(`\\binterface\\s+${name}\\s*\\{`).exec(text);
|
||||
if (!open) return null;
|
||||
const start = open.index + open[0].length;
|
||||
const end = text.indexOf("\n}", start);
|
||||
if (end === -1) return null;
|
||||
const body = text.slice(start, end);
|
||||
return [
|
||||
...new Set(
|
||||
[...body.matchAll(/^ {2}([a-zA-Z_$][\w$]*)\??\s*[?:]/gm)].map(
|
||||
(m) => m[1],
|
||||
),
|
||||
),
|
||||
];
|
||||
}
|
||||
|
||||
describe("account payload consumer guard", () => {
|
||||
const source = readFileSync(ME_ROUTE, "utf8");
|
||||
const fields = payloadFields(source);
|
||||
@@ -294,4 +347,40 @@ describe("account payload consumer guard", () => {
|
||||
).toMatch(new RegExp(`active\\.${field}(?![\\w$])`));
|
||||
}
|
||||
});
|
||||
|
||||
it("every field a client types onto the account payload is published", () => {
|
||||
const published = new Set(fields);
|
||||
const offenders: string[] = [];
|
||||
let claimsChecked = 0;
|
||||
|
||||
for (const file of consumerFiles) {
|
||||
const text = stripComments(readFileSync(file, "utf8"));
|
||||
// The type argument of a read of THIS endpoint. `/api/auth/me/...`
|
||||
// siblings are different payloads, so the literal is anchored closed.
|
||||
for (const call of text.matchAll(
|
||||
/<\s*([A-Za-z_$][\w$]*)\s*>\s*\(\s*"\/api\/auth\/me"\s*\)/g,
|
||||
)) {
|
||||
const declared = declaredMembers(text, call[1]);
|
||||
if (declared === null) continue; // an imported / shared type
|
||||
claimsChecked += 1;
|
||||
for (const member of declared) {
|
||||
if (!published.has(member)) {
|
||||
offenders.push(
|
||||
`${relative(process.cwd(), file)} reads \`${member}\``,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
expect(
|
||||
claimsChecked,
|
||||
"no client names a response type for GET /api/auth/me, so this case matched nothing rather than proving anything",
|
||||
).toBeGreaterThan(0);
|
||||
|
||||
expect(
|
||||
offenders,
|
||||
"these clients type a field onto the account payload that the route does not publish, so it is `undefined` for every account and whatever it decides never happens",
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -30,6 +30,7 @@ import {
|
||||
getOperatorModuleAvailability,
|
||||
} from "@/lib/modules/gate";
|
||||
import { parseTourProgress } from "@/lib/onboarding/tour-progress";
|
||||
import { parseNotificationPrefs } from "@/lib/validations/notification-prefs";
|
||||
import { resolveAccountAccess } from "@/lib/sharing/account-access";
|
||||
import { recordSessionForPayload } from "@/lib/sharing/record-session-fence";
|
||||
|
||||
@@ -145,6 +146,16 @@ export const GET = apiHandler(async () => {
|
||||
// the account has never saved one, which is what makes the panel show the
|
||||
// named template on the first run rather than a silent server default.
|
||||
reportSelection: user.reportSelectionJson ?? null,
|
||||
// The account's resolved notification preferences — the same resolver
|
||||
// `GET /api/auth/me/notification-prefs` answers with, so the two surfaces
|
||||
// cannot disagree. Published here because the medication detail's
|
||||
// notification section decides between the server-side reminder switch and
|
||||
// the client-managed chip on `medication.clientManaged`, and it reads that
|
||||
// decision off THIS payload: the dedicated endpoint is not on the app-boot
|
||||
// path. An account that has never opted in resolves to the documented
|
||||
// defaults rather than to `undefined`, so the switch is what renders.
|
||||
// No extra query — the session already loads the row this column is on.
|
||||
notificationPrefs: parseNotificationPrefs(user.notificationPrefs ?? null),
|
||||
// v1.4.47 W3 — per-user Coach opt-out. Default `false` if the
|
||||
// column is absent (partial-deploy rollback safety, see migration
|
||||
// 0078 commentary). Every Coach mount point on the client checks
|
||||
|
||||
@@ -426,6 +426,17 @@ const accountPayload = z
|
||||
username: z.string(),
|
||||
accountAccess: accountAccessBlock,
|
||||
recordSession: recordSessionState.nullable(),
|
||||
// Enumerated because a client DECIDES on it rather than displays it: the
|
||||
// medication surface hides the server-side reminder switch when
|
||||
// `medication.clientManaged` is true, because the phone owns those
|
||||
// reminders and a switch that no longer decides anything is worse than no
|
||||
// switch. Always fully resolved — an account that never opted in reads
|
||||
// `false`, not absent. `PATCH /api/auth/me/notification-prefs` writes it;
|
||||
// the categories this object does not name are the rest of the open
|
||||
// preference envelope.
|
||||
notificationPrefs: z.looseObject({
|
||||
medication: z.looseObject({ clientManaged: z.boolean() }),
|
||||
}),
|
||||
})
|
||||
.meta({
|
||||
id: "AccountPayload",
|
||||
|
||||
Reference in new issue
Block a user