siddharthd/finance-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Let everyone on a trip see it, and give payments their scope back
ci / lint-test (push) Successful in 52s
Details

Browse Source 
Trips were scoped to trips.owner_id, so Sonu saw no trips at all — despite
having paid for 104 of the tagged rows herself. Her own spending was invisible
on the only page organised around it.

A participant is now anyone with a split on, who paid for, or whose payment is
scoped to, a transaction tagged to the trip. Derived, not stored. A
trip_participants table was designed and rejected: the expenses already carry
the fact, and two records of one fact drift apart. Deriving it also excludes
Singapore + Bangkok 2026 from Sonu for free, which a table would have to be kept
in sync to do. Siddharth 4 trips, Sonu 3, Molina 1.

Everything about a trip is shared except delete. Both trip foreign keys are
ON DELETE SET NULL, so deleting Europe 2026 untags 210 transactions and NULLs
the trip scope on 6 payments — where the hand-derived Europe-first allocation
lives, which nothing recomputes. That stays with the owner.

Trip owed now returns both directions and nets neither. An obligation lives on a
row someone else paid for, so a viewer-as-payer figure can never hold it, and
Sonu's Europe read "you are owed $2,408.24" while omitting the $8,004.04 she
owed. Collapsing the two into a signed net is the tempting next step and would
have corrupted the scope allocation: the grouped-payment allocation cleared each
trip against the one-directional gross, so redefining the debt afterwards turns
$8,004.04 already allocated into an $802.75 over-allocation with household
understated by the same amount. Verified byte-identical — Auckland $1,505.64,
Europe Molina -$816.16, Europe Sonu $0.00, Sonu + Sunny $0.00.

getTransactions gained trip_all_rows so a participant sees the whole trip. It is
opt-in and not implied by trip_id, because the same endpoint backs the main
transactions list and its trip filter must keep owner scoping. Participation is
re-checked in SQL, so passing the flag for someone else's trip returns nothing.

Payments can finally say what they settle. trip_id has existed since migration
0022 but POST never read it and GET never returned it, so every payment made in
the app landed on household and the 9 trip-scoped rows were hand-written SQL.
"Both" needs no new shape — one row per scope sharing a linked_transaction_id.

Three write paths had no authorisation at all and were reachable by any
participant: assignTransactionsToTrip checked nothing, DELETE on a payment
deleted by bare id, and POST accepted any from/to pair. All three now check.

Also fixes the test suite, which was pointing at postgres-pantry: container IPs
move on recreation and 172.22.0.47 stopped being postgres-personal. It only
failed safe because the credentials did not match — resetDB now refuses to
truncate anything not named personal_test.

