fix(splits): make every split account for 100%
ci / lint-test (push) Successful in 1m39s

A 50/50 arrangement was stored as a single row saying "Sonu 50%". The
arithmetic was never wrong — `myShare` resolves the payer's share as
`100 - SUM(everyone else)`, so balances and per-user spend were correct
throughout. It was still a bug, because a ledger is read as well as
computed: on screen that row is a 50% share against a blank, which looks
like half the money is unallocated and is indistinguishable from a split
somebody abandoned half-finished.

It also leaked. `getSharedTransactions` filters by participant with an
EXISTS on an explicit split row, so filtering the Shared view by the
payer silently dropped every transaction where their share was only ever
implied.

Four write paths could produce it, three of them unguarded:

  - the Slack nudge's share button, which inserted one row
  - `POST /api/transactions`, where the add form shows an amber total
    under 100 but saves anyway — this is how Lawn Mowing and Hedge
    Pruning were stored
  - `applyRuleActions`, where ten of the fourteen live split rules name
    only the other person

`completeSplit` is now the single place that writes the remainder, and
every one of those paths ends in it. The remainder goes to the
transaction's *owner*, never to "me": the owner's row on their own
transaction is excluded from both halves of the balance query, so it
cannot create, enlarge or discharge a debt, whereas a row for me on
someone else's transaction is a real obligation. That distinction is
what makes this safe to apply to existing data.

Also fixes the order panel's "Shared 50/50" toggle, which was inert in
both directions: it posted a lone 50% row to share (rejected — must
total 100%) and an empty array to un-share (rejected — array required),
because no way to clear a split existed. DELETE on the splits route is
that way.

Backfill: 7 rows, verified against a row-level dump diff — 2549 -> 2556
rows, none removed, none modified — and participant balances byte
identical before and after (Molina 19556.07, Sonu 20913.35). Every split
in the database now totals 100%.

