From 4fcb135805dabe4e5482475a6765bcf8a2ad9a67 Mon Sep 17 00:00:00 2001 From: siddharthd Date: Mon, 27 Jul 2026 23:40:58 +1000 Subject: [PATCH] fix(trips): a trip figure must only count what the owner is owed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-trip owed number shipped in 689fadc counted every split on every trip transaction regardless of who paid, so it silently mixed debts owed to different people under one label. On Europe 2026 that meant Molina "owed" $21,572.12, of which $1,605.49 was her share of rows Sonu paid for — a real debt, but between the other two participants, and one they had already settled directly (split_payments id 5, Molina -> Sonu, exactly $1,605.49). A participant's own share of a row they themselves paid for was in there too, which is nobody's debt at all. Both sides needed scoping, not just one: the owed side to rows this owner paid for, and the paid side to payments made to this owner. Scoping only the first would have let a Molina -> Sonu payment reduce what Molina owes the owner. The corrected figures reproduce a number derived independently, months of data apart: Molina now reads -$816.16 on Europe, matching her known overpayment to the cent ($19,966.63 of splits against $20,782.79 paid). Sonu goes from $8,793.10 to $1,084.61, and the owner correctly disappears from a list of people who owe the owner. Found by checking a household total against what the app had been showing all along — the query was gross, gave a number about twice the real one, and I had quoted it as "owed". Worth stating plainly: the defect was not in the number the app displayed, it was in the number I computed to explain it. --- src/__tests__/integration/queries.test.ts | 55 +++++++++++++++++++++++ src/lib/queries.ts | 20 ++++++++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/__tests__/integration/queries.test.ts b/src/__tests__/integration/queries.test.ts index 8879421..bdbf477 100644 --- a/src/__tests__/integration/queries.test.ts +++ b/src/__tests__/integration/queries.test.ts @@ -411,3 +411,58 @@ describe("getTripAnalytics — per-trip settlement", () => { expect(bob === undefined || Number(bob.owed) === 0).toBe(true); }); }); + +describe("getTripAnalytics — owner scoping", () => { + it("ignores a trip expense someone else paid for", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const trip = await pool.query( + `INSERT INTO trips (owner_id, name) VALUES ($1, 'Owner Scope Trip') RETURNING id`, + [ownerId] + ); + const tripId = trip.rows[0].id as number; + + // Bob paid this one. Alice's share of it is a debt Alice owes Bob — it is + // not something Bob owes Alice, so it must not appear on Alice's trip view. + const bobPaid = await insertTransaction(pool, otherId, { amount: 500, category: "travel" }); + await pool.query(`INSERT INTO transaction_overrides (transaction_id, trip_id) VALUES ($1, $2)`, [bobPaid, tripId]); + await pool.query( + `INSERT INTO transaction_splits (transaction_id, participant_id, share_percent) + VALUES ($1, $2, 50), ($1, $3, 50)`, + [bobPaid, ownerId, otherId] + ); + + const { participant_splits } = await getTripAnalytics(tripId, ownerId); + const bob = participant_splits.find((r) => r.participant_id === otherId); + expect(bob === undefined || Number(bob.owed) === 0).toBe(true); + }); + + it("ignores a payment settled between the other two participants", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const third = await pool.query( + `INSERT INTO participants (name, email) VALUES ('Carol', 'carol@example.com') RETURNING id` + ); + const carolId = third.rows[0].id as number; + const trip = await pool.query( + `INSERT INTO trips (owner_id, name) VALUES ($1, 'Third Party Trip') RETURNING id`, + [ownerId] + ); + const tripId = trip.rows[0].id as number; + + const txId = await insertTransaction(pool, ownerId, { amount: 300, category: "travel" }); + await pool.query(`INSERT INTO transaction_overrides (transaction_id, trip_id) VALUES ($1, $2)`, [txId, tripId]); + await pool.query( + `INSERT INTO transaction_splits (transaction_id, participant_id, share_percent) VALUES ($1, $2, 50)`, + [txId, carolId] + ); + // Carol pays Bob, not the owner. Carol still owes the owner $150. + await pool.query( + `INSERT INTO split_payments (from_participant_id, to_participant_id, amount, payment_date, trip_id) + VALUES ($1, $2, 150, '2026-03-15', $3)`, + [carolId, otherId, tripId] + ); + + const { participant_splits } = await getTripAnalytics(tripId, ownerId); + const carol = participant_splits.find((r) => r.participant_id === carolId); + expect(Number(carol!.owed)).toBeCloseTo(150); + }); +}); diff --git a/src/lib/queries.ts b/src/lib/queries.ts index bd09e93..65459c9 100644 --- a/src/lib/queries.ts +++ b/src/lib/queries.ts @@ -1,5 +1,5 @@ import { queryRaw } from "./db"; -import { EXCLUDE_RECONCILED_SOURCE, NATIVE_CURRENCY, AMOUNT_UNCONVERTED, ACTIVE_OBLIGATION, STATEMENTS_JOIN } from "./analytics-sql"; +import { EXCLUDE_RECONCILED_SOURCE, NATIVE_CURRENCY, AMOUNT_UNCONVERTED, ACTIVE_OBLIGATION, STATEMENTS_JOIN, OWNER_SCOPE } from "./analytics-sql"; export interface RoutePointRow { label: string; @@ -966,6 +966,16 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise // so netting a EUR figure against AUD ones silently is most likely to // bite exactly here. // + // A fourth, and it is what "owed" actually means: only rows THIS owner paid + // for. Without ${OWNER_SCOPE} the figure sums every split on every trip + // transaction regardless of who paid, so it silently mixes debts owed to + // different people. On Europe 2026 that put $1,605.49 of Molina's share of + // Sonu-paid rows into a number labelled as owed to the owner — a debt that + // is real, but between the other two participants, and which they settled + // directly (split_payments id 5, Molina -> Sonu, exactly $1,605.49). + // A participant's own share of a row they paid for was in there too, which + // is nobody's debt at all. + // // `transactions` is aliased `t` so the shared fragments apply directly — // they assume that alias, and hand-inlining a copy is what let the // reconciled-row exclusion drift out of the analytics routes to begin with. @@ -979,16 +989,22 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise ${STATEMENTS_JOIN} JOIN transaction_splits ts ON ts.transaction_id = t.id WHERE o.trip_id = $1 + AND ${OWNER_SCOPE} = $2 + AND ts.participant_id <> $2 AND t.transaction_type IN ('debit','fee','interest') AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') AND ${ACTIVE_OBLIGATION} AND ${EXCLUDE_RECONCILED_SOURCE} GROUP BY ts.participant_id ), + -- Only payments made TO this owner. Payment 5 on Europe is Molina -> Sonu: + -- a real settlement, but of a debt between those two, so it must not + -- reduce what Molina owes here. Symmetrical with the owner scoping above. paid AS ( SELECT sp.from_participant_id AS pid, SUM(sp.amount) AS amt FROM split_payments sp WHERE sp.trip_id = $1 + AND sp.to_participant_id = $2 GROUP BY sp.from_participant_id ) SELECT p.id AS participant_id, p.name, @@ -999,7 +1015,7 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise LEFT JOIN paid ON paid.pid = p.id WHERE owed.pid IS NOT NULL OR paid.pid IS NOT NULL ORDER BY 3 DESC - `, [tripId]), + `, [tripId, ownerId]), ]); const num_days = (trip.start_date && trip.end_date)