From cb7665ded190cfe20558780ea27cedef9bdc919d Mon Sep 17 00:00:00 2001 From: siddharthd Date: Sun, 2 Aug 2026 19:11:19 +1000 Subject: [PATCH] Let everyone on a trip see it, and give payments their scope back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CLAUDE.md | 90 ++++- src/__tests__/integration/helpers.ts | 28 ++ src/__tests__/integration/queries.test.ts | 371 +++++++++++++++++- src/app/api/split-payments/route.ts | 42 +- .../api/tags/[id]/convert-to-trip/route.ts | 12 +- src/app/api/transactions/bulk/route.ts | 13 +- src/app/api/transactions/route.ts | 1 + src/app/api/trips/[id]/route.ts | 13 + src/app/api/trips/[id]/transactions/route.ts | 11 +- src/app/shared/page.tsx | 29 ++ src/app/trips/[id]/page.tsx | 85 +++- src/lib/hooks.ts | 16 + src/lib/queries.ts | 271 ++++++++++--- 13 files changed, 904 insertions(+), 78 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 83b706f..e416a87 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -249,7 +249,95 @@ a *cancelled* booking has both legs untagged from the trip by hand, because a trip never incurred a cost it cancelled. **Trips:** Europe 2026 (id 1, 19 Mar–12 Apr), Auckland 2026 (id 2), -Europe — Sonu + Sunny (id 3, 12–28 Apr, created 2026-07-28 from tag 5). +Europe — Sonu + Sunny (id 3, 12–28 Apr, created 2026-07-28 from tag 5), +Singapore + Bangkok 2026 (id 4). + +### Trip participation is derived, and a trip is shared + +Rebuilt 2026-08-02. Trips were scoped to `trips.owner_id`, so Sonu saw **no +trips at all** despite paying for 104 of the tagged rows herself — her own +spending was invisible on the only page organised around it. + +**A participant is anyone with a split on, who paid for, or whose payment is +scoped to, a transaction tagged to the trip.** Derived (`TRIP_PARTICIPANT` in +`queries.ts`), never stored. A membership table was designed and rejected: it +would be a second record of a fact the expenses already carry, and two records +of one fact drift — the same reason sharing is a real split rather than a flag. +The derivation also gets the exclusions right for free, which a table has to be +kept in sync to do: Singapore + Bangkok 2026 has no Sonu split and no Sonu +payment, so she is not a participant and never sees it. Live result is +Siddharth 4 trips, Sonu 3, Molina 1. + +**Everything is shared except delete.** Read, edit and assign are open to any +participant. `deleteTrip` stays `owner_id`-only because both trip foreign keys +are `ON DELETE SET NULL`, so deleting Europe 2026 untags 210 transactions *and* +NULLs the trip scope on 6 payments — which is where the hand-derived +Europe-first allocation lives, and nothing recomputes it. The route returns 403 +with the reason rather than a 404 that pretends the trip is missing. + +**`getTransactions` gained `trip_all_rows`, and it is opt-in for a reason.** A +participant sees every row on a trip, not only their own — the trip total +already counts every payer. It must NOT be implied by `trip_id` being present: +`GET /api/transactions` is also the main transactions list, and its trip filter +has to keep owner scoping or filtering your own ledger by "Europe 2026" would +quietly fill it with someone else's rows. Participation is re-checked in SQL, so +passing the flag for a trip you are not on returns nothing rather than +everything. Only `trips/[id]/page.tsx` sets it. + +**Trip owed is pairwise and returns BOTH directions, never 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; an obligation lives on a row someone else paid +for, so a viewer-as-payer figure can never contain it, and Sonu's Europe 2026 +read "you are owed $2,408.24" while omitting the $8,004.04 she owed. + +**Do not collapse the two into one signed net.** It looks like the obvious next +step and it silently corrupts the scope allocation. The grouped-payment +allocation (memory case `allocate_grouped_payments`) cleared Sonu's transfers +against the trip debts chronologically, Europe first with the remainder to +household — and the debt it cleared was the one-directional gross. Netting +redefines Europe's debt as $7,201.30 after the fact, turning the $8,004.04 +already allocated into an $802.75 over-allocation with household understated by +the same amount. The total stays right; the split between scopes stops being. +Both halves therefore carry their gross and payments too (`owed_gross`, +`paid_to_me`, `i_owe_gross`, `paid_by_me`) so a paid-up trip reads "settled" +rather than a bare `0.00`. Verified byte-identical across the change: Auckland +Sonu $1,505.64, Europe Molina −$816.16, Europe Sonu $0.00, Sonu + Sunny Sonu +$0.00. + +### Payment scope reaches the API (2026-08-02) + +`split_payments.trip_id` has existed since migration 0022, but `POST +/api/split-payments` never read it and `GET` never returned it — so **every +payment recorded through the app landed on the household tab**, and the 9 +trip-scoped rows had to be written by hand in SQL. A $11k Europe settlement was +silently reducing the ongoing household balance. + +Both fixed. The modal has a "Settles" selector (Household or a trip) and history +shows each payment's scope as a chip. **"Both" needs no new shape:** one +transfer becomes one row per scope sharing a `linked_transaction_id`, which is +why there is deliberately no unique constraint on it — tx 4121's $4,794.06 sits +as $1,145.52 against Europe — Sonu + Sunny and $3,648.54 against household, and +tx 4111's $3,779.33 spans two trips. All six linked transfers reconcile to the +cent. + +### Three write paths that had no authorisation + +All closed 2026-08-02. Each was reachable by any authenticated participant: + +- **`assignTransactionsToTrip`** took no caller and checked nothing, so + `PATCH /api/trips/[id]/transactions` and `POST /api/transactions/bulk` + (`assign_trip`) let anyone 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. + Now: only rows the caller can already see move, and a non-null destination must + be a trip they participate in — enforced in the query, not the route, so + neither caller can bypass it. Returns the count actually moved. +- **`DELETE /api/split-payments?id=`** deleted by id with no check at all. Erasing + a settlement silently resurrects a discharged debt — the same class of damage + as the split rewrite that reset `settled`. Now limited to the two people the + payment is between. +- **`POST /api/split-payments`** accepted any `from`/`to` pair. Now the payment + must involve the caller, and a trip scope must be a trip they are on. **Partial split coverage inside a category is usually correct, not a gap.** Only *shared* items are split. `utilities` sits at 69% yours because Globird, OVO, GWW diff --git a/src/__tests__/integration/helpers.ts b/src/__tests__/integration/helpers.ts index 21c2319..be7b254 100644 --- a/src/__tests__/integration/helpers.ts +++ b/src/__tests__/integration/helpers.ts @@ -27,8 +27,36 @@ export function mockDbWithPool(p: Pool) { })); } +/** + * Refuse to truncate anything that is not the test database. + * + * `DATABASE_URL` in `.env.test` names the Postgres container by IP, and + * container IPs move on recreation: 172.22.0.47 stopped being + * `postgres-personal` and became `postgres-pantry`, so the suite spent a while + * pointing its TRUNCATE at another app's database. It only failed safe because + * the credentials happened not to match — had they matched, this would have + * wiped pantry-app. + * + * Checked once per process, before the first truncate. + */ +let targetVerified = false; +async function assertTestDatabase(pool: Pool) { + if (targetVerified) return; + const { rows } = await pool.query<{ db: string }>(`SELECT current_database() AS db`); + const db = rows[0]?.db; + if (db !== "personal_test") { + throw new Error( + `Refusing to truncate: connected to "${db}", expected "personal_test". ` + + `Check DATABASE_URL in .env.test — the Postgres container IP may have changed ` + + `(docker inspect postgres-personal --format '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}').` + ); + } + targetVerified = true; +} + /** Wipe all data tables and restart sequences between tests. */ export async function resetDB(pool: Pool) { + await assertTestDatabase(pool); await pool.query(` TRUNCATE split_payments, diff --git a/src/__tests__/integration/queries.test.ts b/src/__tests__/integration/queries.test.ts index 6c99a6a..4f2c83d 100644 --- a/src/__tests__/integration/queries.test.ts +++ b/src/__tests__/integration/queries.test.ts @@ -8,7 +8,10 @@ mockDbWithPool(pool); // Dynamic import AFTER the mock ensures getTransactions / getParticipantBalances // use the test pool rather than Prisma's singleton. -const { getTransactions, getParticipantBalances, getTripAnalytics, getTripById, getStatements } = await import("@/lib/queries"); +const { + getTransactions, getParticipantBalances, getTripAnalytics, getTripById, getStatements, + getTrips, isTripParticipant, assignTransactionsToTrip, deleteTrip, +} = await import("@/lib/queries"); beforeEach(async () => { await resetDB(pool); @@ -778,3 +781,369 @@ describe("the split cutover gates every balance", () => { expect(Number(bob?.total_owed ?? 0)).toBeCloseTo(0); }); }); + +// ── Trip participation ──────────────────────────────────────────────────────── +// +// Trips were scoped to `trips.owner_id`, so a co-traveller saw nothing: Sonu +// could not open a single trip despite paying for 104 of the tagged rows +// herself. Participation is DERIVED from the expenses rather than stored as a +// membership list, because a trip is all the expenses on one trip — and two +// records of one fact drift apart. + +describe("trip participation — visibility", () => { + /** A trip owned by `ownerId` with one row `ownerId` paid for. */ + async function tripWithOwnerRow(ownerId: number, name = "Owned Trip") { + const trip = await pool.query( + `INSERT INTO trips (owner_id, name) VALUES ($1, $2) RETURNING id`, + [ownerId, name] + ); + const tripId = trip.rows[0].id as number; + const txId = await insertTransaction(pool, ownerId, { amount: 200, category: "travel" }); + await pool.query( + `INSERT INTO transaction_overrides (transaction_id, trip_id) VALUES ($1, $2)`, + [txId, tripId] + ); + return { tripId, txId }; + } + + it("shows a trip to its owner", async () => { + const { ownerId } = await seedParticipants(pool); + const { tripId } = await tripWithOwnerRow(ownerId); + + const trips = await getTrips(ownerId); + expect(trips.map((t) => t.id)).toContain(tripId); + }); + + it("shows a trip to someone holding a split on one of its rows", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const { tripId, txId } = await tripWithOwnerRow(ownerId); + await pool.query( + `INSERT INTO transaction_splits (transaction_id, participant_id, share_percent) + VALUES ($1, $2, 50)`, + [txId, otherId] + ); + + const trips = await getTrips(otherId); + expect(trips.map((t) => t.id)).toContain(tripId); + expect(await isTripParticipant(tripId, otherId)).toBe(true); + }); + + it("shows a trip to someone who paid for one of its rows but holds no split", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const { tripId } = await tripWithOwnerRow(ownerId); + const theirTx = await insertTransaction(pool, otherId, { amount: 80, category: "travel" }); + await pool.query( + `INSERT INTO transaction_overrides (transaction_id, trip_id) VALUES ($1, $2)`, + [theirTx, tripId] + ); + + expect((await getTrips(otherId)).map((t) => t.id)).toContain(tripId); + }); + + it("shows a trip to someone whose payment is scoped to it", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const { tripId } = await tripWithOwnerRow(ownerId); + await pool.query( + `INSERT INTO split_payments (from_participant_id, to_participant_id, amount, payment_date, trip_id) + VALUES ($1, $2, 50, '2026-06-20', $3)`, + [otherId, ownerId, tripId] + ); + + expect((await getTrips(otherId)).map((t) => t.id)).toContain(tripId); + }); + + // The Singapore + Bangkok 2026 case. A trip nobody else took must not appear + // just because trips became shareable — this is the whole reason + // participation is derived from the expenses rather than granted. + it("HIDES a trip from someone with no split, no row and no payment on it", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const { tripId } = await tripWithOwnerRow(ownerId, "Solo Trip"); + + expect((await getTrips(otherId)).map((t) => t.id)).not.toContain(tripId); + expect(await isTripParticipant(tripId, otherId)).toBe(false); + expect(await getTripById(tripId, otherId)).toBeNull(); + }); +}); + +describe("trip owed — both directions, never netted", () => { + /** `payerId` paid a $200 travel row on the trip; `splitId` holds 50% of it. */ + async function seed(payerId: number, splitId: number, ownerId: number) { + const trip = await pool.query( + `INSERT INTO trips (owner_id, name) VALUES ($1, 'Pair Trip') RETURNING id`, + [ownerId] + ); + const tripId = trip.rows[0].id as number; + const txId = await insertTransaction(pool, payerId, { amount: 200, 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, splitId] + ); + return tripId; + } + + it("the payer sees it as owed to them, with nothing on the mirror", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const tripId = await seed(ownerId, otherId, ownerId); + + const { participant_splits } = await getTripAnalytics(tripId, ownerId); + const bob = participant_splits.find((r) => r.participant_id === otherId)!; + expect(Number(bob.owed)).toBeCloseTo(100); + expect(Number(bob.i_owe)).toBeCloseTo(0); + expect(Number(bob.i_owe_gross)).toBeCloseTo(0); + }); + + // The figure that could not exist before. An obligation lives on a row someone + // ELSE paid for, so a viewer-as-payer query can never contain it — which is + // why Sonu's Europe page read "you are owed $2,408.24" while omitting the + // $8,004.04 she owed. + it("the split holder sees the same figure as owed BY them", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const tripId = await seed(ownerId, otherId, ownerId); + + const { participant_splits } = await getTripAnalytics(tripId, otherId); + const alice = participant_splits.find((r) => r.participant_id === ownerId)!; + expect(Number(alice.i_owe)).toBeCloseTo(100); + expect(Number(alice.owed)).toBeCloseTo(0); + }); + + // Europe 2026: paid in full, so the net is zero but the gross is not — the UI + // needs both to say "settled" rather than a bare "0.00". + it("keeps gross and paid alongside the net so a paid-up trip reads as settled", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const tripId = await seed(ownerId, otherId, ownerId); + await pool.query( + `INSERT INTO split_payments (from_participant_id, to_participant_id, amount, payment_date, trip_id) + VALUES ($1, $2, 100, '2026-06-20', $3)`, + [otherId, ownerId, tripId] + ); + + const asPayer = await getTripAnalytics(tripId, ownerId); + const bob = asPayer.participant_splits.find((r) => r.participant_id === otherId)!; + expect(Number(bob.owed)).toBeCloseTo(0); + expect(Number(bob.owed_gross)).toBeCloseTo(100); + expect(Number(bob.paid_to_me)).toBeCloseTo(100); + + const asDebtor = await getTripAnalytics(tripId, otherId); + const alice = asDebtor.participant_splits.find((r) => r.participant_id === ownerId)!; + expect(Number(alice.i_owe)).toBeCloseTo(0); + expect(Number(alice.i_owe_gross)).toBeCloseTo(100); + expect(Number(alice.paid_by_me)).toBeCloseTo(100); + }); + + // Netting the two would redefine the debt the grouped-payment allocation was + // computed against, turning a settled trip into an overpayment and leaving + // household understated by the same amount. + it("does not net the two directions against each other", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const tripId = await seed(ownerId, otherId, ownerId); + // A second row, paid the other way, so both directions are live at once. + const theirTx = await insertTransaction(pool, otherId, { amount: 60, category: "travel" }); + await pool.query( + `INSERT INTO transaction_overrides (transaction_id, trip_id) VALUES ($1, $2)`, + [theirTx, tripId] + ); + await pool.query( + `INSERT INTO transaction_splits (transaction_id, participant_id, share_percent) + VALUES ($1, $2, 50)`, + [theirTx, ownerId] + ); + + const { participant_splits } = await getTripAnalytics(tripId, ownerId); + const bob = participant_splits.find((r) => r.participant_id === otherId)!; + expect(Number(bob.owed)).toBeCloseTo(100); + expect(Number(bob.i_owe)).toBeCloseTo(30); + // Emphatically not 70. + expect(Number(bob.owed) - Number(bob.i_owe)).toBeCloseTo(70); + }); + + it("reports whether the viewer owns the trip", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const tripId = await seed(ownerId, otherId, ownerId); + + expect((await getTripAnalytics(tripId, ownerId)).viewer_is_owner).toBe(true); + expect((await getTripAnalytics(tripId, otherId)).viewer_is_owner).toBe(false); + }); +}); + +describe("getTransactions — trip_all_rows", () => { + async function seedSharedTrip() { + const { ownerId, otherId } = await seedParticipants(pool); + const trip = await pool.query( + `INSERT INTO trips (owner_id, name) VALUES ($1, 'Shared Trip') RETURNING id`, + [ownerId] + ); + const tripId = trip.rows[0].id as number; + // One row Bob holds a split on — this is what makes him a participant. + const shared = await insertTransaction(pool, ownerId, { description: "Shared hotel", category: "travel" }); + // One row Bob has no stake in whatsoever. + const solo = await insertTransaction(pool, ownerId, { description: "Alice solo museum", category: "travel" }); + await pool.query( + `INSERT INTO transaction_overrides (transaction_id, trip_id) VALUES ($1, $2), ($3, $2)`, + [shared, tripId, solo] + ); + await pool.query( + `INSERT INTO transaction_splits (transaction_id, participant_id, share_percent) + VALUES ($1, $2, 50)`, + [shared, otherId] + ); + return { ownerId, otherId, tripId }; + } + + it("gives a participant every row on the trip", async () => { + const { otherId, tripId } = await seedSharedTrip(); + + const { data } = await getTransactions(otherId, { + trip_id: String(tripId), trip_all_rows: true, limit: 50, offset: 0, + }); + expect(data.map((r) => r.description).sort()).toEqual(["Alice solo museum", "Shared hotel"]); + }); + + // The main transactions page filters by trip through this same endpoint. If + // the widening were implied by trip_id, filtering your own ledger by a trip + // would silently fill it with someone else's rows and skew its totals. + it("keeps owner scoping when the flag is absent", async () => { + const { otherId, tripId } = await seedSharedTrip(); + + const { data } = await getTransactions(otherId, { + trip_id: String(tripId), limit: 50, offset: 0, + }); + expect(data.map((r) => r.description)).toEqual(["Shared hotel"]); + }); + + it("returns nothing to a non-participant who passes the flag", async () => { + const { ownerId } = await seedSharedTrip(); + const stranger = await pool.query( + `INSERT INTO participants (name) VALUES ('Carol') RETURNING id` + ); + const carolId = stranger.rows[0].id as number; + const trip = await pool.query(`SELECT id FROM trips LIMIT 1`); + + const { data } = await getTransactions(carolId, { + trip_id: String(trip.rows[0].id), trip_all_rows: true, limit: 50, offset: 0, + }); + expect(data).toHaveLength(0); + expect(ownerId).toBeGreaterThan(0); + }); + + it("does not widen anything when trip_id is 'unassigned'", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + await insertTransaction(pool, ownerId, { description: "Alice untripped" }); + await insertTransaction(pool, otherId, { description: "Bob untripped" }); + + const { data } = await getTransactions(otherId, { + trip_id: "unassigned", trip_all_rows: true, limit: 50, offset: 0, + }); + expect(data.map((r) => r.description)).toEqual(["Bob untripped"]); + }); +}); + +describe("assignTransactionsToTrip — authorisation", () => { + it("refuses a trip the caller does not participate in", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const trip = await pool.query( + `INSERT INTO trips (owner_id, name) VALUES ($1, 'Private Trip') RETURNING id`, + [ownerId] + ); + const tripId = trip.rows[0].id as number; + const bobsTx = await insertTransaction(pool, otherId, { description: "Bob lunch" }); + + await expect(assignTransactionsToTrip(tripId, [bobsTx], otherId)).rejects.toThrow(/participant/i); + }); + + // The hole this closed: the function took no caller at all, so any + // authenticated participant could move any transaction id into any trip. + it("silently skips transactions the caller cannot see", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const trip = await pool.query( + `INSERT INTO trips (owner_id, name) VALUES ($1, 'Alice Trip') RETURNING id`, + [ownerId] + ); + const tripId = trip.rows[0].id as number; + const mine = await insertTransaction(pool, ownerId, { description: "Alice flight" }); + const theirs = await insertTransaction(pool, otherId, { description: "Bob private" }); + + const moved = await assignTransactionsToTrip(tripId, [mine, theirs], ownerId); + expect(moved).toBe(1); + + const rows = await pool.query( + `SELECT transaction_id FROM transaction_overrides WHERE trip_id = $1`, [tripId] + ); + expect(rows.rows.map((r) => r.transaction_id)).toEqual([mine]); + }); + + it("lets a participant assign their own transaction to the trip", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const trip = await pool.query( + `INSERT INTO trips (owner_id, name) VALUES ($1, 'Joint Trip') RETURNING id`, + [ownerId] + ); + const tripId = trip.rows[0].id as number; + // Make Bob a participant first. + const seedTx = await insertTransaction(pool, ownerId, { category: "travel" }); + await pool.query( + `INSERT INTO transaction_overrides (transaction_id, trip_id) VALUES ($1, $2)`, + [seedTx, tripId] + ); + await pool.query( + `INSERT INTO transaction_splits (transaction_id, participant_id, share_percent) + VALUES ($1, $2, 50)`, + [seedTx, otherId] + ); + + const bobsTx = await insertTransaction(pool, otherId, { description: "Bob taxi" }); + expect(await assignTransactionsToTrip(tripId, [bobsTx], otherId)).toBe(1); + }); +}); + +// Delete is the one thing that stayed owner-only. Both trip foreign keys are +// ON DELETE SET NULL, so deleting a trip untags every transaction on it and +// drops the trip scope from its payments — including a hand-derived allocation +// that nothing recomputes. +describe("deleteTrip — owner only", () => { + it("does not delete when a non-owner participant asks", async () => { + const { ownerId, otherId } = await seedParticipants(pool); + const trip = await pool.query( + `INSERT INTO trips (owner_id, name) VALUES ($1, 'Precious Trip') RETURNING id`, + [ownerId] + ); + const tripId = trip.rows[0].id as number; + const txId = await insertTransaction(pool, ownerId, { 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, otherId] + ); + // Bob can see it... + expect(await getTripById(tripId, otherId)).not.toBeNull(); + + await deleteTrip(tripId, otherId); + + // ...and still cannot remove it, nor untag its transaction. + expect(await getTripById(tripId, ownerId)).not.toBeNull(); + const still = await pool.query( + `SELECT trip_id FROM transaction_overrides WHERE transaction_id = $1`, [txId] + ); + expect(still.rows[0].trip_id).toBe(tripId); + }); + + it("deletes when the owner asks", async () => { + const { ownerId } = await seedParticipants(pool); + const trip = await pool.query( + `INSERT INTO trips (owner_id, name) VALUES ($1, 'Doomed Trip') RETURNING id`, + [ownerId] + ); + const tripId = trip.rows[0].id as number; + + await deleteTrip(tripId, ownerId); + expect(await getTripById(tripId, ownerId)).toBeNull(); + }); +}); diff --git a/src/app/api/split-payments/route.ts b/src/app/api/split-payments/route.ts index 1fe9d6c..411b494 100644 --- a/src/app/api/split-payments/route.ts +++ b/src/app/api/split-payments/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { getCurrentUser } from "@/lib/auth"; import { queryRaw } from "@/lib/db"; import { prisma } from "@/lib/db"; +import { isTripParticipant } from "@/lib/queries"; export async function GET(req: NextRequest) { const user = await getCurrentUser(req); @@ -21,15 +22,22 @@ export async function GET(req: NextRequest) { payment_date: string; notes: string | null; linked_transaction_id: number | null; + trip_id: number | null; + trip_name: string | null; created_at: string; }>( + // trip_id was stored but never returned, so history could not show which tab + // a payment settled — and a grouped transfer looks like a duplicate until you + // can see that its rows carry different scopes. `SELECT sp.id, sp.from_participant_id, pf.name as from_name, sp.to_participant_id, pt.name as to_name, sp.amount, sp.payment_date, sp.notes, - sp.linked_transaction_id, sp.created_at + sp.linked_transaction_id, sp.trip_id, tr.name as trip_name, + sp.created_at FROM split_payments sp JOIN participants pf ON pf.id = sp.from_participant_id JOIN participants pt ON pt.id = sp.to_participant_id + LEFT JOIN trips tr ON tr.id = sp.trip_id WHERE (sp.from_participant_id = $1 OR sp.to_participant_id = $1) AND (sp.from_participant_id = $2 OR sp.to_participant_id = $2) ORDER BY sp.payment_date DESC, sp.created_at DESC`, @@ -50,9 +58,10 @@ export async function POST(req: NextRequest) { payment_date: string; notes?: string; linked_transaction_id?: number; + trip_id?: number | null; }; - const { from_participant_id, to_participant_id, amount, payment_date, notes, linked_transaction_id } = body; + const { from_participant_id, to_participant_id, amount, payment_date, notes, linked_transaction_id, trip_id } = body; if (!from_participant_id || !to_participant_id || !amount || !payment_date) { return NextResponse.json({ error: "Missing required fields" }, { status: 400 }); @@ -60,6 +69,24 @@ export async function POST(req: NextRequest) { if (amount <= 0) { return NextResponse.json({ error: "Amount must be positive" }, { status: 400 }); } + if (from_participant_id !== user.id && to_participant_id !== user.id) { + return NextResponse.json({ error: "A payment must involve you" }, { status: 403 }); + } + + // Scope. `trip_id` existed in the schema from migration 0022 but this route + // never read it, so every payment recorded in the app landed on the household + // tab and the 9 trip-scoped rows had to be written by hand in SQL. + // + // "Both" needs no extra shape: one transfer becomes one row per scope, all + // carrying the same linked_transaction_id — there is deliberately no unique + // constraint on it. That is how tx 4121's $4,794.06 sits as $1,145.52 against + // Europe — Sonu + Sunny and $3,648.54 against household. + if (trip_id != null && !(await isTripParticipant(trip_id, user.id))) { + return NextResponse.json( + { error: "Cannot scope a payment to a trip you are not on" }, + { status: 403 } + ); + } const payment = await prisma.split_payments.create({ data: { @@ -69,6 +96,7 @@ export async function POST(req: NextRequest) { payment_date: new Date(payment_date), notes: notes || null, linked_transaction_id: linked_transaction_id || null, + trip_id: trip_id ?? null, }, }); @@ -83,6 +111,16 @@ export async function DELETE(req: NextRequest) { const id = Number(sp.get("id")); if (!id) return NextResponse.json({ error: "id required" }, { status: 400 }); + // This deleted by id with no check at all: any authenticated participant could + // erase any settlement, which silently resurrects a discharged debt — the same + // class of damage as the split rewrite that reset `settled`. Deleting a payment + // must be limited to the two people it is between. + const existing = await prisma.split_payments.findUnique({ where: { id } }); + if (!existing) return NextResponse.json({ error: "Not found" }, { status: 404 }); + if (existing.from_participant_id !== user.id && existing.to_participant_id !== user.id) { + return NextResponse.json({ error: "Not your payment to delete" }, { status: 403 }); + } + await prisma.split_payments.delete({ where: { id } }); return NextResponse.json({ ok: true }); } diff --git a/src/app/api/tags/[id]/convert-to-trip/route.ts b/src/app/api/tags/[id]/convert-to-trip/route.ts index 22e5501..3295fd7 100644 --- a/src/app/api/tags/[id]/convert-to-trip/route.ts +++ b/src/app/api/tags/[id]/convert-to-trip/route.ts @@ -28,10 +28,14 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id: }); // Assign all transactions with this tag to the new trip + // The creator owns the new trip, so they participate in it by definition and + // the assignment's participation gate passes. `assigned` is what actually + // moved: rows the creator cannot see are skipped, so a tag spanning someone + // else's transactions converts to a trip holding only the creator's. const transactionIds = await getTagTransactionIds(tagId); - if (transactionIds.length > 0) { - await assignTransactionsToTrip(trip.id, transactionIds); - } + const assigned = transactionIds.length > 0 + ? await assignTransactionsToTrip(trip.id, transactionIds, user.id) + : 0; - return NextResponse.json({ trip, assigned: transactionIds.length }, { status: 201 }); + return NextResponse.json({ trip, assigned, tagged: transactionIds.length }, { status: 201 }); } diff --git a/src/app/api/transactions/bulk/route.ts b/src/app/api/transactions/bulk/route.ts index e8c626d..581aac5 100644 --- a/src/app/api/transactions/bulk/route.ts +++ b/src/app/api/transactions/bulk/route.ts @@ -122,8 +122,17 @@ export async function POST(req: NextRequest) { if (action === "assign_trip") { const { trip_id } = body as { ids: number[]; trip_id: number | null }; - await assignTransactionsToTrip(trip_id, ids); - return NextResponse.json({ updated: ids.length }); + try { + // `updated` is what actually moved, not what was asked for — ids the + // caller cannot see are skipped rather than silently applied. + const updated = await assignTransactionsToTrip(trip_id, ids, user.id); + return NextResponse.json({ updated, requested: ids.length }); + } catch (e) { + return NextResponse.json( + { error: e instanceof Error ? e.message : "Failed to assign" }, + { status: 403 } + ); + } } return NextResponse.json({ error: "Invalid action" }, { status: 400 }); diff --git a/src/app/api/transactions/route.ts b/src/app/api/transactions/route.ts index 3220c06..5e7bf64 100644 --- a/src/app/api/transactions/route.ts +++ b/src/app/api/transactions/route.ts @@ -28,6 +28,7 @@ export async function GET(req: NextRequest) { amount_max: sp.get("amount_max") ? Number(sp.get("amount_max")) : undefined, has_split: sp.get("has_split") || undefined, trip_id: sp.get("trip_id") || undefined, + trip_all_rows: sp.get("trip_all_rows") === "1" || undefined, }); return NextResponse.json(result); diff --git a/src/app/api/trips/[id]/route.ts b/src/app/api/trips/[id]/route.ts index 62a04de..1002fff 100644 --- a/src/app/api/trips/[id]/route.ts +++ b/src/app/api/trips/[id]/route.ts @@ -21,10 +21,23 @@ export async function PATCH(req: NextRequest, { params }: { params: Promise<{ id return NextResponse.json(trip); } +// Everything else about a trip is shared; delete is not. Both trip foreign keys +// are ON DELETE SET NULL, so this untags every transaction on the trip and drops +// the trip scope from its payments — including the hand-derived Europe-first +// allocation, which nothing recomputes. A participant gets a 403 that says so +// rather than a 404 that pretends the trip is not there. export async function DELETE(req: NextRequest, { params }: { params: Promise<{ id: string }> }) { const user = await getCurrentUser(req); if (!user) return NextResponse.json({ error: "Forbidden" }, { status: 403 }); const { id } = await params; + const trip = await getTripById(Number(id), user.id); + if (!trip) return NextResponse.json({ error: "Not found" }, { status: 404 }); + if (trip.owner_id !== user.id) { + return NextResponse.json( + { error: "Only the trip owner can delete a trip. Deleting it would untag every transaction on it and unscope its payments." }, + { status: 403 } + ); + } await deleteTrip(Number(id), user.id); return new NextResponse(null, { status: 204 }); } diff --git a/src/app/api/trips/[id]/transactions/route.ts b/src/app/api/trips/[id]/transactions/route.ts index 37163ce..5a26f98 100644 --- a/src/app/api/trips/[id]/transactions/route.ts +++ b/src/app/api/trips/[id]/transactions/route.ts @@ -10,6 +10,13 @@ export async function PATCH(req: NextRequest, { params }: { params: Promise<{ id if (!Array.isArray(transactionIds) || !transactionIds.length) { return NextResponse.json({ error: "transactionIds must be a non-empty array" }, { status: 400 }); } - await assignTransactionsToTrip(Number(id), transactionIds); - return NextResponse.json({ ok: true }); + try { + const assigned = await assignTransactionsToTrip(Number(id), transactionIds, user.id); + return NextResponse.json({ ok: true, assigned, requested: transactionIds.length }); + } catch (e) { + return NextResponse.json( + { error: e instanceof Error ? e.message : "Failed to assign" }, + { status: 403 } + ); + } } diff --git a/src/app/shared/page.tsx b/src/app/shared/page.tsx index 95de71e..a579c20 100644 --- a/src/app/shared/page.tsx +++ b/src/app/shared/page.tsx @@ -11,6 +11,7 @@ import { useDeletePayment, useCurrentUser, useTags, + useTrips, type SplitPayment, } from "@/lib/hooks"; import type { SharedTransactionRow } from "@/lib/queries"; @@ -147,6 +148,7 @@ function RecordPaymentModal({ onClose: () => void; }) { const record = useRecordPayment(); + const { data: trips = [] } = useTrips(); const theyOweMe = currentBalance > 0; // Default direction matches the debt direction @@ -155,6 +157,8 @@ function RecordPaymentModal({ const [notes, setNotes] = useState(""); // direction: "received" = they paid me, "sent" = I paid them const [direction, setDirection] = useState<"received" | "sent">(theyOweMe ? "received" : "sent"); + // Which tab this settles. "" = the ongoing household tab (trip_id NULL). + const [tripId, setTripId] = useState(""); const [error, setError] = useState(""); async function handleSave() { @@ -168,6 +172,7 @@ function RecordPaymentModal({ amount: amt, payment_date: date, notes: notes || undefined, + trip_id: tripId ? Number(tripId) : null, }); onClose(); } catch (e) { @@ -216,6 +221,24 @@ function RecordPaymentModal({ + {/* Scope. Until now every payment recorded here landed on the household + tab, because the API dropped trip_id — so a $11k Europe settlement + silently reduced the ongoing household balance instead. */} +
+ + +

+ Covering more than one tab? Record it once per tab — the parts add back + up to the transfer. +

+
+
setNotes(e.target.value)} @@ -259,6 +282,12 @@ function PaymentHistory({ participantId, currentUserId }: { participantId: numbe {theyPaidMe ? "+" : "-"}${Number(p.amount).toFixed(2)} {formatDate(p.payment_date)} + {/* Scope, so a grouped transfer stops looking like a duplicate: two + rows of the same amount and date differ only by which tab they + settle, and that was invisible until the API returned trip_id. */} + + {p.trip_name ?? "Household"} + {p.notes && {p.notes}}