fix(transactions): retire stale invoice match pointers when an invoice settles (#1313)
* fix(transactions): retire stale invoice match pointers when an invoice settles potential_invoice_id / potential_supplier_invoice_id are write-once import suggestions: nothing revisited them once written. With recurring same-amount invoices, an earlier suggestion pointed transaction A at invoice X, X was then paid off by transaction B, and A kept pointing at a fully paid invoice. The match dialog computed its amount diff against that invoice's 0 kr remaining_amount and reported a bogus partial payment, and the dead pointer also blocked a fresh suggestion: both re-suggestion scans require the column to be NULL. Add one shared helper, clearSettledInvoiceSuggestions(), that nulls a settled invoice's own suggestion column on every other transaction of the same company, scoped by company_id and by that invoice id only, never widening to the confirmed invoice_id / supplier_invoice_id links. It is best effort by construction: every caller has already booked a payment verifikat, so a failed cleanup logs and returns instead of failing the settle. Wired into every path where an invoice reaches paid through a payment: the dashboard and v1 match-invoice / match-supplier-invoice routes, the dashboard and v1 mark-paid routes, settleInvoicePayment, the batch allocation route (per fully settled allocation), linkInvoiceToVoucher and linkSupplierInvoiceToVoucher, linkTransactionToJournalEntry, and the MCP staged-operation executors for mark_invoice_paid and match_transaction_invoice. Partial payments are deliberately left alone: a partially paid invoice is still matchable. The v1 supplier match route also clears its own row's hint, which it was missing next to its dashboard twin. Read-time revalidation stays as the backstop for the paths not wired up here. countSuggestedMatches now delegates to listSuggestedMatches, which already revalidates candidates, so the worklist badge can no longer claim a number the list refuses to render. A data-only backfill migration retires the pointers already stranded in the database. It touches no journal entry, verifikat or period-locked data, is idempotent, and its status lists mirror lib/invoices/matchable-statuses.ts. Fixes #1259 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(transactions): wire the MCP batch allocation into the settled-pointer cleanup Review follow-up on the #1259 fix. commitMatchBatchAllocate calls the same match_batch_allocate RPC as the dashboard route, and gnubok_match_batch_allocate is a live staged MCP tool, so an agent settling a samlingsbetalning reproduced the issue exactly: the RPC nulls potential_invoice_id / potential_supplier_invoice_id only on the source transaction, leaving every other transaction of the company pointing at an invoice the batch just closed. The per-allocation loop moves into clearSettledBatchAllocationSuggestions() so the HTTP route and the MCP executor run the same code and cannot drift again, with a commit-path test pinning that only the fully settled allocation is retired. The enlarged badge scan is made safe. countSuggestedMatches now feeds up to 200 ids into listSuggestedMatches, past the 150 per .in() that countInboxDocuments already chunks for, so the candidate lookups are chunked at IN_CLAUSE_CHUNK too and their ids deduped. Both lookups now check .error: previously a 414, a 500 or an RLS change produced empty maps, an empty list and a zero badge with nothing logged. Every failure branch here logs companyId, matching the logAndZero convention. Also: restore the anchorSupplierInvoiceDocument doc comment above its own call in the dashboard supplier-invoice mark-paid route (the #1259 block had been inserted between them), and assert the transaction update payload in the v1 match-supplier-invoice test, which now covers the potential_supplier_invoice_id null that the route was missing next to its dashboard twin. Fixes #1259 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
1a5d205bd6
commit
27ae59040e
@@ -109,7 +109,6 @@ describe('countVerifikatMissingDocument', () => {
|
||||
|
||||
describe('simple head counts', () => {
|
||||
it.each([
|
||||
['countSuggestedMatches', countSuggestedMatches, 'transactions'],
|
||||
['countSupplierInvoicesAwaitingApproval', countSupplierInvoicesAwaitingApproval, 'supplier_invoices'],
|
||||
['countOverdueInvoices', countOverdueInvoices, 'invoices'],
|
||||
['countDeadlinesNeedingAction', countDeadlinesNeedingAction, 'deadlines'],
|
||||
@@ -121,6 +120,76 @@ describe('simple head counts', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// Issue #1259: the badge delegates to listSuggestedMatches so it can never
|
||||
// claim a number the list refuses to render. A raw head count over the hint
|
||||
// columns counted pointers at invoices settled by a different transaction.
|
||||
describe('countSuggestedMatches', () => {
|
||||
it('counts only hints whose candidate is still matchable', async () => {
|
||||
enqueue({
|
||||
data: [
|
||||
{
|
||||
id: 'tx-1',
|
||||
date: '2026-06-01',
|
||||
description: 'ICA',
|
||||
amount: 423,
|
||||
currency: 'SEK',
|
||||
potential_invoice_id: 'inv-1',
|
||||
potential_supplier_invoice_id: null,
|
||||
},
|
||||
{
|
||||
id: 'tx-2',
|
||||
date: '2026-05-30',
|
||||
description: 'TELIA',
|
||||
amount: -549,
|
||||
currency: 'SEK',
|
||||
potential_invoice_id: null,
|
||||
potential_supplier_invoice_id: 'sinv-paid',
|
||||
},
|
||||
],
|
||||
})
|
||||
// inv-1 is still open; sinv-paid was settled by another transaction, so the
|
||||
// status/remaining filters exclude it server-side.
|
||||
enqueue({
|
||||
data: [
|
||||
{ id: 'inv-1', invoice_number: 'F-1', total: 423, customer: { name: 'Kund AB' } },
|
||||
],
|
||||
})
|
||||
enqueue({ data: [] })
|
||||
|
||||
await expect(countSuggestedMatches(supabase, COMPANY)).resolves.toBe(1)
|
||||
expect(mockSupabase.from).toHaveBeenCalledWith('transactions')
|
||||
})
|
||||
|
||||
it('returns 0 when the only hint points at an invoice settled elsewhere', async () => {
|
||||
enqueue({
|
||||
data: [
|
||||
{
|
||||
id: 'tx-1',
|
||||
date: '2026-06-01',
|
||||
description: 'MONTHLY FEE',
|
||||
amount: -549,
|
||||
currency: 'SEK',
|
||||
potential_invoice_id: null,
|
||||
potential_supplier_invoice_id: 'sinv-paid',
|
||||
},
|
||||
],
|
||||
})
|
||||
enqueue({ data: [] })
|
||||
await expect(countSuggestedMatches(supabase, COMPANY)).resolves.toBe(0)
|
||||
})
|
||||
|
||||
it('soft-fails to 0 on query error', async () => {
|
||||
enqueue({ error: { message: 'boom' } })
|
||||
await expect(countSuggestedMatches(supabase, COMPANY)).resolves.toBe(0)
|
||||
})
|
||||
|
||||
it('clamps the scan so the badge cannot walk an unbounded hint set', async () => {
|
||||
enqueue({ data: [] })
|
||||
await countSuggestedMatches(supabase, COMPANY)
|
||||
expect(findCall('transactions', 'limit')).toEqual([200])
|
||||
})
|
||||
})
|
||||
|
||||
describe('listSuggestedMatches', () => {
|
||||
it('maps invoice and supplier-invoice hints to confirmable rows', async () => {
|
||||
enqueue({
|
||||
@@ -282,4 +351,65 @@ describe('listSuggestedMatches', () => {
|
||||
|
||||
await expect(listSuggestedMatches(supabase, COMPANY)).resolves.toEqual([])
|
||||
})
|
||||
|
||||
// The badge scans up to 200 hints (SUGGESTED_MATCH_SCAN_CAP), past the 150
|
||||
// ids per .in() that countInboxDocuments already chunks for: PostgREST puts
|
||||
// them in the GET query string, and a 414 would come back as a silent 0.
|
||||
it('chunks the candidate id list at 150 ids per lookup', async () => {
|
||||
const txRows = Array.from({ length: 151 }, (_, i) => ({
|
||||
id: `tx-${i}`,
|
||||
date: '2026-06-01',
|
||||
description: 'X',
|
||||
amount: 100,
|
||||
currency: 'SEK',
|
||||
potential_invoice_id: `inv-${i}`,
|
||||
potential_supplier_invoice_id: null,
|
||||
}))
|
||||
enqueue({ data: txRows })
|
||||
enqueue({ data: [] }) // chunk 1
|
||||
enqueue({ data: [] }) // chunk 2
|
||||
|
||||
await listSuggestedMatches(supabase, COMPANY, 200)
|
||||
|
||||
const idFilters = findCalls('invoices', 'in').filter(([col]) => col === 'id')
|
||||
expect(idFilters).toHaveLength(2)
|
||||
expect((idFilters[0][1] as string[]).length).toBe(150)
|
||||
expect((idFilters[1][1] as string[]).length).toBe(1)
|
||||
})
|
||||
|
||||
// Previously the candidate results were consumed without checking .error, so
|
||||
// a 414 / 500 / RLS change yielded empty maps, an empty list and a zero badge
|
||||
// with nothing logged.
|
||||
it('returns [] and logs with companyId when a candidate lookup fails', async () => {
|
||||
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
enqueue({
|
||||
data: [
|
||||
{
|
||||
id: 'tx-1',
|
||||
date: '2026-06-01',
|
||||
description: 'X',
|
||||
amount: 100,
|
||||
currency: 'SEK',
|
||||
potential_invoice_id: 'inv-1',
|
||||
potential_supplier_invoice_id: null,
|
||||
},
|
||||
],
|
||||
})
|
||||
enqueue({ error: { message: 'boom' } })
|
||||
|
||||
await expect(listSuggestedMatches(supabase, COMPANY)).resolves.toEqual([])
|
||||
expect(consoleError).toHaveBeenCalled()
|
||||
const logged = consoleError.mock.calls.map((c) => String(c[0])).join('\n')
|
||||
expect(logged).toContain('candidate lookup failed')
|
||||
expect(logged).toContain(COMPANY)
|
||||
consoleError.mockRestore()
|
||||
})
|
||||
|
||||
it('logs the tenant with the transaction query failure too', async () => {
|
||||
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
enqueue({ error: { message: 'boom' } })
|
||||
await expect(listSuggestedMatches(supabase, COMPANY)).resolves.toEqual([])
|
||||
expect(String(consoleError.mock.calls[0]?.[0])).toContain(COMPANY)
|
||||
consoleError.mockRestore()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -12,9 +12,10 @@ import {
|
||||
} from './categories'
|
||||
|
||||
/**
|
||||
* All worklist counts in one round-trip burst. Every count is a cheap
|
||||
* head-only query (categories.ts) and individually soft-fails to 0, so this
|
||||
* is safe to call from layouts and server components on every render.
|
||||
* All worklist counts in one round-trip burst. Each count is a bounded query
|
||||
* (mostly head-only; suggested_match revalidates its candidates, see
|
||||
* categories.ts) and individually soft-fails to 0, so this is safe to call
|
||||
* from layouts and server components on every render.
|
||||
*
|
||||
* `total` is the number of distinct actionable items: suggested_match is a
|
||||
* fast path over transactions already counted in book_transaction, so it is
|
||||
|
||||
+95
-49
@@ -129,20 +129,29 @@ export async function countInboxDocuments(
|
||||
const SUGGESTED_MATCH_OR =
|
||||
'potential_invoice_id.not.is.null,potential_supplier_invoice_id.not.is.null'
|
||||
|
||||
/** Unbooked transactions with an invoice/supplier-invoice match hint. */
|
||||
/**
|
||||
* Cap on the hint scan behind countSuggestedMatches; clamps like
|
||||
* INBOX_SCAN_CAP. Above IN_CLAUSE_CHUNK on purpose: the candidate lookups it
|
||||
* feeds are chunked (fetchCandidatesChunked), so the URL length stays bounded
|
||||
* regardless of this number.
|
||||
*/
|
||||
const SUGGESTED_MATCH_SCAN_CAP = 200
|
||||
|
||||
/**
|
||||
* Unbooked transactions with a still-actionable invoice/supplier-invoice match
|
||||
* hint.
|
||||
*
|
||||
* Delegates to listSuggestedMatches so the badge can never claim a number the
|
||||
* list cannot render: a head count over the raw hint columns still counts a
|
||||
* pointer at an invoice settled elsewhere (issue #1259), which is exactly the
|
||||
* divergence lib/worklist exists to prevent (see types.ts).
|
||||
*/
|
||||
export async function countSuggestedMatches(
|
||||
supabase: SupabaseClient,
|
||||
companyId: string,
|
||||
): Promise<number> {
|
||||
const { count, error } = await supabase
|
||||
.from('transactions')
|
||||
.select('id', { count: 'exact', head: true })
|
||||
.eq('company_id', companyId)
|
||||
.is('is_business', null)
|
||||
.eq('is_ignored', false)
|
||||
.or(SUGGESTED_MATCH_OR)
|
||||
if (error) return logAndZero('suggested_match', companyId, error)
|
||||
return count ?? 0
|
||||
const matches = await listSuggestedMatches(supabase, companyId, SUGGESTED_MATCH_SCAN_CAP)
|
||||
return matches.length
|
||||
}
|
||||
|
||||
/** Supplier invoices awaiting approval ("attestera"). */
|
||||
@@ -259,6 +268,37 @@ interface SuggestedMatchTxRow {
|
||||
potential_supplier_invoice_id: string | null
|
||||
}
|
||||
|
||||
type CandidateRow = {
|
||||
id: string
|
||||
invoice_number?: string | null
|
||||
supplier_invoice_number?: string | null
|
||||
total: number | null
|
||||
customer?: { name: string | null } | null
|
||||
supplier?: { name: string | null } | null
|
||||
}
|
||||
|
||||
/**
|
||||
* Run one candidate lookup over a chunked id list. PostgREST serialises .in()
|
||||
* into the GET query string, so a long list can push the URL past proxy limits
|
||||
* (HTTP 414) exactly as countInboxDocuments guards against. The first error
|
||||
* short-circuits: a partial candidate set would silently drop rows from the
|
||||
* list and, through countSuggestedMatches, from the badge.
|
||||
*/
|
||||
async function fetchCandidatesChunked(
|
||||
ids: string[],
|
||||
runChunk: (
|
||||
chunk: string[],
|
||||
) => PromiseLike<{ data: unknown; error: { message?: string } | null }>,
|
||||
): Promise<{ rows: CandidateRow[]; error: { message?: string } | null }> {
|
||||
const rows: CandidateRow[] = []
|
||||
for (let i = 0; i < ids.length; i += IN_CLAUSE_CHUNK) {
|
||||
const { data, error } = await runChunk(ids.slice(i, i + IN_CLAUSE_CHUNK))
|
||||
if (error) return { rows: [], error }
|
||||
rows.push(...((data ?? []) as unknown as CandidateRow[]))
|
||||
}
|
||||
return { rows, error: null }
|
||||
}
|
||||
|
||||
/**
|
||||
* Suggested transaction↔invoice matches with enough candidate context for a
|
||||
* one-click confirm row. Confirm endpoints:
|
||||
@@ -272,9 +312,11 @@ interface SuggestedMatchTxRow {
|
||||
* that has since been paid would otherwise render a one-click confirm row
|
||||
* whose endpoint can only answer ALREADY_PAID.
|
||||
*
|
||||
* Revalidation is done here, at read time, rather than by cleaning up sibling
|
||||
* pointers on settle: the settle paths are many and a missed one leaks, while
|
||||
* this check covers every route into the list.
|
||||
* Revalidation stays here, at read time, even though the high-traffic settle
|
||||
* paths now also retire sibling pointers
|
||||
* (lib/invoices/clear-settled-invoice-suggestions.ts, issue #1259): the settle
|
||||
* paths are many and a missed one leaks, while this check covers every route
|
||||
* into the list.
|
||||
*/
|
||||
export async function listSuggestedMatches(
|
||||
supabase: SupabaseClient,
|
||||
@@ -293,51 +335,55 @@ export async function listSuggestedMatches(
|
||||
.order('date', { ascending: false })
|
||||
.limit(limit)
|
||||
if (error) {
|
||||
log.error('worklist listSuggestedMatches failed', { reason: error.message })
|
||||
// companyId matches logAndZero's convention so repeated failures can be
|
||||
// correlated to a tenant in monitoring.
|
||||
log.error('worklist listSuggestedMatches failed', { companyId, reason: error.message })
|
||||
return []
|
||||
}
|
||||
|
||||
const txs = (txRows ?? []) as SuggestedMatchTxRow[]
|
||||
const invoiceIds = txs.map((t) => t.potential_invoice_id).filter((x): x is string => !!x)
|
||||
const supplierInvoiceIds = txs
|
||||
.map((t) => t.potential_supplier_invoice_id)
|
||||
.filter((x): x is string => !!x)
|
||||
const invoiceIds = [
|
||||
...new Set(txs.map((t) => t.potential_invoice_id).filter((x): x is string => !!x)),
|
||||
]
|
||||
const supplierInvoiceIds = [
|
||||
...new Set(txs.map((t) => t.potential_supplier_invoice_id).filter((x): x is string => !!x)),
|
||||
]
|
||||
|
||||
const [invoiceRes, supplierRes] = await Promise.all([
|
||||
invoiceIds.length > 0
|
||||
? supabase
|
||||
.from('invoices')
|
||||
.select('id, invoice_number, total, customer:customers(name)')
|
||||
.eq('company_id', companyId)
|
||||
.in('id', invoiceIds)
|
||||
.in('status', [...MATCHABLE_INVOICE_STATUSES])
|
||||
.gt('remaining_amount', 0)
|
||||
: Promise.resolve({ data: [], error: null }),
|
||||
supplierInvoiceIds.length > 0
|
||||
? supabase
|
||||
.from('supplier_invoices')
|
||||
.select('id, supplier_invoice_number, total, supplier:suppliers(name)')
|
||||
.eq('company_id', companyId)
|
||||
.in('id', supplierInvoiceIds)
|
||||
.in('status', [...MATCHABLE_SUPPLIER_INVOICE_STATUSES])
|
||||
.gt('remaining_amount', 0)
|
||||
: Promise.resolve({ data: [], error: null }),
|
||||
fetchCandidatesChunked(invoiceIds, (chunk) =>
|
||||
supabase
|
||||
.from('invoices')
|
||||
.select('id, invoice_number, total, customer:customers(name)')
|
||||
.eq('company_id', companyId)
|
||||
.in('id', chunk)
|
||||
.in('status', [...MATCHABLE_INVOICE_STATUSES])
|
||||
.gt('remaining_amount', 0),
|
||||
),
|
||||
fetchCandidatesChunked(supplierInvoiceIds, (chunk) =>
|
||||
supabase
|
||||
.from('supplier_invoices')
|
||||
.select('id, supplier_invoice_number, total, supplier:suppliers(name)')
|
||||
.eq('company_id', companyId)
|
||||
.in('id', chunk)
|
||||
.in('status', [...MATCHABLE_SUPPLIER_INVOICE_STATUSES])
|
||||
.gt('remaining_amount', 0),
|
||||
),
|
||||
])
|
||||
|
||||
type CandidateRow = {
|
||||
id: string
|
||||
invoice_number?: string | null
|
||||
supplier_invoice_number?: string | null
|
||||
total: number | null
|
||||
customer?: { name: string | null } | null
|
||||
supplier?: { name: string | null } | null
|
||||
// A failed candidate lookup must not pass for "nothing is matchable": that
|
||||
// would render an empty list and, through countSuggestedMatches, a silent
|
||||
// zero badge. Log it (with companyId) and bail, same as the tx query above.
|
||||
const candidateError = invoiceRes.error ?? supplierRes.error
|
||||
if (candidateError) {
|
||||
log.error('worklist listSuggestedMatches candidate lookup failed', {
|
||||
companyId,
|
||||
reason: candidateError.message,
|
||||
})
|
||||
return []
|
||||
}
|
||||
const invoiceById = new Map<string, CandidateRow>(
|
||||
((invoiceRes.data ?? []) as unknown as CandidateRow[]).map((r) => [r.id, r]),
|
||||
)
|
||||
const supplierById = new Map<string, CandidateRow>(
|
||||
((supplierRes.data ?? []) as unknown as CandidateRow[]).map((r) => [r.id, r]),
|
||||
)
|
||||
|
||||
const invoiceById = new Map<string, CandidateRow>(invoiceRes.rows.map((r) => [r.id, r]))
|
||||
const supplierById = new Map<string, CandidateRow>(supplierRes.rows.map((r) => [r.id, r]))
|
||||
|
||||
const matches: SuggestedMatch[] = []
|
||||
for (const tx of txs) {
|
||||
|
||||
Reference in New Issue
Block a user