From 8c21893cc2059af9c3bd80d2165f0b7a9aec5783 Mon Sep 17 00:00:00 2001 From: siddharthd Date: Tue, 28 Jul 2026 11:25:03 +1000 Subject: [PATCH] fix(trips): money that came back is not what the trip cost Every trip figure filtered on transaction_type IN ('debit','fee','interest'), which drops refunds and credits outright. A partly-refunded booking therefore read at its full price and the refund subtracted nothing, anywhere: the headline total_spend, the category breakdown, the daily chart, top merchants and the tag breakdown were all gross. This is the same defect the general analytics fixed once already, which is why NET_SPEND_ROWS and SPEND_SIGNED exist -- a refunded Expedia purchase read as $2,888.92 of spend until they did. Trip analytics never adopted them. Doing so now costs one predicate and one expression per query. getTrips/getTripById needed the trips alias moved to `tr`: the fragments assume `t` is `transactions`, and hand-inlining a copy rather than renaming is exactly how the reconciled-row exclusion drifted out of the analytics routes before. On Europe 2026 this is $821.12 -- a LuxuryEscapes booking with two part-credits against it, and a FreeNow hold adjustment. Fully cancelled bookings are a different case and are handled by untagging both legs from the trip by hand, because a trip never incurred a cost it cancelled. No balance moves: the owed query already excludes credits and a refund carries no split. There is a test asserting exactly that, and it passes with or without this change -- it is a guard, not a proof. The three that do prove it fail without it. --- src/__tests__/integration/queries.test.ts | 74 ++++++++++++++- src/lib/queries.ts | 111 +++++++++++++--------- 2 files changed, 139 insertions(+), 46 deletions(-) diff --git a/src/__tests__/integration/queries.test.ts b/src/__tests__/integration/queries.test.ts index bdbf477..868544c 100644 --- a/src/__tests__/integration/queries.test.ts +++ b/src/__tests__/integration/queries.test.ts @@ -8,7 +8,7 @@ mockDbWithPool(pool); // Dynamic import AFTER the mock ensures getTransactions / getParticipantBalances // use the test pool rather than Prisma's singleton. -const { getTransactions, getParticipantBalances, getTripAnalytics } = await import("@/lib/queries"); +const { getTransactions, getParticipantBalances, getTripAnalytics, getTripById } = await import("@/lib/queries"); beforeEach(async () => { await resetDB(pool); @@ -466,3 +466,75 @@ describe("getTripAnalytics — owner scoping", () => { expect(Number(carol!.owed)).toBeCloseTo(150); }); }); + +// A refunded trip expense must not still read as trip cost. These queries +// filtered on debit/fee/interest, so a refund was dropped entirely and the +// original purchase stood at full value. +describe("getTripAnalytics — refunds reduce trip cost", () => { + async function seedTripWithRefund(ownerId: number) { + const trip = await pool.query( + `INSERT INTO trips (owner_id, name, start_date, end_date) + VALUES ($1, 'Refund Trip', '2026-03-01', '2026-03-10') RETURNING id`, + [ownerId] + ); + const tripId = trip.rows[0].id as number; + + const spend = await insertTransaction(pool, ownerId, { + amount: 200, category: "travel", description: "Hotel booking", transaction_date: "2026-03-02", + }); + const refund = await insertTransaction(pool, ownerId, { + amount: 50, category: "travel", description: "Hotel partial refund", + transaction_type: "refund", transaction_date: "2026-03-05", + }); + await pool.query( + `INSERT INTO transaction_overrides (transaction_id, trip_id) VALUES ($1, $3), ($2, $3)`, + [spend, refund, tripId] + ); + return tripId; + } + + it("nets the refund out of the category total", async () => { + const { ownerId } = await seedParticipants(pool); + const tripId = await seedTripWithRefund(ownerId); + + const { category_breakdown } = await getTripAnalytics(tripId, ownerId); + const travel = category_breakdown.find((c) => c.category === "travel"); + expect(Number(travel!.amount)).toBeCloseTo(150); + }); + + it("nets the refund out of the trip's headline total_spend", async () => { + const { ownerId } = await seedParticipants(pool); + const tripId = await seedTripWithRefund(ownerId); + + const trip = await getTripById(tripId, ownerId); + expect(Number(trip!.total_spend)).toBeCloseTo(150); + }); + + it("shows the refund as a negative on its own day", async () => { + const { ownerId } = await seedParticipants(pool); + const tripId = await seedTripWithRefund(ownerId); + + const { daily_spend } = await getTripAnalytics(tripId, ownerId); + const refundDay = daily_spend.find((d) => d.date === "2026-03-05"); + expect(Number(refundDay!.amount)).toBeCloseTo(-50); + }); + + // The owed side must be untouched: a refund carries no split, and the owed + // query deliberately excludes credits. Netting cost must not move a balance. + it("leaves what the other participant owes unchanged", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const tripId = await seedTripWithRefund(ownerId); + const rows = await pool.query( + `SELECT transaction_id FROM transaction_overrides WHERE trip_id = $1 ORDER BY transaction_id`, + [tripId] + ); + await pool.query( + `INSERT INTO transaction_splits (transaction_id, participant_id, share_percent) VALUES ($1, $2, 50)`, + [rows.rows[0].transaction_id, otherId] + ); + + const { participant_splits } = await getTripAnalytics(tripId, ownerId); + const bob = participant_splits.find((r) => r.participant_id === otherId); + expect(Number(bob!.owed)).toBeCloseTo(100); + }); +}); diff --git a/src/lib/queries.ts b/src/lib/queries.ts index 65459c9..61d8348 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, OWNER_SCOPE } from "./analytics-sql"; +import { EXCLUDE_RECONCILED_SOURCE, NATIVE_CURRENCY, AMOUNT_UNCONVERTED, ACTIVE_OBLIGATION, STATEMENTS_JOIN, OWNER_SCOPE, NET_SPEND_ROWS, SPEND_SIGNED } from "./analytics-sql"; export interface RoutePointRow { label: string; @@ -844,40 +844,42 @@ export interface TripAnalytics { }[]; } +// `total_spend` is the headline figure on the trips list and the trip header, +// and it nets refunds for the same reason getTripAnalytics does — see the note +// there. Trips are aliased `tr` so that `t` can be `transactions`, which is the +// alias the shared fragments assume. +const TRIP_TOTAL_SPEND = `COALESCE(SUM( + CASE WHEN ${NET_SPEND_ROWS} + AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') + THEN ${SPEND_SIGNED} ELSE 0 END + ), 0)::float AS total_spend`; + export async function getTrips(ownerId: number): Promise { return queryRaw(` SELECT - t.*, - COALESCE(SUM( - CASE WHEN tx.transaction_type IN ('debit','fee','interest') - AND COALESCE(o.category_override, tx.category, 'other') NOT IN ('transfers', 'investment') - THEN COALESCE(tx.amount_aud, tx.amount) ELSE 0 END - ), 0)::float AS total_spend, + tr.*, + ${TRIP_TOTAL_SPEND}, COUNT(o.transaction_id)::int AS transaction_count - FROM trips t - LEFT JOIN transaction_overrides o ON o.trip_id = t.id - LEFT JOIN transactions tx ON tx.id = o.transaction_id - WHERE t.owner_id = $1 - GROUP BY t.id - ORDER BY t.created_at DESC + FROM trips tr + LEFT JOIN transaction_overrides o ON o.trip_id = tr.id + LEFT JOIN transactions t ON t.id = o.transaction_id + WHERE tr.owner_id = $1 + GROUP BY tr.id + ORDER BY tr.created_at DESC `, [ownerId]); } export async function getTripById(id: number, ownerId: number): Promise { const rows = await queryRaw(` SELECT - t.*, - COALESCE(SUM( - CASE WHEN tx.transaction_type IN ('debit','fee','interest') - AND COALESCE(o.category_override, tx.category, 'other') NOT IN ('transfers', 'investment') - THEN COALESCE(tx.amount_aud, tx.amount) ELSE 0 END - ), 0)::float AS total_spend, + tr.*, + ${TRIP_TOTAL_SPEND}, COUNT(o.transaction_id)::int AS transaction_count - FROM trips t - LEFT JOIN transaction_overrides o ON o.trip_id = t.id - LEFT JOIN transactions tx ON tx.id = o.transaction_id - WHERE t.id = $1 AND t.owner_id = $2 - GROUP BY t.id + FROM trips tr + LEFT JOIN transaction_overrides o ON o.trip_id = tr.id + LEFT JOIN transactions t ON t.id = o.transaction_id + WHERE tr.id = $1 AND tr.owner_id = $2 + GROUP BY tr.id `, [id, ownerId]); return rows[0] ?? null; } @@ -886,44 +888,63 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise const trip = await getTripById(tripId, ownerId); if (!trip) throw new Error("Trip not found"); + // What the trip cost, with refunds subtracted. + // + // These four queries filtered on `transaction_type IN ('debit','fee','interest')`, + // which drops every refund and credit — so money that came back was still + // counted as trip spend. A partly-refunded booking read at its full price and + // a fully-refunded one read as pure cost. + // + // NET_SPEND_ROWS admits the refunds and SPEND_SIGNED carries their direction, + // the same pair the general analytics adopted after a refunded Expedia + // purchase read as $2,888.92 of spend. `transactions` is aliased `t` because + // the fragments assume that alias. + // + // A cancelled booking is a different case and is NOT handled here: both its + // legs are untagged from the trip by hand, because a trip the booking was + // cancelled from never incurred that cost at all. This nets the partial + // refunds — a price adjustment on a booking that did happen. + // + // COUNT(*) deliberately still counts refund rows: a refund is a transaction + // that occurred on the trip, even though it subtracts from the total. const [categoryRows, dailyRows, merchantRows, tagRows, splitRows] = await Promise.all([ queryRaw<{ category: string; amount: number; count: number }>(` SELECT - COALESCE(o.category_override, tx.category, 'other') AS category, - SUM(COALESCE(tx.amount_aud, tx.amount))::float AS amount, + COALESCE(o.category_override, t.category, 'other') AS category, + SUM(${SPEND_SIGNED})::float AS amount, COUNT(*)::int AS count FROM transaction_overrides o - JOIN transactions tx ON tx.id = o.transaction_id + JOIN transactions t ON t.id = o.transaction_id WHERE o.trip_id = $1 - AND tx.transaction_type IN ('debit','fee','interest') - AND COALESCE(o.category_override, tx.category, 'other') NOT IN ('transfers', 'investment') + AND ${NET_SPEND_ROWS} + AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') GROUP BY 1 ORDER BY 2 DESC `, [tripId]), queryRaw<{ date: string; amount: number }>(` SELECT - tx.transaction_date::text AS date, - SUM(COALESCE(tx.amount_aud, tx.amount))::float AS amount + t.transaction_date::text AS date, + SUM(${SPEND_SIGNED})::float AS amount FROM transaction_overrides o - JOIN transactions tx ON tx.id = o.transaction_id + JOIN transactions t ON t.id = o.transaction_id WHERE o.trip_id = $1 - AND tx.transaction_type IN ('debit','fee','interest') - AND COALESCE(o.category_override, tx.category, 'other') NOT IN ('transfers', 'investment') + AND ${NET_SPEND_ROWS} + AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') GROUP BY 1 ORDER BY 1 `, [tripId]), queryRaw<{ merchant: string; amount: number; count: number }>(` SELECT - COALESCE(o.merchant_normalized, tx.merchant_normalized, tx.merchant_name, tx.description) AS merchant, - SUM(COALESCE(tx.amount_aud, tx.amount))::float AS amount, + COALESCE(o.merchant_normalized, t.merchant_normalized, t.merchant_name, t.description) AS merchant, + SUM(${SPEND_SIGNED})::float AS amount, COUNT(*)::int AS count FROM transaction_overrides o - JOIN transactions tx ON tx.id = o.transaction_id + JOIN transactions t ON t.id = o.transaction_id WHERE o.trip_id = $1 - AND tx.transaction_type IN ('debit','fee','interest') - AND COALESCE(o.category_override, tx.category, 'other') NOT IN ('transfers', 'investment') + AND ${NET_SPEND_ROWS} + AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') GROUP BY 1 ORDER BY 2 DESC LIMIT 10 @@ -932,15 +953,15 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise queryRaw<{ tag_id: number; name: string; color: string; amount: number; count: number }>(` SELECT tg.id AS tag_id, tg.name, tg.color, - SUM(COALESCE(tx.amount_aud, tx.amount))::float AS amount, - COUNT(DISTINCT tx.id)::int AS count + SUM(${SPEND_SIGNED})::float AS amount, + COUNT(DISTINCT t.id)::int AS count FROM transaction_overrides o - JOIN transactions tx ON tx.id = o.transaction_id - JOIN transaction_tags tt ON tt.transaction_id = tx.id + JOIN transactions t ON t.id = o.transaction_id + JOIN transaction_tags tt ON tt.transaction_id = t.id JOIN tags tg ON tg.id = tt.tag_id WHERE o.trip_id = $1 - AND tx.transaction_type IN ('debit','fee','interest') - AND COALESCE(o.category_override, tx.category, 'other') NOT IN ('transfers', 'investment') + AND ${NET_SPEND_ROWS} + AND COALESCE(o.category_override, t.category, 'other') NOT IN ('transfers', 'investment') GROUP BY tg.id ORDER BY 4 DESC `, [tripId]),