Not done: a database-level constraint. Enforcing the sum needs a
deferred constraint trigger, and the rule path commits its DELETE and
INSERT as separate statements, so the trigger would reject the
intermediate state. Making it work means wrapping every write path in a
transaction, which is a larger change than the defect warrants.
This commit is contained in:
2026-07-29 10:23:06 +10:00
parent dd0462a5f9
commit 22e4a1ead0
9 changed files with 490 additions and 7 deletions
+304
View File
@@ -0,0 +1,304 @@
import { describe, it, expect, beforeAll, afterAll, beforeEach } from "vitest";
import type { Pool } from "pg";
import {
createPool,
mockDbWithPool,
resetDB,
seedParticipants,
insertTransaction,
} from "./helpers";
/**
* Every split adds up to 100%.
*
* The failure this guards against is not an arithmetic one — `myShare` has
* always treated the payer's share as the remainder, so the numbers were right.
* It is that the remainder was never written down, so a 50/50 arrangement was
* stored as a single row reading "Sonu 50%" and displayed as half a split.
*/
describe("completeSplit", () => {
let pool: Pool;
let completeSplit: (id: number) => Promise<void>;
beforeAll(async () => {
pool = createPool();
mockDbWithPool(pool);
({ completeSplit } = await import("@/lib/splits"));
});
afterAll(async () => {
await pool.end();
});
beforeEach(async () => {
await resetDB(pool);
});
const sharesOf = async (txId: number) => {
const r = await pool.query(
`SELECT participant_id, share_percent::float FROM transaction_splits
WHERE transaction_id = $1 ORDER BY participant_id`,
[txId]
);
return r.rows as { participant_id: number; share_percent: number }[];
};
it("writes the payer's half of a 50/50 recorded as one row", async () => {
const { ownerId, otherId } = await seedParticipants(pool);
const txId = await insertTransaction(pool, ownerId);
await pool.query(
`INSERT INTO transaction_splits (transaction_id, participant_id, share_percent)
VALUES ($1, $2, 50)`,
[txId, otherId]
);
await completeSplit(txId);
expect(await sharesOf(txId)).toEqual([
{ participant_id: ownerId, share_percent: 50 },
{ participant_id: otherId, share_percent: 50 },
]);
});
it("leaves an unsplit transaction unsplit", async () => {
// A transaction nobody shares is not a 100% split of itself. Writing one
// would put every row in the Shared view.
const { ownerId } = await seedParticipants(pool);
const txId = await insertTransaction(pool, ownerId);
await completeSplit(txId);
expect(await sharesOf(txId)).toEqual([]);
});
it("adds nothing when the other party owes all of it", async () => {
const { ownerId, otherId } = await seedParticipants(pool);
const txId = await insertTransaction(pool, ownerId);
await pool.query(
`INSERT INTO transaction_splits (transaction_id, participant_id, share_percent)
VALUES ($1, $2, 100)`,
[txId, otherId]
);
await completeSplit(txId);
expect(await sharesOf(txId)).toEqual([
{ participant_id: otherId, share_percent: 100 },
]);
});
it("removes the owner's row when the others grow to cover the whole amount", async () => {
// A 50/50 revised to "they owe all of it". The owner's share becomes zero,
// and a 0% row cannot be stored anyway — `share_percent > 0` is a CHECK
// constraint — so the row has to go rather than be zeroed.
const { ownerId, otherId } = await seedParticipants(pool);
const txId = await insertTransaction(pool, ownerId);
await pool.query(
`INSERT INTO transaction_splits (transaction_id, participant_id, share_percent)
VALUES ($1, $2, 100), ($1, $3, 50)`,
[txId, otherId, ownerId]
);
await completeSplit(txId);
expect(await sharesOf(txId)).toEqual([
{ participant_id: otherId, share_percent: 100 },
]);
});
it("fills the remainder for a three-way split, not a half", async () => {
const { ownerId, otherId } = await seedParticipants(pool);
const third = (
await pool.query(`INSERT INTO participants (name) VALUES ('Carol') RETURNING id`)
).rows[0].id as number;
const txId = await insertTransaction(pool, ownerId);
await pool.query(
`INSERT INTO transaction_splits (transaction_id, participant_id, share_percent)
VALUES ($1, $2, 50), ($1, $3, 25)`,
[txId, otherId, third]
);
await completeSplit(txId);
expect(await sharesOf(txId)).toEqual([
{ participant_id: ownerId, share_percent: 25 },
{ participant_id: otherId, share_percent: 50 },
{ participant_id: third, share_percent: 25 },
]);
});
it("leaves an over-allocated split alone instead of trimming someone's share", async () => {
// >100% is a caller's mistake. Silently deleting a share to force the total
// down would destroy the evidence of it.
// No single row may exceed 100 (CHECK constraint), but two can add up past
// it — 60 + 60 is how an over-allocated split actually arrives.
const { ownerId, otherId } = await seedParticipants(pool);
const third = (
await pool.query(`INSERT INTO participants (name) VALUES ('Dave') RETURNING id`)
).rows[0].id as number;
const txId = await insertTransaction(pool, ownerId);
await pool.query(
`INSERT INTO transaction_splits (transaction_id, participant_id, share_percent)
VALUES ($1, $2, 60), ($1, $3, 60)`,
[txId, otherId, third]
);
await completeSplit(txId);
expect(await sharesOf(txId)).toEqual([
{ participant_id: otherId, share_percent: 60 },
{ participant_id: third, share_percent: 60 },
]);
});
it("gives the remainder to the statement's owner when the row has none", async () => {
// Statement rows carry no owner_id of their own; it comes from the
// statement. Transaction 3828 was one of these.
const { ownerId, otherId } = await seedParticipants(pool);
const stmt = await pool.query(
`INSERT INTO statements (owner_id, filename, bank_name, account_number, billing_start_date, billing_end_date)
VALUES ($1, 'test.pdf', 'Test Bank', '0001', '2026-06-01', '2026-06-30') RETURNING id`,
[ownerId]
);
const tx = await pool.query(
`INSERT INTO transactions (owner_id, statement_id, transaction_date, description, amount, transaction_type, row_index)
VALUES (NULL, $1, '2026-06-15', 'Statement row', 29.17, 'debit', 0) RETURNING id`,
[stmt.rows[0].id]
);
const txId = tx.rows[0].id as number;
await pool.query(
`INSERT INTO transaction_splits (transaction_id, participant_id, share_percent)
VALUES ($1, $2, 50)`,
[txId, otherId]
);
await completeSplit(txId);
expect(await sharesOf(txId)).toEqual([
{ participant_id: ownerId, share_percent: 50 },
{ participant_id: otherId, share_percent: 50 },
]);
});
it("never puts my share on someone else's transaction", async () => {
// The remainder goes to the transaction's owner, never to "me". A row for
// me on a transaction I do not own is a debt I owe, and this helper must
// not invent one.
const { ownerId, otherId } = await seedParticipants(pool);
const txId = await insertTransaction(pool, otherId); // they paid
await pool.query(
`INSERT INTO transaction_splits (transaction_id, participant_id, share_percent)
VALUES ($1, $2, 50)`,
[txId, ownerId]
);
await completeSplit(txId);
const shares = await sharesOf(txId);
expect(shares).toEqual([
{ participant_id: ownerId, share_percent: 50 },
{ participant_id: otherId, share_percent: 50 },
]);
// My share is unchanged — the new row belongs to the payer.
expect(shares.find((s) => s.participant_id === ownerId)?.share_percent).toBe(50);
});
it("does not disturb a settled split", async () => {
// Adding the payer's row must not touch anyone else's `settled` flag —
// that is how $37k of discharged debt gets resurrected.
const { ownerId, otherId } = await seedParticipants(pool);
const txId = await insertTransaction(pool, ownerId);
await pool.query(
`INSERT INTO transaction_splits (transaction_id, participant_id, share_percent, settled)
VALUES ($1, $2, 50, true)`,
[txId, otherId]
);
await completeSplit(txId);
const r = await pool.query(
`SELECT settled FROM transaction_splits
WHERE transaction_id = $1 AND participant_id = $2`,
[txId, otherId]
);
expect(r.rows[0].settled).toBe(true);
});
it("is idempotent", async () => {
const { ownerId, otherId } = await seedParticipants(pool);
const txId = await insertTransaction(pool, ownerId);
await pool.query(
`INSERT INTO transaction_splits (transaction_id, participant_id, share_percent)
VALUES ($1, $2, 50)`,
[txId, otherId]
);
await completeSplit(txId);
const once = await sharesOf(txId);
await completeSplit(txId);
expect(await sharesOf(txId)).toEqual(once);
});
});
/**
* The rule path, which is where ten of the live rules write a single share.
*/
describe("applyRuleActions completes the split", () => {
let pool: Pool;
let applyRuleActions: (
id: number,
actions: { apply_split?: { participant_id: number; share_percent: number }[] }
) => Promise<void>;
beforeAll(async () => {
pool = createPool();
mockDbWithPool(pool);
({ applyRuleActions } = await import("@/lib/rule-actions"));
});
afterAll(async () => {
await pool.end();
});
beforeEach(async () => {
await resetDB(pool);
});
it("writes the payer's half for a rule that names only the other person", async () => {
const { ownerId, otherId } = await seedParticipants(pool);
const txId = await insertTransaction(pool, ownerId, { description: "Woolworths" });
await applyRuleActions(txId, {
apply_split: [{ participant_id: otherId, share_percent: 50 }],
});
const r = await pool.query(
`SELECT participant_id, share_percent::float FROM transaction_splits
WHERE transaction_id = $1 ORDER BY participant_id`,
[txId]
);
expect(r.rows).toEqual([
{ participant_id: ownerId, share_percent: 50 },
{ participant_id: otherId, share_percent: 50 },
]);
});
it("leaves a rule that already names both alone", async () => {
const { ownerId, otherId } = await seedParticipants(pool);
const txId = await insertTransaction(pool, ownerId);
await applyRuleActions(txId, {
apply_split: [
{ participant_id: ownerId, share_percent: 50 },
{ participant_id: otherId, share_percent: 50 },
],
});
const r = await pool.query(
`SELECT sum(share_percent)::float AS total, count(*)::int AS n
FROM transaction_splits WHERE transaction_id = $1`,
[txId]
);
expect(r.rows[0]).toEqual({ total: 100, n: 2 });
});
});
@@ -1,6 +1,7 @@
import { NextRequest, NextResponse } from "next/server"; import { NextRequest, NextResponse } from "next/server";
import { getCurrentUser } from "@/lib/auth"; import { getCurrentUser } from "@/lib/auth";
import { queryRaw } from "@/lib/db"; import { queryRaw } from "@/lib/db";
import { completeSplit } from "@/lib/splits";
interface SnapshotEntry { interface SnapshotEntry {
transaction_id: number; transaction_id: number;
@@ -91,6 +92,11 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
[txId, s.participant_id, s.share_percent, s.settled] [txId, s.participant_id, s.share_percent, s.settled]
); );
} }
// A snapshot taken before splits were required to total 100% holds the old
// partial shape, and restoring it verbatim would reintroduce exactly what
// this run is being undone from. The owner's share is balance-neutral, so
// completing it cannot change what the revert owes anyone.
await completeSplit(txId);
} }
await queryRaw( await queryRaw(
+6
View File
@@ -6,6 +6,7 @@ import {
slackUserForParticipant, slackUserForParticipant,
} from "@/lib/slack-verify"; } from "@/lib/slack-verify";
import { nudgeBlocks, detailsModal, partnerNudgeBlocks } from "@/lib/slack-blocks"; import { nudgeBlocks, detailsModal, partnerNudgeBlocks } from "@/lib/slack-blocks";
import { completeSplit } from "@/lib/splits";
import { import {
RATINGS, RATINGS,
OWNER_PARTICIPANT_ID, OWNER_PARTICIPANT_ID,
@@ -330,6 +331,11 @@ async function toggleShare(transactionId: number): Promise<string | null> {
DO UPDATE SET share_percent = 50`, DO UPDATE SET share_percent = 50`,
[transactionId, SECOND_CONSUMER_ID] [transactionId, SECOND_CONSUMER_ID]
); );
// Both halves, not just theirs. The button means "50/50", and a lone row for
// the other person renders as a 50% share against a blank. The payer's half
// comes from completeSplit rather than a second hardcoded insert, so it lands
// on whoever actually owns the row instead of assuming that is me.
await completeSplit(transactionId);
return null; return null;
} }
@@ -41,6 +41,30 @@ export async function GET(
return NextResponse.json(splits); return NextResponse.json(splits);
} }
/**
* Remove every split — un-share the transaction.
*
* POST cannot express this: it requires shares totalling 100%, and an empty
* array is not that. Without this the order panel's "Shared 50/50" toggle had
* no way back, and pressing it to un-share failed with "splits array required".
*/
export async function DELETE(
req: NextRequest,
{ params }: { params: Promise<{ id: string }> }
) {
const user = await getCurrentUser(req);
if (!user) return NextResponse.json({ error: "Unauthorized" }, { status: 403 });
const { id } = await params;
const transactionId = Number(id);
if (!(await canAccessTransactions(user.id, [transactionId]))) {
return NextResponse.json({ error: "Not found" }, { status: 404 });
}
const removed = await prisma.transaction_splits.deleteMany({
where: { transaction_id: transactionId },
});
return NextResponse.json({ removed: removed.count });
}
export async function POST( export async function POST(
req: NextRequest, req: NextRequest,
{ params }: { params: Promise<{ id: string }> } { params }: { params: Promise<{ id: string }> }
+6
View File
@@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from "next/server";
import { getCurrentUser } from "@/lib/auth"; import { getCurrentUser } from "@/lib/auth";
import { getTransactions } from "@/lib/queries"; import { getTransactions } from "@/lib/queries";
import { queryRaw } from "@/lib/db"; import { queryRaw } from "@/lib/db";
import { completeSplit } from "@/lib/splits";
export async function GET(req: NextRequest) { export async function GET(req: NextRequest) {
const user = await getCurrentUser(req); const user = await getCurrentUser(req);
@@ -79,6 +80,11 @@ export async function POST(req: NextRequest) {
[transactionId, s.participant_id, s.share_percent] [transactionId, s.participant_id, s.share_percent]
); );
} }
// The form lets you name just the other person and shows the total in amber
// when it is under 100 — which is how "Lawn Mowing, Sonu 50%" was saved with
// the other half nowhere. Fill in the payer's share rather than refusing:
// naming only the other person is a reasonable thing to mean.
await completeSplit(transactionId);
} }
return NextResponse.json({ id: transactionId }, { status: 201 }); return NextResponse.json({ id: transactionId }, { status: 201 });
+13 -5
View File
@@ -7,6 +7,7 @@ import {
useParticipants, useParticipants,
useSetOrderReview, useSetOrderReview,
useSetSplits, useSetSplits,
useClearSplits,
type ItemOpinion, type ItemOpinion,
type ItemVerdict, type ItemVerdict,
type OrderReceipt, type OrderReceipt,
@@ -249,19 +250,26 @@ function SharedToggle({
otherName: string; otherName: string;
}) { }) {
const setSplits = useSetSplits(); const setSplits = useSetSplits();
const clearSplits = useClearSplits();
const shared = splits.some((s) => s.participant_id === SECOND_CONSUMER_ID); const shared = splits.some((s) => s.participant_id === SECOND_CONSUMER_ID);
return ( return (
<div className="mb-2 flex items-center gap-2"> <div className="mb-2 flex items-center gap-2">
<button <button
type="button" type="button"
disabled={setSplits.isPending} disabled={setSplits.isPending || clearSplits.isPending}
onClick={() => onClick={() =>
setSplits.mutate({ // Both halves on the way in, and a real delete on the way out. This
// used to post one 50% row to share and an empty array to un-share,
// and the endpoint rejected both — the toggle did nothing either way.
shared
? clearSplits.mutate(transactionId)
: setSplits.mutate({
transactionId, transactionId,
splits: shared splits: [
? [] { participant_id: OWNER_PARTICIPANT_ID, share_percent: 50 },
: [{ participant_id: SECOND_CONSUMER_ID, share_percent: 50 }], { participant_id: SECOND_CONSUMER_ID, share_percent: 50 },
],
}) })
} }
className={`rounded border px-2 py-1 text-xs transition-colors disabled:opacity-50 ${ className={`rounded border px-2 py-1 text-xs transition-colors disabled:opacity-50 ${
+30
View File
@@ -405,6 +405,36 @@ export function useSetSplits() {
}); });
} }
/**
* Remove every split from a transaction — the un-share half of a toggle.
*
* Separate from `useSetSplits` because that endpoint requires a set of shares
* totalling 100%, and "no split at all" is not a set of shares. Posting `[]` to
* it was rejected, which is why the order panel's toggle could not be turned
* off.
*/
export function useClearSplits() {
const qc = useQueryClient();
return useMutation({
mutationFn: async (transactionId: number) => {
const res = await fetch(`/api/transactions/${transactionId}/splits`, {
method: "DELETE",
});
if (!res.ok) {
const err = await res.json();
throw new Error(err.error || "Failed to clear splits");
}
return res.json();
},
onSuccess: () => {
qc.invalidateQueries({ queryKey: ["splits"] });
qc.invalidateQueries({ queryKey: ["shared-transactions"] });
qc.invalidateQueries({ queryKey: ["participant-balances"] });
qc.invalidateQueries({ queryKey: ["order-review"] });
},
});
}
export interface SplitPayment { export interface SplitPayment {
id: number; id: number;
from_participant_id: number; from_participant_id: number;
+5
View File
@@ -9,6 +9,7 @@
*/ */
import { queryRaw } from "@/lib/db"; import { queryRaw } from "@/lib/db";
import { completeSplit } from "@/lib/splits";
import type { Actions } from "@/lib/rules"; import type { Actions } from "@/lib/rules";
export interface SnapshotEntry { export interface SnapshotEntry {
@@ -68,6 +69,10 @@ export async function applyRuleActions(
[transactionId, s.participant_id, s.share_percent] [transactionId, s.participant_id, s.share_percent]
); );
} }
// A rule may name only the other person — "split Woolworths with Sonu 50%"
// is a complete thought, and ten of the original rules are written that way.
// The payer's half is implied by it, so write the implication down.
await completeSplit(transactionId);
} }
} }
+94
View File
@@ -0,0 +1,94 @@
import { queryRaw, queryRow } from "@/lib/db";
/**
* A split has to add up to 100%.
*
* The database was happy to hold a transaction whose only split row said
* "Sonu 50%", because everything that *reads* a split treats the payer's share
* as whatever is left over — see `myShare` in analytics-sql.ts, which falls back
* to `100 - SUM(everyone else)`. The arithmetic was never wrong.
*
* It was still a bug, because a ledger is read as well as computed. On screen
* that row is a 50% share and a blank, which looks like half the money is
* unallocated, and there is no way to tell it apart from a split someone left
* half-finished. It also leaks: the participant filter on the Shared view
* (`getSharedTransactions`) selects transactions that have an explicit row for
* that participant, so filtering by the payer silently drops every transaction
* where their share was only ever implied.
*
* So the remainder gets written down. `completeSplit` is the single place that
* does it, and every write path ends in it.
*
* **This is deliberately balance-neutral.** The owner's row on their own
* transaction is excluded from both halves of the balance query in
* `getParticipantBalances` — "they owe me" reads `ts.participant_id != $1` on
* transactions I own, "I owe them" reads transactions I do not own. So adding a
* row for the transaction's own owner cannot create, enlarge or discharge a
* debt; it makes explicit the number every reader was already inferring. That
* is why the remainder is assigned to the *owner* rather than to "me": a row
* for me on someone else's transaction is a real obligation, and this helper
* must never invent one of those.
*/
/** The owner whose share is implied: the transaction's, falling back to its statement's. */
async function effectiveOwner(transactionId: number): Promise<number | null> {
const row = await queryRow<{ owner_id: number | null }>(
`SELECT COALESCE(t.owner_id, s.owner_id) AS owner_id
FROM transactions t
LEFT JOIN statements s ON s.id = t.statement_id
WHERE t.id = $1`,
[transactionId]
);
return row?.owner_id ?? null;
}
/**
* Materialise the owner's share so the split totals 100%.
*
* Does nothing to a transaction with no splits — an unshared transaction is not
* a 100% split of itself, and writing one would put every row in the Shared
* view. Does nothing when the other shares already total 100% (they owe all of
* it, and the owner's share is a genuine zero), beyond removing a stale owner
* row if one is left behind.
*
* Over-allocated splits (>100%) are left exactly as they are. That is a
* mistake the caller should have rejected, and quietly deleting someone's share
* to force the total down would destroy the evidence of it.
*/
export async function completeSplit(transactionId: number): Promise<void> {
const owner = await effectiveOwner(transactionId);
if (owner === null) return;
const rows = await queryRaw<{ participant_id: number; share_percent: string }>(
`SELECT participant_id, share_percent FROM transaction_splits WHERE transaction_id = $1`,
[transactionId]
);
if (rows.length === 0) return;
const others = rows
.filter((r) => r.participant_id !== owner)
.reduce((sum, r) => sum + Number(r.share_percent), 0);
const remainder = Number((100 - others).toFixed(2));
if (remainder > 0.01) {
await queryRaw(
`INSERT INTO transaction_splits (transaction_id, participant_id, share_percent)
VALUES ($1, $2, $3)
ON CONFLICT (transaction_id, participant_id)
DO UPDATE SET share_percent = EXCLUDED.share_percent`,
[transactionId, owner, remainder]
);
} else if (remainder > -0.01) {
// Exactly nothing left for the owner. Drop their row rather than storing a
// 0% share, so "they owe all of it" keeps looking like one row, not two.
await queryRaw(
`DELETE FROM transaction_splits WHERE transaction_id = $1 AND participant_id = $2`,
[transactionId, owner]
);
}
}
/** `completeSplit` over many transactions, for bulk and rule-run paths. */
export async function completeSplits(transactionIds: number[]): Promise<void> {
for (const id of transactionIds) await completeSplit(id);
}