fix(trips): a trip figure must only count what the owner is owed
ci / lint-test (push) Successful in 46s
ci / lint-test (push) Successful in 46s
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.
This commit is contained in:
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
+18
-2
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user