From 845b89255d6aa47f4e50f4efacb2d99d29ee06d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 30 Jul 2026 23:17:53 +0200 Subject: [PATCH] fix(nutrients): audit every batch ingest call, not only the ones that wrote a row nutrient.batch.ingest only reached the durable AuditLog table when inserted + updated > 0, so a batch where every entry was rejected (unit mismatch, out-of-range value, invalid day, a write failure) left the exact same empty trail as a phone that never called the endpoint at all. That made an empty nutrient_intake_days table unanswerable from the durable record - the question this surface exists to answer. Record the call unconditionally, the same shape measurement.batch.ingest and workout.batch.ingest already use (counts only, no per-entry health values, no user-typed content), plus an aggregate skip-reason breakdown so an operator can tell, from the row alone, that the phone posted and why nothing landed. HTTP status and the per-entry response shape are unchanged - a batch endpoint returning 200 with per-entry outcomes on a partial failure is the correct, documented contract and native clients already decode it. --- src/app/api/nutrients/batch/route.ts | 44 +++++++++++++++++------ tests/integration/nutrients-batch.test.ts | 43 ++++++++++++++++++++++ 2 files changed, 77 insertions(+), 10 deletions(-) diff --git a/src/app/api/nutrients/batch/route.ts b/src/app/api/nutrients/batch/route.ts index 68e6ce217..cdfadcc57 100644 --- a/src/app/api/nutrients/batch/route.ts +++ b/src/app/api/nutrients/batch/route.ts @@ -306,17 +306,40 @@ async function postBatch(request: NextRequest): Promise { } } + // v1.34.3 — per-reason skip breakdown, durable AND ephemeral. Computed + // once from the closed `reason` set (never raw exception text — see the + // upsert `catch` blocks above), so this carries only counts and reason + // codes, never a per-entry health value or anything user-typed. + const skippedByReason: Record = {}; + for (const s of skipped) { + skippedByReason[s.reason] = (skippedByReason[s.reason] ?? 0) + 1; + } + + // v1.34.3 — audit UNCONDITIONALLY. This used to fire only when + // `inserted + updated > 0`, which made it the ONLY durable trace of the + // call — so a batch where every entry was skipped (say, a systemic unit + // mismatch) left the exact same empty AuditLog record as a phone that + // never posted at all. An operator investigating an empty + // `nutrient_intake_days` table could not tell "the client never called" + // from "the client called and every entry was rejected" without that + // record. Mirrors `measurement.batch.ingest` / `workout.batch.ingest` + // (unconditional, counts-only, no per-entry health values) plus the + // aggregate skip-reason breakdown so "did it post, and why did nothing + // land" is answerable from the durable row alone, without a Wide-Event + // trail that (unlike this table) isn't guaranteed to still exist. + await auditLog("nutrient.batch.ingest", { + userId: user.id, + ipAddress: getClientIp(request), + details: { + processed: entries.length, + inserted, + updated, + skipped: skipped.length, + skippedByReason, + }, + }); + if (inserted + updated > 0) { - await auditLog("nutrient.batch.ingest", { - userId: user.id, - ipAddress: getClientIp(request), - details: { - processed: entries.length, - inserted, - updated, - skipped: skipped.length, - }, - }); // v1.30.3 (QA F9) — a landed row changes the dashboard's water/nutrient // tile; without this the snapshot kept serving the pre-sync figure // until the analytics fresh TTL lapsed (~3 min). Mirrors the manual @@ -333,6 +356,7 @@ async function postBatch(request: NextRequest): Promise { inserted, updated, skipped: skipped.length, + skipped_by_reason: skippedByReason, }, }); diff --git a/tests/integration/nutrients-batch.test.ts b/tests/integration/nutrients-batch.test.ts index 49529bc8d..10836c34a 100644 --- a/tests/integration/nutrients-batch.test.ts +++ b/tests/integration/nutrients-batch.test.ts @@ -187,6 +187,49 @@ describe("POST /api/nutrients/batch (real Postgres)", () => { expect(count).toBe(0); }); + // Before this, `nutrient.batch.ingest` audited only when + // `inserted + updated > 0` — the ONLY durable record of the call — so a + // batch where every entry was rejected left the exact same empty + // AuditLog trail as a phone that never posted at all. An operator could + // not tell the two apart from the durable record. + it("records an audit-log row even when every entry is rejected", async () => { + const { POST } = await import("@/app/api/nutrients/batch/route"); + const res = await POST( + postReq({ + entries: [ + { day: recentDay(1), nutrient: "vitamin_d", unit: "mg", amount: 20 }, + { day: recentDay(2), nutrient: "iron", unit: "mg", amount: 999 }, + ], + }), + ); + const json = (await res.json()) as BatchEnvelope; + expect(json.data.inserted).toBe(0); + expect(json.data.updated).toBe(0); + expect(json.data.skipped).toHaveLength(2); + + const rows = await getPrismaClient().auditLog.findMany({ + where: { userId: OPTED_IN_USER, action: "nutrient.batch.ingest" }, + }); + expect(rows).toHaveLength(1); + const details = JSON.parse(rows[0].details ?? "{}") as { + processed: number; + inserted: number; + updated: number; + skipped: number; + skippedByReason: Record; + }; + expect(details).toMatchObject({ + processed: 2, + inserted: 0, + updated: 0, + skipped: 2, + }); + expect(details.skippedByReason).toMatchObject({ + unit_mismatch: 1, + value_out_of_range: 1, + }); + }); + it("refuses ingest 403 module.disabled for an account that never opted in", async () => { const { POST } = await import("@/app/api/nutrients/batch/route"); cookieJar.clear();