line-items: one metadata row, agreed on by the write and the read
Three defects on one surface, all invisible while a transaction has exactly one expense_metadata row — which is almost always. 1. ORDER BY (transaction_id = $1) DESC did the opposite of its comment. Postgres orders DESC as NULLS FIRST, and the row that merely POINTS at the transaction has a NULL transaction_id, so the clause written to prefer the owned row preferred the other one — in exactly the case it exists for. matchTransaction already spelled out NULLS LAST; now so do this and GET /api/transactions/[id]/order, which had no ORDER BY at all and could read a different row from the one an enrichment had just written. 2. replaceLineItems takes an optional metadataId. The match resolves a row through a platform-filtered join; re-picking independently here can land the write on a row the match ruled out. 3. previous_line_items on intake_batches is the undo baseline, recorded once per batch under COALESCE and read back. Returning whatever was just read hands a pantry retry its own write as the "previous" state, so undo restores the thing it undid. Migration 0034 is unapplied anywhere, so the column is added there rather than in a 0035.
This commit is contained in:
@@ -32,10 +32,20 @@ CREATE TABLE IF NOT EXISTS intake_batches (
|
|||||||
status text NOT NULL,
|
status text NOT NULL,
|
||||||
transaction_id integer,
|
transaction_id integer,
|
||||||
capture_id integer,
|
capture_id integer,
|
||||||
|
-- The undo baseline: what `expense_metadata.line_items` held before this
|
||||||
|
-- batch first replaced it. Recorded here rather than returned from the write,
|
||||||
|
-- because pantry re-calls the enrich endpoint on every confirm and after a
|
||||||
|
-- timeout — a caller that hands back whatever it just read gives a retry its
|
||||||
|
-- OWN write as the "previous" state, and undo then restores the thing it was
|
||||||
|
-- meant to reverse. Written under COALESCE, so only the first snapshot sticks.
|
||||||
|
previous_line_items jsonb,
|
||||||
created_at timestamptz NOT NULL DEFAULT now(),
|
created_at timestamptz NOT NULL DEFAULT now(),
|
||||||
resolved_at timestamptz
|
resolved_at timestamptz
|
||||||
);
|
);
|
||||||
|
|
||||||
|
-- For a database that already ran an earlier draft of this file.
|
||||||
|
ALTER TABLE intake_batches ADD COLUMN IF NOT EXISTS previous_line_items jsonb;
|
||||||
|
|
||||||
COMMENT ON TABLE intake_batches IS
|
COMMENT ON TABLE intake_batches IS
|
||||||
'One row per upload batch through POST /api/intake. Persisted before '
|
'One row per upload batch through POST /api/intake. Persisted before '
|
||||||
'extraction so Slack taps can act on it later and re-shares are detected. '
|
'extraction so Slack taps can act on it later and re-shares are detected. '
|
||||||
|
|||||||
@@ -34,6 +34,14 @@ export async function GET(
|
|||||||
-- through matched_transaction_id. Both directions have to resolve or the
|
-- through matched_transaction_id. Both directions have to resolve or the
|
||||||
-- detail is missing on exactly the orders that were paid by card.
|
-- detail is missing on exactly the orders that were paid by card.
|
||||||
WHERE transaction_id = $1 OR matched_transaction_id = $1
|
WHERE transaction_id = $1 OR matched_transaction_id = $1
|
||||||
|
-- The same order as replaceLineItems in line-items.ts and matchTransaction
|
||||||
|
-- in intake-router.ts. Both directions can resolve at once — a statement
|
||||||
|
-- line owning its own row while a delivery receipt points at it — and an
|
||||||
|
-- unordered LIMIT 1 then reads a different row from the one an enrichment
|
||||||
|
-- wrote, so a PUT that reported success showed nothing on the screen.
|
||||||
|
-- NULLS LAST because DESC is NULLS FIRST and the pointing row's
|
||||||
|
-- transaction_id is NULL.
|
||||||
|
ORDER BY (transaction_id = $1) DESC NULLS LAST, id
|
||||||
LIMIT 1`,
|
LIMIT 1`,
|
||||||
[Number(id)]
|
[Number(id)]
|
||||||
);
|
);
|
||||||
|
|||||||
+57
-9
@@ -15,6 +15,12 @@ import { queryRow, queryRaw } from "@/lib/db";
|
|||||||
* transaction has no row at all (a manual transaction, a statement line), one
|
* transaction has no row at all (a manual transaction, a statement line), one
|
||||||
* is created so the items have somewhere to live; a transaction is NEVER
|
* is created so the items have somewhere to live; a transaction is NEVER
|
||||||
* created here.
|
* created here.
|
||||||
|
*
|
||||||
|
* `opts.metadataId` pins WHICH row when the caller already resolved one. A
|
||||||
|
* statement transaction can own its own `expense_metadata` row AND be the
|
||||||
|
* `matched_transaction_id` of a delivery row: the match query and this write
|
||||||
|
* then re-pick independently and can land on different rows, so the enrichment
|
||||||
|
* reports success against a row nobody reads.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
export type LineItemsSource = "intake" | "pantry";
|
export type LineItemsSource = "intake" | "pantry";
|
||||||
@@ -55,10 +61,35 @@ export type ReplaceResult = {
|
|||||||
line_items: LineItemRecord[];
|
line_items: LineItemRecord[];
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The undo baseline, recorded once per batch.
|
||||||
|
*
|
||||||
|
* Returning the row that was just read makes a RETRY destroy the thing undo
|
||||||
|
* exists to restore: pantry re-calls this on every confirm and after a timeout,
|
||||||
|
* and the second call would hand back its own write as the "previous" state.
|
||||||
|
* The first snapshot a batch ever takes is the only one worth keeping, so it is
|
||||||
|
* written under COALESCE and read back rather than trusted.
|
||||||
|
*/
|
||||||
|
async function undoBaseline(batchId: string | null | undefined, snapshot: unknown[]): Promise<unknown[]> {
|
||||||
|
// Pantry passes back whatever batch id it was given; only an intake batch has
|
||||||
|
// a row to record against, and a non-uuid would fail the cast.
|
||||||
|
if (!batchId || !UUID.test(batchId)) return snapshot;
|
||||||
|
const row = await queryRow<{ previous_line_items: unknown }>(
|
||||||
|
`UPDATE intake_batches
|
||||||
|
SET previous_line_items = COALESCE(previous_line_items, $2::jsonb)
|
||||||
|
WHERE id = $1::uuid
|
||||||
|
RETURNING previous_line_items`,
|
||||||
|
[batchId, JSON.stringify(snapshot)]
|
||||||
|
);
|
||||||
|
return Array.isArray(row?.previous_line_items) ? row.previous_line_items : snapshot;
|
||||||
|
}
|
||||||
|
|
||||||
export async function replaceLineItems(
|
export async function replaceLineItems(
|
||||||
transactionId: number,
|
transactionId: number,
|
||||||
lineItems: LineItemRecord[],
|
lineItems: LineItemRecord[],
|
||||||
opts: { source: LineItemsSource; platform?: string | null; batchId?: string | null }
|
opts: { source: LineItemsSource; platform?: string | null; batchId?: string | null; metadataId?: number | null }
|
||||||
): Promise<ReplaceResult> {
|
): Promise<ReplaceResult> {
|
||||||
const txn = await queryRow<{
|
const txn = await queryRow<{
|
||||||
id: number;
|
id: number;
|
||||||
@@ -73,25 +104,42 @@ export async function replaceLineItems(
|
|||||||
);
|
);
|
||||||
if (!txn) throw new TransactionNotFoundError(`transaction ${transactionId} does not exist`);
|
if (!txn) throw new TransactionNotFoundError(`transaction ${transactionId} does not exist`);
|
||||||
|
|
||||||
// Prefer the row that IS this transaction over one that merely points at it —
|
// The caller's row wins where it still exists; otherwise prefer the row that
|
||||||
// a statement line can be the `matched_transaction_id` of a receipt row and
|
// IS this transaction over one that merely points at it — a statement line
|
||||||
// carry its own row too, and the items belong on the receipt.
|
// can be the `matched_transaction_id` of a receipt row and carry its own row
|
||||||
const existing = await queryRow<{ id: number; line_items: unknown }>(
|
// too, and the items belong on the receipt.
|
||||||
|
const pinned = opts.metadataId
|
||||||
|
? await queryRow<{ id: number; line_items: unknown }>(
|
||||||
|
`SELECT id, line_items FROM expense_metadata
|
||||||
|
WHERE id = $1 AND (transaction_id = $2 OR matched_transaction_id = $2)`,
|
||||||
|
[opts.metadataId, transactionId]
|
||||||
|
)
|
||||||
|
: null;
|
||||||
|
const existing =
|
||||||
|
pinned ??
|
||||||
|
(await queryRow<{ id: number; line_items: unknown }>(
|
||||||
`SELECT id, line_items FROM expense_metadata
|
`SELECT id, line_items FROM expense_metadata
|
||||||
WHERE transaction_id = $1 OR matched_transaction_id = $1
|
WHERE transaction_id = $1 OR matched_transaction_id = $1
|
||||||
ORDER BY (transaction_id = $1) DESC, id
|
-- 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`,
|
LIMIT 1`,
|
||||||
[transactionId]
|
[transactionId]
|
||||||
);
|
));
|
||||||
|
|
||||||
const json = JSON.stringify(lineItems);
|
const json = JSON.stringify(lineItems);
|
||||||
if (existing) {
|
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]);
|
await queryRaw(`UPDATE expense_metadata SET line_items = $2::jsonb WHERE id = $1`, [existing.id, json]);
|
||||||
return {
|
return {
|
||||||
transaction_id: transactionId,
|
transaction_id: transactionId,
|
||||||
metadata_id: existing.id,
|
metadata_id: existing.id,
|
||||||
created_metadata: false,
|
created_metadata: false,
|
||||||
previous_line_items: Array.isArray(existing.line_items) ? existing.line_items : [],
|
previous_line_items: previous,
|
||||||
line_items: lineItems,
|
line_items: lineItems,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -118,7 +166,7 @@ export async function replaceLineItems(
|
|||||||
transaction_id: transactionId,
|
transaction_id: transactionId,
|
||||||
metadata_id: created!.id,
|
metadata_id: created!.id,
|
||||||
created_metadata: true,
|
created_metadata: true,
|
||||||
previous_line_items: [],
|
previous_line_items: await undoBaseline(opts.batchId, []),
|
||||||
line_items: lineItems,
|
line_items: lineItems,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user