From e68041a3aff111c573646bf8414ed57616f37ef9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 21:39:19 +0100 Subject: [PATCH] fix(forms): keep one Idempotency-Key for the remaining create forms A screening, an illness episode, a life event, a symptom, a custom metric or an address-book entry created on a weak signal could be stored twice: the POST committed, the answer missed the timeout, and the second tap went out under a new key. Their routes already honour the key; the forms now post through useIdempotentPost like the other create forms, so the retry replays the first answer. The screening's save moves into a small hook beside MentalWellbeing, which sat at the function-size cap and now drops below it; the custom metric form's create-or-update call moves into one as well. --- .../baselines/source-function-lines.json | 5 +- .../custom-metrics/custom-metric-form.tsx | 23 +++++--- .../use-illness-save-settles.test.ts | 33 +++++++++++- src/components/illness/use-illness.ts | 6 ++- .../mental-health/mental-wellbeing.tsx | 52 ++++++++++++------- src/components/symptoms/use-symptoms.ts | 7 ++- src/components/timeline/use-timeline.ts | 8 ++- src/hooks/use-practitioners.ts | 6 ++- 8 files changed, 102 insertions(+), 38 deletions(-) diff --git a/src/__tests__/baselines/source-function-lines.json b/src/__tests__/baselines/source-function-lines.json index 7ac0176fa..ec9a32f35 100644 --- a/src/__tests__/baselines/source-function-lines.json +++ b/src/__tests__/baselines/source-function-lines.json @@ -1,7 +1,7 @@ { "$comment": "Production functions over 200 lines, keyed \"file :: name\", and their length. Maintained by src/__tests__/source-size-ratchet.test.ts.", - "count": 339, - "total": 110086, + "count": 338, + "total": 109885, "entries": { "app/achievements/page.tsx :: AchievementsPage": 206, "app/api/admin/backups/upload/route.ts :: apiHandler(arg)": 275, @@ -168,7 +168,6 @@ "components/medications/wizard/medication-wizard-dialog.tsx :: WizardDialogShell": 516, "components/medications/wizard/steps/step3-dose.tsx :: Step3Dose": 222, "components/medications/wizard/steps/step8-summary.tsx :: Step8Summary": 230, - "components/mental-health/mental-wellbeing.tsx :: MentalWellbeing": 201, "components/mood/manage/tag-groups-card.tsx :: TagGroupsCard": 256, "components/mood/manage/tag-manager-card.tsx :: TagManagerCard": 270, "components/mood/mood-form.tsx :: MoodForm": 514, diff --git a/src/components/custom-metrics/custom-metric-form.tsx b/src/components/custom-metrics/custom-metric-form.tsx index ee599f7b1..e0cb0a053 100644 --- a/src/components/custom-metrics/custom-metric-form.tsx +++ b/src/components/custom-metrics/custom-metric-form.tsx @@ -10,7 +10,8 @@ import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Switch } from "@/components/ui/switch"; import { Textarea } from "@/components/ui/textarea"; -import { ApiError, apiPatch, apiPost } from "@/lib/api/api-fetch"; +import { ApiError, apiPatch } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { localizedApiError } from "@/lib/api/localized-error"; import { useTranslations } from "@/lib/i18n/context"; @@ -34,6 +35,18 @@ function parseInteger(raw: string): number | null { return Number.isInteger(n) && n >= 0 ? n : null; } +/** + * Update the metric, or define a new one under one Idempotency-Key until the + * create lands, so a retry after a timeout is not a second metric. + */ +function useSaveCustomMetric(existing: CustomMetricDto | undefined) { + const post = useIdempotentPost(); + return (body: unknown) => + existing + ? apiPatch(`/api/custom-metrics/${existing.id}`, body) + : post("/api/custom-metrics", body); +} + interface CustomMetricFormProps { /** When set the form edits this metric; otherwise it defines a new one. */ existing?: CustomMetricDto; @@ -80,6 +93,7 @@ export function CustomMetricForm({ ); const [submitting, setSubmitting] = useState(false); const [error, setError] = useState(null); + const save = useSaveCustomMetric(existing); async function handleSubmit(e: React.FormEvent) { e.preventDefault(); @@ -110,12 +124,7 @@ export function CustomMetricForm({ setSubmitting(true); try { - const saved = existing - ? await apiPatch( - `/api/custom-metrics/${existing.id}`, - body, - ) - : await apiPost("/api/custom-metrics", body); + const saved = await save(body); toast.success( existing ? t("customMetrics.form.updatedToast") diff --git a/src/components/illness/__tests__/use-illness-save-settles.test.ts b/src/components/illness/__tests__/use-illness-save-settles.test.ts index d5212fe30..59a4b91c7 100644 --- a/src/components/illness/__tests__/use-illness-save-settles.test.ts +++ b/src/components/illness/__tests__/use-illness-save-settles.test.ts @@ -1,9 +1,11 @@ /** - * An episode save settles once the server stored it. + * An episode save settles once the server stored it, and stores it once. * * Every episode hook's `onSuccess` used to return the refresh of the illness * reads and the dashboard. A mutation waits for what `onSuccess` returns, so * the episode sheet kept its spinner until each of those reads had refetched. + * And a create retried after a timeout went out under a new key, so the + * server stored the episode twice. */ import { describe, expect, it, vi } from "vitest"; @@ -14,6 +16,11 @@ const client = vi.hoisted(() => ({ getQueryCache: () => ({ findAll: () => [] }), })); +vi.mock("react", async (importOriginal) => ({ + ...(await importOriginal()), + useRef: (initial: T) => ({ current: initial }), +})); + vi.mock("@tanstack/react-query", () => ({ // The hook's options are what is under test; return them as they are. useQuery: (options: unknown) => options, @@ -29,6 +36,7 @@ vi.mock("@/lib/api/api-fetch", () => ({ apiDelete: vi.fn(), })); +import { apiPost } from "@/lib/api/api-fetch"; import { useCreateEpisode, useDeleteEpisode, @@ -54,3 +62,26 @@ describe("episode saves", () => { expect(client.invalidateQueries).toHaveBeenCalled(); }); }); + +describe("creating an episode", () => { + it("retries under the same Idempotency-Key and starts the next one fresh", async () => { + const create = useCreateEpisode() as unknown as { + mutationFn: (input: unknown) => Promise; + }; + const post = vi.mocked(apiPost); + post.mockRejectedValueOnce(new TypeError("timed out")); + post.mockResolvedValue({ id: "e1" }); + const keyOf = (call: number) => + new Headers((post.mock.calls[call]![2] as RequestInit).headers).get( + "Idempotency-Key", + ); + + await expect(create.mutationFn({ name: "Flu" })).rejects.toThrow(); + await create.mutationFn({ name: "Flu" }); + await create.mutationFn({ name: "Cold" }); + + expect(keyOf(0)).toBeTruthy(); + expect(keyOf(1)).toBe(keyOf(0)); + expect(keyOf(2)).not.toBe(keyOf(0)); + }); +}); diff --git a/src/components/illness/use-illness.ts b/src/components/illness/use-illness.ts index f333c04ea..c0ed5d8d7 100644 --- a/src/components/illness/use-illness.ts +++ b/src/components/illness/use-illness.ts @@ -17,6 +17,7 @@ import { apiPatch, apiPost, } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { illnessDependentKeys, invalidateKeys, @@ -164,9 +165,12 @@ function useInvalidateIllness() { /** Create an episode. */ export function useCreateEpisode() { const invalidate = useInvalidateIllness(); + // One key until a save lands, so a retry after a timeout is not a second + // episode. + const post = useIdempotentPost(); return useMutation({ mutationFn: (input: IllnessEpisodeCreateInput) => - apiPost("/api/illness/episodes", input), + post("/api/illness/episodes", input), onSuccess: invalidate, }); } diff --git a/src/components/mental-health/mental-wellbeing.tsx b/src/components/mental-health/mental-wellbeing.tsx index 0cdebd557..88a14a5b6 100644 --- a/src/components/mental-health/mental-wellbeing.tsx +++ b/src/components/mental-health/mental-wellbeing.tsx @@ -31,7 +31,8 @@ import { useMemo, useState } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useTranslations } from "@/lib/i18n/context"; -import { apiGetList, apiPost } from "@/lib/api/api-fetch"; +import { apiGetList } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { invalidateKeys, queryKeys, @@ -57,9 +58,35 @@ import type { } from "./types"; import { ListCapNotice } from "@/components/ui/list-cap-notice"; +/** + * The check-in's save, under one Idempotency-Key until it lands, so a second + * tap after a timeout replays the stored screening instead of adding one. + */ +function useSubmitAssessment(onSaved: (data: CreateResponse) => void) { + const queryClient = useQueryClient(); + const post = useIdempotentPost(); + return useMutation({ + mutationFn: (body: unknown) => + post("/api/mental-health/assessments", body), + onSuccess: (data) => { + onSaved(data); + // A screening is listed on its day, and it can satisfy a due checkup + // reminder, so the checkups list and the Today rail must repaint. + // Honest constraint: the server satisfy runs async via pg-boss, so + // this invalidation narrows the stale window but cannot close it — a + // refetch racing the job may still see the pre-satisfy row. + void invalidateKeys(queryClient, [ + queryKeys.mentalHealthAssessments(), + queryKeys.measurementReminders(), + queryKeys.dayRoot(), + ]); + void refetchInactiveDailyReads(queryClient); + }, + }); +} + export function MentalWellbeing() { const { t, locale } = useTranslations(); - const queryClient = useQueryClient(); const [phase, setPhase] = useState("choose"); const [instrument, setInstrument] = useState("PHQ9"); @@ -94,24 +121,9 @@ export function MentalWellbeing() { }); const history = historyPage?.data; - const mutation = useMutation({ - mutationFn: (body: unknown) => - apiPost("/api/mental-health/assessments", body), - onSuccess: (data) => { - setResult(data); - setPhase("result"); - // A screening is listed on its day, and it can satisfy a due checkup - // reminder, so the checkups list and the Today rail must repaint. - // Honest constraint: the server satisfy runs async via pg-boss, so - // this invalidation narrows the stale window but cannot close it — a - // refetch racing the job may still see the pre-satisfy row. - void invalidateKeys(queryClient, [ - queryKeys.mentalHealthAssessments(), - queryKeys.measurementReminders(), - queryKeys.dayRoot(), - ]); - void refetchInactiveDailyReads(queryClient); - }, + const mutation = useSubmitAssessment((data) => { + setResult(data); + setPhase("result"); }); // Most-recent assessment per instrument for the landing's "last result" line. diff --git a/src/components/symptoms/use-symptoms.ts b/src/components/symptoms/use-symptoms.ts index 286cdaed1..a5f3094d7 100644 --- a/src/components/symptoms/use-symptoms.ts +++ b/src/components/symptoms/use-symptoms.ts @@ -16,7 +16,7 @@ import { type QueryClient, } from "@tanstack/react-query"; -import { apiDelete, apiGet, apiPatch, apiPost } from "@/lib/api/api-fetch"; +import { apiDelete, apiGet, apiPatch } from "@/lib/api/api-fetch"; import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { invalidateKeys, queryKeys } from "@/lib/query-keys"; import type { @@ -147,9 +147,12 @@ function useInvalidateSymptoms() { export function useCreateSymptomDefinition() { const invalidate = useInvalidateSymptoms(); + // One key until a save lands, so a retry after a timeout is not a second + // symptom. + const post = useIdempotentPost(); return useMutation({ mutationFn: (input: { label: string; icon?: string | null }) => - apiPost("/api/symptoms/definitions", input), + post("/api/symptoms/definitions", input), onSuccess: invalidate, }); } diff --git a/src/components/timeline/use-timeline.ts b/src/components/timeline/use-timeline.ts index e3a49e242..ab0aaae45 100644 --- a/src/components/timeline/use-timeline.ts +++ b/src/components/timeline/use-timeline.ts @@ -10,7 +10,8 @@ */ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { apiDelete, apiGet, apiPatch, apiPost } from "@/lib/api/api-fetch"; +import { apiDelete, apiGet, apiPatch } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import type { LifeEventCreateInput, LifeEventDTO, @@ -75,9 +76,12 @@ export function useLifeEventMutations() { const invalidate = () => invalidateKeys(qc, [queryKeys.timelineRoot(), queryKeys.dayRoot()]); + // One key until a save lands, so a retry after a timeout is not a second + // event. + const post = useIdempotentPost(); const create = useMutation({ mutationFn: (body: LifeEventCreateInput) => - apiPost("/api/life-events", body), + post("/api/life-events", body), onSuccess: invalidate, }); const update = useMutation({ diff --git a/src/hooks/use-practitioners.ts b/src/hooks/use-practitioners.ts index 3fdd4145e..3e9bedd26 100644 --- a/src/hooks/use-practitioners.ts +++ b/src/hooks/use-practitioners.ts @@ -11,6 +11,7 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { apiDelete, apiGetList, apiPatch, apiPost } from "@/lib/api/api-fetch"; +import { useIdempotentPost } from "@/lib/api/use-idempotent-post"; import { encounterDependentKeys, invalidateKeys, @@ -70,10 +71,11 @@ export function usePractitionerMutations() { refetchInactiveDailyReads(qc), ]); + // One key for as long as the sheet that mounts this is open. + const post = useIdempotentPost(); const create = useMutation({ mutationKey: queryKeys.practitionerCreate(), - mutationFn: (body: PractitionerWriteBody) => - apiPost(BASE, body), + mutationFn: (body: PractitionerWriteBody) => post(BASE, body), onSuccess: invalidate, });