fix(inbox): sync matched_transaction_id on MCP-staged document attach (#549)
* fix(inbox): sync matched_transaction_id on MCP-staged document attach The REST attach route (app/api/transactions/[id]/attach-document) already updates invoice_inbox_items.matched_transaction_id so the inbox list shows the "Kopplad till transaktion" indicator. The MCP-staged path through commitAttachDocumentToTransaction did not, leaving inbox rows looking unprocessed after a /pending approval. Mirror the REST behaviour with a best-effort UPDATE (idempotent, gated on both FK columns being NULL). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(review): inspect supabase error on best-effort inbox link The Supabase client resolves with { error } rather than rejecting on RLS/DB failures, so the previous try/catch wrapping the inbox UPDATE never fired for the documented failure modes — the console.error was dead code. Switch to destructured { error } + if-block, matching the pattern used elsewhere in commit.ts. Applied to both the new MCP-staged path (lib/pending-operations/commit.ts) and the pre-existing REST route (app/api/transactions/[id]/attach-document/ route.ts) it was mirrored from. Both tests now assert console.error fires with the expected payload, so the swallow-and-log path is exercised for real instead of passing for the wrong reason. Addresses Greptile P1 + P2 on PR #549. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
39204cc0de
commit
2879f6ed17
@@ -126,6 +126,13 @@ describe('POST /api/transactions/[id]/attach-document', () => {
|
||||
// Side-effect failure must not roll back the (compliant) document attach.
|
||||
expect(status).toBe(200)
|
||||
expect(body.data.transaction_id).toBe('tx-1')
|
||||
// The Supabase client resolves with { error } rather than rejecting, so
|
||||
// we additionally assert that the error was actually inspected and logged
|
||||
// (not silently dropped by a try/catch that never fires).
|
||||
expect(spy).toHaveBeenCalledWith(
|
||||
'[attach-document] Failed to link inbox item:',
|
||||
expect.objectContaining({ message: 'rls denied' }),
|
||||
)
|
||||
spy.mockRestore()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -87,16 +87,18 @@ export async function POST(
|
||||
// as matched so the inbox UI can show it as "Kopplad" + link back to the
|
||||
// transaction. Best-effort: a failure here must not roll back the
|
||||
// (compliant) document attach.
|
||||
try {
|
||||
await supabase
|
||||
.from('invoice_inbox_items')
|
||||
.update({ matched_transaction_id: transactionId })
|
||||
.eq('document_id', document_id)
|
||||
.eq('company_id', companyId)
|
||||
.is('matched_transaction_id', null)
|
||||
.is('created_supplier_invoice_id', null)
|
||||
} catch (linkErr) {
|
||||
console.error('[attach-document] Failed to link inbox item:', linkErr)
|
||||
//
|
||||
// The Supabase client resolves with { error } rather than rejecting on
|
||||
// RLS/DB errors, so we destructure rather than try/catch.
|
||||
const { error: inboxLinkErr } = await supabase
|
||||
.from('invoice_inbox_items')
|
||||
.update({ matched_transaction_id: transactionId })
|
||||
.eq('document_id', document_id)
|
||||
.eq('company_id', companyId)
|
||||
.is('matched_transaction_id', null)
|
||||
.is('created_supplier_invoice_id', null)
|
||||
if (inboxLinkErr) {
|
||||
console.error('[attach-document] Failed to link inbox item:', inboxLinkErr)
|
||||
}
|
||||
|
||||
// Rättelse audit trail (BFL 5 kap 5 §): record swaps where a non-null doc
|
||||
|
||||
@@ -493,6 +493,7 @@ describe('commitPendingOperation: attach_document_to_transaction', () => {
|
||||
enqueue({ data: { id: 'tx-1', document_id: null, journal_entry_id: null }, error: null })
|
||||
enqueue({ data: { id: 'doc-1' }, error: null }) // doc fetch
|
||||
enqueue({ data: { journal_entry_id: null }, error: null }) // UPDATE returning
|
||||
enqueue({ data: null, error: null }) // invoice_inbox_items best-effort link
|
||||
enqueue({ data: null, error: null }) // dispatcher commit update
|
||||
|
||||
const result = await commitPendingOperation(
|
||||
@@ -510,6 +511,7 @@ describe('commitPendingOperation: attach_document_to_transaction', () => {
|
||||
enqueue({ data: { id: 'tx-1', document_id: null, journal_entry_id: null }, error: null })
|
||||
enqueue({ data: { id: 'doc-1' }, error: null }) // doc fetch
|
||||
enqueue({ data: { journal_entry_id: 'je-7' }, error: null }) // UPDATE returning post-state
|
||||
enqueue({ data: null, error: null }) // invoice_inbox_items best-effort link
|
||||
enqueue({ data: null, error: null }) // doc propagation update
|
||||
enqueue({ data: null, error: null }) // dispatcher commit update
|
||||
|
||||
@@ -521,4 +523,55 @@ describe('commitPendingOperation: attach_document_to_transaction', () => {
|
||||
)
|
||||
expect(result.status).toBe('committed')
|
||||
})
|
||||
|
||||
it('still commits when the inbox-link best-effort update errors', async () => {
|
||||
// Inbox sync is best-effort — a failure to mark the inbox row as matched
|
||||
// must not roll back the (compliant) doc→tx attach. Mirrors the REST
|
||||
// route's swallow-and-log behaviour. The Supabase client resolves with
|
||||
// { error } rather than rejecting, so we both confirm the op commits AND
|
||||
// that the error was actually inspected and logged (not silently dropped).
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
|
||||
enqueue({ data: { id: 'tx-1', document_id: null, journal_entry_id: null }, error: null })
|
||||
enqueue({ data: { id: 'doc-1' }, error: null }) // doc fetch
|
||||
enqueue({ data: { journal_entry_id: null }, error: null }) // tx UPDATE returning
|
||||
enqueue({ data: null, error: { message: 'inbox row missing or RLS-blocked' } }) // inbox link — errors
|
||||
enqueue({ data: null, error: null }) // dispatcher commit update
|
||||
|
||||
const spy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const result = await commitPendingOperation(
|
||||
supabase as never,
|
||||
'user-1',
|
||||
'company-1',
|
||||
makePendingOp(baseOp),
|
||||
)
|
||||
expect(result.status).toBe('committed')
|
||||
expect(spy).toHaveBeenCalledWith(
|
||||
'[commitAttach] Failed to link inbox item:',
|
||||
expect.objectContaining({ message: 'inbox row missing or RLS-blocked' }),
|
||||
)
|
||||
spy.mockRestore()
|
||||
})
|
||||
|
||||
it('touches the invoice_inbox_items table to sync matched_transaction_id', async () => {
|
||||
// Argument-level check that the executor actually reaches into
|
||||
// invoice_inbox_items to keep the inbox UI in sync with the staged-write
|
||||
// path. Otherwise the inbox row stays in "Behöver åtgärd" forever.
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
|
||||
enqueue({ data: { id: 'tx-1', document_id: null, journal_entry_id: null }, error: null })
|
||||
enqueue({ data: { id: 'doc-1' }, error: null }) // doc fetch
|
||||
enqueue({ data: { journal_entry_id: null }, error: null }) // tx UPDATE returning
|
||||
enqueue({ data: null, error: null }) // invoice_inbox_items link
|
||||
enqueue({ data: null, error: null }) // dispatcher commit update
|
||||
|
||||
await commitPendingOperation(
|
||||
supabase as never,
|
||||
'user-1',
|
||||
'company-1',
|
||||
makePendingOp(baseOp),
|
||||
)
|
||||
const tablesTouched = (supabase.from as ReturnType<typeof vi.fn>).mock.calls.map(c => c[0])
|
||||
expect(tablesTouched).toContain('invoice_inbox_items')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1059,6 +1059,25 @@ async function commitAttachDocumentToTransaction(
|
||||
}
|
||||
if (!postUpdate) return { error: 'Transaction not found', status: 404 }
|
||||
|
||||
// If the attached doc came from an invoice_inbox_items row, mark that row
|
||||
// as matched so the inbox UI shows "Kopplad till transaktion". Best-effort:
|
||||
// a failure must not roll back the (compliant) attach. Mirrors the REST
|
||||
// route in app/api/transactions/[id]/attach-document/route.ts so MCP-staged
|
||||
// and REST attaches converge on the same inbox state.
|
||||
//
|
||||
// The Supabase client resolves with { error } rather than rejecting on
|
||||
// RLS/DB errors, so we destructure rather than try/catch.
|
||||
const { error: inboxLinkErr } = await supabase
|
||||
.from('invoice_inbox_items')
|
||||
.update({ matched_transaction_id: txId })
|
||||
.eq('document_id', documentId)
|
||||
.eq('company_id', companyId)
|
||||
.is('matched_transaction_id', null)
|
||||
.is('created_supplier_invoice_id', null)
|
||||
if (inboxLinkErr) {
|
||||
console.error('[commitAttach] Failed to link inbox item:', inboxLinkErr)
|
||||
}
|
||||
|
||||
const journalEntryId = postUpdate.journal_entry_id as string | null
|
||||
if (journalEntryId) {
|
||||
const { error: linkErr } = await supabase
|
||||
|
||||
Reference in New Issue
Block a user