diff --git a/prisma/migrations/0023_supersede_duplicates/migration.sql b/prisma/migrations/0023_supersede_duplicates/migration.sql new file mode 100644 index 0000000..026627d --- /dev/null +++ b/prisma/migrations/0023_supersede_duplicates/migration.sql @@ -0,0 +1,37 @@ +-- A transaction imported twice cannot simply be deleted. +-- +-- Every child of `transactions` is ON DELETE CASCADE — splits, tags, overrides, +-- expense_metadata, order_reviews. Deleting a row said to be "the duplicate" +-- therefore destroys whatever curation happens to sit on it, silently and +-- unrecoverably. The curation is not reliably on the surviving side either: of +-- the 31 known duplicate pairs, one carries splits and six carry overrides, and +-- which member holds them is an accident of import order. +-- +-- So a duplicate is superseded, never removed. The row stays, keeps its +-- children, and points at the row that replaces it. Reversing a mistake is then +-- one UPDATE rather than a restore from backup. +-- +-- This is the statement-vs-statement case. `reconciled_with_id` already covers +-- manual-vs-statement, and deliberately cannot be reused: the predicate that +-- hides a reconciled row is scoped to `statement_id IS NULL`, because a +-- statement line pointing at something else is the survivor, not the duplicate. +-- Both of these rows are statement lines. + +ALTER TABLE transactions + ADD COLUMN IF NOT EXISTS superseded_by_id integer + REFERENCES transactions(id) ON DELETE SET NULL; + +COMMENT ON COLUMN transactions.superseded_by_id IS + 'This row was imported twice; the named row is the one that counts. Excluded from every figure, kept for its children and its audit trail. NULL = live.'; + +CREATE INDEX IF NOT EXISTS idx_transactions_superseded + ON transactions (superseded_by_id) + WHERE superseded_by_id IS NOT NULL; + +-- A row cannot supersede itself, and a survivor cannot itself be superseded +-- (that would hide both members of the pair and lose the amount entirely). +ALTER TABLE transactions + DROP CONSTRAINT IF EXISTS transactions_no_self_supersede; +ALTER TABLE transactions + ADD CONSTRAINT transactions_no_self_supersede + CHECK (superseded_by_id IS NULL OR superseded_by_id <> id); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 8777beb..910345e 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -183,11 +183,14 @@ model transactions { payment_method String? // card | cash | bank_transfer | other; NULL = unknown (migration 0016) owner_id Int? reconciled_with_id Int? + superseded_by_id Int? principal_amount Decimal? @db.Decimal(12, 2) interest_amount Decimal? @db.Decimal(12, 2) statement statements? @relation(fields: [statement_id], references: [id], onDelete: Cascade) reconciled_with transactions? @relation("reconciled", fields: [reconciled_with_id], references: [id], onDelete: SetNull) reconciled_by transactions[] @relation("reconciled") + superseded_by transactions? @relation("superseded", fields: [superseded_by_id], references: [id], onDelete: SetNull) + supersedes transactions[] @relation("superseded") expense_metadata expense_metadata? order_review order_reviews? } diff --git a/src/__tests__/integration/queries.test.ts b/src/__tests__/integration/queries.test.ts index f39b7d0..4ff8693 100644 --- a/src/__tests__/integration/queries.test.ts +++ b/src/__tests__/integration/queries.test.ts @@ -608,3 +608,61 @@ describe("getStatements — overlapping billing periods", () => { expect(rows.every((r) => r.overlaps.length === 0)).toBe(true); }); }); + +// A statement imported twice puts every transaction in the overlap in the +// ledger twice. The duplicate is superseded rather than deleted, because every +// child of `transactions` cascades on delete. +describe("superseded duplicates are excluded but kept", () => { + it("hides a superseded row from the transaction list", async () => { + const { ownerId } = await seedParticipants(pool); + const keep = await insertTransaction(pool, ownerId, { description: "RAIZ INVESTMENT", amount: 1500 }); + const dup = await insertTransaction(pool, ownerId, { description: "RAIZ INVESTMENT", amount: 1500 }); + await pool.query(`UPDATE transactions SET superseded_by_id = $1 WHERE id = $2`, [keep, dup]); + + const { data, total } = await getTransactions(ownerId, { limit: 50, offset: 0 }); + expect(total).toBe(1); + expect(data.map((t) => t.id)).toEqual([keep]); + }); + + it("keeps the superseded row and its children in the database", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const keep = await insertTransaction(pool, ownerId, { amount: 100 }); + const dup = await insertTransaction(pool, ownerId, { amount: 100 }); + await pool.query( + `INSERT INTO transaction_splits (transaction_id, participant_id, share_percent) VALUES ($1, $2, 50)`, + [dup, otherId] + ); + await pool.query(`UPDATE transactions SET superseded_by_id = $1 WHERE id = $2`, [keep, dup]); + + const rows = await pool.query(`SELECT superseded_by_id FROM transactions WHERE id = $1`, [dup]); + expect(rows.rows[0].superseded_by_id).toBe(keep); + const kids = await pool.query(`SELECT count(*)::int AS n FROM transaction_splits WHERE transaction_id = $1`, [dup]); + expect(kids.rows[0].n).toBe(1); + }); + + // The point of excluding it: a split on a duplicate must not be owed twice. + it("does not count a superseded row towards what someone owes", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const keep = await insertTransaction(pool, ownerId, { amount: 100 }); + const dup = await insertTransaction(pool, ownerId, { amount: 100 }); + for (const id of [keep, dup]) { + await pool.query( + `INSERT INTO transaction_splits (transaction_id, participant_id, share_percent) VALUES ($1, $2, 50)`, + [id, otherId] + ); + } + await pool.query(`UPDATE transactions SET superseded_by_id = $1 WHERE id = $2`, [keep, dup]); + + const balances = await getParticipantBalances(ownerId); + const bob = balances.find((b) => b.id === otherId); + expect(Number(bob!.total_owed)).toBeCloseTo(50); + }); + + it("refuses to let a row supersede itself", async () => { + const { ownerId } = await seedParticipants(pool); + const id = await insertTransaction(pool, ownerId); + await expect( + pool.query(`UPDATE transactions SET superseded_by_id = $1 WHERE id = $1`, [id]) + ).rejects.toThrow(); + }); +}); diff --git a/src/lib/analytics-sql.ts b/src/lib/analytics-sql.ts index a2da1f8..baa05f9 100644 --- a/src/lib/analytics-sql.ts +++ b/src/lib/analytics-sql.ts @@ -36,7 +36,9 @@ export const OWNER_SCOPE = `COALESCE(t.owner_id, s.owner_id)`; export const STATEMENTS_JOIN = `LEFT JOIN statements s ON s.id = t.statement_id`; /** - * Drops the manual/CSV row that a statement line has superseded. + * Drops rows that a different row has replaced. Two distinct cases: + * + * **1. The manual/CSV row a statement line superseded** (`reconciled_with_id`). * * Reconciliation keeps both rows: the manual one the user entered and the * statement line it turned out to be. Only the statement line should count, or @@ -52,8 +54,26 @@ export const STATEMENTS_JOIN = `LEFT JOIN statements s ON s.id = t.statement_id` * inserted with `reconciled_with_id` NULL and are held out of the reconcile * queue by `needsCardMatch()`, so nothing ever sets it. If one is reconciled by * hand against a card line, this is what stops it double-counting. + * + * **2. The statement row imported twice** (`superseded_by_id`, migration 0023). + * + * When two statements for one account bill overlapping periods, every + * transaction in the overlap arrives twice — 31 pairs on ANZ `4085-56264`, from + * statements 107/142/143. Case 1 cannot express this: its predicate is scoped + * to `statement_id IS NULL` on purpose, and here BOTH rows are statement lines. + * + * The superseded row is excluded rather than deleted because every child of + * `transactions` cascades on delete, and the curation is not reliably on the + * surviving side. + * + * Adding it to this fragment rather than making a new one is deliberate: every + * query that already asks "count each purchase once" now excludes both kinds + * without being edited. Anything summing transactions without this fragment + * still double-counts — that is the same gap that let reconciled rows into the + * analytics routes in the first place. */ -export const EXCLUDE_RECONCILED_SOURCE = `NOT (t.statement_id IS NULL AND t.reconciled_with_id IS NOT NULL)`; +export const EXCLUDE_RECONCILED_SOURCE = `NOT (t.statement_id IS NULL AND t.reconciled_with_id IS NOT NULL) + AND t.superseded_by_id IS NULL`; /** * The currency `t.amount` is actually denominated in. diff --git a/src/lib/queries.ts b/src/lib/queries.ts index bde97f1..4bac104 100644 --- a/src/lib/queries.ts +++ b/src/lib/queries.ts @@ -897,6 +897,7 @@ export interface TripAnalytics { // alias the shared fragments assume. const TRIP_TOTAL_SPEND = `COALESCE(SUM( CASE WHEN ${NET_SPEND_ROWS} + AND ${EXCLUDE_RECONCILED_SOURCE} AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') THEN ${SPEND_SIGNED} ELSE 0 END ), 0)::float AS total_spend`; @@ -964,6 +965,7 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise JOIN transactions t ON t.id = o.transaction_id WHERE o.trip_id = $1 AND ${NET_SPEND_ROWS} + AND ${EXCLUDE_RECONCILED_SOURCE} AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') GROUP BY 1 ORDER BY 2 DESC @@ -977,6 +979,7 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise JOIN transactions t ON t.id = o.transaction_id WHERE o.trip_id = $1 AND ${NET_SPEND_ROWS} + AND ${EXCLUDE_RECONCILED_SOURCE} AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') GROUP BY 1 ORDER BY 1 @@ -991,6 +994,7 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise JOIN transactions t ON t.id = o.transaction_id WHERE o.trip_id = $1 AND ${NET_SPEND_ROWS} + AND ${EXCLUDE_RECONCILED_SOURCE} AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') GROUP BY 1 ORDER BY 2 DESC @@ -1008,6 +1012,7 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise JOIN tags tg ON tg.id = tt.tag_id WHERE o.trip_id = $1 AND ${NET_SPEND_ROWS} + AND ${EXCLUDE_RECONCILED_SOURCE} AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') GROUP BY tg.id ORDER BY 4 DESC