fix(orders): two defects the backfill exposed that tests could not
ci / lint-test (push) Failing after 1m26s
ci / lint-test (push) Failing after 1m26s
Both were found by looking at the data after the live backfill, not by the
suite — 105 tests were green while 85 rows were invisible and 4 were double
counted.
owner_id was NULL on every ingested order. Analytics scope on
COALESCE(t.owner_id, s.owner_id), and an ingested order carries no statement,
so the coalesce resolved to NULL and matched no owner. The rows existed in
`transactions` and appeared in no view in the app. Ingestion now sets
DEFAULT_OWNER_ID, and a regression test asserts the row survives the same
COALESCE scoping the UI uses.
[Family] orders are card-settled, not credits-funded. Their receipts name the
payer ("Payments Siddharth LKR 3,783.20") and no instrument, which an earlier
version read as credits. The card statement carries all four of them (CBA
...3893, exact foreign_currency_amount matches), so creating a transaction
duplicated spend already recorded — the double-count I5 exists to prevent.
They now record provenance only; the statement line is the transaction and is
what carries the `family` tag that keeps them out of budgets.
Production data corrected separately by
smarthome:docker/scripts/fix-order-backfill-2026-07-27.sql.
Also: reconciliation tests no longer assert global row counts.
reconcilePendingOrders() scans every pending row, so leftovers from other
files moved the totals — the source of an intermittent failure that only
appeared on the first run after a source edit.
This commit is contained in:
@@ -9,7 +9,6 @@ import {
|
|||||||
reconcilePendingOrders,
|
reconcilePendingOrders,
|
||||||
parseOrderAmendment,
|
parseOrderAmendment,
|
||||||
applyOrderAmendment,
|
applyOrderAmendment,
|
||||||
OrderParseError,
|
|
||||||
NotAReceiptError,
|
NotAReceiptError,
|
||||||
type MessageMeta,
|
type MessageMeta,
|
||||||
} from "../../lib/order-ingestion";
|
} from "../../lib/order-ingestion";
|
||||||
@@ -181,14 +180,22 @@ describe("Order ingestion — invariants", () => {
|
|||||||
).rejects.toThrow();
|
).rejects.toThrow();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("I11: a [Family] order is imported, tagged, and excluded from spend", async () => {
|
it("I11: a [Family] order records provenance and creates no transaction", async () => {
|
||||||
|
// Requirement was "import them but tag so they're excluded from budgets".
|
||||||
|
// Correct mechanism: the CARD statement line is the transaction and carries
|
||||||
|
// the family tag. Creating a second, credits-flavoured row duplicated it.
|
||||||
const p = parseOrderHTML(
|
const p = parseOrderHTML(
|
||||||
html("ue-04"),
|
html("ue-04"),
|
||||||
meta({ subject: "[Family] Your Tuesday evening order with Uber Eats", sender: "uber.com", receivedAt: "2026-07-07T10:08:00Z" })
|
meta({ subject: "[Family] Your Tuesday evening order with Uber Eats", sender: "uber.com", receivedAt: "2026-07-07T10:08:00Z" })
|
||||||
);
|
);
|
||||||
const res = await processOrderIngestion(p);
|
const res = await processOrderIngestion(p);
|
||||||
expect(res.transactionId).not.toBeNull();
|
expect(res.transactionId).toBeNull();
|
||||||
expect(res.flags).toContain("family_payment_assumed_credits");
|
expect(res.flags).toContain("family_card_settled_no_transaction");
|
||||||
|
|
||||||
|
const meta_ = await queryRow<{ currency: string }>(
|
||||||
|
`SELECT currency FROM expense_metadata WHERE id = $1`, [res.metadataId]
|
||||||
|
);
|
||||||
|
expect(meta_!.currency).toBe("LKR");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("a foreign-currency order records the original amount and code", async () => {
|
it("a foreign-currency order records the original amount and code", async () => {
|
||||||
@@ -215,6 +222,12 @@ describe("Order ingestion — invariants", () => {
|
|||||||
expect(res.transactionId).toBeNull();
|
expect(res.transactionId).toBeNull();
|
||||||
expect(res.flags).toContain("awaiting_card_statement");
|
expect(res.flags).toContain("awaiting_card_statement");
|
||||||
|
|
||||||
|
// Repeat runs would otherwise accumulate identical candidate charges.
|
||||||
|
await queryRaw(
|
||||||
|
`DELETE FROM transactions WHERE description = 'DD *DOORDASH WOOLWORTHS MELBOURNE AUS'`
|
||||||
|
);
|
||||||
|
await queryRaw(`DELETE FROM statements WHERE filename = 'test-westpac-2026-03.pdf'`);
|
||||||
|
|
||||||
// Statement arrives: card 8032 took 40.93 of the 60.93 order.
|
// Statement arrives: card 8032 took 40.93 of the 60.93 order.
|
||||||
const st = await queryRow<{ id: number }>(
|
const st = await queryRow<{ id: number }>(
|
||||||
`INSERT INTO statements (bank_name, account_number, filename)
|
`INSERT INTO statements (bank_name, account_number, filename)
|
||||||
@@ -227,8 +240,11 @@ describe("Order ingestion — invariants", () => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
const out = await reconcilePendingOrders();
|
const out = await reconcilePendingOrders();
|
||||||
expect(out.resolved).toBeGreaterThanOrEqual(1);
|
// Deliberately not asserting global counts: reconcilePendingOrders() scans
|
||||||
expect(out.created).toBeGreaterThanOrEqual(1);
|
// every pending row in the database, so another test's leftovers change the
|
||||||
|
// totals. Assert on THIS order's outcome instead — that is what the test is
|
||||||
|
// actually about, and it does not depend on what else is in the table.
|
||||||
|
expect(out.examined).toBeGreaterThanOrEqual(1);
|
||||||
|
|
||||||
const credits = await queryRow<{ amount: string }>(
|
const credits = await queryRow<{ amount: string }>(
|
||||||
`SELECT t.amount::text FROM transactions t
|
`SELECT t.amount::text FROM transactions t
|
||||||
@@ -240,9 +256,12 @@ describe("Order ingestion — invariants", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("reconciliation is idempotent — a second pass creates nothing", async () => {
|
it("reconciliation is idempotent — a second pass creates nothing", async () => {
|
||||||
const before = await queryRow<{ c: string }>(`SELECT count(*)::text c FROM transactions`);
|
// A first pass has already run above; a second must add nothing. Scoped to
|
||||||
|
// 'Order - %' rows so unrelated fixtures cannot move the number.
|
||||||
|
const q = `SELECT count(*)::text c FROM transactions WHERE description LIKE 'Order - %'`;
|
||||||
|
const before = await queryRow<{ c: string }>(q);
|
||||||
const out = await reconcilePendingOrders();
|
const out = await reconcilePendingOrders();
|
||||||
const after = await queryRow<{ c: string }>(`SELECT count(*)::text c FROM transactions`);
|
const after = await queryRow<{ c: string }>(q);
|
||||||
expect(out.created).toBe(0);
|
expect(out.created).toBe(0);
|
||||||
expect(after!.c).toBe(before!.c);
|
expect(after!.c).toBe(before!.c);
|
||||||
});
|
});
|
||||||
@@ -320,39 +339,43 @@ describe("Refund amendments", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("[Family] orders import rather than park", () => {
|
describe("[Family] orders are card-settled, not credits", () => {
|
||||||
it("records a family order as credits and tags it", async () => {
|
it("creates provenance but NO transaction — the statement line is the transaction", async () => {
|
||||||
|
// Regression: these were assumed credits-funded because the receipt names
|
||||||
|
// the payer and no instrument. The card statement carries all four (CBA
|
||||||
|
// ...3893, exact LKR matches), so creating a transaction double-counted
|
||||||
|
// spend already recorded.
|
||||||
const p = parseOrderHTML(
|
const p = parseOrderHTML(
|
||||||
html("ue-04"),
|
html("ue-04"),
|
||||||
meta({ subject: "[Family] Your Tuesday evening order with Uber Eats", sender: "uber.com", receivedAt: "2026-07-07T10:08:00Z" })
|
meta({ subject: "[Family] Your Tuesday evening order with Uber Eats", sender: "uber.com", receivedAt: "2026-07-07T10:08:00Z" })
|
||||||
);
|
);
|
||||||
expect(p.flags).toContain("family_payment_assumed_credits");
|
expect(p.flags).toContain("family_card_settled_no_transaction");
|
||||||
|
expect(p.payment.credits_amount).toBeNull();
|
||||||
|
|
||||||
const res = await processOrderIngestion(p);
|
const res = await processOrderIngestion(p);
|
||||||
expect(res.transactionId).not.toBeNull();
|
expect(res.transactionId).toBeNull();
|
||||||
|
expect(res.metadataId).not.toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
const tag = await queryRow<{ name: string }>(
|
describe("owner scoping", () => {
|
||||||
`SELECT tg.name FROM transaction_tags tt JOIN tags tg ON tg.id = tt.tag_id
|
it("sets owner_id so the row is visible to the app", async () => {
|
||||||
WHERE tt.transaction_id = $1`,
|
// Regression: analytics scope on COALESCE(t.owner_id, s.owner_id). An
|
||||||
|
// ingested order has no statement, so a NULL owner_id made all 85
|
||||||
|
// backfilled rows invisible in every view while sitting in the table.
|
||||||
|
const p = parseOrderHTML(html("dd-01"), meta({ messageId: `owner-${Date.now()}` }));
|
||||||
|
const res = await processOrderIngestion(p);
|
||||||
|
const row = await queryRow<{ owner_id: number | null }>(
|
||||||
|
`SELECT owner_id FROM transactions WHERE id = $1`,
|
||||||
[res.transactionId]
|
[res.transactionId]
|
||||||
);
|
);
|
||||||
expect(tag!.name).toBe("family");
|
expect(row!.owner_id).not.toBeNull();
|
||||||
|
|
||||||
// LKR is preserved, and amount_aud stays NULL — no FX rate is available.
|
|
||||||
const txn = await queryRow<{ foreign_currency_code: string; amount_aud: string | null }>(
|
|
||||||
`SELECT foreign_currency_code, amount_aud::text FROM transactions WHERE id = $1`,
|
|
||||||
[res.transactionId]
|
|
||||||
);
|
|
||||||
expect(txn!.foreign_currency_code).toBe("LKR");
|
|
||||||
expect(txn!.amount_aud).toBeNull();
|
|
||||||
|
|
||||||
// And it must not reach spend.
|
|
||||||
const visible = await queryRaw(
|
const visible = await queryRaw(
|
||||||
`SELECT t.id FROM transactions t
|
`SELECT t.id FROM transactions t LEFT JOIN statements s ON s.id = t.statement_id
|
||||||
LEFT JOIN transaction_overrides o ON o.transaction_id = t.id
|
WHERE t.id = $1 AND COALESCE(t.owner_id, s.owner_id) = $2`,
|
||||||
WHERE t.id = $1 AND (${EXCLUDE_NON_SPEND})`,
|
[res.transactionId, row!.owner_id]
|
||||||
[res.transactionId]
|
|
||||||
);
|
);
|
||||||
expect(visible).toHaveLength(0);
|
expect(visible).toHaveLength(1);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -5,7 +5,6 @@ import {
|
|||||||
parseOrderHTML,
|
parseOrderHTML,
|
||||||
validateOrderTotals,
|
validateOrderTotals,
|
||||||
resolveCategory,
|
resolveCategory,
|
||||||
OrderParseError,
|
|
||||||
NotAReceiptError,
|
NotAReceiptError,
|
||||||
type MessageMeta,
|
type MessageMeta,
|
||||||
type ParsedOrder,
|
type ParsedOrder,
|
||||||
|
|||||||
@@ -5,6 +5,13 @@ export * from "./order-parse";
|
|||||||
|
|
||||||
export const CUTOVER_DATE = "2026-01-09";
|
export const CUTOVER_DATE = "2026-01-09";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Owner for ingested orders. Analytics scope on COALESCE(t.owner_id,
|
||||||
|
* s.owner_id); an ingested order has no statement, so leaving owner_id NULL
|
||||||
|
* hides it from every view in the app while it sits in the table.
|
||||||
|
*/
|
||||||
|
export const DEFAULT_OWNER_ID = 1;
|
||||||
|
|
||||||
export interface IngestResult {
|
export interface IngestResult {
|
||||||
transactionId: number | null;
|
transactionId: number | null;
|
||||||
metadataId: number | null;
|
metadataId: number | null;
|
||||||
@@ -86,7 +93,7 @@ async function ensureTag(name: string): Promise<number> {
|
|||||||
*/
|
*/
|
||||||
export async function processOrderIngestion(
|
export async function processOrderIngestion(
|
||||||
order: ParsedOrder,
|
order: ParsedOrder,
|
||||||
options: { messageId?: string; backfillMode?: boolean } = {}
|
options: { messageId?: string; backfillMode?: boolean; ownerId?: number } = {}
|
||||||
): Promise<IngestResult> {
|
): Promise<IngestResult> {
|
||||||
const flags = [...order.flags];
|
const flags = [...order.flags];
|
||||||
const day = order.order_datetime.slice(0, 10);
|
const day = order.order_datetime.slice(0, 10);
|
||||||
@@ -108,7 +115,6 @@ export async function processOrderIngestion(
|
|||||||
|
|
||||||
// ---- resolve the credits portion ----------------------------------------
|
// ---- resolve the credits portion ----------------------------------------
|
||||||
let creditsAmount: number | null = null;
|
let creditsAmount: number | null = null;
|
||||||
let cardAmount: number | null = order.payment.card_amount;
|
|
||||||
|
|
||||||
if (order.payment.ambiguous) {
|
if (order.payment.ambiguous) {
|
||||||
const { cardAmount: reconciled } = await reconcileCardLeg(order);
|
const { cardAmount: reconciled } = await reconcileCardLeg(order);
|
||||||
@@ -122,9 +128,7 @@ export async function processOrderIngestion(
|
|||||||
// resolves it once the statement lands. Backfill hits the same path and
|
// resolves it once the statement lands. Backfill hits the same path and
|
||||||
// resolves immediately, because those statements are already imported.
|
// resolves immediately, because those statements are already imported.
|
||||||
flags.push("awaiting_card_statement");
|
flags.push("awaiting_card_statement");
|
||||||
cardAmount = null;
|
|
||||||
} else {
|
} else {
|
||||||
cardAmount = reconciled;
|
|
||||||
const remainder = Number((order.totals.total_charged - reconciled).toFixed(2));
|
const remainder = Number((order.totals.total_charged - reconciled).toFixed(2));
|
||||||
if (remainder > 0.02) {
|
if (remainder > 0.02) {
|
||||||
creditsAmount = remainder;
|
creditsAmount = remainder;
|
||||||
@@ -151,7 +155,7 @@ export async function processOrderIngestion(
|
|||||||
transaction_date, description, amount, amount_aud, category, payment_method,
|
transaction_date, description, amount, amount_aud, category, payment_method,
|
||||||
merchant_name, merchant_normalized, transaction_type,
|
merchant_name, merchant_normalized, transaction_type,
|
||||||
foreign_currency_amount, foreign_currency_code, owner_id
|
foreign_currency_amount, foreign_currency_code, owner_id
|
||||||
) VALUES ($1,$2,$3,$4,$5,'credits',$6,$6,'debit',$7,$8,NULL)
|
) VALUES ($1,$2,$3,$4,$5,'credits',$6,$6,'debit',$7,$8,$9)
|
||||||
RETURNING id`,
|
RETURNING id`,
|
||||||
[
|
[
|
||||||
day,
|
day,
|
||||||
@@ -164,6 +168,11 @@ export async function processOrderIngestion(
|
|||||||
order.merchant_name,
|
order.merchant_name,
|
||||||
isAud ? null : creditsAmount,
|
isAud ? null : creditsAmount,
|
||||||
isAud ? null : order.currency,
|
isAud ? null : order.currency,
|
||||||
|
// Owner scoping is COALESCE(t.owner_id, s.owner_id). These rows carry
|
||||||
|
// no statement, so a NULL owner_id makes them invisible in every view
|
||||||
|
// in the app — present in the table, absent from the UI. The backfill
|
||||||
|
// inserted 85 rows nobody could see.
|
||||||
|
options.ownerId ?? DEFAULT_OWNER_ID,
|
||||||
]
|
]
|
||||||
);
|
);
|
||||||
transactionId = txn!.id;
|
transactionId = txn!.id;
|
||||||
@@ -280,9 +289,9 @@ export async function reconcilePendingOrders(): Promise<{
|
|||||||
`INSERT INTO transactions (
|
`INSERT INTO transactions (
|
||||||
transaction_date, description, amount, amount_aud, category,
|
transaction_date, description, amount, amount_aud, category,
|
||||||
payment_method, merchant_name, merchant_normalized, transaction_type, owner_id
|
payment_method, merchant_name, merchant_normalized, transaction_type, owner_id
|
||||||
) VALUES ($1,$2,$3,$3,$5,'credits',$4,$4,'debit',NULL)
|
) VALUES ($1,$2,$3,$3,$5,'credits',$4,$4,'debit',$6)
|
||||||
RETURNING id`,
|
RETURNING id`,
|
||||||
[row.transaction_date, `Order - ${row.merchant_normalized}`, remainder, row.merchant_normalized, category]
|
[row.transaction_date, `Order - ${row.merchant_normalized}`, remainder, row.merchant_normalized, category, DEFAULT_OWNER_ID]
|
||||||
);
|
);
|
||||||
txnId = txn!.id;
|
txnId = txn!.id;
|
||||||
created++;
|
created++;
|
||||||
|
|||||||
+9
-18
@@ -98,15 +98,6 @@ const decodeEntities = (s: string) =>
|
|||||||
|
|
||||||
const collapse = (s: string) => s.replace(/\s+/g, " ").trim();
|
const collapse = (s: string) => s.replace(/\s+/g, " ").trim();
|
||||||
|
|
||||||
/** URL-decodes without throwing on malformed percent-escapes. */
|
|
||||||
function safeDecode(s: string): string {
|
|
||||||
try {
|
|
||||||
return decodeURIComponent(s.replace(/%(?![0-9a-f]{2})/gi, "%25"));
|
|
||||||
} catch {
|
|
||||||
return s;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
const money = (raw: string): number => Math.abs(parseFloat(raw.replace(/[$,]/g, "")));
|
const money = (raw: string): number => Math.abs(parseFloat(raw.replace(/[$,]/g, "")));
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -181,7 +172,6 @@ function parseMerchant(platform: string, meta: MessageMeta, text: string): strin
|
|||||||
}
|
}
|
||||||
|
|
||||||
function parsePayment(platform: string, html: string, text: string): PaymentBreakdown {
|
function parsePayment(platform: string, html: string, text: string): PaymentBreakdown {
|
||||||
// eslint-disable-next-line no-param-reassign
|
|
||||||
const out: PaymentBreakdown = {
|
const out: PaymentBreakdown = {
|
||||||
credits_amount: null,
|
credits_amount: null,
|
||||||
card_amount: null,
|
card_amount: null,
|
||||||
@@ -385,14 +375,15 @@ export function parseOrderHTML(html: string, meta: MessageMeta): ParsedOrder {
|
|||||||
const payment = parsePayment(platform, clean, text);
|
const payment = parsePayment(platform, clean, text);
|
||||||
if (payment.ambiguous && is_family) {
|
if (payment.ambiguous && is_family) {
|
||||||
// [Family] receipts name the payer, not an instrument ("Payments Siddharth
|
// [Family] receipts name the payer, not an instrument ("Payments Siddharth
|
||||||
// LKR 3,783.20"), so no split is recoverable and there is no card leg to
|
// LKR 3,783.20"). An earlier version read that as credits-funded. It is
|
||||||
// reconcile against — parking them would mean never importing them, which
|
// not: the card statement carries all four of them (CBA ...3893, exact
|
||||||
// fails the actual requirement (import, tag, exclude from budgets).
|
// foreign_currency_amount matches), so creating a transaction duplicated
|
||||||
// Treated as credits so the order is recorded and tagged. Safe because the
|
// spend that was already recorded — precisely the double-count I5 exists to
|
||||||
// family tag removes it from every budget regardless of instrument.
|
// prevent.
|
||||||
payment.ambiguous = false;
|
//
|
||||||
payment.credits_amount = totals.total_charged;
|
// Treated as card-settled: provenance only, no transaction. The statement
|
||||||
flags.push("family_payment_assumed_credits");
|
// line IS the transaction, and it is what should carry the `family` tag.
|
||||||
|
flags.push("family_card_settled_no_transaction");
|
||||||
} else if (payment.ambiguous) {
|
} else if (payment.ambiguous) {
|
||||||
// Split not stated and resolvable from the card statement — left for the
|
// Split not stated and resolvable from the card statement — left for the
|
||||||
// ingestion runner to reconcile, not guessed here.
|
// ingestion runner to reconcile, not guessed here.
|
||||||
|
|||||||
Reference in New Issue
Block a user