From 8a41b5dbf21c4673b4e4d6a71cf4d197c484c05a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Flod=C3=A9n?= Date: Sun, 12 Jul 2026 21:14:16 +0200 Subject: [PATCH] fix(v1): resolve supplier-payment/categorize settlement account from cash_account_id (#986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(transactions): extract shared settlement-account resolution helper Dedupe the identical cash_account_id -> ledger_account lookup across match-supplier-invoice (POST + preview) and categorize into resolveSettlementAccount, per CodeRabbit's nitpick on PR #985. Pure extraction, no behavior change. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Jonas Flodén * fix(v1): resolve supplier-payment/categorize settlement account from cash_account_id Closes the v1/MCP-facing half of the settlement-account gap left open by PR #985 (which only fixed the dashboard routes): - match-supplier-invoice: the pure-SEK accrual path always called createSupplierInvoicePaymentEntry with no paymentAccount at all (hardcoded internal default 1930), never reading the transaction's cash_account_id. Now resolves it via resolveSettlementAccount, same as the dashboard route post-#985. - categorize: never called applySettlementAccount after building the mapping result, so every categorization booked the bank leg to 1930 regardless of which cash account the transaction was linked to. Left the FX/foreign-currency branch (createSupplierInvoicePaymentEntry) and the cash-method branch (createSupplierInvoiceCashEntry) on their pre-existing internal 1930 default, matching #985's own scope decision on the equivalent dashboard route. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Jonas Flodén * fix(bookkeeping): abort instead of silently defaulting to 1930 when settlement-account lookup errors Same shared-helper fix as PR #985: resolveSettlementAccount now throws BookkeepingDatabaseError on a genuine cash_accounts query error instead of warning and falling back to 1930. An explicit cash_account_id almost certainly resolves to a non-1930 account, so a transient failure masking it risked the same class of misbooking this whole PR series exists to fix, just via infra flakiness instead of a stale setting. No route changes needed: both v1 call sites (match-supplier-invoice, categorize) already run under withApiV1, whose existing catch-all converts any isBookkeepingError() throw into the correct structured 500. Added regression tests confirming the abort for both. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Jonas Flodén * fix(v1): guard resolved settlement account against chart of accounts CodeRabbit and jakobwennberg's triage on #986 both flagged that resolveSettlementAccount() returns cash_accounts.ledger_account unvalidated, so an inactive/removed account surfaced as the generic MATCH_SI_RECORD_PAYMENT_FAILED instead of an actionable error. Add the same findUnresolvableAccounts pre-check and AccountsNotInChartError race-guard the categorize routes already use. Signed-off-by: Jonas Flodén * test(bookkeeping): align settlement-account error assertion with #985 Use .rejects.toBeInstanceOf(BookkeepingDatabaseError) instead of toMatchObject({ constructor: ... }), matching #985's edef79d follow-up (the assertion was correct either way, but this is the more idiomatic check and now makes the shared helper's test file byte-identical across #985/#986/#987, removing the add/add merge conflict between them noted in the merge-order validation. Signed-off-by: Jonas Flodén Signed-off-by: Jonas Flodén --------- Signed-off-by: Jonas Flodén Co-authored-by: Jakob Wennberg --- DECISIONS.md | 3 + .../transactions/[id]/__tests__/route.test.ts | 396 +++++++++++++++++- .../transactions/[id]/categorize/route.ts | 19 +- .../[id]/match-supplier-invoice/route.ts | 52 ++- 4 files changed, 465 insertions(+), 5 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 34e37318..3fb68eb1 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -71,7 +71,10 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-11] Momsdeklaration UI overhaul: deleted VatCompositionChart (donut mixed utgående/omvänd/ingående moms as slices of one pie, answering no filing question) and reduced the VAT ReportExportMenu to xlsx-only (XML/PDF are filing artifacts, now owned solely by the "Lämna in" card): both are one-commit reverts if vetoed. [2026-07-11] Hoisted local VAT checks + RC-gap worklist out of SkatteverketPanel into ungated VatChecksCard: the panel's paywall/not-connected early-returns hid compliance errors from exactly the users who file manually. [2026-07-11] NE/INK2 amounts display in whole kronor (matches filed SRU values per SFL); momsdeklaration keeps öre (reconciles against ledger and settlement verifikat). Numbered h2 section headers instead of a stepper component on the VAT page: same sequencing legibility, a tenth of the diff. +[2026-07-11] Closed the v1/MCP-facing half of the #985 settlement-account gap (PR #985 itself only fixed the dashboard routes): v1 match-supplier-invoice now resolves paymentAccount via resolveSettlementAccount for the pure-SEK accrual path (was always hardcoded 1930, no call site even read cash_account_id); v1 categorize now calls applySettlementAccount after building mappingResult, which it never did before. Left the FX/foreign-currency branch (createSupplierInvoicePaymentEntry) and the cash-method branch (createSupplierInvoiceCashEntry) on their pre-existing internal 1930 default, matching #985's own scope decision on the dashboard route. Follow-ups tracked separately: #1000 (closing the FX/cash-method gap) and #1001 (detecting/remediating historical mis-bookings). [2026-07-12] Compliance-review triage on the payment-link PR: finding 1 (email pay button on kreditfaktura) verified FALSE: invoice-templates.ts derives isCreditNote from credited_invoice_id and hidePayment already gates both HTML and text builders; no change. Finding 2 was the real deferred v1 gap but misfiled against invoice-columns.ts (which already carries deduction_total): the actual hole was the v1 send route's hand-rolled fetch projection, now replaced with the shared INVOICE_FULL_COLUMNS/INVOICE_ITEM_FULL_COLUMNS so PDF/email inputs cannot drift from the GET shape again (closes the [2026-07-10] deferred ROT/RUT send fix; also gives v1 sends the pay button + deduction box). Finding 3 accepted as a robustness fix only: the non-ok path already reflected true server state, but a thrown fetch left the Godkann spinner stuck; approve handler now try/catch/finally with a server refetch on failure. +[2026-07-12] resolveSettlementAccount now throws BookkeepingDatabaseError('resolve_settlement_account', ...) instead of warning-and-falling-back-to-1930 when the cash_accounts lookup itself errors (compliance-bot finding, same change applied identically across #985/#986/#987 since it's the shared helper file): an explicit cash_account_id almost certainly resolves to a non-1930 account, so a transient DB blip masking it must not silently misbook a real payment. No route code changes needed here either -- v1 match-supplier-invoice and categorize both already run under withApiV1, whose existing catch-all converts any isBookkeepingError() throw into the correct structured 500 via v1ErrorResponse. Added regression tests for both v1 call sites confirming the abort (status 500, code BOOKKEEPING_DATABASE_ERROR, no JE created) rather than assuming the shared infrastructure handles it silently. +[2026-07-12] #986 review follow-up (CodeRabbit + jakobwennberg triage): v1 match-supplier-invoice now pre-validates the resolved settlement account against chart_of_accounts before booking the pure-SEK accrual entry, returning ACCOUNTS_NOT_IN_CHART instead of the generic MATCH_SI_RECORD_PAYMENT_FAILED for a deactivated cash_accounts.ledger_account; same AccountsNotInChartError race-guard added to the catch block, mirroring the categorize routes' existing pattern. [2026-07-11] match-supplier-invoice (POST + preview) misbooked a real bank payment to 2893 (skuld till aktieägare) instead of 1930: both routes defaulted paymentAccount from company_settings.last_supplier_payment_account, a sticky setting only meant to remember the manual mark-paid "betald med privata medel" account choice. Once that setting held 2893 from an unrelated private payment, every subsequent real bank-transaction match reused it. Fixed by resolving the credit account from the matched transaction's own cash_account_id -> cash_accounts.ledger_account (falling back to 1930 when unlinked), mirroring the existing settlement-account lookup in transactions/[id]/categorize/route.ts. last_supplier_payment_account is no longer read by either route; it stays scoped to seeding the manual mark-paid UI's default picker. Did not touch the FX branch (createSupplierInvoicePaymentEntry, still defaults paymentAccount internally to 1930) or the cash-method branch (createSupplierInvoiceCashEntry, called with paymentAccount=undefined): both are pre-existing, separate gaps outside this bug's repro (a pure-SEK accrual match). [2026-07-11] Extracted the cash_account_id -> ledger_account resolution (identical in match-supplier-invoice POST, its preview, and transactions/[id]/categorize) into resolveSettlementAccount (lib/bookkeeping/settlement-account.ts), per CodeRabbit's dedup nitpick on PR #985. Pure behavior extraction, no logic change. Investigated whether other transaction actions should adopt it: bulk-book/book already resolve the account client-side (components' shared resolveAccount in lib/cash-accounts/resolve-account.ts) before the manual lines reach the server, so no gap there. Found two real gaps left open, NOT fixed here (bigger surface, deserve their own review): (1) the customer-side match-invoice route (POST + preview) and the underlying createInvoiceCashEntry/buildInvoicePaymentClearingLines (lib/bookkeeping/invoice-entries.ts, invoice-payment-lines.ts) hardcode account_number: '1930' unconditionally, never reading cash_account_id at all, so any customer receipt landing in a non-primary bank account is misbooked, same defect class as this PR fixed but present unconditionally rather than only when a stale setting fires; (2) the /api/v1 (MCP-facing) match-supplier-invoice route still calls createSupplierInvoiceCashEntry/createSupplierInvoicePaymentEntry with paymentAccount left undefined (defaults to 1930 internally), i.e. the pre-#985 bug's underlying gap is reachable through the public API/MCP tool surface even after this fix merges. The v1 categorize route has the analogous gap: it never calls applySettlementAccount after building its mapping result. [2026-07-11] Closed the remaining items from the Swedish-accounting-compliance bot review on PR #985: (1) the FX branch (createSupplierInvoicePaymentEntry) and cash-method branch (createSupplierInvoiceCashEntry) in match-supplier-invoice/route.ts were already computing `paymentAccount` via resolveSettlementAccount but not passing it through to those two calls (only the pure-SEK clearing branch used it) -- both functions already accepted an optional paymentAccount param (`paymentAccount || '1930'` internally), so this was a one-line threading fix per call site, not a new code path; the preview route already threaded it everywhere, confirmed by reading its cash/FX preview branches. (2) resolveSettlementAccount now also warns (and still falls back to 1930) when cash_account_id resolves to a row with no ledger_account, not just on a hard query error: a bound-but-empty ledger_account is a data-integrity gap, not a normal unlinked-transaction case, and previously fell back silently. (3) Added a column comment on company_settings.last_supplier_payment_account (migration 20260711140000) documenting that it must never be read to resolve a matched transaction's settlement account. diff --git a/app/api/v1/companies/[companyId]/transactions/[id]/__tests__/route.test.ts b/app/api/v1/companies/[companyId]/transactions/[id]/__tests__/route.test.ts index b965f812..1dcb7515 100644 --- a/app/api/v1/companies/[companyId]/transactions/[id]/__tests__/route.test.ts +++ b/app/api/v1/companies/[companyId]/transactions/[id]/__tests__/route.test.ts @@ -57,9 +57,18 @@ vi.mock('@/lib/bookkeeping/supplier-invoice-entries', () => ({ vi.mock('@/lib/invoices/match-log', () => ({ logMatchEvent: vi.fn(), })) -vi.mock('@/lib/bookkeeping/mapping-engine', () => ({ - saveUserMappingRule: vi.fn().mockResolvedValue(undefined), -})) +vi.mock('@/lib/bookkeeping/mapping-engine', async () => { + // Keep the real applySettlementAccount: it's a pure rewrite (1930 -> the + // resolved bank leg) and the v1 categorize route's settlement-account fix + // depends on it actually running, not a stub. + const actual = await vi.importActual( + '@/lib/bookkeeping/mapping-engine', + ) + return { + ...actual, + saveUserMappingRule: vi.fn().mockResolvedValue(undefined), + } +}) vi.mock('@/lib/bookkeeping/counterparty-templates', () => ({ upsertCounterpartyTemplate: vi.fn().mockResolvedValue(undefined), buildMappingResultFromCounterpartyTemplate: vi.fn(), @@ -315,6 +324,123 @@ describe('POST :id/categorize', () => { expect(body.error.code).toBe('ACCOUNTS_NOT_IN_CHART') expect(body.error.details.account_numbers).toEqual(['5410']) }) + + // Regression for the v1/MCP-facing half of the settlement-account gap + // fixed on the dashboard route by PR #985: this v1 route never called + // applySettlementAccount at all, so every category booking hardcoded the + // bank leg to 1930 even when the transaction was linked to a different + // cash account (e.g. a savings or EUR account). + it('books the bank leg to the transaction\'s linked cash account, not hardcoded 1930', async () => { + mockServiceClient.mockReturnValue( + makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: [ + { + data: { + id: TX_ID, + company_id: COMPANY_ID, + date: '2026-05-12', + amount: -349.5, + currency: 'SEK', + merchant_name: 'ICA', + journal_entry_id: null, + cash_account_id: 'ca-1940', + }, + error: null, + }, + { data: [{ id: TX_ID }], error: null }, // CAS update select + ], + company_settings: { data: { entity_type: 'enskild_firma' }, error: null }, + cash_accounts: { data: { ledger_account: '1940' }, error: null }, + }), + ) + + const res = await categorizePOST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/${TX_ID}/categorize`, + { is_business: true, category: 'expense_office' }, + ), + txParams(TX_ID), + ) + expect(res.status).toBe(200) + expect(createTxJE).toHaveBeenCalledTimes(1) + const mappingResult = createTxJE.mock.calls[0][4] as { + debit_account: string + credit_account: string + } + expect(mappingResult.credit_account).toBe('1940') + }) + + it('falls back to 1930 when the transaction has no linked cash account', async () => { + mockServiceClient.mockReturnValue( + makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: [ + { + data: { + id: TX_ID, + company_id: COMPANY_ID, + date: '2026-05-12', + amount: -349.5, + currency: 'SEK', + merchant_name: 'ICA', + journal_entry_id: null, + cash_account_id: null, + }, + error: null, + }, + { data: [{ id: TX_ID }], error: null }, + ], + company_settings: { data: { entity_type: 'enskild_firma' }, error: null }, + }), + ) + + const res = await categorizePOST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/${TX_ID}/categorize`, + { is_business: true, category: 'expense_office' }, + ), + txParams(TX_ID), + ) + expect(res.status).toBe(200) + const mappingResult = createTxJE.mock.calls[0][4] as { credit_account: string } + expect(mappingResult.credit_account).toBe('1930') + }) + + it('aborts with 500 BOOKKEEPING_DATABASE_ERROR when the cash_accounts lookup errors, mutating nothing', async () => { + mockServiceClient.mockReturnValue( + makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: { + data: { + id: TX_ID, + company_id: COMPANY_ID, + date: '2026-05-12', + amount: -349.5, + currency: 'SEK', + merchant_name: 'ICA', + journal_entry_id: null, + cash_account_id: 'ca-broken', + }, + error: null, + }, + company_settings: { data: { entity_type: 'enskild_firma' }, error: null }, + cash_accounts: { data: null, error: { message: 'connection reset' } }, + }), + ) + + const res = await categorizePOST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/${TX_ID}/categorize`, + { is_business: true, category: 'expense_office' }, + ), + txParams(TX_ID), + ) + expect(res.status).toBe(500) + const body = await res.json() + expect(body.error.code).toBe('BOOKKEEPING_DATABASE_ERROR') + expect(createTxJE).not.toHaveBeenCalled() + }) }) describe('POST :id/uncategorize', () => { @@ -512,4 +638,268 @@ describe('POST :id/match-supplier-invoice', () => { expect(res.status).toBe(400) expect((await res.json()).error.code).toBe('MATCH_SI_NOT_EXPENSE') }) + + // Regression for the v1/MCP-facing half of the settlement-account gap + // fixed on the dashboard route by PR #985: this route always called + // createSupplierInvoicePaymentEntry with no paymentAccount argument at all + // (hardcoded internal default 1930) for every pure-SEK accrual match, + // regardless of which cash account the transaction was actually linked to. + describe('settlement account resolution (pure-SEK accrual path)', () => { + it('credits the transaction\'s own linked cash account when it is not the primary 1930', async () => { + mockServiceClient.mockReturnValue( + makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: { + data: { + id: TX_ID, + amount: -5000, + date: '2026-05-12', + currency: 'SEK', + supplier_invoice_id: null, + journal_entry_id: null, + cash_account_id: 'ca-1940', + }, + error: null, + }, + supplier_invoices: [ + { + data: { + id: SI_ID, + status: 'approved', + total: 5000, + paid_amount: 0, + remaining_amount: 5000, + currency: 'SEK', + exchange_rate: null, + supplier: { name: 'Acme', supplier_type: 'swedish_business' }, + items: [], + }, + error: null, + }, + { data: [{ id: SI_ID }], error: null }, + ], + company_settings: { data: { accounting_method: 'accrual' }, error: null }, + cash_accounts: { data: { ledger_account: '1940' }, error: null }, + supplier_invoice_payments: { data: null, error: null }, + }), + ) + const res = await matchSIPOST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/${TX_ID}/match-supplier-invoice`, + { supplier_invoice_id: SI_ID }, + ), + txParams(TX_ID), + ) + expect(res.status).toBe(200) + expect(createSupplierInvPmtJE).toHaveBeenCalledTimes(1) + const paymentAccountArg = createSupplierInvPmtJE.mock.calls[0][8] + expect(paymentAccountArg).toBe('1940') + }) + + it('ignores a stale last_supplier_payment_account setting and uses the linked cash account', async () => { + mockServiceClient.mockReturnValue( + makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: { + data: { + id: TX_ID, + amount: -1001, + date: '2026-02-01', + currency: 'SEK', + supplier_invoice_id: null, + journal_entry_id: null, + cash_account_id: 'ca-1930', + }, + error: null, + }, + supplier_invoices: [ + { + data: { + id: SI_ID, + status: 'registered', + total: 1001, + paid_amount: 0, + remaining_amount: 1001, + currency: 'SEK', + exchange_rate: null, + supplier: { name: 'Acme', supplier_type: 'swedish_business' }, + items: [], + }, + error: null, + }, + { data: [{ id: SI_ID }], error: null }, + ], + // Stale sticky setting from an earlier private-funds mark-paid + // payment: must be ignored, this route never reads it. + company_settings: { + data: { accounting_method: 'accrual', last_supplier_payment_account: '2893' }, + error: null, + }, + cash_accounts: { data: { ledger_account: '1930' }, error: null }, + supplier_invoice_payments: { data: null, error: null }, + }), + ) + const res = await matchSIPOST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/${TX_ID}/match-supplier-invoice`, + { supplier_invoice_id: SI_ID }, + ), + txParams(TX_ID), + ) + expect(res.status).toBe(200) + const paymentAccountArg = createSupplierInvPmtJE.mock.calls[0][8] + expect(paymentAccountArg).toBe('1930') + expect(paymentAccountArg).not.toBe('2893') + }) + + it('defaults to 1930 when the transaction has no linked cash account', async () => { + mockServiceClient.mockReturnValue( + makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: { + data: { + id: TX_ID, + amount: -750, + date: '2026-02-01', + currency: 'SEK', + supplier_invoice_id: null, + journal_entry_id: null, + cash_account_id: null, + }, + error: null, + }, + supplier_invoices: [ + { + data: { + id: SI_ID, + status: 'registered', + total: 750, + paid_amount: 0, + remaining_amount: 750, + currency: 'SEK', + exchange_rate: null, + supplier: { name: 'Acme', supplier_type: 'swedish_business' }, + items: [], + }, + error: null, + }, + { data: [{ id: SI_ID }], error: null }, + ], + company_settings: { data: { accounting_method: 'accrual' }, error: null }, + supplier_invoice_payments: { data: null, error: null }, + }), + ) + const res = await matchSIPOST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/${TX_ID}/match-supplier-invoice`, + { supplier_invoice_id: SI_ID }, + ), + txParams(TX_ID), + ) + expect(res.status).toBe(200) + const paymentAccountArg = createSupplierInvPmtJE.mock.calls[0][8] + expect(paymentAccountArg).toBe('1930') + }) + + it('aborts with 500 BOOKKEEPING_DATABASE_ERROR when the cash_accounts lookup errors, mutating nothing', async () => { + mockServiceClient.mockReturnValue( + makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: { + data: { + id: TX_ID, + amount: -600, + date: '2026-02-01', + currency: 'SEK', + supplier_invoice_id: null, + journal_entry_id: null, + cash_account_id: 'ca-broken', + }, + error: null, + }, + supplier_invoices: { + data: { + id: SI_ID, + status: 'registered', + total: 600, + paid_amount: 0, + remaining_amount: 600, + currency: 'SEK', + exchange_rate: null, + supplier: { name: 'Acme', supplier_type: 'swedish_business' }, + items: [], + }, + error: null, + }, + company_settings: { data: { accounting_method: 'accrual' }, error: null }, + cash_accounts: { data: null, error: { message: 'connection reset' } }, + }), + ) + const res = await matchSIPOST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/${TX_ID}/match-supplier-invoice`, + { supplier_invoice_id: SI_ID }, + ), + txParams(TX_ID), + ) + expect(res.status).toBe(500) + const body = await res.json() + expect(body.error.code).toBe('BOOKKEEPING_DATABASE_ERROR') + expect(createSupplierInvPmtJE).not.toHaveBeenCalled() + }) + + it('returns 400 ACCOUNTS_NOT_IN_CHART when the linked cash account is deactivated in the kontoplan', async () => { + mockServiceClient.mockReturnValue( + makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: { + data: { + id: TX_ID, + amount: -5000, + date: '2026-05-12', + currency: 'SEK', + supplier_invoice_id: null, + journal_entry_id: null, + cash_account_id: 'ca-1940', + }, + error: null, + }, + supplier_invoices: { + data: { + id: SI_ID, + status: 'approved', + total: 5000, + paid_amount: 0, + remaining_amount: 5000, + currency: 'SEK', + exchange_rate: null, + supplier: { name: 'Acme', supplier_type: 'swedish_business' }, + items: [], + }, + error: null, + }, + company_settings: { data: { accounting_method: 'accrual' }, error: null }, + cash_accounts: { data: { ledger_account: '1940' }, error: null }, + }), + ) + // Simulate the 1940 account existing in cash_accounts but having been + // deactivated in chart_of_accounts since. + findMissingAccountsMock.mockResolvedValueOnce(['1940']) + + const res = await matchSIPOST( + makeRequest( + `https://x.test/api/v1/companies/${COMPANY_ID}/transactions/${TX_ID}/match-supplier-invoice`, + { supplier_invoice_id: SI_ID }, + ), + txParams(TX_ID), + ) + expect(res.status).toBe(400) + const body = await res.json() + expect(body.error.code).toBe('ACCOUNTS_NOT_IN_CHART') + expect(body.error.details.account_numbers).toEqual(['1940']) + // Engine and invoice/transaction updates must NOT run: the match stays + // retryable rather than posting a payment against a dead account. + expect(createSupplierInvPmtJE).not.toHaveBeenCalled() + }) + }) }) diff --git a/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts b/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts index 65c5fa94..d86037a5 100644 --- a/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts +++ b/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts @@ -33,7 +33,8 @@ import { } from '@/lib/bookkeeping/counterparty-templates' import { createTransactionJournalEntry } from '@/lib/bookkeeping/transaction-entries' import { reverseEntry } from '@/lib/bookkeeping/engine' -import { saveUserMappingRule } from '@/lib/bookkeeping/mapping-engine' +import { saveUserMappingRule, applySettlementAccount } from '@/lib/bookkeeping/mapping-engine' +import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' import { AccountsNotInChartError, isBookkeepingError } from '@/lib/bookkeeping/errors' import { collectMappingResultAccounts, findUnresolvableAccounts } from '@/lib/bookkeeping/account-validation' import { getErrorMessage } from '@/lib/errors/get-error-message' @@ -240,6 +241,22 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string ) } + // Book the bank leg against the transaction's ACTUAL settlement account + // rather than the hardcoded 1930 in the templates. Without this, interest + // or fees that landed on a savings/EUR account mis-book to 1930 and the + // real bank line never reconciles. applySettlementAccount only rewrites a + // 1930 leg and is a no-op when the settlement account is 1930, so legacy + // rows with no cash_account_id behave exactly as before. Mirrors the + // internal dashboard route (app/api/transactions/[id]/categorize); this + // v1 surface previously never called applySettlementAccount at all. + const settlementAccount = await resolveSettlementAccount( + ctx.supabase, + ctx.companyId!, + transaction.cash_account_id, + txLog, + ) + mappingResult = applySettlementAccount(mappingResult, settlementAccount) + if ( is_business && body.account_override && diff --git a/app/api/v1/companies/[companyId]/transactions/[id]/match-supplier-invoice/route.ts b/app/api/v1/companies/[companyId]/transactions/[id]/match-supplier-invoice/route.ts index 882abfd5..be37b719 100644 --- a/app/api/v1/companies/[companyId]/transactions/[id]/match-supplier-invoice/route.ts +++ b/app/api/v1/companies/[companyId]/transactions/[id]/match-supplier-invoice/route.ts @@ -16,8 +16,10 @@ import { createSupplierInvoicePaymentEntry, createSupplierInvoiceCashEntry, } from '@/lib/bookkeeping/supplier-invoice-entries' +import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' import { reverseEntry, createJournalEntry, findFiscalPeriod } from '@/lib/bookkeeping/engine' -import { isBookkeepingError } from '@/lib/bookkeeping/errors' +import { AccountsNotInChartError, isBookkeepingError } from '@/lib/bookkeeping/errors' +import { findUnresolvableAccounts } from '@/lib/bookkeeping/account-validation' import { getErrorMessage } from '@/lib/errors/get-error-message' import { logMatchEvent } from '@/lib/invoices/match-log' import { eventBus } from '@/lib/events/bus' @@ -219,12 +221,48 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string .single() const accountingMethod = settings?.accounting_method || 'accrual' + // Credit the cash account THIS transaction actually belongs to, never a + // hardcoded 1930: cash_account_id -> cash_accounts.ledger_account is the + // only source of truth for which bank account a matched transaction + // settled from (mirrors the dashboard route's #985 fix). Only applied to + // the pure-SEK accrual path below; the FX path keeps its pre-existing + // internal 1930 default, matching that fix's scope. + const paymentAccount = await resolveSettlementAccount( + ctx.supabase, + ctx.companyId!, + transaction.cash_account_id, + txLog, + ) + // Route on the supplier invoice's actual booking state. An invoice // booked at receipt (registration_journal_entry_id set) must clear // 2440 regardless of the company's current setting. const siAlreadyBooked = !!(invoice as { registration_journal_entry_id?: string | null }).registration_journal_entry_id const useCashEntry = !siAlreadyBooked && accountingMethod === 'cash' + // Pure SEK: both legs of the match are SEK, so the payment account + // resolved above can safely replace the 1930 default. Kept out of scope + // for foreign-currency matches, same as the dashboard route. + const isPureSek = transaction.currency === 'SEK' && invoice.currency === 'SEK' + + // Guard the resolved account against the chart (mirrors the categorize + // routes): an inactive cash_accounts.ledger_account would otherwise reach + // the engine as a generic MATCH_SI_RECORD_PAYMENT_FAILED instead of + // ACCOUNTS_NOT_IN_CHART. Only reachable where the account is actually used. + if (isPureSek && !useCashEntry && !customLines) { + const missingAccounts = await findUnresolvableAccounts( + ctx.supabase, + ctx.companyId!, + [paymentAccount], + ) + if (missingAccounts.length > 0) { + txLog.warn('resolved settlement account is inactive/unknown', { missingAccounts }) + return v1ErrorResponse(new AccountsNotInChartError(missingAccounts), txLog, { + requestId: ctx.requestId, + }) + } + } + // Full settlement = the bank amount pays off the whole remaining balance. // Cross-currency always settles the remaining (paymentAmountInvoiceCurrency // is clamped to invoice.remaining_amount above). @@ -307,11 +345,23 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string paymentAmountSek, transaction.date, exchangeRateDifference !== 0 ? exchangeRateDifference : undefined, + undefined, // supplierName (unchanged default) + // Resolved settlement account, pure-SEK matches only: the FX + // branch keeps defaulting internally to 1930, out of scope here + // just as it was for the dashboard route's #985 fix. + isPureSek ? paymentAccount : undefined, ) if (je) journalEntryId = je.id } } catch (err) { txLog.error('match-supplier-invoice: payment JE creation failed: aborting before state mutation', err as Error) + // AccountsNotInChartError means the account was deactivated between our + // pre-validation above and the engine call (race): return the same + // structured error rather than falling through to the generic + // MATCH_SI_RECORD_PAYMENT_FAILED, mirroring the categorize routes. + if (err instanceof AccountsNotInChartError) { + return v1ErrorResponse(err, txLog, { requestId: ctx.requestId }) + } const message = isBookkeepingError(err) ? getErrorMessage(err, { context: 'supplier_invoice' }) : err instanceof Error