diff --git a/DECISIONS.md b/DECISIONS.md index 516a2200..34e37318 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -72,6 +72,10 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [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-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-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. +[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 (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 -- a failed request the caller can retry beats a wrong verifikat needing a storno). Left the "row found but ledger_account is empty" case as warn+fallback: that's a data-integrity gap, not a query failure, and a prior compliance-bot round only asked for a warning there. No caller changes needed: every route here already runs under withRouteContext/withApiV1, whose outer catch-all already converts any isBookkeepingError() throw into the correct structured 500 via errorResponse/v1ErrorResponse, and lib/pending-operations/commit.ts's dispatcher already has the identical generic bookkeeping-error handling for every other engine failure. Same change applied identically across #985/#986/#987 (shared helper file). Filed #1000 to track the still-open FX/cash-method paymentAccount gap in the /api/v1 and MCP-facing match-supplier-invoice route (the main match-supplier-invoice route's FX/cash-method branches already thread paymentAccount via resolveSettlementAccount, per the entry above; deliberately out of scope, matches this PR's own scope decision) and a new issue to track historical-mis-booking remediation (PR #986 compliance bot finding #3, a distinct detection/correction initiative never claimed in scope by any of these three PRs). [2026-07-12] pain.001 salary dialect hardened to the Swedish Common Interpretation (Bankforeningen "Common Payment Types in Sweden" Appendix 1, Example 4 Salaries; cross-checked vs Nordea Corporate Access pain.001 examples v2.6, 2026-06-22): dropped SvcLvl SEPA (SEPA credit transfer is EUR-only; omitting SvcLvl gets the domestic NURG default), dropped RmtInf (Nordea: remittance info not allowed for SALA; statement text comes from the Dataclearing LON code), creditor addressed as CdtrAgt ClrSysMmbId SESBA + CdtrAcct Othr SchmeNm BBAN with the account WITHOUT clearing, Dbtr now carries OrgId, all ids clamped to Max35Text. The clearing/account split (Swedbank 5-digit shift, Nordea personkonto prefix dedup) is extracted to splitDomesticBankAccount in lib/salary/payment/bank-account.ts and shared by BOTH the bg-lb and pain001 generators so the two formats can never route a payment differently again (pain001 previously concatenated raw digits and duplicated the personkonto clearing). Swedbank MmbId = first 4 clearing digits with the 5th shifted into the account, mirroring the production-proven LB encoding and the appendix's 4-digit MmbId salary example; run a generated file through Swedbank Validex (and the other banks' test uploads) before the 1 Aug Bg Lon campaign. [2026-07-12] ESG/CO2 reporting parked (no build): external revisor review flagged its absence; not a purchase criterion for the target segment (tech-native sjalvbokforare). Revisit on real customer demand; likely shape then is a spend-based CO2 estimate on supplier invoices as an extension, not core. [2026-07-12] "Projektredovisning" split into two scopes after the revisor review: full project accounting (WIP, successiv vinstavrakning, budget follow-up) parked indefinitely; light time-to-invoice (time entries to invoice rows; schema support already exists via project_time_entries and dimension FKs) stays an OPEN 2026 positioning decision, deliberately not committed yet. diff --git a/app/api/transactions/[id]/categorize/route.ts b/app/api/transactions/[id]/categorize/route.ts index 76a9abb5..c2626bc6 100644 --- a/app/api/transactions/[id]/categorize/route.ts +++ b/app/api/transactions/[id]/categorize/route.ts @@ -8,6 +8,7 @@ import { createTransactionJournalEntry } from '@/lib/bookkeeping/transaction-ent import { detectBookingDuplicate } from '@/lib/transactions/booking-duplicate-detection' import { appendProcessingHistory } from '@/lib/processing-history/append' import { saveUserMappingRule, applySettlementAccount } from '@/lib/bookkeeping/mapping-engine' +import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' import { upsertCounterpartyTemplate, buildMappingResultFromCounterpartyTemplate } from '@/lib/bookkeeping/counterparty-templates' import { withRouteContext } from '@/lib/api/with-route-context' import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error' @@ -314,26 +315,12 @@ export const POST = withRouteContext( // 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. - let settlementAccount = '1930' - if (transaction.cash_account_id) { - const { data: txCashAccount, error: cashAccountError } = await supabase - .from('cash_accounts') - .select('ledger_account') - .eq('id', transaction.cash_account_id) - .eq('company_id', companyId) - .maybeSingle() - if (cashAccountError) { - // Don't fail the booking: fall back to 1930, but surface the lookup - // failure so a silent mis-booking to the wrong bank leg stays auditable. - txLog.warn('settlement-account lookup failed; defaulting to 1930', { - cashAccountId: transaction.cash_account_id, - error: cashAccountError.message, - }) - } - if (txCashAccount?.ledger_account) { - settlementAccount = txCashAccount.ledger_account as string - } - } + const settlementAccount = await resolveSettlementAccount( + supabase, + companyId!, + transaction.cash_account_id, + txLog, + ) mappingResult = applySettlementAccount(mappingResult, settlementAccount) txLog.info('mapping resolved', { diff --git a/app/api/transactions/[id]/match-supplier-invoice/__tests__/route.test.ts b/app/api/transactions/[id]/match-supplier-invoice/__tests__/route.test.ts index 5c3854cb..b8bd45e2 100644 --- a/app/api/transactions/[id]/match-supplier-invoice/__tests__/route.test.ts +++ b/app/api/transactions/[id]/match-supplier-invoice/__tests__/route.test.ts @@ -6,6 +6,16 @@ import { } from '@/tests/helpers' import { AccountsNotInChartError } from '@/lib/bookkeeping/errors' +const { mockLoggerWarn } = vi.hoisted(() => ({ mockLoggerWarn: vi.fn() })) +vi.mock('@/lib/logger', () => ({ + createLogger: () => ({ + info: vi.fn(), + warn: mockLoggerWarn, + error: vi.fn(), + child: vi.fn().mockReturnThis(), + }), +})) + const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase() vi.mock('@/lib/supabase/server', () => ({ createClient: () => Promise.resolve(mockSupabase), @@ -202,6 +212,156 @@ describe('POST /api/transactions/[id]/match-supplier-invoice: FX residual', () = }) }) +describe('POST /api/transactions/[id]/match-supplier-invoice: settlement account resolution', () => { + // Regression for a real misbooking: a company whose last few supplier + // invoices were paid privately (mark-paid sets company_settings + // .last_supplier_payment_account = '2893') later matched a genuine bank + // transaction to a supplier invoice. The route used to default + // paymentAccount from that sticky setting, so the payment credited 2893 + // (shareholder loan) instead of the transaction's real 1930 bank account. + it('credits the account this transaction is linked to, even with a stale last_supplier_payment_account on file', async () => { + enqueue({ + data: { + id: TX_UUID, + company_id: 'company-1', + amount: -1001, + currency: 'SEK', + amount_sek: null, + supplier_invoice_id: null, + cash_account_id: 'ca-1930', + date: '2026-02-01', + }, + error: null, + }) + enqueue({ + data: { + id: SI_UUID, + currency: 'SEK', + exchange_rate: null, + status: 'registered', + remaining_amount: 1001, + paid_amount: 0, + supplier: { supplier_type: 'swedish_business' }, + items: [], + }, + error: null, + }) + // Stale sticky setting from an earlier private-funds payment: must be + // ignored now that the route no longer selects it. + enqueue({ data: { accounting_method: 'accrual', last_supplier_payment_account: '2893' }, error: null }) + enqueue({ data: { ledger_account: '1930' }, error: null }) // cash_accounts lookup + enqueue({ data: [{ id: SI_UUID }], error: null }) + enqueue({ data: null, error: null }) + enqueue({ data: null, error: null }) + + await POST(makeReq(), createMockRouteParams({ id: TX_UUID })) + + expect(mockCreateJournalEntry).toHaveBeenCalledTimes(1) + const input = mockCreateJournalEntry.mock.calls[0][3] as { + lines: Array<{ account_number: string; debit_amount: number; credit_amount: number }> + } + expect(input.lines.find((l) => l.account_number === '1930')?.credit_amount).toBe(1001) + expect(input.lines.some((l) => l.account_number === '2893')).toBe(false) + }) + + it('credits the transaction\'s own linked cash account when it is not the primary 1930', async () => { + enqueue({ + data: { + id: TX_UUID, + company_id: 'company-1', + amount: -500, + currency: 'SEK', + amount_sek: null, + supplier_invoice_id: null, + cash_account_id: 'ca-1940', + date: '2026-02-01', + }, + error: null, + }) + enqueue({ + data: { + id: SI_UUID, + currency: 'SEK', + exchange_rate: null, + status: 'registered', + remaining_amount: 500, + paid_amount: 0, + supplier: { supplier_type: 'swedish_business' }, + items: [], + }, + error: null, + }) + enqueue({ data: { accounting_method: 'accrual' }, error: null }) + enqueue({ data: { ledger_account: '1940' }, error: null }) // cash_accounts lookup + enqueue({ data: [{ id: SI_UUID }], error: null }) + enqueue({ data: null, error: null }) + enqueue({ data: null, error: null }) + + await POST(makeReq(), createMockRouteParams({ id: TX_UUID })) + + const input = mockCreateJournalEntry.mock.calls[0][3] as { + lines: Array<{ account_number: string; debit_amount: number; credit_amount: number }> + } + expect(input.lines.find((l) => l.account_number === '1940')?.credit_amount).toBe(500) + }) + + it('defaults to 1930 when the transaction has no linked cash account', async () => { + enqueueHappyPath({ + transaction: { amount: -750, currency: 'SEK' }, + invoice: { currency: 'SEK', remaining_amount: 750 }, + }) + await POST(makeReq(), createMockRouteParams({ id: TX_UUID })) + const input = mockCreateJournalEntry.mock.calls[0][3] as { + lines: Array<{ account_number: string; debit_amount: number; credit_amount: number }> + } + expect(input.lines.find((l) => l.account_number === '1930')?.credit_amount).toBe(750) + }) + + it('aborts with 500 BOOKKEEPING_DATABASE_ERROR (mutates nothing) when the cash_accounts lookup errors', async () => { + // Regression: an explicit cash_account_id almost certainly resolves to a + // non-1930 account, so a transient lookup failure must not silently + // degrade to 1930 (same misbooking risk this whole fix exists to close, + // just triggered by infra flakiness instead of a stale setting). The + // request should fail before any state mutation, not book to a guessed + // account. + enqueue({ + data: { + id: TX_UUID, + company_id: 'company-1', + amount: -600, + currency: 'SEK', + amount_sek: null, + supplier_invoice_id: null, + cash_account_id: 'ca-broken', + date: '2026-02-01', + }, + error: null, + }) + enqueue({ + data: { + id: SI_UUID, + currency: 'SEK', + exchange_rate: null, + status: 'registered', + remaining_amount: 600, + paid_amount: 0, + supplier: { supplier_type: 'swedish_business' }, + items: [], + }, + error: null, + }) + enqueue({ data: { accounting_method: 'accrual' }, error: null }) + enqueue({ data: null, error: { message: 'connection reset' } }) // cash_accounts lookup errors + + const res = await POST(makeReq(), createMockRouteParams({ id: TX_UUID })) + const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res) + + expect(status).toBe(500) + expect(body.error.code).toBe('BOOKKEEPING_DATABASE_ERROR') + expect(mockCreateJournalEntry).not.toHaveBeenCalled() + }) +}) + describe('POST /api/transactions/[id]/match-supplier-invoice: non-FX paths', () => { it('returns 200 with the expected body shape on the happy path', async () => { enqueueHappyPath({ diff --git a/app/api/transactions/[id]/match-supplier-invoice/preview/__tests__/route.test.ts b/app/api/transactions/[id]/match-supplier-invoice/preview/__tests__/route.test.ts new file mode 100644 index 00000000..f9b217a8 --- /dev/null +++ b/app/api/transactions/[id]/match-supplier-invoice/preview/__tests__/route.test.ts @@ -0,0 +1,149 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { + createQueuedMockSupabase, + createMockRouteParams, + parseJsonResponse, +} from '@/tests/helpers' + +const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase() +vi.mock('@/lib/supabase/server', () => ({ + createClient: () => Promise.resolve(mockSupabase), +})) + +vi.mock('@/lib/company/context', () => ({ + requireCompanyId: vi.fn().mockResolvedValue('company-1'), + getActiveCompanyId: vi.fn().mockResolvedValue('company-1'), +})) + +import { GET } from '../route' + +const mockUser = { id: 'user-1', email: 'test@test.se' } +const TX_UUID = '11111111-1111-4111-8111-111111111111' +const SI_UUID = '22222222-2222-4222-8222-222222222222' + +beforeEach(() => { + vi.clearAllMocks() + reset() + mockSupabase.auth.getUser.mockResolvedValue({ data: { user: mockUser } }) +}) + +function makeReq() { + return new Request( + `http://localhost/api/transactions/${TX_UUID}/match-supplier-invoice/preview?supplier_invoice_id=${SI_UUID}`, + ) +} + +// Regression: the sticky company_settings.last_supplier_payment_account +// (written whenever a supplier invoice is marked paid "with private funds", +// e.g. crediting 2893) used to be the previewed credit account for ANY +// matched transaction, including one linked to the company's real 1930 bank +// account. The preview must credit the transaction's own linked cash +// account, not that unrelated sticky setting. +describe('GET /api/transactions/[id]/match-supplier-invoice/preview: settlement account resolution', () => { + it('previews a credit to the transaction\'s linked cash account, ignoring a stale last_supplier_payment_account', async () => { + enqueue({ + data: { + id: TX_UUID, + date: '2026-02-01', + amount: -1001, + currency: 'SEK', + amount_sek: null, + cash_account_id: 'ca-1930', + }, + error: null, + }) + enqueue({ + data: { + id: SI_UUID, + currency: 'SEK', + exchange_rate: null, + total: 1001, + remaining_amount: 1001, + registration_journal_entry_id: 'je-registered', + items: [], + }, + error: null, + }) + // Stale sticky setting from an earlier private-funds payment: must be + // ignored now that the route resolves the account from the transaction. + enqueue({ data: { accounting_method: 'accrual', last_supplier_payment_account: '2893' }, error: null }) + enqueue({ data: { ledger_account: '1930' }, error: null }) // cash_accounts lookup + + const res = await GET(makeReq(), createMockRouteParams({ id: TX_UUID })) + const { body } = await parseJsonResponse<{ + lines: Array<{ account_number: string; debit_amount: number; credit_amount: number }> + }>(res) + + expect(body.lines.find((l) => l.account_number === '1930')?.credit_amount).toBe(1001) + expect(body.lines.some((l) => l.account_number === '2893')).toBe(false) + }) + + it('defaults to 1930 when the transaction has no linked cash account', async () => { + enqueue({ + data: { + id: TX_UUID, + date: '2026-02-01', + amount: -750, + currency: 'SEK', + amount_sek: null, + cash_account_id: null, + }, + error: null, + }) + enqueue({ + data: { + id: SI_UUID, + currency: 'SEK', + exchange_rate: null, + total: 750, + remaining_amount: 750, + registration_journal_entry_id: 'je-registered', + items: [], + }, + error: null, + }) + enqueue({ data: { accounting_method: 'accrual' }, error: null }) + + const res = await GET(makeReq(), createMockRouteParams({ id: TX_UUID })) + const { body } = await parseJsonResponse<{ + lines: Array<{ account_number: string; debit_amount: number; credit_amount: number }> + }>(res) + + expect(body.lines.find((l) => l.account_number === '1930')?.credit_amount).toBe(750) + }) + + it('previews a credit to the linked cash account when it is not the primary 1930', async () => { + enqueue({ + data: { + id: TX_UUID, + date: '2026-02-01', + amount: -500, + currency: 'SEK', + amount_sek: null, + cash_account_id: 'ca-1940', + }, + error: null, + }) + enqueue({ + data: { + id: SI_UUID, + currency: 'SEK', + exchange_rate: null, + total: 500, + remaining_amount: 500, + registration_journal_entry_id: 'je-registered', + items: [], + }, + error: null, + }) + enqueue({ data: { accounting_method: 'accrual' }, error: null }) + enqueue({ data: { ledger_account: '1940' }, error: null }) // cash_accounts lookup + + const res = await GET(makeReq(), createMockRouteParams({ id: TX_UUID })) + const { body } = await parseJsonResponse<{ + lines: Array<{ account_number: string; debit_amount: number; credit_amount: number }> + }>(res) + + expect(body.lines.find((l) => l.account_number === '1940')?.credit_amount).toBe(500) + }) +}) diff --git a/app/api/transactions/[id]/match-supplier-invoice/preview/route.ts b/app/api/transactions/[id]/match-supplier-invoice/preview/route.ts index 6d0c92f2..ef57d535 100644 --- a/app/api/transactions/[id]/match-supplier-invoice/preview/route.ts +++ b/app/api/transactions/[id]/match-supplier-invoice/preview/route.ts @@ -13,6 +13,7 @@ import { withRouteContext } from '@/lib/api/with-route-context' import { errorResponseFromCode } from '@/lib/errors/get-structured-error' import { resolveSekAmount } from '@/lib/bookkeeping/currency-utils' import { buildSupplierPaymentClearingLines } from '@/lib/bookkeeping/supplier-payment-lines' +import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' import { ORE_TOLERANCE } from '@/lib/money' import type { SupplierInvoice, SupplierInvoiceItem } from '@/types' @@ -50,7 +51,9 @@ export const GET = withRouteContext( // amount_sek is needed for the cash-method preview: a foreign-currency // settlement is translated at the payment-date rate (the SEK that left // the bank), mirroring the committed verifikat from the POST handler. - .select('id, date, amount, currency, amount_sek') + // cash_account_id resolves which BAS account this bank line actually + // settles from, mirroring the POST handler's settlement-account lookup. + .select('id, date, amount, currency, amount_sek, cash_account_id') .eq('id', transactionId) .eq('company_id', companyId) .single() @@ -70,13 +73,22 @@ export const GET = withRouteContext( const { data: settings } = await supabase .from('company_settings') - .select('accounting_method, last_supplier_payment_account') + .select('accounting_method') .eq('company_id', companyId) .single() const accountingMethod = settings?.accounting_method || 'accrual' - const paymentAccount = - (settings as { last_supplier_payment_account?: string } | null)?.last_supplier_payment_account || '1930' + + // Same resolution as the POST handler: credit the cash account this + // transaction is actually linked to, never the sticky + // last_supplier_payment_account (that setting reflects the manual + // mark-paid/private-funds flow, not a real matched bank transaction). + const paymentAccount = await resolveSettlementAccount( + supabase, + companyId!, + transaction.cash_account_id, + log, + ) const siAlreadyBooked = !!(invoice as { registration_journal_entry_id?: string | null }).registration_journal_entry_id const useCashEntry = !siAlreadyBooked && accountingMethod === 'cash' diff --git a/app/api/transactions/[id]/match-supplier-invoice/route.ts b/app/api/transactions/[id]/match-supplier-invoice/route.ts index f62451ac..242df20f 100644 --- a/app/api/transactions/[id]/match-supplier-invoice/route.ts +++ b/app/api/transactions/[id]/match-supplier-invoice/route.ts @@ -4,6 +4,7 @@ import { createSupplierInvoiceCashEntry, } from '@/lib/bookkeeping/supplier-invoice-entries' import { buildSupplierPaymentClearingLines } from '@/lib/bookkeeping/supplier-payment-lines' +import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' import { cancelOrphanedPaymentEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' import { planSupplierPayment } from '@/lib/invoices/apply-supplier-payment' import { createJournalEntry, findFiscalPeriod } from '@/lib/bookkeeping/engine' @@ -104,15 +105,25 @@ export const POST = withRouteContext( const { data: settings } = await supabase .from('company_settings') - .select('accounting_method, last_supplier_payment_account') + .select('accounting_method') .eq('company_id', companyId) .single() const accountingMethod = settings?.accounting_method || 'accrual' - // Same default the preview route uses, so the committed verifikat credits the - // same account the user saw previewed (the old path hardcoded 1930 here). - const paymentAccount = - (settings as { last_supplier_payment_account?: string } | null)?.last_supplier_payment_account || '1930' + + // Credit the cash account THIS transaction actually belongs to, never a + // company-wide "last used" preference: last_supplier_payment_account is + // written by the manual mark-paid flow (e.g. a private-funds payment + // booked to 2893) and has no relationship to which bank account a real, + // matched transaction settled from. Reusing it here silently misbooked a + // genuine 1930 bank payment to 2893 once a private payment had set that + // sticky default. + const paymentAccount = await resolveSettlementAccount( + supabase, + companyId!, + transaction.cash_account_id, + txLog, + ) // Route on the supplier invoice's actual booking state: if 2440 was posted // at receipt (accrual), the match clears 2440 regardless of the company's @@ -254,10 +265,11 @@ export const POST = withRouteContext( transaction.date, invoice.supplier?.supplier_type || 'swedish_business', undefined, // supplierName (unchanged default) - undefined, // paymentAccount (unchanged default 1930) - // Pin a foreign-currency settlement to the payment-date rate so 1930 - // equals the bank movement (kontantmetoden books the expense at - // payment). No-op for SEK invoices and same-rate settlements. + paymentAccount, + // Pin a foreign-currency settlement to the payment-date rate so the + // settlement account equals the bank movement (kontantmetoden books + // the expense at payment). No-op for SEK invoices and same-rate + // settlements. exchangeRateDifference !== 0 && fullSettlement ? actualBankSek : undefined, ) if (journalEntry) journalEntryId = journalEntry.id @@ -292,6 +304,8 @@ export const POST = withRouteContext( supabase, companyId, user.id, invoice as SupplierInvoice, paymentAmountSek, transaction.date, exchangeRateDifference !== 0 ? exchangeRateDifference : undefined, + undefined, // supplierName (unchanged default) + paymentAccount, ) if (journalEntry) journalEntryId = journalEntry.id } diff --git a/lib/bookkeeping/__tests__/settlement-account.test.ts b/lib/bookkeeping/__tests__/settlement-account.test.ts new file mode 100644 index 00000000..f0a590b3 --- /dev/null +++ b/lib/bookkeeping/__tests__/settlement-account.test.ts @@ -0,0 +1,67 @@ +import { describe, it, expect, vi } from 'vitest' +import { createMockSupabase } from '@/tests/helpers' +import { resolveSettlementAccount } from '../settlement-account' +import { BookkeepingDatabaseError } from '../errors' + +const noopLog = { warn: vi.fn() } as unknown as import('@/lib/logger').Logger + +describe('resolveSettlementAccount', () => { + it('returns 1930 when the transaction has no cash_account_id', async () => { + const { supabase } = createMockSupabase() + + const result = await resolveSettlementAccount(supabase as never, 'company-1', null, noopLog) + + expect(result).toBe('1930') + expect(supabase.from).not.toHaveBeenCalled() + }) + + it('returns the linked cash account ledger_account', async () => { + const { supabase, mockResult } = createMockSupabase() + mockResult({ data: { ledger_account: '1940' }, error: null }) + + const result = await resolveSettlementAccount(supabase as never, 'company-1', 'ca-1', noopLog) + + expect(result).toBe('1940') + expect(supabase.from).toHaveBeenCalledWith('cash_accounts') + }) + + it('throws a BookkeepingDatabaseError instead of silently falling back when the lookup errors', async () => { + const { supabase, mockResult } = createMockSupabase() + mockResult({ data: null, error: { message: 'boom' } }) + + await expect( + resolveSettlementAccount(supabase as never, 'company-1', 'ca-1', noopLog), + ).rejects.toBeInstanceOf(BookkeepingDatabaseError) + await expect( + resolveSettlementAccount(supabase as never, 'company-1', 'ca-1', noopLog), + ).rejects.toMatchObject({ + operation: 'resolve_settlement_account', + message: expect.stringContaining('boom'), + }) + }) + + it('falls back to 1930 when cash_account_id does not match any row', async () => { + const { supabase, mockResult } = createMockSupabase() + mockResult({ data: null, error: null }) + + const result = await resolveSettlementAccount(supabase as never, 'company-1', 'ca-unknown', noopLog) + + expect(result).toBe('1930') + }) + + it('falls back to 1930 and warns when the row has no ledger_account', async () => { + const { supabase, mockResult } = createMockSupabase() + mockResult({ data: { ledger_account: null }, error: null }) + const warn = vi.fn() + + const result = await resolveSettlementAccount(supabase as never, 'company-1', 'ca-1', { + warn, + } as unknown as import('@/lib/logger').Logger) + + expect(result).toBe('1930') + expect(warn).toHaveBeenCalledWith( + 'settlement-account lookup returned no ledger_account; defaulting to 1930', + expect.objectContaining({ cashAccountId: 'ca-1' }), + ) + }) +}) diff --git a/lib/bookkeeping/errors.ts b/lib/bookkeeping/errors.ts index abc06cbd..c6aa4cb4 100644 --- a/lib/bookkeeping/errors.ts +++ b/lib/bookkeeping/errors.ts @@ -296,6 +296,7 @@ export type BookkeepingOperation = | 'fetch_currency_receivables' | 'fetch_currency_payables' | 'check_existing_revaluation' + | 'resolve_settlement_account' export class BookkeepingDatabaseError extends Error { readonly code = BOOKKEEPING_DATABASE_ERROR diff --git a/lib/bookkeeping/settlement-account.ts b/lib/bookkeeping/settlement-account.ts new file mode 100644 index 00000000..db1c2082 --- /dev/null +++ b/lib/bookkeeping/settlement-account.ts @@ -0,0 +1,54 @@ +import type { SupabaseClient } from '@supabase/supabase-js' +import type { Logger } from '@/lib/logger' +import { BookkeepingDatabaseError } from '@/lib/bookkeeping/errors' + +const FALLBACK_ACCOUNT = '1930' + +/** + * Resolve the BAS ledger account a transaction actually settles from/to. + * + * Never fall back to a company-wide "last used" setting (e.g. + * last_supplier_payment_account, written by the manual mark-paid + * private-funds flow): those reflect unrelated flows with no relationship + * to which bank account a specific transaction is linked to. + * cash_account_id -> cash_accounts.ledger_account is the only source of + * truth for a real transaction's settlement account. + */ +export async function resolveSettlementAccount( + supabase: SupabaseClient, + companyId: string, + cashAccountId: string | null, + log: Logger, +): Promise { + if (!cashAccountId) return FALLBACK_ACCOUNT + + const { data, error } = await supabase + .from('cash_accounts') + .select('ledger_account') + .eq('id', cashAccountId) + .eq('company_id', companyId) + .maybeSingle() + + if (error) { + // An EXPLICIT cash_account_id exists: it almost certainly resolves to a + // non-1930 account, so silently degrading to 1930 on a transient lookup + // failure risks the exact class of misbooking this helper exists to + // prevent, just triggered by infra flakiness instead of a stale setting. + // Fail the request instead: the caller can retry, whereas a wrongly + // booked verifikat needs a storno to correct (BFL 5 kap). + throw new BookkeepingDatabaseError('resolve_settlement_account', error.message) + } + + // A transaction with a cash_account_id that resolves to no row, or a row + // with no ledger_account, is a data-integrity gap (not a normal "no cash + // account linked" case): the fallback fires silently otherwise, masking a + // bad cash_accounts row behind a plausible-looking 1930 verifikat. + if (!data?.ledger_account) { + log.warn('settlement-account lookup returned no ledger_account; defaulting to 1930', { + cashAccountId, + }) + return FALLBACK_ACCOUNT + } + + return data.ledger_account as string +} diff --git a/supabase/migrations/20260712120000_document_last_supplier_payment_account_scope.sql b/supabase/migrations/20260712120000_document_last_supplier_payment_account_scope.sql new file mode 100644 index 00000000..1bd86de5 --- /dev/null +++ b/supabase/migrations/20260712120000_document_last_supplier_payment_account_scope.sql @@ -0,0 +1,13 @@ +-- Clarify scope after a real misbooking (see PR #985 / DECISIONS.md +-- 2026-07-11): this column only seeds the manual mark-paid "betald med +-- privata medel" dialog's default account picker. It must NEVER be read to +-- resolve the settlement account for a real matched bank transaction: that +-- comes from transactions.cash_account_id -> cash_accounts.ledger_account +-- (see lib/bookkeeping/settlement-account.ts). Reusing this sticky, +-- company-wide value for a real payment previously misbooked a genuine bank +-- payment to 2893 (skuld till aktieägare) once an unrelated private-funds +-- payment had set it. +COMMENT ON COLUMN company_settings.last_supplier_payment_account IS + 'Default account seed for the manual mark-paid "betald med privata medel" picker only. Never read to resolve the settlement account for a matched bank transaction (use cash_accounts.ledger_account via cash_account_id instead).'; + +NOTIFY pgrst, 'reload schema';