From f761500247eb3bccc98905eba4eacc197af47b2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 20:02:40 +0100 Subject: [PATCH] fix(forms): send one Idempotency-Key per opened create form A mood, symptom, lab value, custom metric value, unscheduled dose, allergy, vaccination, visit or measurement saved on a weak signal could be stored twice: the POST committed, the answer missed the 15 s timeout, the form said it could not save, and the second tap was a new call with a new key, so the server ran it again. useIdempotentPost keeps one key until a save lands and renews it for the next entry, so the retry replays the first answer. The create forms and the three create hooks post through it; the comment on the per-call key in api-fetch now says what that key does and does not cover. LabForm and MoodForm each gain the one line that mounts the key, so their function-size baselines go up by one. The intake toast guard now recognises a dose posted through the hook. --- .../baselines/source-function-lines.json | 4 +- .../intake-toast-single-decision.test.ts | 21 +++++- .../custom-metric-entry-form.tsx | 5 +- .../medication-intake-quick-add.test.tsx | 2 +- .../dashboard/medication-intake-quick-add.tsx | 6 +- src/components/labs/lab-form.tsx | 6 +- .../measurements/measurement-form.tsx | 9 ++- src/components/mood/mood-form.tsx | 5 +- src/components/records/allergy-form.tsx | 6 +- src/components/symptoms/use-symptoms.ts | 5 +- .../vaccinations/use-vaccinations.ts | 6 +- src/hooks/use-encounters.ts | 13 ++-- .../__tests__/use-idempotent-post.test.tsx | 75 +++++++++++++++++++ src/lib/api/api-fetch.ts | 11 ++- src/lib/api/use-idempotent-post.ts | 33 ++++++++ 15 files changed, 172 insertions(+), 35 deletions(-) create mode 100644 src/lib/api/__tests__/use-idempotent-post.test.tsx create mode 100644 src/lib/api/use-idempotent-post.ts diff --git a/src/__tests__/baselines/source-function-lines.json b/src/__tests__/baselines/source-function-lines.json index 7c156b9d6..97ec5e369 100644 --- a/src/__tests__/baselines/source-function-lines.json +++ b/src/__tests__/baselines/source-function-lines.json @@ -134,7 +134,7 @@ "components/labs/biomarker-merge-sheet.tsx :: BiomarkerMergeSheet": 239, "components/labs/lab-biomarker-chart.tsx :: LabBiomarkerChart": 370, "components/labs/lab-biomarker-detail.tsx :: LabBiomarkerDetail": 481, - "components/labs/lab-form.tsx :: LabForm": 473, + "components/labs/lab-form.tsx :: LabForm": 474, "components/labs/lab-history-list.tsx :: LabHistoryList": 534, "components/labs/ocr-review-dialog.tsx :: OcrReviewDialog": 359, "components/labs/ocr-row-editor.tsx :: OcrRowEditor": 378, @@ -173,7 +173,7 @@ "components/mental-health/mental-wellbeing.tsx :: MentalWellbeing": 202, "components/mood/manage/tag-groups-card.tsx :: TagGroupsCard": 256, "components/mood/manage/tag-manager-card.tsx :: TagManagerCard": 270, - "components/mood/mood-form.tsx :: MoodForm": 513, + "components/mood/mood-form.tsx :: MoodForm": 514, "components/mood/mood-list.tsx :: MoodList": 1109, "components/mood/mood-tag-picker.tsx :: MoodTagPicker": 257, "components/onboarding/confirm-screen.tsx :: ConfirmScreen": 255, diff --git a/src/__tests__/intake-toast-single-decision.test.ts b/src/__tests__/intake-toast-single-decision.test.ts index 0c3927a20..37d5faf6d 100644 --- a/src/__tests__/intake-toast-single-decision.test.ts +++ b/src/__tests__/intake-toast-single-decision.test.ts @@ -111,12 +111,31 @@ export function intakeFacts(source: string, fileName: string): IntakeFacts { callsHelper: false, }; + // A create form posts through the function `useIdempotentPost()` returns + // (one key per opened form), bound to a local name. + const posters = new Set(); + const findPosters = (node: ts.Node): void => { + if ( + ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && + node.initializer && + ts.isCallExpression(node.initializer) && + ts.isIdentifier(node.initializer.expression) && + (aliases.get(node.initializer.expression.text) ?? + node.initializer.expression.text) === "useIdempotentPost" + ) { + posters.add(node.name.text); + } + node.forEachChild(findPosters); + }; + parsed.forEachChild(findPosters); + const visit = (node: ts.Node): void => { if (ts.isCallExpression(node)) { const callee = node.expression; if (ts.isIdentifier(callee)) { const name = aliases.get(callee.text) ?? callee.text; - if (name === "apiPost") { + if (name === "apiPost" || posters.has(callee.text)) { const url = urlText(node); if (url !== null && url.endsWith("/intake")) facts.postsIntake = true; } diff --git a/src/components/custom-metrics/custom-metric-entry-form.tsx b/src/components/custom-metrics/custom-metric-entry-form.tsx index 01dbc2212..15078ba6e 100644 --- a/src/components/custom-metrics/custom-metric-entry-form.tsx +++ b/src/components/custom-metrics/custom-metric-entry-form.tsx @@ -11,8 +11,8 @@ import { DateTimeField } from "@/components/ui/date-time-field"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Textarea } from "@/components/ui/textarea"; -import { apiPost } from "@/lib/api/api-fetch"; import { localizedApiError } from "@/lib/api/localized-error"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; @@ -56,6 +56,7 @@ export function CustomMetricEntryForm({ const { t } = useTranslations(); const formId = useId(); const queryClient = useQueryClient(); + const post = useIdempotentPost(); const [value, setValue] = useState(""); const [measuredAt, setMeasuredAt] = useState(() => nowLocal()); @@ -86,7 +87,7 @@ export function CustomMetricEntryForm({ setSubmitting(true); try { - await apiPost( + await post( `/api/custom-metrics/${customMetricId}/entries`, body, ); diff --git a/src/components/dashboard/__tests__/medication-intake-quick-add.test.tsx b/src/components/dashboard/__tests__/medication-intake-quick-add.test.tsx index 1d7d1eed5..28ab5a317 100644 --- a/src/components/dashboard/__tests__/medication-intake-quick-add.test.tsx +++ b/src/components/dashboard/__tests__/medication-intake-quick-add.test.tsx @@ -399,7 +399,7 @@ describe("submit invalidation — the quick-add is a capture-picker surface, not // AFTER the intake POST landed, and after the sheet was told it is done: // the save does not wait for the refetches before it closes. expect(componentSrc).toMatch( - /async function handleSubmit\([\s\S]*?apiPost[\s\S]*?onSuccess\?\.\(\);\s*(?:\/\/[^\n]*\n\s*)*void invalidateMedicationReads\(queryClient\);/, + /async function handleSubmit\([\s\S]*?await post<[\s\S]*?onSuccess\?\.\(\);\s*(?:\/\/[^\n]*\n\s*)*void invalidateMedicationReads\(queryClient\);/, ); // The bundle carries every inline compliance chart, so no second sweep. expect(medicationDependentKeys).toContainEqual( diff --git a/src/components/dashboard/medication-intake-quick-add.tsx b/src/components/dashboard/medication-intake-quick-add.tsx index 1c63d9ae6..0c662b618 100644 --- a/src/components/dashboard/medication-intake-quick-add.tsx +++ b/src/components/dashboard/medication-intake-quick-add.tsx @@ -27,7 +27,8 @@ import { } from "@/lib/medications/default-medication"; import { useAuth } from "@/hooks/use-auth"; import { useActiveRecordName } from "@/hooks/use-record-capabilities"; -import { apiGet, apiPost } from "@/lib/api/api-fetch"; +import { apiGet } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { localizedApiError } from "@/lib/api/localized-error"; import { intakeToastOptions, @@ -119,6 +120,7 @@ export function MedicationIntakeQuickAdd({ }: MedicationIntakeQuickAddProps) { const { t } = useTranslations(); const queryClient = useQueryClient(); + const post = useIdempotentPost(); // v1.16.9 — the auto-pick reasons in the PROFILE timezone (the same // source the cards use); Berlin stays the last-resort fallback so // logged-out mounts behave unchanged. @@ -226,7 +228,7 @@ export function MedicationIntakeQuickAdd({ // v1.30.1 — the route returns the created event // (`apiSuccess(event, 201)`), same shape the card's take/skip path // already reads; the id drives the new Undo action below. - const created = await apiPost<{ id?: string } | undefined>( + const created = await post<{ id?: string } | undefined>( `/api/medications/${medicationId}/intake`, { takenAt: timestamp, diff --git a/src/components/labs/lab-form.tsx b/src/components/labs/lab-form.tsx index 83652c303..f28ccc36e 100644 --- a/src/components/labs/lab-form.tsx +++ b/src/components/labs/lab-form.tsx @@ -21,7 +21,8 @@ import { SelectValue, } from "@/components/ui/select"; import { Textarea } from "@/components/ui/textarea"; -import { apiGet, apiPost } from "@/lib/api/api-fetch"; +import { apiGet } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { localizedApiError } from "@/lib/api/localized-error"; import { isUnreadableRange, @@ -144,6 +145,7 @@ export function LabForm({ const labNumber = useLabNumber(); const recordName = useActiveRecordName(); const queryClient = useQueryClient(); + const post = useIdempotentPost(); const formId = useId(); const { data: catalog, isLoading: catalogLoading } = useQuery({ @@ -265,7 +267,7 @@ export function LabForm({ const printed = isQualitative ? null : parseReferenceRange(sourceRange, selected?.unit); - const created = await apiPost("/api/labs", { + const created = await post("/api/labs", { biomarkerId, ...(isQualitative ? { valueText: qualitativeValue } diff --git a/src/components/measurements/measurement-form.tsx b/src/components/measurements/measurement-form.tsx index 9f7ba309b..c3a89b4a6 100644 --- a/src/components/measurements/measurement-form.tsx +++ b/src/components/measurements/measurement-form.tsx @@ -33,8 +33,8 @@ import { measurementDependentKeys, refetchInactiveDailyReads, } from "@/lib/query-keys"; -import { apiPost } from "@/lib/api/api-fetch"; import { localizedApiError } from "@/lib/api/localized-error"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { MEASUREMENT_NOTES_MAX_LENGTH } from "@/lib/measurements/notes-limit"; import { getLastUsedMeasurementType, @@ -327,6 +327,7 @@ export function MeasurementForm({ const { t } = useTranslations(); const recordName = useActiveRecordName(); const queryClient = useQueryClient(); + const post = useIdempotentPost(); const unitDisplay = useUnitDisplay(); const { user } = useAuth(); // Glucose is its own display axis, separate from metric/imperial: it is @@ -448,7 +449,7 @@ export function MeasurementForm({ }); } - await apiPost("/api/measurements", batch); + await post("/api/measurements", batch); } else if (isKetoneMode) { // One fingerstick, posted as one transaction (`ketone-fingerstick.ts`). const batch = ketoneFingerstickBatch({ @@ -463,7 +464,7 @@ export function MeasurementForm({ setError(t("measurements.saveError")); return; } - await apiPost("/api/measurements", batch); + await post("/api/measurements", batch); } else { // Single measurement. Canonical storage stays SI: an imperial user // types in lb / in / °F and we invert to the canonical unit at the @@ -508,7 +509,7 @@ export function MeasurementForm({ : inverted; } canonicalValue = entryValueToCanonical(type, canonicalValue); - await apiPost("/api/measurements", { + await post("/api/measurements", { type, value: canonicalValue, measuredAt: timestamp, diff --git a/src/components/mood/mood-form.tsx b/src/components/mood/mood-form.tsx index e79bf95c6..28c14dbc3 100644 --- a/src/components/mood/mood-form.tsx +++ b/src/components/mood/mood-form.tsx @@ -42,8 +42,8 @@ import { moodDependentKeys, refetchInactiveDailyReads, } from "@/lib/query-keys"; -import { apiPost } from "@/lib/api/api-fetch"; import { localizedApiError } from "@/lib/api/localized-error"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { SheetSection, SheetSectionCount } from "@/components/ui/sheet-section"; import { MoodTagPicker } from "./mood-tag-picker"; import { @@ -125,6 +125,7 @@ export function MoodForm({ }: MoodFormProps) { const { t } = useTranslations(); const queryClient = useQueryClient(); + const post = useIdempotentPost(); const [mood, setMood] = useState(""); const { getRadioProps: getMoodRadioProps } = useRovingRadioGroup({ @@ -236,7 +237,7 @@ export function MoodForm({ const trimmedNote = note.trim(); - const saved = await apiPost("/api/mood-entries", { + const saved = await post("/api/mood-entries", { mood, tags: tags.length > 0 ? tags : undefined, tagKeys: tagKeys.length > 0 ? tagKeys : undefined, diff --git a/src/components/records/allergy-form.tsx b/src/components/records/allergy-form.tsx index 1270ce001..c4b979e2a 100644 --- a/src/components/records/allergy-form.tsx +++ b/src/components/records/allergy-form.tsx @@ -17,7 +17,8 @@ import { SelectValue, } from "@/components/ui/select"; import { Textarea } from "@/components/ui/textarea"; -import { apiPost, apiPut } from "@/lib/api/api-fetch"; +import { apiPut } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { localizedApiError } from "@/lib/api/localized-error"; import { allergyCategoryEnum, @@ -67,6 +68,7 @@ export function AllergyForm({ footerSlot, }: AllergyFormProps) { const { t } = useTranslations(); + const post = useIdempotentPost(); const formId = useId(); const [substance, setSubstance] = useState(existing?.substance ?? ""); @@ -106,7 +108,7 @@ export function AllergyForm({ try { const saved = existing ? await apiPut(`/api/allergies/${existing.id}`, body) - : await apiPost("/api/allergies", body); + : await post("/api/allergies", body); toast.success( existing ? t("records.allergies.form.updatedToast") diff --git a/src/components/symptoms/use-symptoms.ts b/src/components/symptoms/use-symptoms.ts index 711a7c96a..286cdaed1 100644 --- a/src/components/symptoms/use-symptoms.ts +++ b/src/components/symptoms/use-symptoms.ts @@ -17,6 +17,7 @@ import { } from "@tanstack/react-query"; import { apiDelete, apiGet, apiPatch, apiPost } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { invalidateKeys, queryKeys } from "@/lib/query-keys"; import type { SymptomDefinitionDTO, @@ -184,11 +185,13 @@ export function useDeleteSymptomDefinition() { }); } +/** Under one key for as long as the entry form that mounts it is open. */ export function useLogSymptomEvent() { const invalidate = useInvalidateSymptoms(); + const post = useIdempotentPost(); return useMutation({ mutationFn: (input: SymptomEventCreateInput) => - apiPost("/api/symptoms/events", input), + post("/api/symptoms/events", input), onSuccess: invalidate, }); } diff --git a/src/components/vaccinations/use-vaccinations.ts b/src/components/vaccinations/use-vaccinations.ts index a2c9c8656..5d4258401 100644 --- a/src/components/vaccinations/use-vaccinations.ts +++ b/src/components/vaccinations/use-vaccinations.ts @@ -21,6 +21,7 @@ import { apiPatch, apiPost, } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { invalidateReminderReads } from "@/hooks/use-measurement-reminders"; import { invalidateKeys, @@ -133,10 +134,11 @@ export function useVaccinationMutations() { invalidateReminderReads(qc), ]); + // One key for as long as the sheet that mounts this is open. + const post = useIdempotentPost(); const create = useMutation({ mutationKey: queryKeys.vaccinationCreate(), - mutationFn: (body: VaccinationWriteBody) => - apiPost(BASE, body), + mutationFn: (body: VaccinationWriteBody) => post(BASE, body), onSuccess: invalidate, }); diff --git a/src/hooks/use-encounters.ts b/src/hooks/use-encounters.ts index d140be832..6cb207374 100644 --- a/src/hooks/use-encounters.ts +++ b/src/hooks/use-encounters.ts @@ -19,13 +19,8 @@ import { useQueryClient, } from "@tanstack/react-query"; -import { - apiDelete, - apiGet, - apiGetList, - apiPatch, - apiPost, -} from "@/lib/api/api-fetch"; +import { apiDelete, apiGet, apiGetList, apiPatch } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { invalidateReminderReads } from "@/hooks/use-measurement-reminders"; import { encounterDependentKeys, @@ -175,9 +170,11 @@ export function useEncounterMutations() { invalidateReminderReads(qc), ]); + // One key for as long as the sheet that mounts this is open. + const post = useIdempotentPost(); const create = useMutation({ mutationKey: queryKeys.encounterCreate(), - mutationFn: (body: EncounterWriteBody) => apiPost(BASE, body), + mutationFn: (body: EncounterWriteBody) => post(BASE, body), onSuccess: invalidate, }); diff --git a/src/lib/api/__tests__/use-idempotent-post.test.tsx b/src/lib/api/__tests__/use-idempotent-post.test.tsx new file mode 100644 index 000000000..cd307c480 --- /dev/null +++ b/src/lib/api/__tests__/use-idempotent-post.test.tsx @@ -0,0 +1,75 @@ +/** + * A second tap on Save after the first answer was lost must not store the + * entry twice. The per-call key the transport mints changes with every + * call, so a create form sends one key until a save lands. + */ +import { renderToStaticMarkup } from "react-dom/server"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { useIdempotentPost } from "../use-idempotent-post"; + +type Post = ReturnType; + +function openForm(): Post { + let post: Post | null = null; + function Probe() { + post = useIdempotentPost(); + return null; + } + renderToStaticMarkup(); + return post!; +} + +const fetchMock = vi.fn(); + +function keyOf(call: number): string | null { + const init = fetchMock.mock.calls[call]![1] as RequestInit; + return new Headers(init.headers).get("Idempotency-Key"); +} + +const stored = () => + new Response(JSON.stringify({ data: { id: "e1" }, error: null }), { + status: 201, + headers: { "Content-Type": "application/json" }, + }); + +beforeEach(() => vi.stubGlobal("fetch", fetchMock)); +afterEach(() => { + fetchMock.mockReset(); + vi.unstubAllGlobals(); +}); + +describe("useIdempotentPost", () => { + it("sends the first key again when the first answer never arrived", async () => { + fetchMock + .mockRejectedValueOnce(new DOMException("timed out", "TimeoutError")) + .mockResolvedValueOnce(stored()); + const post = openForm(); + + await expect(post("/api/mood-entries", { mood: "GOOD" })).rejects.toThrow(); + await post("/api/mood-entries", { mood: "GOOD" }); + + expect(keyOf(0)).toBeTruthy(); + expect(keyOf(1)).toBe(keyOf(0)); + }); + + it("takes a new key for the next entry once a save landed", async () => { + fetchMock.mockResolvedValueOnce(stored()).mockResolvedValueOnce(stored()); + const post = openForm(); + + await post("/api/mood-entries", { mood: "GOOD" }); + await post("/api/mood-entries", { mood: "GOOD" }); + + expect(keyOf(1)).not.toBe(keyOf(0)); + }); + + it("keeps a header the caller set", async () => { + fetchMock.mockResolvedValueOnce(stored()); + const post = openForm(); + + await post("/api/x", {}, { headers: { Prefer: "respond-async" } }); + + const init = fetchMock.mock.calls[0]![1] as RequestInit; + expect(new Headers(init.headers).get("Prefer")).toBe("respond-async"); + }); +}); diff --git a/src/lib/api/api-fetch.ts b/src/lib/api/api-fetch.ts index 2a6adcd8b..15f2e28a2 100644 --- a/src/lib/api/api-fetch.ts +++ b/src/lib/api/api-fetch.ts @@ -262,12 +262,11 @@ export function apiGet( * Attach a per-call `Idempotency-Key` unless the caller already set one. * * Every `apiHandler` route accepts the header, and the write routes - * wrapped in `withIdempotency` dedupe on it — but almost no client call - * site sent one, so a transport-level replay of a create (a retried - * request, a re-sent form submission) could land a duplicate dose or - * vaccination and shift every derived series. Minting the key here - * protects every `apiPost` / `apiPatch` surface at once; a caller with a - * real natural key still wins by setting the header itself. + * wrapped in `withIdempotency` dedupe on it. A key minted per call covers a + * transport-level replay of that one request. A second tap on Save is a + * new call and gets a new key, so a create form sends one key for as long + * as it is open (`useIdempotentPost`); a caller that sets the header + * itself always wins. */ function withDefaultIdempotencyKey(init: RequestInit): RequestInit { const headers = new Headers(init.headers); diff --git a/src/lib/api/use-idempotent-post.ts b/src/lib/api/use-idempotent-post.ts new file mode 100644 index 000000000..4bd1ac40a --- /dev/null +++ b/src/lib/api/use-idempotent-post.ts @@ -0,0 +1,33 @@ +"use client"; + +import { useRef } from "react"; + +import { apiPost } from "@/lib/api/api-fetch"; +import { randomId } from "@/lib/random-id"; + +/** + * `apiPost` for a create form, under one `Idempotency-Key` until a save lands. + * + * The transport mints a fresh key per call, which covers a replayed request + * but not a second tap on Save: when the first answer never arrived (the 15 s + * timeout on a weak signal) the entry is already stored, and the retry is a + * new call with a new key, so the server stored it twice. Every submit + * through this function sends the same key until one succeeds, so the retry + * replays the first answer instead; a retry with a changed body is a + * different request and runs as one. The next entry gets a new key. + */ +export function useIdempotentPost() { + const key = useRef(null); + return async function post( + path: string, + body?: unknown, + init?: RequestInit, + ): Promise { + key.current ??= randomId(); + const headers = new Headers(init?.headers); + headers.set("Idempotency-Key", key.current); + const result = await apiPost(path, body, { ...init, headers }); + key.current = null; + return result; + }; +}