diff --git a/apps/caramel-app/src/app/api/account/data/delete/route.ts b/apps/caramel-app/src/app/api/account/data/delete/route.ts index 9d46cb13..47dcd50f 100644 --- a/apps/caramel-app/src/app/api/account/data/delete/route.ts +++ b/apps/caramel-app/src/app/api/account/data/delete/route.ts @@ -1,5 +1,6 @@ import { withRoute } from '@/lib/api/withRoute' import prisma from '@/lib/prisma' +import type { Prisma } from '@prisma/client' import { NextResponse } from 'next/server' import { z } from 'zod' @@ -7,12 +8,20 @@ import { z } from 'zod' // // Removes the three login-features tables' rows for the caller: their synced // savings history, the stores they follow, and the coupon reports they made. +// It also SCRUBS the caller's identity off their site suggestions — see below. // // WHAT IT DELIBERATELY DOES NOT DO: // - It does NOT delete the account or the login. That is a larger job (Better // Auth session teardown, extension session revocation, an email // confirmation step) and is out of scope here. See the TODO in // DataPrivacySection.tsx. +// - It does NOT delete site_suggestions rows, it SCRUBS them. The row's +// identifying half (user_id, requester_email, user_agent) is nulled; the +// domain and the status stay. A "please support this store" request is not +// personal data once the requester is off it, and it is the pipeline's +// input: deleting the row would silently retract a store request that other +// people may also have made, and would corrupt a queue this user does not +// own. Nothing new is stamped — a scrub is not an ANSWER to the request. // - It does NOT flip the savings-sync preference. Turning sync OFF and // DELETING history are two deliberately separate acts: a delete that also // changed a setting the user never touched would make the danger zone do @@ -46,18 +55,59 @@ export const POST = withRoute( return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } const userId = session.user.id + const email = session.user.email ?? null + + // A site suggestion is matched TWO ways, and the second is the one that + // matters: `user_id` finds the requests made while signed in, and the + // requester email finds the ones made while signed OUT — where the row + // carries no user id at all and the address the person typed is the + // ONLY thing on it. Matching by user id alone is the fix that looks + // right and leaves exactly the email this route exists to remove. + // + // Case-insensitive, because the suggest form records what the visitor + // typed (`Shopper@Example.com`) while the account holds its own + // spelling; a case-sensitive compare would walk straight past the row. + // An account with no email on it (the schema allows one) contributes no + // email branch rather than a `null` one, which would match every + // anonymous suggestion ever made. + const suggestionIdentity: Prisma.SiteSuggestionWhereInput[] = [ + { userId }, + ...(email + ? [ + { + requesterEmail: { + equals: email, + mode: 'insensitive' as const, + }, + }, + ] + : []), + ] // ONE transaction. A partial delete is the worst outcome available // here: the user is told their data is gone while some of it remains, // and the counts the UI just showed them become a lie. If any of the - // three fails, Prisma rolls the whole interactive batch back and + // four fails, Prisma rolls the whole interactive batch back and // withRoute's catch turns it into a 500 + Sentry — nothing deleted, // and the failure toast ("Nothing was removed") is then true. - const [savingsEvents, favoriteStores, couponReports] = + // + // The scrub belongs IN here for exactly that reason: run as a fourth + // loose await after a successful transaction, a failure would leave the + // three tables emptied and the email still sitting in site_suggestions, + // with a 500 telling the user nothing had been removed. + const [savingsEvents, favoriteStores, couponReports, siteSuggestions] = await prisma.$transaction([ prisma.savingsEvent.deleteMany({ where: { userId } }), prisma.favoriteStore.deleteMany({ where: { userId } }), prisma.couponReport.deleteMany({ where: { userId } }), + prisma.siteSuggestion.updateMany({ + where: { OR: suggestionIdentity }, + data: { + userId: null, + requesterEmail: null, + userAgent: null, + }, + }), ]) return NextResponse.json({ @@ -66,6 +116,10 @@ export const POST = withRoute( favoriteStores: favoriteStores.count, couponReports: couponReports.count, }, + // Reported under its own key, never folded into `deleted`: these + // rows still exist, and calling that a deletion would be a lie the + // next reader of this response would believe. + scrubbed: { siteSuggestions: siteSuggestions.count }, }) }, ) diff --git a/apps/caramel-app/tests/integration/site-suggestions.itest.ts b/apps/caramel-app/tests/integration/site-suggestions.itest.ts index b0a885e8..681f5774 100644 --- a/apps/caramel-app/tests/integration/site-suggestions.itest.ts +++ b/apps/caramel-app/tests/integration/site-suggestions.itest.ts @@ -1,3 +1,4 @@ +import { POST as deleteMyData } from '@/app/api/account/data/delete/route' import { POST } from '@/app/api/sites/suggest/route' import prisma from '@/lib/prisma' import { @@ -392,3 +393,108 @@ describe('site suggestions — answering a request (real Postgres)', () => { ).resolves.toEqual({ notifiedAt: toldAt }) }) }) + +// "Delete my data" and the requester identity (#227), on real Postgres. +// +// The unit suite imitates `mode: 'insensitive'` in its in-memory fake. Whether +// Postgres actually folds the case — and whether the OR really reaches a row +// that carries no user id at all — is a property of the database, so it is +// settled here. +describe('delete-my-data scrubs the requester identity (real Postgres)', () => { + it('scrubs the signed-in row AND the email-only row typed in a different case, and leaves a third user alone', async () => { + const mine = await createRow(`mine.${ITEST_DOMAIN_SUFFIX}`) + await prisma.siteSuggestion.update({ + where: { id: mine }, + data: { + userId, + requesterEmail: USER_EMAIL, + userAgent: 'their laptop', + }, + }) + // Made while SIGNED OUT: no user id, and the address as THEY typed it. + const anonymous = await createRow(`anon.${ITEST_DOMAIN_SUFFIX}`) + await prisma.siteSuggestion.update({ + where: { id: anonymous }, + data: { + requesterEmail: USER_EMAIL.toUpperCase(), + userAgent: 'their phone', + }, + }) + const stranger = await createRow(`stranger.${ITEST_DOMAIN_SUFFIX}`) + await prisma.siteSuggestion.update({ + where: { id: stranger }, + data: { + requesterEmail: `someone-else@${ITEST_DOMAIN_SUFFIX}`, + userAgent: 'not theirs', + }, + }) + + getSessionMock.mockResolvedValue({ + user: { id: userId, email: USER_EMAIL }, + }) + const res = await deleteMyData( + new NextRequest('http://localhost/api/account/data/delete', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ confirm: 'DELETE' }), + }), + ) + expect(res.status).toBe(200) + expect((await res.json()).scrubbed).toEqual({ siteSuggestions: 2 }) + + for (const id of [mine, anonymous]) { + await expect( + prisma.siteSuggestion.findUniqueOrThrow({ + where: { id }, + select: { + userId: true, + requesterEmail: true, + userAgent: true, + }, + }), + ).resolves.toEqual({ + userId: null, + requesterEmail: null, + userAgent: null, + }) + } + await expect( + prisma.siteSuggestion.findUniqueOrThrow({ + where: { id: stranger }, + select: { requesterEmail: true, userAgent: true }, + }), + ).resolves.toEqual({ + requesterEmail: `someone-else@${ITEST_DOMAIN_SUFFIX}`, + userAgent: 'not theirs', + }) + }) + + it('the store request itself survives the scrub, and stays drainable by the pipeline', async () => { + const id = await createRow(`survives.${ITEST_DOMAIN_SUFFIX}`) + await prisma.siteSuggestion.update({ + where: { id }, + data: { userId, requesterEmail: USER_EMAIL }, + }) + getSessionMock.mockResolvedValue({ + user: { id: userId, email: USER_EMAIL }, + }) + await deleteMyData( + new NextRequest('http://localhost/api/account/data/delete', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ confirm: 'DELETE' }), + }), + ) + + // Still a `new` row the pipeline will hand out, with its domain intact. + const listed = ( + await listSiteSuggestions({ status: 'new', limit: 500 }) + ).filter(s => s.id === id) + expect(listed).toHaveLength(1) + expect(listed[0]).toMatchObject({ + domain: `survives.${ITEST_DOMAIN_SUFFIX}`, + requesterEmail: null, + status: 'new', + }) + }) +}) diff --git a/apps/caramel-app/tests/unit/account-data-delete-suggestion-scrub.test.ts b/apps/caramel-app/tests/unit/account-data-delete-suggestion-scrub.test.ts new file mode 100644 index 00000000..ab577da1 --- /dev/null +++ b/apps/caramel-app/tests/unit/account-data-delete-suggestion-scrub.test.ts @@ -0,0 +1,317 @@ +import { POST as deletePOST } from '@/app/api/account/data/delete/route' +import { POST as notifyPOST } from '@/app/api/ingest/site-suggestions/notify/route' +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + matchesWhere, + resetTable, + seedRow, + siteSuggestionFake, + table, +} from './support/siteSuggestionsPrismaFake' + +// "Delete my data" and the requester identity on site_suggestions. +// +// The row is SCRUBBED, not deleted: a "please support this store" request is +// not personal data once the requester is off it, and it is the coupons +// pipeline's input — deleting it would silently retract a store request other +// people may also have made. +// +// The bug this file exists to keep fixed is the SIGNED-OUT suggestion. That row +// carries no user id at all: the address the visitor typed is the only thing on +// it. A scrub matched on `user_id` alone is the fix that looks right and leaves +// behind exactly the email the route was written to remove — so the pair below +// keeps both losing predicates verbatim and runs them against the same rows. +// +// ANNOUNCED FAKE, but an EXECUTING one: `$transaction` really awaits the batch +// and `siteSuggestion.updateMany` really evaluates the where against an +// in-memory table (support/siteSuggestionsPrismaFake.ts). A recording-only mock +// could not tell a matching predicate from a missing one, which is the whole +// question here. The case-insensitive match is Postgres behaviour imitated in +// that fake and pinned for real in tests/integration/site-suggestions.itest.ts. + +const { prismaMock } = vi.hoisted(() => ({ + prismaMock: { + $transaction: vi.fn(async (ops: Promise[]) => + Promise.all(ops), + ), + savingsEvent: { deleteMany: vi.fn(async () => ({ count: 0 })) }, + favoriteStore: { deleteMany: vi.fn(async () => ({ count: 0 })) }, + couponReport: { deleteMany: vi.fn(async () => ({ count: 0 })) }, + user: { update: vi.fn(), delete: vi.fn() }, + siteSuggestion: {} as Record, + }, +})) +vi.mock('@/lib/prisma', async () => { + const fake = await import('./support/siteSuggestionsPrismaFake') + prismaMock.siteSuggestion = fake.siteSuggestionFake as unknown as Record< + string, + unknown + > + return { default: prismaMock } +}) + +const { getSessionMock } = vi.hoisted(() => ({ + getSessionMock: vi.fn( + async (_opts: { headers: Headers }) => null as unknown, + ), +})) +vi.mock('@/lib/auth/auth', () => ({ + auth: { api: { getSession: getSessionMock } }, +})) + +vi.mock('@/lib/rateLimit', async importOriginal => { + const actual = await importOriginal() + return { ...actual, checkRateLimit: vi.fn(async () => null) } +}) + +const { envMock } = vi.hoisted(() => ({ + envMock: { + INGEST_API_KEY: 'test-ingest-key-scrub', + SITE_SUGGESTIONS_AUTO_NOTIFY: 'false', + ALLOWED_ORIGINS: '', + }, +})) +vi.mock('@/lib/env', () => ({ env: envMock })) + +const { sendEmailMock } = vi.hoisted(() => ({ + sendEmailMock: vi.fn(async (_payload: Record) => {}), +})) +vi.mock('@/lib/email', async importOriginal => ({ + ...(await importOriginal>()), + sendEmail: sendEmailMock, +})) + +const USER_ID = 'user-under-test' +const USER_EMAIL = 'shopper@example.com' + +function deleteRequest(body: unknown = { confirm: 'DELETE' }) { + return new NextRequest('http://localhost/api/account/data/delete', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + }) +} + +function notifyRequest(ids: string[]) { + return new NextRequest( + 'http://localhost/api/ingest/site-suggestions/notify', + { + method: 'POST', + headers: { + 'content-type': 'application/json', + authorization: `Bearer ${envMock.INGEST_API_KEY}`, + }, + body: JSON.stringify({ ids }), + }, + ) +} + +function row(id: string) { + return table.find(r => r.id === id)! +} + +/** The three rows every case below needs: one made while signed IN, one made + * while signed OUT (email only, and typed in a different case), and one + * belonging to somebody else entirely. */ +function seedTheThreeRows() { + seedRow('signed-in', { + domain: 'worldofbooks.com', + userId: USER_ID, + requesterEmail: USER_EMAIL, + userAgent: 'Mozilla/5.0 (their laptop)', + }) + seedRow('signed-out', { + domain: 'peepers.com', + userId: null, + // What they TYPED — the account's own spelling is lower case. + requesterEmail: 'Shopper@Example.COM', + userAgent: 'Mozilla/5.0 (their phone)', + }) + seedRow('somebody-else', { + domain: 'worldofbooks.com', + userId: 'a-different-user', + requesterEmail: 'stranger@example.com', + userAgent: 'Mozilla/5.0 (not theirs)', + }) +} + +beforeEach(() => { + resetTable() + sendEmailMock.mockClear() + prismaMock.$transaction.mockClear() + prismaMock.savingsEvent.deleteMany.mockClear() + prismaMock.favoriteStore.deleteMany.mockClear() + prismaMock.couponReport.deleteMany.mockClear() + getSessionMock.mockResolvedValue({ + user: { id: USER_ID, email: USER_EMAIL }, + }) +}) + +describe('delete-my-data scrubs the requester identity — THE PAIR', () => { + it('the PRE-CHANGE route touched no suggestion at all, so both of the caller’s rows kept their email', async () => { + seedTheThreeRows() + // The transaction the route ran before this change, kept VERBATIM. + await prismaMock.$transaction([ + prismaMock.savingsEvent.deleteMany(), + prismaMock.favoriteStore.deleteMany(), + prismaMock.couponReport.deleteMany(), + ]) + + expect(row('signed-in').requesterEmail).toBe(USER_EMAIL) + expect(row('signed-out').requesterEmail).toBe('Shopper@Example.COM') + }) + + it('the NAIVE fix — matching on user_id alone — leaves the signed-out row’s email in place', async () => { + seedTheThreeRows() + // The predicate everybody writes first, kept VERBATIM and run against + // the same rows through the same fake. + await siteSuggestionFake.updateMany({ + where: { userId: USER_ID }, + data: { userId: null, requesterEmail: null, userAgent: null }, + }) + + expect(row('signed-in').requesterEmail).toBeNull() + // ...and here is the address the route exists to remove, still there. + expect(row('signed-out').requesterEmail).toBe('Shopper@Example.COM') + }) + + it('the SHIPPED route scrubs BOTH — the user_id-matched row and the email-only one', async () => { + seedTheThreeRows() + const res = await deletePOST(deleteRequest()) + + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ + deleted: { + savingsEvents: 0, + favoriteStores: 0, + couponReports: 0, + }, + // Its own key: these rows still exist. Calling it a deletion would + // be a lie the next reader of this response would believe. + scrubbed: { siteSuggestions: 2 }, + }) + for (const id of ['signed-in', 'signed-out']) { + expect(row(id).userId).toBeNull() + expect(row(id).requesterEmail).toBeNull() + expect(row(id).userAgent).toBeNull() + } + }) +}) + +describe('delete-my-data scrubs the requester identity — scope', () => { + it('a third user’s suggestion is untouched, identity and all', async () => { + seedTheThreeRows() + await deletePOST(deleteRequest()) + + expect(row('somebody-else')).toMatchObject({ + userId: 'a-different-user', + requesterEmail: 'stranger@example.com', + userAgent: 'Mozilla/5.0 (not theirs)', + }) + }) + + it('the store request SURVIVES: domain, status and created_at are left alone, and nothing new is stamped', async () => { + seedRow('signed-in', { + domain: 'worldofbooks.com', + status: 'supported', + userId: USER_ID, + requesterEmail: USER_EMAIL, + }) + const before = { ...row('signed-in') } + await deletePOST(deleteRequest()) + + const after = row('signed-in') + expect(after.domain).toBe('worldofbooks.com') + expect(after.rawUrl).toBe(before.rawUrl) + expect(after.status).toBe('supported') + expect(after.createdAt).toEqual(before.createdAt) + // A scrub is not an ANSWER to the request, so it dates nothing. + expect(after.statusChangedAt).toBeNull() + expect(after.notifiedAt).toBeNull() + expect(after.importedAt).toBeNull() + }) + + it('an account with NO email contributes no email branch — it can never match every anonymous suggestion', async () => { + seedRow('mine', { userId: USER_ID, requesterEmail: null }) + seedRow('anonymous-stranger', { + userId: null, + requesterEmail: null, + userAgent: 'somebody else entirely', + }) + getSessionMock.mockResolvedValue({ + user: { id: USER_ID, email: null }, + }) + + const res = await deletePOST(deleteRequest()) + expect((await res.json()).scrubbed).toEqual({ siteSuggestions: 1 }) + expect(row('anonymous-stranger').userAgent).toBe( + 'somebody else entirely', + ) + }) + + it('an empty OR would match NOTHING, not everything — the failure mode a conditional where must not have', () => { + // Guards the fake's own semantics, which the pins above rest on: if an + // empty OR matched every row, "the scrub is correctly scoped" would be + // unfalsifiable here. + seedRow('any') + expect(matchesWhere(row('any'), { OR: [] })).toBe(false) + }) +}) + +describe('delete-my-data scrubs the requester identity — transactional', () => { + it('the scrub is the FOURTH member of the SAME batch, never a loose await after it', async () => { + seedTheThreeRows() + await deletePOST(deleteRequest()) + + expect(prismaMock.$transaction).toHaveBeenCalledTimes(1) + const batch = prismaMock.$transaction.mock.calls[0]![0] + expect(Array.isArray(batch)).toBe(true) + expect(batch).toHaveLength(4) + expect(siteSuggestionFake.updateMany).toHaveBeenCalledTimes(1) + }) + + it('a failing batch scrubs NOTHING — the email cannot be left behind by a partial run', async () => { + seedTheThreeRows() + prismaMock.$transaction.mockRejectedValueOnce( + new Error('deadlock detected on favorite_stores'), + ) + + const res = await deletePOST(deleteRequest()) + expect(res.status).toBe(500) + // The route builds the batch before handing it over, so the operation + // object exists; what must NOT have happened is the row changing. + expect(row('signed-in').requesterEmail).toBe(USER_EMAIL) + expect(row('signed-out').requesterEmail).toBe('Shopper@Example.COM') + }) +}) + +describe('a scrubbed suggestion still behaves — the notify route', () => { + it('a scrubbed `supported` row is `not_eligible` for want of an email, never a crash', async () => { + seedRow('was-theirs', { + domain: 'worldofbooks.com', + status: 'supported', + userId: USER_ID, + requesterEmail: USER_EMAIL, + }) + await deletePOST(deleteRequest()) + expect(row('was-theirs').requesterEmail).toBeNull() + + const res = await notifyPOST(notifyRequest(['was-theirs'])) + expect(res.status).toBe(200) + expect(await res.json()).toMatchObject({ + ok: true, + sent: 0, + notEligible: 1, + results: [ + { + id: 'was-theirs', + outcome: 'not_eligible', + reason: 'no_email', + }, + ], + }) + // The person asked to be forgotten. Nothing may be mailed to them. + expect(sendEmailMock).not.toHaveBeenCalled() + }) +}) diff --git a/apps/caramel-app/tests/unit/account-data-delete.test.ts b/apps/caramel-app/tests/unit/account-data-delete.test.ts index c5f89908..6abf5fb1 100644 --- a/apps/caramel-app/tests/unit/account-data-delete.test.ts +++ b/apps/caramel-app/tests/unit/account-data-delete.test.ts @@ -10,9 +10,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' // 1. The literal confirmation string is required SERVER-side. The typed // dialog is a second lock, not the only one. // 2. It deletes only the caller's rows, and only the three login-features -// tables — never the account, never the sync preference. +// tables — never the account, never the sync preference. Site suggestions +// are the one exception and are SCRUBBED rather than deleted (the store +// request is not personal data once the requester is off it). // 3. It is TRANSACTIONAL: a partial failure leaves nothing deleted, which is // what makes the failure toast ("Nothing was removed") a true statement. +// +// This file pins the SHAPE of the batch. What the scrub's predicate actually +// SELECTS — the signed-out, email-only row a user_id match walks past — is +// pinned against an executing in-memory table in +// account-data-delete-suggestion-scrub.test.ts. const { prismaMock, transactionMock } = vi.hoisted(() => { const transactionMock = vi.fn() @@ -23,6 +30,20 @@ const { prismaMock, transactionMock } = vi.hoisted(() => { savingsEvent: { deleteMany: vi.fn(() => ({ op: 'savings' })) }, favoriteStore: { deleteMany: vi.fn(() => ({ op: 'favorites' })) }, couponReport: { deleteMany: vi.fn(() => ({ op: 'reports' })) }, + siteSuggestion: { + // Typed args, so the `data` key assertion below reads the real + // call rather than an untyped `any` the compiler cannot check. + updateMany: vi.fn( + (_args: { + where: unknown + data: Record + }) => ({ op: 'suggestions' }), + ), + // Never called by this route: the store request must survive + // the requester. Mocked so the "not called" assertion below is + // a statement about the route, not about the fixture. + deleteMany: vi.fn(), + }, // `delete` is mocked even though the route must never call it: a // "not called" assertion against a method the fixture does not // define is a statement about the fixture, not about the route. @@ -47,6 +68,7 @@ vi.mock('@/lib/rateLimit', async importOriginal => { }) const USER_ID = 'user-under-test' +const USER_EMAIL = 'shopper@example.com' function deleteRequest(body: unknown) { return new NextRequest('http://localhost/api/account/data/delete', { @@ -58,11 +80,14 @@ function deleteRequest(body: unknown) { beforeEach(() => { vi.clearAllMocks() - getSessionMock.mockResolvedValue({ user: { id: USER_ID } }) + getSessionMock.mockResolvedValue({ + user: { id: USER_ID, email: USER_EMAIL }, + }) transactionMock.mockResolvedValue([ { count: 14 }, { count: 6 }, { count: 7 }, + { count: 3 }, ]) }) @@ -73,6 +98,7 @@ describe('POST /api/account/data/delete — the confirmation gate', () => { expect(res.status).toBe(200) expect(await res.json()).toEqual({ deleted: { savingsEvents: 14, favoriteStores: 6, couponReports: 7 }, + scrubbed: { siteSuggestions: 3 }, }) expect(transactionMock).toHaveBeenCalledTimes(1) }) @@ -128,6 +154,38 @@ describe('POST /api/account/data/delete — scope', () => { expect(prismaMock.couponReport.deleteMany).toHaveBeenCalledWith({ where: { userId: USER_ID }, }) + // The suggestion scrub is matched TWO ways — the second is the whole + // point, because a signed-out request carries no user id at all. + expect(prismaMock.siteSuggestion.updateMany).toHaveBeenCalledWith({ + where: { + OR: [ + { userId: USER_ID }, + { + requesterEmail: { + equals: USER_EMAIL, + mode: 'insensitive', + }, + }, + ], + }, + data: { userId: null, requesterEmail: null, userAgent: null }, + }) + }) + + it('a site suggestion is SCRUBBED, never deleted — the store request survives its requester', async () => { + await POST(deleteRequest({ confirm: 'DELETE' })) + + expect(prismaMock.siteSuggestion.updateMany).toHaveBeenCalledTimes(1) + expect(prismaMock.siteSuggestion.deleteMany).not.toHaveBeenCalled() + // Only the identifying half is nulled. domain/status/created_at are + // absent from `data`, so the pipeline's input is untouched, and nothing + // new is stamped: a scrub is not an ANSWER to the request. + const [args] = prismaMock.siteSuggestion.updateMany.mock.calls[0]! + expect(Object.keys(args.data).sort()).toEqual([ + 'requesterEmail', + 'userAgent', + 'userId', + ]) }) it('does NOT delete the account and does NOT touch the sync preference', async () => { @@ -154,18 +212,22 @@ describe('POST /api/account/data/delete — scope', () => { }) describe('POST /api/account/data/delete — transactional', () => { - it('runs all three deletes inside ONE $transaction, never as loose awaits', async () => { + it('runs all three deletes AND the suggestion scrub inside ONE $transaction, never as loose awaits', async () => { await POST(deleteRequest({ confirm: 'DELETE' })) const batch = transactionMock.mock.calls[0]![0] expect(Array.isArray(batch)).toBe(true) - expect(batch).toHaveLength(3) - // The delete builders were invoked to BUILD the batch, and their - // results were handed to $transaction rather than awaited separately. + expect(batch).toHaveLength(4) + // The builders were invoked to BUILD the batch, and their results were + // handed to $transaction rather than awaited separately. The scrub is + // IN here on purpose: run as a fourth loose await after a successful + // transaction, a failure would empty the three tables and leave the + // email sitting in site_suggestions. expect(batch).toEqual([ { op: 'savings' }, { op: 'favorites' }, { op: 'reports' }, + { op: 'suggestions' }, ]) }) diff --git a/apps/caramel-app/tests/unit/support/siteSuggestionsPrismaFake.ts b/apps/caramel-app/tests/unit/support/siteSuggestionsPrismaFake.ts index 6f5c2cdf..ef932a3f 100644 --- a/apps/caramel-app/tests/unit/support/siteSuggestionsPrismaFake.ts +++ b/apps/caramel-app/tests/unit/support/siteSuggestionsPrismaFake.ts @@ -53,6 +53,24 @@ function matchesField(value: unknown, cond: Condition): boolean { if (value < (record.gte as Date)) return false } else if (key === 'not') { if (matchesField(value, record.not)) return false + } else if (key === 'equals') { + // `mode: 'insensitive'` is a real Postgres behaviour; imitated + // here so the unit suite can exercise the case-folded match, + // and pinned for real in the integration suite. + if (record.mode === 'insensitive') { + if ( + typeof value !== 'string' || + typeof record.equals !== 'string' || + value.toLowerCase() !== record.equals.toLowerCase() + ) { + return false + } + } else if (!matchesField(value, record.equals)) { + return false + } + } else if (key === 'mode') { + // Read by the `equals` branch above; never a filter on its own. + continue } else { throw new Error(`fake prisma: unsupported operator "${key}"`) } @@ -72,6 +90,15 @@ export function matchesWhere( if (matchesWhere(row, cond as Record)) return false continue } + if (key === 'OR') { + const branches = cond as Record[] + // An EMPTY OR matches nothing in Prisma. Spelling that out matters: + // the scrub builds its OR conditionally, so a bug that produced an + // empty list must select no rows here rather than every row. + if (!branches.some(branch => matchesWhere(row, branch))) + return false + continue + } if (!(key in row)) { throw new Error(`fake prisma: unknown column "${key}"`) } @@ -106,58 +133,87 @@ function orderedByCreatedAt(rows: FakeSuggestionRow[]): FakeSuggestionRow[] { return ordered } +/** + * Prisma's array-form `$transaction` takes LAZY PrismaPromises: they are built + * by the caller and only executed when the transaction awaits them, which is + * precisely why a rejected batch leaves the rows untouched. An eager `async` + * fake would apply every write while the batch was merely being ASSEMBLED, and + * "a partial failure changes nothing" would be untestable here — so these + * return a thenable that runs on await instead. + */ +function lazy(run: () => T): PromiseLike { + return { + // 2026-09-08: a hand-made thenable is the POINT here, not an + // accident. Prisma's own PrismaPromise is exactly this, and modelling + // it is what lets the suite prove that a rejected `$transaction` batch + // leaves the rows untouched — the property "the scrub lives inside the + // transaction" depends on it. Test-fixture scope only; nothing ships. + // The directive must be the LAST comment line above the code it + // covers (CLAUDE.md gotcha: prettier reorders otherwise). + // oxlint-disable-next-line unicorn/no-thenable + then: (onFulfilled, onRejected) => + Promise.resolve().then(run).then(onFulfilled, onRejected), + } +} + export const siteSuggestionFake = { findMany: vi.fn( - async (args: { + (args: { where?: Record orderBy?: { createdAt: 'asc' } take?: number select?: Record - }) => { - let rows = table.filter(row => matchesWhere(row, args.where)) - if (args.orderBy) rows = orderedByCreatedAt(rows) - if (typeof args.take === 'number') rows = rows.slice(0, args.take) - return rows.map(row => project(row, args.select)) - }, + }) => + lazy(() => { + let rows = table.filter(row => matchesWhere(row, args.where)) + if (args.orderBy) rows = orderedByCreatedAt(rows) + if (typeof args.take === 'number') { + rows = rows.slice(0, args.take) + } + return rows.map(row => project(row, args.select)) + }), ), updateMany: vi.fn( - async (args: { + (args: { where?: Record data: Partial - }) => { - let count = 0 - for (const row of table) { - if (!matchesWhere(row, args.where)) continue - Object.assign(row, args.data) - count += 1 - } - return { count } - }, + }) => + lazy(() => { + let count = 0 + for (const row of table) { + if (!matchesWhere(row, args.where)) continue + Object.assign(row, args.data) + count += 1 + } + return { count } + }), ), updateManyAndReturn: vi.fn( - async (args: { + (args: { where?: Record data: Partial select?: Record - }) => { - const updated: Record[] = [] - for (const row of table) { - if (!matchesWhere(row, args.where)) continue - Object.assign(row, args.data) - updated.push(project(row, args.select)) - } - return updated - }, + }) => + lazy(() => { + const updated: Record[] = [] + for (const row of table) { + if (!matchesWhere(row, args.where)) continue + Object.assign(row, args.data) + updated.push(project(row, args.select)) + } + return updated + }), ), create: vi.fn( - async (args: { + (args: { data: Partial select?: Record - }) => { - const row = makeRow(`created-${table.length}`, args.data) - table.push(row) - return project(row, args.select) - }, + }) => + lazy(() => { + const row = makeRow(`created-${table.length}`, args.data) + table.push(row) + return project(row, args.select) + }), ), }