From 6db2f6f9ae7acfab63ae4721b54a686b7c9f5313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 7 Aug 2026 15:03:10 +0200 Subject: [PATCH] fix(cycle): make removing a period start restore the neighbour it moved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening a cycle closes the one before it: the start write stamps the previous cycle's endDate and lengthDays, and a back-filled start takes its own end from its successor. Nothing undid any of that. Deleting the start left the previous cycle as a closed torso holding an end date from a cycle that no longer existed, and a record whose last cycle is closed has no open cycle to forecast from — the prediction went away and there was no way to bring it back from the app. Both removal paths now re-derive the boundary from the cycles that remain: a cycle ends the day before the next one begins, and with no next one it is open again. That is a re-derivation rather than an undo log, so it is correct for a history that came from an import or a restore, and running it twice changes nothing the second time. Deleting the day-log that opened a cycle now removes that cycle too. The one-tap start writes both rows and the log sheet's delete is the only way the web offers to take it back; leaving the cycle standing was how the broken state survived the undo. - soft-delete and re-anchor run in one transaction - the audit row names the neighbour's moved boundary, the only part of the write the tombstone cannot reconstruct - round trip through the real routes: photograph the cycle rows, log the start, remove it either way, compare field by field — including that the forecast comes back --- src/app/api/cycle/cycles/[id]/route.ts | 44 ++- src/app/api/cycle/day-logs/[id]/route.ts | 56 +++- src/lib/cycle/cycle-boundaries.ts | 85 +++++ .../cycle-start-round-trip.test.ts | 296 ++++++++++++++++++ 4 files changed, 471 insertions(+), 10 deletions(-) create mode 100644 src/lib/cycle/cycle-boundaries.ts create mode 100644 tests/integration/cycle-start-round-trip.test.ts diff --git a/src/app/api/cycle/cycles/[id]/route.ts b/src/app/api/cycle/cycles/[id]/route.ts index 642d5a81c..4b8900d00 100644 --- a/src/app/api/cycle/cycles/[id]/route.ts +++ b/src/app/api/cycle/cycles/[id]/route.ts @@ -2,6 +2,12 @@ * `DELETE /api/cycle/cycles/{id}` — soft-delete a cycle * (ios-contract §2.F): set `deletedAt` + bump `syncVersion`, emit a * tombstone on the next sync page. 204. Idempotent. Owner-scoped + gated. + * + * The delete also hands the span back to the preceding cycle. Opening a cycle + * closes the one before it, so removing it without re-deriving that boundary + * left the previous cycle standing as a closed torso with an end date taken + * from a cycle that no longer exists — and a record whose last cycle is closed + * has no open cycle to forecast from. */ import { NextRequest } from "next/server"; @@ -9,8 +15,10 @@ import { prisma } from "@/lib/db"; import { apiHandler, requireRecordAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; +import { overwriteDetails } from "@/lib/sharing/audit-details"; import { apiError, getClientIp } from "@/lib/api-response"; import { requireCycleEnabled } from "@/lib/cycle/gate"; +import { reanchorAfterRemovedStart } from "@/lib/cycle/cycle-boundaries"; type RouteParams = { params: Promise<{ id: string }> }; @@ -28,21 +36,46 @@ export const DELETE = apiHandler( const existing = await prisma.menstrualCycle.findUnique({ where: { id }, - select: { id: true, userId: true }, + select: { id: true, userId: true, startDate: true }, }); if (!existing || existing.userId !== user.id) { return apiError("Cycle not found", 404); } - await prisma.menstrualCycle.update({ - where: { id }, - data: { deletedAt: new Date(), syncVersion: { increment: 1 } }, + // Tombstone + re-anchor as one unit: a neighbour re-derived against a + // half-applied delete would read the row being removed as still live. + const reanchored = await prisma.$transaction(async (db) => { + await db.menstrualCycle.update({ + where: { id }, + data: { deletedAt: new Date(), syncVersion: { increment: 1 } }, + }); + return reanchorAfterRemovedStart(db, user.id, existing.startDate); }); await auditLog("cycle.cycle.delete", { userId: user.id, ipAddress: getClientIp(request), - details: { cycleId: id }, + // The tombstone carries the deleted cycle's own columns, so the only + // unrecoverable part of this write is what it moved on the NEIGHBOUR. + details: { + cycleId: id, + startDate: existing.startDate, + ...(reanchored + ? { + reanchoredCycleId: reanchored.cycleId, + ...overwriteDetails({ + before: { + endDate: reanchored.endDateBefore, + lengthDays: reanchored.lengthDaysBefore, + }, + after: { + endDate: reanchored.endDateAfter, + lengthDays: reanchored.lengthDaysAfter, + }, + }), + } + : {}), + }, }); annotate({ @@ -51,6 +84,7 @@ export const DELETE = apiHandler( entity_type: "menstrual_cycle", entity_id: id, }, + meta: { reanchored_prior: reanchored !== null }, }); return new Response(null, { status: 204 }); diff --git a/src/app/api/cycle/day-logs/[id]/route.ts b/src/app/api/cycle/day-logs/[id]/route.ts index 7cd6fb3b8..7b88e0479 100644 --- a/src/app/api/cycle/day-logs/[id]/route.ts +++ b/src/app/api/cycle/day-logs/[id]/route.ts @@ -23,6 +23,7 @@ import { safeJson, } from "@/lib/api-response"; import { requireCycleEnabled } from "@/lib/cycle/gate"; +import { reanchorAfterRemovedStart } from "@/lib/cycle/cycle-boundaries"; import { encrypt, decrypt } from "@/lib/crypto"; import { getOrCreateCycleProfile } from "@/lib/cycle/profile"; import { replaceSymptomLinks } from "@/lib/cycle/day-log-write"; @@ -202,7 +203,7 @@ export const DELETE = apiHandler( const existing = await prisma.cycleDayLog.findUnique({ where: { id }, - select: { id: true, userId: true }, + select: { id: true, userId: true, date: true }, }); if (!existing || existing.userId !== user.id) { return apiError("Day-log not found", 404); @@ -211,15 +212,56 @@ export const DELETE = apiHandler( // Soft-delete: leave the row in place so the `/api/sync/changes` // delta feed surfaces it as a tombstone. A re-delete re-bumps // `syncVersion` harmlessly (idempotent). - await prisma.cycleDayLog.update({ - where: { id }, - data: { deletedAt: new Date(), syncVersion: { increment: 1 } }, + // + // A day that OPENS a cycle takes that cycle with it. The one-tap "started + // my period" writes both rows, and this delete is the only way the web + // offers to take it back — leaving the cycle behind meant the previous one + // stayed closed against a start whose last trace had just been removed, and + // the forecast never came back. The cycle boundary is then re-derived for + // the preceding cycle, the same repair the cycle delete does. + const removed = await prisma.$transaction(async (db) => { + await db.cycleDayLog.update({ + where: { id }, + data: { deletedAt: new Date(), syncVersion: { increment: 1 } }, + }); + const openedCycle = await db.menstrualCycle.findFirst({ + where: { userId: user.id, deletedAt: null, startDate: existing.date }, + select: { id: true }, + }); + if (!openedCycle) return { cycleId: null, reanchored: null }; + await db.menstrualCycle.update({ + where: { id: openedCycle.id }, + data: { deletedAt: new Date(), syncVersion: { increment: 1 } }, + }); + return { + cycleId: openedCycle.id, + reanchored: await reanchorAfterRemovedStart(db, user.id, existing.date), + }; }); await auditLog("cycle.day-log.delete", { userId: user.id, ipAddress: getClientIp(request), - details: { dayLogId: id }, + details: { + dayLogId: id, + date: existing.date, + ...(removed.cycleId ? { removedCycleId: removed.cycleId } : {}), + ...(removed.reanchored + ? { + reanchoredCycleId: removed.reanchored.cycleId, + ...overwriteDetails({ + before: { + endDate: removed.reanchored.endDateBefore, + lengthDays: removed.reanchored.lengthDaysBefore, + }, + after: { + endDate: removed.reanchored.endDateAfter, + lengthDays: removed.reanchored.lengthDaysAfter, + }, + }), + } + : {}), + }, }); annotate({ @@ -228,6 +270,10 @@ export const DELETE = apiHandler( entity_type: "cycle_day_log", entity_id: id, }, + meta: { + removed_cycle: removed.cycleId !== null, + reanchored_prior: removed.reanchored !== null, + }, }); return new Response(null, { status: 204 }); diff --git a/src/lib/cycle/cycle-boundaries.ts b/src/lib/cycle/cycle-boundaries.ts new file mode 100644 index 000000000..9d9e7f84a --- /dev/null +++ b/src/lib/cycle/cycle-boundaries.ts @@ -0,0 +1,85 @@ +/** + * Cycle boundary re-derivation — the inverse of what a period-start write does + * to its neighbours. + * + * `POST /api/cycle/period` with `action:"start"` is not an insert. It closes + * the previous open cycle (stamps `endDate` and `lengthDays` from the new + * start) and, when the start is back-filled between two existing cycles, takes + * the inserted cycle's own end from its successor. Removing that start left + * every one of those overwrites standing: the previous cycle kept the end date + * a cycle that no longer exists gave it, and a record whose last cycle is + * closed has no open cycle for the engine to forecast from. The prediction + * disappeared and there was no way to bring it back from the app. + * + * The repair is a re-derivation rather than an undo log, which is why it is + * safe to run on a record whose history came from an import or a restore + * instead of from the one-tap boundary: a cycle ends the day before the next + * one begins, and when there is no next one it is open. That statement is true + * of every cycle in the table regardless of how the row got there, so applying + * it to the surviving neighbour restores exactly the state the start write + * moved — and applying it twice changes nothing the second time. + */ +import { addDays, dayDiff } from "./day-math"; +import type { Prisma, PrismaClient } from "@/generated/prisma/client"; + +/** The Prisma client or an interactive-transaction handle. */ +type CycleDb = PrismaClient | Prisma.TransactionClient; + +/** The boundary columns the re-derivation moved, for the audit row. */ +export interface ReanchoredCycle { + cycleId: string; + endDateBefore: string | null; + endDateAfter: string | null; + lengthDaysBefore: number | null; + lengthDaysAfter: number | null; +} + +/** + * Re-derive the boundary of the cycle that precedes `removedStartDate` from + * the cycles that remain. Call it AFTER the cycle at that date is soft-deleted + * (the queries below read live rows only, so a still-live row at the removed + * start would re-derive against itself and change nothing). + * + * Returns the columns it moved, or null when there is no preceding cycle or + * the preceding cycle already holds the derived boundary — an idempotent + * re-run must not bump `syncVersion` and make every device re-pull a row that + * did not change. + */ +export async function reanchorAfterRemovedStart( + db: CycleDb, + userId: string, + removedStartDate: string, +): Promise { + const prior = await db.menstrualCycle.findFirst({ + where: { userId, deletedAt: null, startDate: { lt: removedStartDate } }, + orderBy: { startDate: "desc" }, + select: { id: true, startDate: true, endDate: true, lengthDays: true }, + }); + if (!prior) return null; + + const next = await db.menstrualCycle.findFirst({ + where: { userId, deletedAt: null, startDate: { gt: removedStartDate } }, + orderBy: { startDate: "asc" }, + select: { startDate: true }, + }); + + // A cycle ends the day before the next one starts; with no next one it is + // open again, which is the state the removed start took away from it. + const endDate = next ? addDays(next.startDate, -1) : null; + const lengthDays = next ? dayDiff(next.startDate, prior.startDate) : null; + + if (prior.endDate === endDate && prior.lengthDays === lengthDays) return null; + + await db.menstrualCycle.update({ + where: { id: prior.id }, + data: { endDate, lengthDays, syncVersion: { increment: 1 } }, + }); + + return { + cycleId: prior.id, + endDateBefore: prior.endDate, + endDateAfter: endDate, + lengthDaysBefore: prior.lengthDays, + lengthDaysAfter: lengthDays, + }; +} diff --git a/tests/integration/cycle-start-round-trip.test.ts b/tests/integration/cycle-start-round-trip.test.ts new file mode 100644 index 000000000..8595df6f1 --- /dev/null +++ b/tests/integration/cycle-start-round-trip.test.ts @@ -0,0 +1,296 @@ +/** + * Logging a period start and taking it back has to leave the record where it + * found it. + * + * `action:"start"` does not only insert a cycle. It CLOSES the previous open + * one — stamps its `endDate` and `lengthDays` from the new start — and, when + * the start is back-filled between two existing cycles, re-anchors the one + * after it too. Nothing undid any of that. Deleting the start left the + * previous cycle standing as a closed torso, and a record whose last cycle is + * closed has no open cycle to forecast from, so the prediction disappeared and + * did not come back. There is no way to repair that from the app. + * + * So this is a round trip, not a delete test: it photographs every cycle row + * before the start is logged, logs it, removes it, and asserts the rows are + * back to the photograph field by field. Two removal paths reach a start — + * deleting the cycle, and deleting the day-log that opened it from the log + * sheet, which is the only one the web actually offers. + */ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +process.env.API_TOKEN_HMAC_KEY ??= + "test-hmac-key-cycle-round-trip-integration-32-bytes-min-1234567890"; +process.env.ENCRYPTION_KEY ??= + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + +const USER_ID = "user-cycle-round-trip"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => cookieJar.set(name, value), + delete: (name: string) => cookieJar.delete(name), + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +async function loginAs(userId: string): Promise { + cookieJar.clear(); + headerJar.clear(); + const session = await getPrismaClient().session.create({ + data: { userId, expiresAt: new Date(Date.now() + 60 * 60 * 1000) }, + }); + cookieJar.set("healthlog_session", session.id); +} + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + cookieJar.clear(); + headerJar.clear(); + await getPrismaClient().user.create({ + data: { + id: USER_ID, + username: "cycle-round-trip", + email: "cycle-round-trip@example.test", + gender: "FEMALE", + }, + }); +}); + +function jsonRequest( + path: string, + method: string, + body?: unknown, +): NextRequest { + return new NextRequest(`http://localhost${path}`, { + method, + headers: { "content-type": "application/json" }, + body: body !== undefined ? JSON.stringify(body) : undefined, + }); +} + +/** The cycle boundary state, which is exactly what a start write moves. */ +interface CycleShape { + startDate: string; + endDate: string | null; + lengthDays: number | null; + periodEndDate: string | null; + deletedAt: boolean; +} + +async function cycleShapes(): Promise { + const rows = await getPrismaClient().menstrualCycle.findMany({ + where: { userId: USER_ID }, + orderBy: { startDate: "asc" }, + select: { + startDate: true, + endDate: true, + lengthDays: true, + periodEndDate: true, + deletedAt: true, + }, + }); + return rows.map((r) => ({ + startDate: r.startDate, + endDate: r.endDate, + lengthDays: r.lengthDays, + periodEndDate: r.periodEndDate, + deletedAt: r.deletedAt !== null, + })); +} + +/** Only the cycles a read would see — a tombstone is not part of the record. */ +async function liveShapes(): Promise { + return (await cycleShapes()).filter((c) => !c.deletedAt); +} + +async function startPeriod(date: string): Promise { + const { POST } = await import("@/app/api/cycle/period/route"); + return POST( + jsonRequest("/api/cycle/period", "POST", { + action: "start", + date, + loggedAt: `${date}T08:00:00.000Z`, + }), + ); +} + +/** The three months of history the maintainer's record carried. */ +async function seedHistory(): Promise { + await startPeriod("2026-05-04"); + await startPeriod("2026-06-01"); + await startPeriod("2026-07-01"); +} + +describe("logging a period start and taking it back", () => { + it("re-opens the previous cycle when the start's own cycle is deleted", async () => { + await loginAs(USER_ID); + await seedHistory(); + const before = await liveShapes(); + // The July cycle is open — that is the state the record has to return to. + expect(before.at(-1)).toEqual({ + startDate: "2026-07-01", + endDate: null, + lengthDays: null, + periodEndDate: null, + deletedAt: false, + }); + + await startPeriod("2026-08-07"); + const during = await liveShapes(); + expect(during).toHaveLength(4); + // The July cycle really was closed by the new start, so the delete below + // has something to undo. + expect(during.at(-2)).toMatchObject({ + startDate: "2026-07-01", + endDate: "2026-08-06", + lengthDays: 37, + }); + + const added = await getPrismaClient().menstrualCycle.findFirstOrThrow({ + where: { userId: USER_ID, startDate: "2026-08-07" }, + select: { id: true }, + }); + const { DELETE } = await import("@/app/api/cycle/cycles/[id]/route"); + const res = await DELETE( + jsonRequest(`/api/cycle/cycles/${added.id}`, "DELETE"), + { params: Promise.resolve({ id: added.id }) }, + ); + expect(res.status).toBe(204); + + expect(await liveShapes()).toEqual(before); + }); + + it("re-opens the previous cycle when the start's day-log is deleted", async () => { + // The path the web actually offers: the log sheet's delete button removes + // the day-log, and the cycle that one tap opened has to go with it. + await loginAs(USER_ID); + await seedHistory(); + const before = await liveShapes(); + + await startPeriod("2026-08-07"); + expect(await liveShapes()).toHaveLength(4); + + const dayLog = await getPrismaClient().cycleDayLog.findFirstOrThrow({ + where: { userId: USER_ID, date: "2026-08-07" }, + select: { id: true }, + }); + const { DELETE } = await import("@/app/api/cycle/day-logs/[id]/route"); + const res = await DELETE( + jsonRequest(`/api/cycle/day-logs/${dayLog.id}`, "DELETE"), + { params: Promise.resolve({ id: dayLog.id }) }, + ); + expect(res.status).toBe(204); + + expect(await liveShapes()).toEqual(before); + }); + + it("restores the boundary a back-filled start took from BOTH neighbours", async () => { + // A start inserted between two existing cycles closes the one before it + // and takes its own end from the one after. Removing it has to hand the + // whole span back to the earlier cycle. + await loginAs(USER_ID); + await seedHistory(); + await startPeriod("2026-08-07"); + const before = await liveShapes(); + + await startPeriod("2026-06-15"); + const during = await liveShapes(); + expect(during).toHaveLength(5); + expect(during[1]).toMatchObject({ + startDate: "2026-06-01", + endDate: "2026-06-14", + lengthDays: 14, + }); + expect(during[2]).toMatchObject({ + startDate: "2026-06-15", + endDate: "2026-06-30", + lengthDays: 16, + }); + + const inserted = await getPrismaClient().menstrualCycle.findFirstOrThrow({ + where: { userId: USER_ID, startDate: "2026-06-15" }, + select: { id: true }, + }); + const { DELETE } = await import("@/app/api/cycle/cycles/[id]/route"); + await DELETE(jsonRequest(`/api/cycle/cycles/${inserted.id}`, "DELETE"), { + params: Promise.resolve({ id: inserted.id }), + }); + + expect(await liveShapes()).toEqual(before); + }); + + it("leaves a day-log that opened no cycle alone", async () => { + // A plain entry on a day that is not a cycle start must not take a cycle + // with it when it is deleted. + await loginAs(USER_ID); + await seedHistory(); + const before = await liveShapes(); + + const { POST } = await import("@/app/api/cycle/day-logs/route"); + await POST( + jsonRequest("/api/cycle/day-logs", "POST", { + date: "2026-07-14", + flow: "SPOTTING", + loggedAt: "2026-07-14T08:00:00.000Z", + }), + ); + const dayLog = await getPrismaClient().cycleDayLog.findFirstOrThrow({ + where: { userId: USER_ID, date: "2026-07-14" }, + select: { id: true }, + }); + const { DELETE } = await import("@/app/api/cycle/day-logs/[id]/route"); + await DELETE(jsonRequest(`/api/cycle/day-logs/${dayLog.id}`, "DELETE"), { + params: Promise.resolve({ id: dayLog.id }), + }); + + expect(await liveShapes()).toEqual(before); + }); + + it("gives the forecast back", async () => { + // The symptom the round trip exists for: after the start is taken back the + // engine must forecast again, from the same open cycle as before. + await loginAs(USER_ID); + await seedHistory(); + + const { GET } = await import("@/app/api/cycle/calendar/route"); + const readForecast = async (): Promise => { + const res = await GET( + new NextRequest("http://localhost/api/cycle/calendar"), + ); + const body = JSON.parse(await res.text()); + return body.data?.prediction?.nextPeriodStart ?? null; + }; + + const before = await readForecast(); + expect(before).not.toBeNull(); + + await startPeriod("2026-08-07"); + const dayLog = await getPrismaClient().cycleDayLog.findFirstOrThrow({ + where: { userId: USER_ID, date: "2026-08-07" }, + select: { id: true }, + }); + const { DELETE } = await import("@/app/api/cycle/day-logs/[id]/route"); + await DELETE(jsonRequest(`/api/cycle/day-logs/${dayLog.id}`, "DELETE"), { + params: Promise.resolve({ id: dayLog.id }), + }); + + expect(await readForecast()).toBe(before); + }); +});