fix(api): finish the pagination tiebreaker, and state the tie premise correctly

The comment on the measurements list justified the tiebreaker with an Apple
Health import stamping long runs with one instant, and said the unique index
forbids two live measurements of the same type at that instant. The index is
`(userId, type, measuredAt, source, sleepStage)`, so the named example is the
one case it does block — while ties survive across `type` (the `stats:` daily
rollups), across `source` (two writers landing together) and across
`sleepStage`. Same conclusion, honest premise.

The per-day drill-down on the same route still sorted on `measuredAt` alone.
It slices in memory rather than with `skip`, so nothing drifts between pages,
but two identical requests were free to render a tied block in different
orders, and a list that reshuffles on refresh reads as data changing.

Three of the tiebreakered routes had nothing pinning the order: custom-metric
entries, the per-medication intake history (both sort arms, so the NULLS LAST
pin cannot be lost to the tiebreaker) and the mental-health assessment
history. Each now asserts the `orderBy` the way the illness day-log test does.
This commit is contained in:
Marc-André Bombeck committed 2026-09-10 05:07:02 +02:00
1 parent 2c63b0c4a7
commit 42100d94a7
4 files changed
+70 -8

No files matched your search

@@ -90,10 +90,18 @@ describe("GET /api/custom-metrics/[id]/entries", () => {
expect(body.data.meta.total).toBe(5);
expect(body.data.meta.limit).toBe(1);
const findCall = vi.mocked(prisma.customMetricEntry.findMany).mock
.calls[0][0] as { where: Record<string, unknown>; take: number };
.calls[0][0] as {
where: Record<string, unknown>;
take: number;
orderBy: unknown;
};
expect(findCall.where.userId).toBe("user-1");
expect(findCall.where.customMetricId).toBe("cm-1");
expect(findCall.take).toBe(1);
// `measuredAt` is not unique on a custom metric — a batch of readings can
// share one instant — so offset paging over it alone can repeat a row on
// one page and drop another.
expect(findCall.orderBy).toEqual([{ measuredAt: "desc" }, { id: "desc" }]);
});
it("404s a cross-user / unknown metric", async () => {
+12 -5
View File
@@ -190,7 +190,12 @@ export const GET = apiHandler(async (request: NextRequest) => {
measuredAt: { gte: dayStart, lt: dayEnd },
deletedAt: null,
},
orderBy: { measuredAt: sortDir },
// Same tiebreaker as the paged list. This one slices in memory rather
// than with `skip`, so there is no page drift to prevent — but two
// identical requests would still be free to render a tied block in
// different orders, and a drill-down that reshuffles on refresh reads
// as data changing.
orderBy: [{ measuredAt: sortDir }, { id: sortDir }],
// v1.4.38 — the 1000-row cap now lives on the validator (refine
// on `(limit, dayKey)` returning 422 when a caller asks for
// more). The route reads `limit` straight through because the
@@ -545,10 +550,12 @@ export const GET = apiHandler(async (request: NextRequest) => {
prisma.measurement.findMany({
where,
// `{ id: sortDir }` is the tiebreaker, not decoration. `measuredAt`
// is not unique here and ties are the normal case — an Apple Health
// import stamps long runs with one instant, and the
// `stats:<id>:YYYY-MM-DD` daily-rollup convention puts every daily
// total at the same one. Without a unique secondary key Postgres is
// is not unique here and ties are the normal case. Only the exact
// tuple `(userId, type, measuredAt, source, sleepStage)` is unique,
// so a tie survives across `type` (the `stats:<id>:YYYY-MM-DD`
// daily-rollup convention puts every daily total at one instant),
// across `source` (two writers landing on the same instant) and
// across `sleepStage`. Without a unique secondary key Postgres is
// free to order the tied block differently between the query for page
// N and the query for page N+1, so a client paging its own history
// silently gets one row twice and never gets another.
@@ -208,6 +208,27 @@ describe("GET /api/medications/[id]/intake — status filter", () => {
});
});
it("orders on a unique tiebreaker, keeping NULLS LAST on the takenAt arm", async () => {
// Scheduled slots share an instant by construction — a twice-daily
// medication puts every morning dose on the same `scheduledFor` — so
// offset paging over the sort key alone can repeat a row on one page and
// drop another. The `takenAt` arm keeps its NULLS LAST pin as well: the
// tiebreaker must not push skipped rows back to the top.
await GET(makeRequest("sortBy=takenAt&sortDir=desc"), ROUTE_PARAMS);
expect(
vi.mocked(prisma.medicationIntakeEvent.findMany).mock.calls[0][0]
?.orderBy,
).toEqual([{ takenAt: { sort: "desc", nulls: "last" } }, { id: "desc" }]);
vi.mocked(prisma.medicationIntakeEvent.findMany).mockClear();
await GET(makeRequest("sortBy=scheduledFor&sortDir=asc"), ROUTE_PARAMS);
expect(
vi.mocked(prisma.medicationIntakeEvent.findMany).mock.calls[0][0]
?.orderBy,
).toEqual([{ scheduledFor: "asc" }, { id: "asc" }]);
});
it("rejects unknown status values with a 422", async () => {
const res = await GET(makeRequest("status=junk"), ROUTE_PARAMS);
expect(res.status).toBe(422);
@@ -15,7 +15,11 @@ import { NextRequest } from "next/server";
vi.mock("@/lib/db", () => ({
prisma: {
appSettings: { findUnique: vi.fn().mockResolvedValue(null) },
mentalHealthAssessment: { create: vi.fn(), findFirst: vi.fn() },
mentalHealthAssessment: {
create: vi.fn(),
findFirst: vi.fn(),
findMany: vi.fn(),
},
measurement: { create: vi.fn().mockResolvedValue({}) },
},
}));
@@ -60,7 +64,7 @@ vi.mock("next/headers", () => ({
})),
}));
import { POST } from "../route";
import { GET, POST } from "../route";
import { enqueueReminderSatisfy } from "@/lib/jobs/reminder-satisfy";
import { getSession } from "@/lib/auth/session";
import { requireModuleEnabled } from "@/lib/modules/gate";
@@ -449,3 +453,25 @@ describe("POST /api/mental-health/assessments — external-id stability floor",
expect(prisma.mentalHealthAssessment.create).toHaveBeenCalledTimes(1);
});
});
describe("GET /api/mental-health/assessments", () => {
it("orders the history on a unique tiebreaker", async () => {
vi.mocked(prisma.mentalHealthAssessment.findMany).mockResolvedValue(
[] as never,
);
const res = await (GET as unknown as (r: NextRequest) => Promise<Response>)(
new NextRequest(
new URL("http://localhost/api/mental-health/assessments"),
),
);
expect(res.status).toBe(200);
// `takenAt` is day-resolution, so two instruments answered in one sitting
// tie; without the unique secondary key offset paging can repeat a row on
// one page and drop another.
const findArgs = vi.mocked(prisma.mentalHealthAssessment.findMany).mock
.calls[0][0] as { orderBy: unknown };
expect(findArgs.orderBy).toEqual([{ takenAt: "desc" }, { id: "desc" }]);
});
});