22 new tests, 276 passing, build clean.
This commit is contained in:
siddharthd
2026-08-02 19:11:19 +10:00
parent 6e179d3a0a
commit cb7665ded1
13 changed files with 904 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/lib/queries.ts
+225 -46
View File
@@ -137,13 +137,33 @@ interface TransactionFilters {
amount_max?: number;
has_split?: string;
trip_id?: string;
/**
* With a `trip_id` set, show every row on the trip rather than only the
* viewer's own. A trip is all the expenses on one trip, so a participant sees
* the whole thing — the trip total already counts every payer.
*
* Opt-in, and deliberately NOT implied by `trip_id` being present, because
* `GET /api/transactions` is also the main transactions list and its trip
* filter must keep owner scoping — otherwise filtering your own ledger by
* "Europe 2026" would quietly fill it with someone else's rows and skew every
* total on the page. Only the trip detail view sets this.
*/
trip_all_rows?: boolean;
}
export async function getTransactions(ownerId: number, filters: TransactionFilters) {
const conditions: string[] = [
`(COALESCE(t.owner_id, s.owner_id) = $1 OR EXISTS (SELECT 1 FROM transaction_splits ts_me WHERE ts_me.transaction_id = t.id AND ts_me.participant_id = $1))`,
EXCLUDE_RECONCILED_SOURCE,
];
// A real trip id, not "unassigned" — there is no trip to participate in for
// rows that belong to none, so the owner scoping has to stand there.
const tripAllRows = Boolean(
filters.trip_all_rows && filters.trip_id && filters.trip_id !== "unassigned"
);
const conditions: string[] = [EXCLUDE_RECONCILED_SOURCE];
if (!tripAllRows) {
conditions.push(
`(COALESCE(t.owner_id, s.owner_id) = $1 OR EXISTS (SELECT 1 FROM transaction_splits ts_me WHERE ts_me.transaction_id = t.id AND ts_me.participant_id = $1))`
);
}
const params: unknown[] = [ownerId];
let paramIdx = 2;
@@ -231,8 +251,15 @@ export async function getTransactions(ownerId: number, filters: TransactionFilte
if (filters.trip_id === "unassigned") {
conditions.push(`o.trip_id IS NULL`);
} else if (filters.trip_id) {
conditions.push(`o.trip_id = $${paramIdx++}`);
const tripParam = paramIdx++;
conditions.push(`o.trip_id = $${tripParam}`);
params.push(Number(filters.trip_id));
if (tripAllRows) {
// The gate for having dropped the owner filter above. Enforced in SQL
// rather than trusted from the route, so passing trip_all_rows with a
// trip you are not on returns nothing instead of everything.
conditions.push(TRIP_PARTICIPANT(`$${tripParam}`, `$1`));
}
}
const where = `WHERE ${conditions.join(" AND ")}`;
@@ -904,11 +931,69 @@ export interface TripAnalytics {
participant_splits: {
participant_id: number;
name: string;
/** Their share of this trip, net of payments scoped to it. */
/** Their share of rows the VIEWER paid, net of payments scoped to this trip. */
owed: number;
/** That same figure before payments, so the UI can say "settled" rather than just "0.00". */
owed_gross: number;
/** Payments from them to the viewer, scoped to this trip. */
paid_to_me: number;
/** The VIEWER's share of rows THIS PARTICIPANT paid, net of the viewer's payments to them. */
i_owe: number;
/** Ditto, before payments. */
i_owe_gross: number;
/** Payments from the viewer to them, scoped to this trip. */
paid_by_me: number;
/** Splits counted at a non-AUD figure because no converted amount exists. */
unconverted_count: number;
i_owe_unconverted_count: number;
}[];
/** True when the viewer is a participant but not the trip's owner. */
viewer_is_owner: boolean;
}
/**
* Who counts as a participant on a trip. Derived, never stored.
*
* A trip is *all the expenses on one trip*, so being a participant is a fact
* about those expenses: you hold a split on one, you paid for one, or a payment
* of yours is scoped to the trip. Storing it as a membership list would be a
* second record of the same fact, and two records of one fact drift — the same
* reason `CLAUDE.md` insists sharing is a real split rather than a flag, and the
* reason the 2026-07-27 settlement design concluded an extra party on a trip
* "needs no schema at all".
*
* It also gets the exclusions right for free. Singapore + Bangkok 2026 has no
* Sonu split and no Sonu payment, so she is not a participant and never sees it,
* with no backfill to keep in sync as transactions are assigned and unassigned.
*
* Aliased `p*` throughout so it can be inlined anywhere without colliding with
* the `t`/`s`/`o` aliases the shared fragments assume.
*/
const TRIP_PARTICIPANT = (tripIdExpr: string, participantExpr: string) => `(
EXISTS (
SELECT 1 FROM transaction_overrides po
JOIN transactions pt ON pt.id = po.transaction_id
LEFT JOIN statements ps ON ps.id = pt.statement_id
LEFT JOIN transaction_splits pts ON pts.transaction_id = pt.id
WHERE po.trip_id = ${tripIdExpr}
AND (COALESCE(pt.owner_id, ps.owner_id) = ${participantExpr}
OR pts.participant_id = ${participantExpr})
)
OR EXISTS (
SELECT 1 FROM split_payments psp
WHERE psp.trip_id = ${tripIdExpr}
AND (psp.from_participant_id = ${participantExpr}
OR psp.to_participant_id = ${participantExpr})
)
)`;
/** Exported for the routes that gate a write on participation. */
export async function isTripParticipant(tripId: number, participantId: number): Promise<boolean> {
const rows = await queryRaw<{ ok: boolean }>(`
SELECT (tr.owner_id = $2 OR ${TRIP_PARTICIPANT('tr.id', '$2')}) AS ok
FROM trips tr WHERE tr.id = $1
`, [tripId, participantId]);
return rows[0]?.ok === true;
}
// `total_spend` is the headline figure on the trips list and the trip header,
@@ -922,7 +1007,11 @@ const TRIP_TOTAL_SPEND = `COALESCE(SUM(
THEN ${SPEND_SIGNED} ELSE 0 END
), 0)::float AS total_spend`;
export async function getTrips(ownerId: number): Promise<TripRow[]> {
// A trip is visible to its owner and to anyone who participates in it. Scoping
// visibility to `owner_id` alone meant Sonu could not see a single trip despite
// paying for 104 of the tagged rows herself — her own spending was invisible on
// the only page organised around it.
export async function getTrips(viewerId: number): Promise<TripRow[]> {
return queryRaw<TripRow>(`
SELECT
tr.*,
@@ -931,13 +1020,13 @@ export async function getTrips(ownerId: number): Promise<TripRow[]> {
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
WHERE tr.owner_id = $1 OR ${TRIP_PARTICIPANT('tr.id', '$1')}
GROUP BY tr.id
ORDER BY tr.created_at DESC
`, [ownerId]);
`, [viewerId]);
}
export async function getTripById(id: number, ownerId: number): Promise<TripRow | null> {
export async function getTripById(id: number, viewerId: number): Promise<TripRow | null> {
const rows = await queryRaw<TripRow>(`
SELECT
tr.*,
@@ -946,14 +1035,14 @@ export async function getTripById(id: number, ownerId: number): Promise<TripRow
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
WHERE tr.id = $1 AND (tr.owner_id = $2 OR ${TRIP_PARTICIPANT('tr.id', '$2')})
GROUP BY tr.id
`, [id, ownerId]);
`, [id, viewerId]);
return rows[0] ?? null;
}
export async function getTripAnalytics(tripId: number, ownerId: number): Promise<TripAnalytics> {
const trip = await getTripById(tripId, ownerId);
export async function getTripAnalytics(tripId: number, viewerId: number): Promise<TripAnalytics> {
const trip = await getTripById(tripId, viewerId);
if (!trip) throw new Error("Trip not found");
// What the trip cost, with refunds subtracted.
@@ -1059,7 +1148,7 @@ 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
// A fourth, and it is what "owed" actually means: only rows THIS viewer 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
@@ -1072,43 +1161,87 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise
// `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.
queryRaw<{ participant_id: number; name: string; owed: number; unconverted_count: number }>(`
WITH owed AS (
SELECT ts.participant_id AS pid,
SUM(ts.share_percent / 100.0 * COALESCE(t.amount_aud, t.amount)) AS gross,
SUM(CASE WHEN ${AMOUNT_UNCONVERTED} THEN 1 ELSE 0 END) AS unconverted
//
// ── Both directions, and deliberately NOT netted ──
//
// `owed` is unchanged: their share of rows the viewer paid. `i_owe` is the
// mirror: the viewer's share of rows THAT participant paid. Rendering the
// pair from the viewer's side is the whole fix — a non-owner used to get a
// page where their own obligation could not appear, so Sonu's Europe 2026
// read "you are owed $2,408.24" while omitting the $8,004.04 she owed.
//
// Collapsing the two into one signed net was the obvious next step and is
// WRONG. The grouped-payment allocation (memory case `allocate_grouped_payments`)
// cleared Sonu's transfers against the trip debts chronologically, Europe
// first, remainder to household — and the debt it cleared was this
// one-directional gross. Netting redefines Europe's debt as $7,201.30 after
// the fact, which turns the $8,004.04 already allocated into an $802.75
// over-allocation and leaves household understated by the same amount. The
// total stays right and the split between scopes silently stops being. So
// both halves are returned whole, with their gross and payments, and the UI
// states them separately.
queryRaw<{
participant_id: number; name: string;
owed: number; owed_gross: number; paid_to_me: number;
i_owe: number; i_owe_gross: number; paid_by_me: number;
unconverted_count: number; i_owe_unconverted_count: number;
}>(`
WITH scoped AS (
SELECT ts.participant_id AS split_pid,
${OWNER_SCOPE} AS payer_pid,
ts.share_percent / 100.0 * COALESCE(t.amount_aud, t.amount) AS amt,
CASE WHEN ${AMOUNT_UNCONVERTED} THEN 1 ELSE 0 END AS unconverted
FROM transaction_overrides o
JOIN transactions t ON t.id = o.transaction_id
${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 (
owed AS (
SELECT split_pid AS pid, SUM(amt) AS gross, SUM(unconverted) AS unconverted
FROM scoped WHERE payer_pid = $2 AND split_pid <> $2 GROUP BY 1
),
mine AS (
SELECT payer_pid AS pid, SUM(amt) AS gross, SUM(unconverted) AS unconverted
FROM scoped WHERE split_pid = $2 AND payer_pid <> $2 GROUP BY 1
),
-- Only payments made TO the viewer. Payment 5 is Molina -> Sonu: a real
-- settlement, but of a debt between those two, so it must not reduce what
-- Molina owes the viewer. Symmetrical with the payer scoping above.
paid_to_me 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
WHERE sp.trip_id = $1 AND sp.to_participant_id = $2
GROUP BY 1
),
paid_by_me AS (
SELECT sp.to_participant_id AS pid, SUM(sp.amount) AS amt
FROM split_payments sp
WHERE sp.trip_id = $1 AND sp.from_participant_id = $2
GROUP BY 1
)
SELECT p.id AS participant_id, p.name,
(COALESCE(owed.gross, 0) - COALESCE(paid.amt, 0))::float AS owed,
COALESCE(owed.unconverted, 0)::int AS unconverted_count
(COALESCE(owed.gross, 0) - COALESCE(paid_to_me.amt, 0))::float AS owed,
COALESCE(owed.gross, 0)::float AS owed_gross,
COALESCE(paid_to_me.amt, 0)::float AS paid_to_me,
(COALESCE(mine.gross, 0) - COALESCE(paid_by_me.amt, 0))::float AS i_owe,
COALESCE(mine.gross, 0)::float AS i_owe_gross,
COALESCE(paid_by_me.amt, 0)::float AS paid_by_me,
COALESCE(owed.unconverted, 0)::int AS unconverted_count,
COALESCE(mine.unconverted, 0)::int AS i_owe_unconverted_count
FROM participants p
LEFT JOIN owed ON owed.pid = p.id
LEFT JOIN paid ON paid.pid = p.id
WHERE owed.pid IS NOT NULL OR paid.pid IS NOT NULL
LEFT JOIN owed ON owed.pid = p.id
LEFT JOIN mine ON mine.pid = p.id
LEFT JOIN paid_to_me ON paid_to_me.pid = p.id
LEFT JOIN paid_by_me ON paid_by_me.pid = p.id
WHERE owed.pid IS NOT NULL OR mine.pid IS NOT NULL
OR paid_to_me.pid IS NOT NULL OR paid_by_me.pid IS NOT NULL
ORDER BY 3 DESC
`, [tripId, ownerId]),
`, [tripId, viewerId]),
]);
const num_days = (trip.start_date && trip.end_date)
@@ -1126,6 +1259,7 @@ export async function getTripAnalytics(tripId: number, ownerId: number): Promise
top_merchants: merchantRows,
tag_breakdown: tagRows,
participant_splits: splitRows,
viewer_is_owner: trip.owner_id === viewerId,
};
}
@@ -1141,9 +1275,11 @@ export async function createTrip(
return rows[0];
}
// Editable by any participant: a trip is a shared record of a shared journey,
// and dates or a colour are not the owner's private property.
export async function updateTrip(
id: number,
ownerId: number,
viewerId: number,
data: Partial<{ name: string; description: string | null; start_date: string | null; end_date: string | null; color: string; archived: boolean }>
): Promise<TripRow | null> {
const setClauses: string[] = [];
@@ -1155,31 +1291,74 @@ export async function updateTrip(
if ('end_date' in data) { setClauses.push(`end_date = $${idx++}`); params.push(data.end_date ?? null); }
if (data.color !== undefined) { setClauses.push(`color = $${idx++}`); params.push(data.color); }
if (data.archived !== undefined) { setClauses.push(`archived = $${idx++}`); params.push(data.archived); }
if (!setClauses.length) return getTripById(id, ownerId);
params.push(id, ownerId);
if (!setClauses.length) return getTripById(id, viewerId);
const idParam = idx++;
const viewerParam = idx;
params.push(id, viewerId);
const rows = await queryRaw<TripRow>(`
UPDATE trips SET ${setClauses.join(', ')}
WHERE id = $${idx++} AND owner_id = $${idx}
WHERE id = $${idParam}
AND (owner_id = $${viewerParam} OR ${TRIP_PARTICIPANT(`$${idParam}`, `$${viewerParam}`)})
RETURNING *, 0::float AS total_spend, 0::int AS transaction_count
`, params);
return rows[0] ?? null;
}
/**
* Delete stays OWNER-ONLY, deliberately, even though everything else about a
* trip is now shared.
*
* Both trip foreign keys are ON DELETE SET NULL, so deleting Europe 2026 untags
* 210 transactions *and* NULLs the trip scope on 6 payments. That scope is where
* the Europe-first allocation of Sonu's grouped transfers lives, and it was
* derived by hand — nothing in the app recomputes it. Handing that to any
* participant makes an unrecoverable loss one click away.
*/
export async function deleteTrip(id: number, ownerId: number): Promise<void> {
await queryRaw(`DELETE FROM trips WHERE id = $1 AND owner_id = $2`, [id, ownerId]);
}
/**
* Assign transactions to a trip, or to none when `tripId` is null.
*
* Both the scoping clauses here are new and both closed a live hole: this took
* no viewer at all and checked nothing, so `PATCH /api/trips/[id]/transactions`
* let any authenticated participant move any transaction id into any trip id.
* Not being able to *see* a trip was no obstacle, because the write path never
* read one.
*
* - Only rows the viewer can already see may be moved (`owner OR split`, the
* same test getTransactions applies).
* - A non-null destination must be a trip the viewer participates in. Checked
* here rather than only in the route so the guarantee cannot be bypassed by
* the other caller (`POST /api/transactions/bulk`, `assign_trip`).
*
* Returns the number of rows actually moved, which is how a caller detects that
* some ids were silently out of reach.
*/
export async function assignTransactionsToTrip(
tripId: number | null,
transactionIds: number[]
): Promise<void> {
if (!transactionIds.length) return;
await queryRaw(`
transactionIds: number[],
viewerId: number
): Promise<number> {
if (!transactionIds.length) return 0;
if (tripId !== null && !(await isTripParticipant(tripId, viewerId))) {
throw new Error("Not a participant on that trip");
}
const rows = await queryRaw<{ transaction_id: number }>(`
INSERT INTO transaction_overrides (transaction_id, trip_id)
SELECT unnest($1::int[]), $2
SELECT t.id, $2
FROM transactions t
LEFT JOIN statements s ON s.id = t.statement_id
WHERE t.id = ANY($1::int[])
AND (COALESCE(t.owner_id, s.owner_id) = $3
OR EXISTS (SELECT 1 FROM transaction_splits ts
WHERE ts.transaction_id = t.id AND ts.participant_id = $3))
ON CONFLICT (transaction_id)
DO UPDATE SET trip_id = EXCLUDED.trip_id
`, [transactionIds, tripId]);
RETURNING transaction_id
`, [transactionIds, tripId, viewerId]);
return rows.length;
}
export async function getTagTransactionIds(tagId: number): Promise<number[]> {