From 67ca5267080b7dde823109cfa2a233f48002f81e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 17 May 2026 15:21:28 +0200 Subject: [PATCH] fix(measurements): reject offset>0 on groupBy=day / dayKey branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The groupBy=day collapsed branch runs the per-user-day bucketing AFTER the per-sample findMany, so it can't honour a non-zero offset without real Postgres-side date_trunc grouping plus a distinct count — both v1.4.38 backlog items. The dayKey drill-down has the same shape: it returns a single bounded page, not a cursor. Both branches previously accepted offset silently and hard-coded offset:0 in the response meta, so a consumer threading the v1 pagination contract (admin tools, any future iOS list view) would paint "showing 1-25 of N" with the wrong N. Reject the combination at the validator instead with a clear 422. --- .../__tests__/group-by-day.test.ts | 26 ++++ src/lib/validations/measurement.ts | 129 ++++++++++-------- 2 files changed, 101 insertions(+), 54 deletions(-) diff --git a/src/app/api/measurements/__tests__/group-by-day.test.ts b/src/app/api/measurements/__tests__/group-by-day.test.ts index 19e817603..50f43de51 100644 --- a/src/app/api/measurements/__tests__/group-by-day.test.ts +++ b/src/app/api/measurements/__tests__/group-by-day.test.ts @@ -259,3 +259,29 @@ describe("GET /api/measurements — dayKey drill-down (W7c)", () => { expect(json.data.meta.dayKey).toBeUndefined(); }); }); + +describe("GET /api/measurements — schema rejections (W10 reconcile)", () => { + // v1.4.37 W10 H1 — the groupBy=day collapsed path runs the collapse + // AFTER the per-sample scan, so it can't honour a non-zero offset + // without a Postgres-side date_trunc grouping that's a v1.4.38 + // backlog item. The validator must reject the combination instead + // of silently rendering "showing 1-25 of N" with the wrong N. + it("rejects offset>0 when groupBy=day is set", async () => { + const res = await GET( + getRequest("type=ACTIVITY_STEPS&groupBy=day&offset=25"), + ); + expect(res.status).toBe(422); + expect(prisma.measurement.findMany).not.toHaveBeenCalled(); + }); + + // v1.4.37 W10 H1 — same restriction for the drill-down branch; it + // returns a single bounded page per dayKey, not a cursor. + it("rejects offset>0 when dayKey is set", async () => { + const res = await GET( + getRequest("type=ACTIVITY_STEPS&dayKey=2026-05-15&offset=25"), + ); + expect(res.status).toBe(422); + expect(prisma.measurement.findMany).not.toHaveBeenCalled(); + }); + +}); diff --git a/src/lib/validations/measurement.ts b/src/lib/validations/measurement.ts index e44416788..081541700 100644 --- a/src/lib/validations/measurement.ts +++ b/src/lib/validations/measurement.ts @@ -235,60 +235,81 @@ export const updateMeasurementSchema = z.object({ .optional(), }); -export const listMeasurementsSchema = z.object({ - type: measurementTypeEnum.optional(), - from: z.iso - .datetime({ offset: true }) - .transform((s) => new Date(s)) - .optional(), - to: z.iso - .datetime({ offset: true }) - .transform((s) => new Date(s)) - .optional(), - // v1.4.28 FB-D2 — when the chart sends an explicit from/to window the - // payload is already bounded; lift the per-request ceiling to 5000 - // (still cheaper than the legacy unbounded `while (true)` walk). - // Callers that omit from/to keep the historical 500-row cap. - limit: z.coerce.number().int().min(1).max(5000).optional().default(100), - offset: z.coerce.number().int().min(0).optional().default(0), - sortBy: z - .enum(["type", "value", "measuredAt", "source"]) - .optional() - .default("measuredAt"), - sortDir: z.enum(["asc", "desc"]).optional().default("desc"), - // v1.4.28 FB-D2 — bucket hint for range-aware queries. When set the - // GET handler runs a server-side `date_trunc` aggregation and returns - // one row per bucket per type rather than the raw measurement rows. - // Omitting `aggregate` keeps the raw wire shape (iOS contract); the - // chart-data client must opt in explicitly. - aggregate: z.enum(["raw", "daily", "weekly", "monthly"]).optional(), - // v1.4.36 W1 — opt-in source switch for daily-aggregate reads. When - // `source=rollup` + `aggregate=daily`, the route reads from the - // persistent `measurement_rollups` DAY buckets instead of running a - // live `date_trunc` GROUP BY scan over the raw `measurements` table. - // The chart-data client opts in for the trends-row strip + every - // sub-page chart so the three parallel daily-aggregate requests stop - // burning a full table scan each. The route falls back to live SQL - // when the rollup bucket set is empty for the requested window so - // brand-new accounts still see correct data on their first chart. - source: z.enum(["rollup"]).optional(), - // v1.4.37 W7c — list-view "one row per day" mode for cumulative - // types (steps, active energy, distance, flights, daylight). When - // `groupBy=day` is set and `type` is a cumulative HK type, the route - // returns one synthesised row per user-TZ day with `value` = SUM - // and `sampleCount` = number of per-sample rows behind the bucket. - // Omitted = legacy per-sample list behaviour (iOS contract stable). - groupBy: z.enum(["day"]).optional(), - // v1.4.37 W7c — drill-down to per-sample rows for a single day in - // the user's IANA timezone. Format `YYYY-MM-DD`; the route resolves - // the day boundary against the user's `User.timezone`. Used by the - // expandable list row to reveal the chunks that contributed to the - // collapsed daily total. - dayKey: z - .string() - .regex(/^\d{4}-\d{2}-\d{2}$/, "dayKey must be YYYY-MM-DD") - .optional(), -}); +export const listMeasurementsSchema = z + .object({ + type: measurementTypeEnum.optional(), + from: z.iso + .datetime({ offset: true }) + .transform((s) => new Date(s)) + .optional(), + to: z.iso + .datetime({ offset: true }) + .transform((s) => new Date(s)) + .optional(), + // v1.4.28 FB-D2 — when the chart sends an explicit from/to window the + // payload is already bounded; lift the per-request ceiling to 5000 + // (still cheaper than the legacy unbounded `while (true)` walk). + // Callers that omit from/to keep the historical 500-row cap. + limit: z.coerce.number().int().min(1).max(5000).optional().default(100), + offset: z.coerce.number().int().min(0).optional().default(0), + sortBy: z + .enum(["type", "value", "measuredAt", "source"]) + .optional() + .default("measuredAt"), + sortDir: z.enum(["asc", "desc"]).optional().default("desc"), + // v1.4.28 FB-D2 — bucket hint for range-aware queries. When set the + // GET handler runs a server-side `date_trunc` aggregation and returns + // one row per bucket per type rather than the raw measurement rows. + // Omitting `aggregate` keeps the raw wire shape (iOS contract); the + // chart-data client must opt in explicitly. + aggregate: z.enum(["raw", "daily", "weekly", "monthly"]).optional(), + // v1.4.36 W1 — opt-in source switch for daily-aggregate reads. When + // `source=rollup` + `aggregate=daily`, the route reads from the + // persistent `measurement_rollups` DAY buckets instead of running a + // live `date_trunc` GROUP BY scan over the raw `measurements` table. + // The chart-data client opts in for the trends-row strip + every + // sub-page chart so the three parallel daily-aggregate requests stop + // burning a full table scan each. The route falls back to live SQL + // when the rollup bucket set is empty for the requested window so + // brand-new accounts still see correct data on their first chart. + source: z.enum(["rollup"]).optional(), + // v1.4.37 W7c — list-view "one row per day" mode for cumulative + // types (steps, active energy, distance, flights, daylight). When + // `groupBy=day` is set and `type` is a cumulative HK type, the route + // returns one synthesised row per user-TZ day with `value` = SUM + // and `sampleCount` = number of per-sample rows behind the bucket. + // Omitted = legacy per-sample list behaviour (iOS contract stable). + // + // v1.4.37 W10 — the route's groupBy=day branch hard-codes `offset:0` + // in the meta because the collapse runs after the per-sample scan; + // real pagination would require Postgres-side `date_trunc` grouping + // and a separate `prisma.count({ distinct: ["dayKey"] })`. Until + // that lands, reject any caller threading a non-zero offset so the + // pagination contract isn't silently dropped. + groupBy: z.enum(["day"]).optional(), + // v1.4.37 W7c — drill-down to per-sample rows for a single day in + // the user's IANA timezone. Format `YYYY-MM-DD`; the route resolves + // the day boundary against the user's `User.timezone`. Used by the + // expandable list row to reveal the chunks that contributed to the + // collapsed daily total. + // + // v1.4.37 W10 — same offset restriction as `groupBy=day`; the + // drill-down branch returns a single bounded page rather than a + // cursor. + dayKey: z + .string() + .regex(/^\d{4}-\d{2}-\d{2}$/, "dayKey must be YYYY-MM-DD") + .optional(), + }) + .refine( + ({ offset, groupBy, dayKey }) => + !(offset > 0 && (groupBy === "day" || dayKey != null)), + { + message: + "offset is not supported with groupBy=day or dayKey; use a smaller window or omit offset", + path: ["offset"], + }, + ); export const createBatchMeasurementSchema = z.object({ measurements: z.array(createMeasurementSchema).min(1).max(5),