mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(mood): write tag links inside the entry transaction
The mood entry was committed before createTagLinks/replaceTagLinks ran, so a tag-link failure 5xx'd after the entry already existed and a client retry minted a duplicate entry. Wrap the entry write and the tag-link write in one $transaction and thread the tx client through both helpers so they commit or roll back together.
This commit is contained in:
1 parent
fcf5114620
commit
d5580917cc
4 files changed
+165
-38
No files matched your search
@@ -134,17 +134,26 @@ export const PUT = apiHandler(
|
||||
// monotonic value and paired clients reconcile higher-wins.
|
||||
updateData.syncVersion = { increment: 1 };
|
||||
|
||||
const entry = await prisma.moodEntry.update({
|
||||
where: { id },
|
||||
data: updateData,
|
||||
});
|
||||
// v1.8.5 — update the entry and replace its structured-tag links in
|
||||
// one transaction. A tag-link failure must roll the entry update back
|
||||
// too — otherwise a client retry on the 5xx double-applies the edit
|
||||
// (and the `syncVersion` increment). The tx client is threaded through
|
||||
// the helper so both writes commit (or abort) together.
|
||||
const entry = await prisma.$transaction(async (tx) => {
|
||||
const updated = await tx.moodEntry.update({
|
||||
where: { id },
|
||||
data: updateData,
|
||||
});
|
||||
|
||||
// v1.8.5 — full replacement of the structured-tag link set when
|
||||
// `tagKeys` is present in the body. `null` clears every link; an
|
||||
// omitted field leaves the links untouched.
|
||||
if (data.tagKeys !== undefined) {
|
||||
await replaceTagLinks(id, data.tagKeys ?? []);
|
||||
}
|
||||
// Full replacement of the structured-tag link set when `tagKeys`
|
||||
// is present in the body. `null` clears every link; an omitted
|
||||
// field leaves the links untouched.
|
||||
if (data.tagKeys !== undefined) {
|
||||
await replaceTagLinks(id, data.tagKeys ?? [], tx);
|
||||
}
|
||||
|
||||
return updated;
|
||||
});
|
||||
|
||||
await auditLog("moodEntry.update", {
|
||||
userId: user.id,
|
||||
|
||||
@@ -4,6 +4,19 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
// v1.8.5 — the POST happy path writes the entry + its structured-tag
|
||||
// links inside one `$transaction`. The mock hands the same client object
|
||||
// to the interactive-transaction callback so the route's tx-scoped writes
|
||||
// (entry create + tag-link read-back) resolve against the same fakes.
|
||||
const txClient = {
|
||||
moodEntry: {
|
||||
create: vi.fn(),
|
||||
},
|
||||
moodEntryTagLink: {
|
||||
findMany: vi.fn().mockResolvedValue([]),
|
||||
},
|
||||
};
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
moodEntry: {
|
||||
@@ -11,10 +24,20 @@ vi.mock("@/lib/db", () => ({
|
||||
count: vi.fn().mockResolvedValue(0),
|
||||
create: vi.fn(),
|
||||
},
|
||||
moodEntryTagLink: {
|
||||
findMany: vi.fn().mockResolvedValue([]),
|
||||
},
|
||||
auditLog: { create: vi.fn() },
|
||||
$transaction: vi.fn(
|
||||
async (fn: (tx: typeof txClient) => unknown) => fn(txClient),
|
||||
),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/mood/tag-links", () => ({
|
||||
createTagLinks: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
@@ -31,6 +54,9 @@ vi.mock("@/lib/cache/invalidate", () => ({
|
||||
vi.mock("@/lib/rollups/mood-rollups", () => ({
|
||||
recomputeMoodBucketsForEntry: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/moodlog/push", () => ({
|
||||
pushMoodEntriesToMoodLog: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
@@ -47,6 +73,9 @@ vi.mock("next/headers", () => ({
|
||||
import { GET, POST } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
import { createTagLinks } from "@/lib/mood/tag-links";
|
||||
import { recomputeMoodBucketsForEntry } from "@/lib/rollups/mood-rollups";
|
||||
import { pushMoodEntriesToMoodLog } from "@/lib/moodlog/push";
|
||||
|
||||
const SESSION_OK = {
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) },
|
||||
@@ -69,6 +98,20 @@ beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
vi.mocked(prisma.auditLog.create).mockResolvedValue({} as never);
|
||||
// Re-establish the interactive-transaction passthrough after the reset.
|
||||
vi.mocked(prisma.$transaction).mockImplementation(
|
||||
async (fn: unknown) => (fn as (tx: typeof txClient) => unknown)(txClient),
|
||||
);
|
||||
txClient.moodEntryTagLink.findMany.mockResolvedValue([]);
|
||||
vi.mocked(createTagLinks).mockResolvedValue(undefined);
|
||||
// `reset` blanks the post-commit best-effort mocks; restore the promise
|
||||
// returns so `recompute(...)` awaits and `push(...).catch()` is callable.
|
||||
vi.mocked(recomputeMoodBucketsForEntry).mockResolvedValue(undefined);
|
||||
vi.mocked(pushMoodEntriesToMoodLog).mockResolvedValue({
|
||||
pushed: 0,
|
||||
skipped: 0,
|
||||
status: "ok",
|
||||
});
|
||||
});
|
||||
|
||||
describe("GET /api/mood-entries — 422 multi-issue (v1.4.43 W6)", () => {
|
||||
@@ -174,3 +217,55 @@ describe("POST /api/mood-entries — 422 multi-issue (v1.4.43 W6)", () => {
|
||||
expect(res.status).toBe(422);
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /api/mood-entries — entry + tag-links transaction (v1.8.5)", () => {
|
||||
const VALID_BODY = {
|
||||
mood: "GUT",
|
||||
moodLoggedAt: "2026-06-01T08:00:00.000Z",
|
||||
tagKeys: ["happy"],
|
||||
};
|
||||
|
||||
it("writes the entry + tag links through the transaction client", async () => {
|
||||
txClient.moodEntry.create.mockResolvedValue({
|
||||
id: "mood-1",
|
||||
tags: null,
|
||||
moodLoggedAt: new Date(VALID_BODY.moodLoggedAt),
|
||||
mood: "GUT",
|
||||
note: null,
|
||||
source: "MANUAL",
|
||||
date: "2026-06-01",
|
||||
});
|
||||
|
||||
const res = await POST(postReq(VALID_BODY));
|
||||
expect(res.status).toBe(201);
|
||||
// The tag-link write runs against the same `$transaction` client that
|
||||
// created the entry, so both commit (or roll back) together.
|
||||
expect(createTagLinks).toHaveBeenCalledWith("mood-1", ["happy"], txClient);
|
||||
});
|
||||
|
||||
it("rolls the entry back when the tag-link write fails (no commit)", async () => {
|
||||
txClient.moodEntry.create.mockResolvedValue({
|
||||
id: "mood-2",
|
||||
tags: null,
|
||||
moodLoggedAt: new Date(VALID_BODY.moodLoggedAt),
|
||||
mood: "GUT",
|
||||
note: null,
|
||||
source: "MANUAL",
|
||||
date: "2026-06-01",
|
||||
});
|
||||
// A failing tag-link write must propagate out of the transaction so
|
||||
// Prisma aborts the entry create — the route surfaces it as a 5xx and
|
||||
// the entry is never committed (no audit row, no rollup recompute).
|
||||
vi.mocked(createTagLinks).mockRejectedValueOnce(new Error("link write"));
|
||||
|
||||
const res = await POST(postReq(VALID_BODY));
|
||||
expect(res.status).toBe(500);
|
||||
|
||||
const { auditLog } = await import("@/lib/auth/audit");
|
||||
expect(auditLog).not.toHaveBeenCalled();
|
||||
const { recomputeMoodBucketsForEntry } = await import(
|
||||
"@/lib/rollups/mood-rollups"
|
||||
);
|
||||
expect(recomputeMoodBucketsForEntry).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -167,27 +167,34 @@ async function postMoodEntry(request: NextRequest) {
|
||||
const score = getScoreForMood(mood);
|
||||
|
||||
try {
|
||||
const entry = await prisma.moodEntry.create({
|
||||
data: {
|
||||
userId: user.id,
|
||||
date,
|
||||
tz,
|
||||
mood,
|
||||
score,
|
||||
tags: tags ? JSON.stringify(tags) : null,
|
||||
note: note ?? null,
|
||||
source: source ?? "MANUAL",
|
||||
moodLoggedAt,
|
||||
},
|
||||
});
|
||||
// v1.8.5 — write the entry and its structured-tag links in one
|
||||
// transaction. The links are user-intended content, not a cache, so a
|
||||
// tag-link failure must roll the entry back too — otherwise a client
|
||||
// retry on the 5xx mints a duplicate entry. The tx client is threaded
|
||||
// through the helper so both writes commit (or abort) together.
|
||||
const entry = await prisma.$transaction(async (tx) => {
|
||||
const created = await tx.moodEntry.create({
|
||||
data: {
|
||||
userId: user.id,
|
||||
date,
|
||||
tz,
|
||||
mood,
|
||||
score,
|
||||
tags: tags ? JSON.stringify(tags) : null,
|
||||
note: note ?? null,
|
||||
source: source ?? "MANUAL",
|
||||
moodLoggedAt,
|
||||
},
|
||||
});
|
||||
|
||||
// v1.8.5 — write the structured-tag links. Unknown keys are dropped
|
||||
// inside the helper (the catalog is the source of truth). Best-effort
|
||||
// is wrong here — the links are user-intended content, not a cache —
|
||||
// so a failure surfaces as a 5xx via the outer try/catch.
|
||||
if (tagKeys && tagKeys.length > 0) {
|
||||
await createTagLinks(entry.id, tagKeys);
|
||||
}
|
||||
if (tagKeys && tagKeys.length > 0) {
|
||||
// Unknown keys are dropped inside the helper (the catalog is the
|
||||
// source of truth).
|
||||
await createTagLinks(created.id, tagKeys, tx);
|
||||
}
|
||||
|
||||
return created;
|
||||
});
|
||||
|
||||
await auditLog("moodEntry.create", {
|
||||
userId: user.id,
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Prisma, PrismaClient } from "@/generated/prisma/client";
|
||||
import { prisma } from "@/lib/db";
|
||||
|
||||
/**
|
||||
@@ -7,16 +8,29 @@ import { prisma } from "@/lib/db";
|
||||
* write the `mood_entry_tag_links` join for a mood entry. The catalog is
|
||||
* the source of truth: unknown keys are dropped silently so a stale
|
||||
* client can never mint a link to a tag the deployment doesn't carry.
|
||||
*
|
||||
* Every helper accepts an optional `db` client so the caller can thread
|
||||
* the same `$transaction` client that wrote the entry — the entry row and
|
||||
* its links then commit (or roll back) together.
|
||||
*/
|
||||
|
||||
/**
|
||||
* A Prisma client the helpers can run against: the singleton or a
|
||||
* `$transaction` interactive client.
|
||||
*/
|
||||
type TagLinkDb = PrismaClient | Prisma.TransactionClient;
|
||||
|
||||
/**
|
||||
* Resolve catalog tag keys to ids, dropping unknown / inactive keys.
|
||||
* Returns the ids in catalog order (deduped).
|
||||
*/
|
||||
export async function resolveTagKeysToIds(keys: string[]): Promise<string[]> {
|
||||
export async function resolveTagKeysToIds(
|
||||
keys: string[],
|
||||
db: TagLinkDb = prisma,
|
||||
): Promise<string[]> {
|
||||
const unique = Array.from(new Set(keys));
|
||||
if (unique.length === 0) return [];
|
||||
const rows = await prisma.moodTag.findMany({
|
||||
const rows = await db.moodTag.findMany({
|
||||
where: { key: { in: unique }, isActive: true },
|
||||
select: { id: true },
|
||||
});
|
||||
@@ -30,10 +44,11 @@ export async function resolveTagKeysToIds(keys: string[]): Promise<string[]> {
|
||||
export async function createTagLinks(
|
||||
moodEntryId: string,
|
||||
keys: string[],
|
||||
db: TagLinkDb = prisma,
|
||||
): Promise<void> {
|
||||
const tagIds = await resolveTagKeysToIds(keys);
|
||||
const tagIds = await resolveTagKeysToIds(keys, db);
|
||||
if (tagIds.length === 0) return;
|
||||
await prisma.moodEntryTagLink.createMany({
|
||||
await db.moodEntryTagLink.createMany({
|
||||
data: tagIds.map((moodTagId) => ({ moodEntryId, moodTagId })),
|
||||
skipDuplicates: true,
|
||||
});
|
||||
@@ -48,9 +63,10 @@ export async function createTagLinks(
|
||||
export async function replaceTagLinks(
|
||||
moodEntryId: string,
|
||||
keys: string[],
|
||||
db: TagLinkDb = prisma,
|
||||
): Promise<void> {
|
||||
const desiredIds = new Set(await resolveTagKeysToIds(keys));
|
||||
const existing = await prisma.moodEntryTagLink.findMany({
|
||||
const desiredIds = new Set(await resolveTagKeysToIds(keys, db));
|
||||
const existing = await db.moodEntryTagLink.findMany({
|
||||
where: { moodEntryId },
|
||||
select: { moodTagId: true },
|
||||
});
|
||||
@@ -60,12 +76,12 @@ export async function replaceTagLinks(
|
||||
const toCreate = [...desiredIds].filter((id) => !existingIds.has(id));
|
||||
|
||||
if (toDelete.length > 0) {
|
||||
await prisma.moodEntryTagLink.deleteMany({
|
||||
await db.moodEntryTagLink.deleteMany({
|
||||
where: { moodEntryId, moodTagId: { in: toDelete } },
|
||||
});
|
||||
}
|
||||
if (toCreate.length > 0) {
|
||||
await prisma.moodEntryTagLink.createMany({
|
||||
await db.moodEntryTagLink.createMany({
|
||||
data: toCreate.map((moodTagId) => ({ moodEntryId, moodTagId })),
|
||||
skipDuplicates: true,
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user