line-items: snapshot before writing, and stop reporting writes that did not happen
Three faults on the same function, all of them "the write is assumed to have worked". - The create path snapshotted the undo baseline AFTER its INSERT. A crash in between left the retry recording the intake's own write as "previous", so undo restored the thing it was meant to reverse — the exact failure previous_line_items exists to prevent. Both paths now snapshot first. - An UPDATE against a row deleted between the select and the write affects nothing and still returned a metadata_id, so the batch was marked enriched against a row nobody had written to. The write is now checked and falls through to the ordered lookup, then to a create. - Two calls for one transaction could both find no row and both INSERT; expense_metadata.transaction_id is UNIQUE, so one lost with a constraint error that reached the user as an error card. ON CONFLICT (transaction_id) DO UPDATE, with (xmax = 0) so created_metadata stays truthful. And the new row's currency comes from the batch instead of a hard-coded 'AUD'. OrderDetails prefers expense_metadata.currency, so a USD order — booked correctly on the transaction with foreign_currency_code — printed "$".
This commit is contained in:
+59
-31
@@ -1,4 +1,4 @@
|
||||
import { queryRow, queryRaw } from "@/lib/db";
|
||||
import { queryRow } from "@/lib/db";
|
||||
|
||||
/**
|
||||
* Replace the itemised receipt behind a transaction (plan §11 C2).
|
||||
@@ -89,7 +89,7 @@ async function undoBaseline(batchId: string | null | undefined, snapshot: unknow
|
||||
export async function replaceLineItems(
|
||||
transactionId: number,
|
||||
lineItems: LineItemRecord[],
|
||||
opts: { source: LineItemsSource; platform?: string | null; batchId?: string | null; metadataId?: number | null }
|
||||
opts: { source: LineItemsSource; platform?: string | null; batchId?: string | null; metadataId?: number | null; currency?: string | null }
|
||||
): Promise<ReplaceResult> {
|
||||
const txn = await queryRow<{
|
||||
id: number;
|
||||
@@ -115,42 +115,67 @@ export async function replaceLineItems(
|
||||
[opts.metadataId, transactionId]
|
||||
)
|
||||
: null;
|
||||
const existing =
|
||||
pinned ??
|
||||
(await queryRow<{ id: number; line_items: unknown }>(
|
||||
`SELECT id, line_items FROM expense_metadata
|
||||
WHERE transaction_id = $1 OR matched_transaction_id = $1
|
||||
-- NULLS LAST is load-bearing, and its absence inverted this clause.
|
||||
-- Postgres orders DESC as NULLS FIRST, and the row that merely POINTS
|
||||
-- at the transaction has a NULL transaction_id — so the comparison
|
||||
-- meant to prefer the owned row preferred the other one, silently and
|
||||
-- in exactly the case it was written for. matchTransaction spells it
|
||||
-- out; this did not, and the two therefore disagreed.
|
||||
ORDER BY (transaction_id = $1) DESC NULLS LAST, id
|
||||
LIMIT 1`,
|
||||
[transactionId]
|
||||
));
|
||||
|
||||
const json = JSON.stringify(lineItems);
|
||||
|
||||
/**
|
||||
* Snapshot the baseline, then write. In that order on BOTH paths: the create
|
||||
* path used to snapshot after its INSERT, so a crash in between left a retry
|
||||
* recording the intake's own write as "previous" — undo restoring the thing
|
||||
* it was meant to reverse, which is the failure `previous_line_items` exists
|
||||
* to prevent.
|
||||
*
|
||||
* Returns null when the row is gone between the read and the write. That
|
||||
* UPDATE reported success and the batch was marked enriched against a row
|
||||
* nothing had written to.
|
||||
*/
|
||||
const write = async (row: { id: number; line_items: unknown }): Promise<ReplaceResult | null> => {
|
||||
const previous = await undoBaseline(opts.batchId, Array.isArray(row.line_items) ? row.line_items : []);
|
||||
const updated = await queryRow<{ id: number }>(
|
||||
`UPDATE expense_metadata SET line_items = $2::jsonb WHERE id = $1 RETURNING id`,
|
||||
[row.id, json]
|
||||
);
|
||||
if (!updated) return null;
|
||||
return { transaction_id: transactionId, metadata_id: row.id, created_metadata: false, previous_line_items: previous, line_items: lineItems };
|
||||
};
|
||||
|
||||
if (pinned) {
|
||||
const done = await write(pinned);
|
||||
if (done) return done;
|
||||
}
|
||||
const existing = await queryRow<{ id: number; line_items: unknown }>(
|
||||
`SELECT id, line_items FROM expense_metadata
|
||||
WHERE transaction_id = $1 OR matched_transaction_id = $1
|
||||
-- NULLS LAST is load-bearing, and its absence inverted this clause.
|
||||
-- Postgres orders DESC as NULLS FIRST, and the row that merely POINTS at
|
||||
-- the transaction has a NULL transaction_id — so the comparison meant to
|
||||
-- prefer the owned row preferred the other one, silently and in exactly
|
||||
-- the case it was written for. matchTransaction spells it out; this did
|
||||
-- not, and the two therefore disagreed.
|
||||
ORDER BY (transaction_id = $1) DESC NULLS LAST, id
|
||||
LIMIT 1`,
|
||||
[transactionId]
|
||||
);
|
||||
if (existing) {
|
||||
const previous = await undoBaseline(opts.batchId, Array.isArray(existing.line_items) ? existing.line_items : []);
|
||||
await queryRaw(`UPDATE expense_metadata SET line_items = $2::jsonb WHERE id = $1`, [existing.id, json]);
|
||||
return {
|
||||
transaction_id: transactionId,
|
||||
metadata_id: existing.id,
|
||||
created_metadata: false,
|
||||
previous_line_items: previous,
|
||||
line_items: lineItems,
|
||||
};
|
||||
const done = await write(existing);
|
||||
if (done) return done;
|
||||
}
|
||||
|
||||
// Nothing to update. Snapshot the empty baseline BEFORE creating the row.
|
||||
const previous = await undoBaseline(opts.batchId, []);
|
||||
const reference = opts.batchId ? `intake:${opts.batchId}` : `intake:txn:${transactionId}`;
|
||||
const created = await queryRow<{ id: number }>(
|
||||
// ON CONFLICT because two calls for one transaction can both find no row and
|
||||
// both INSERT; expense_metadata.transaction_id is UNIQUE, so one of them lost
|
||||
// with a constraint error that reached the user as an error card. `xmax = 0`
|
||||
// is how a plain insert is told from a conflict-update, so the reported
|
||||
// `created_metadata` stays true.
|
||||
const created = await queryRow<{ id: number; inserted: boolean }>(
|
||||
`INSERT INTO expense_metadata (
|
||||
transaction_id, source, platform, order_reference, line_items,
|
||||
merchant_normalized, amount, transaction_date, currency, flags, extraction_model
|
||||
) VALUES ($1, $2, $3, $4, $5::jsonb, $6, $7::numeric, $8::date, 'AUD', '[]'::jsonb, 'cloud-budget')
|
||||
RETURNING id`,
|
||||
) VALUES ($1, $2, $3, $4, $5::jsonb, $6, $7::numeric, $8::date, $9, '[]'::jsonb, 'cloud-budget')
|
||||
ON CONFLICT (transaction_id) DO UPDATE SET line_items = EXCLUDED.line_items
|
||||
RETURNING id, (xmax = 0) AS inserted`,
|
||||
[
|
||||
transactionId,
|
||||
opts.source,
|
||||
@@ -160,13 +185,16 @@ export async function replaceLineItems(
|
||||
txn.merchant_normalized ?? txn.merchant_name ?? null,
|
||||
txn.amount,
|
||||
txn.transaction_date,
|
||||
// Not hard-coded 'AUD': the transaction may carry a foreign figure, and
|
||||
// OrderDetails prefers this column, so a USD order printed "$".
|
||||
opts.currency || "AUD",
|
||||
]
|
||||
);
|
||||
return {
|
||||
transaction_id: transactionId,
|
||||
metadata_id: created!.id,
|
||||
created_metadata: true,
|
||||
previous_line_items: await undoBaseline(opts.batchId, []),
|
||||
created_metadata: created!.inserted,
|
||||
previous_line_items: previous,
|
||||
line_items: lineItems,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user