diff --git a/DECISIONS.md b/DECISIONS.md index b850c497..49af4979 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1333,3 +1333,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-28] Employee-save failure reported inline (role=alert in the dialog footer, carrying the requestId) in addition to the single destructive toast, and a missing PERSONNUMMER_ENCRYPTION_KEY typed as 503 PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED rather than INTERNAL_ERROR (#1996): the Radix modal aria-hides the root-layout Toaster while the dialog is open, so the toast is invisible to assistive tech and E2E drivers; TOAST_LIMIT is 1, so a second toast is not an option; and the missing key is a permanent configuration gap where "try again later" is wrong and "contact support" is right (same reasoning as CUSTOMER_PERSONAL_NUMBER_UNREADABLE and INVOICE_SEND_EMAIL_NOT_CONFIGURED). The shared postAction helper was not extended (it takes no body and exposes no requestId): keeping the change local to the dialog avoids widening a helper other panels rely on. [2026-08-28] /migrate SIE guard extended to every provider (Fortnox exemption removed) as "a completed SIE import must exist for the company", not "must be part of this run", plus a wizard hint that disables Start when SIE is unchecked and never imported; chose this over forcing the checkbox on because the route is the only seam a direct API call or a stale client cannot bypass, and "must exist" keeps entities-only re-runs after a full migration working (#2000). [2026-08-28] /migrate SIE guard skips company-info-only runs (all entity flags false) and the wizard derives "SIE already imported" from the preview OR this session's successful /import-sie results: company info writes no accounts, balances or subledger rows, so the BFL rationale does not apply; and the one-shot preview went stale after phase 1 succeeded and phase 2 failed, falsely blocking an entities-only retry (#2000 review). +[2026-08-28] Bankavstamning NULL-link fix scoped to transfer legs with contradicting sign (20260828220000): the naive rule (NULL counts only for the primary account) and the formula-only variant (drop far-leg-settled vouchers from unexplained) were both simulated against prod and rejected; the naive rule worsened 4 of 11 affected cards (worst -37 000 kr false alarm on single-leg vouchers with no user action available), the formula variant blew up healthy cards by up to 474 550 kr. The shipped three-condition rule changes 24 vouchers on 7 cards in 6 companies, all verified per-card. diff --git a/lib/reconciliation/bank-reconciliation.ts b/lib/reconciliation/bank-reconciliation.ts index bb09e15c..14f577e4 100644 --- a/lib/reconciliation/bank-reconciliation.ts +++ b/lib/reconciliation/bank-reconciliation.ts @@ -957,7 +957,12 @@ export async function getReconciliationStatus( // Account-scoped since 20260723160000: a voucher whose links all sit on another // cash account (a transfer's other leg) counts as unmatched HERE, keeping this // number in agreement with the "Omatchade verifikationer" table the - // reconciliation view derives from the same RPC. + // reconciliation view derives from the same RPC. Direction-aware since + // 20260828220000: on a NON-primary account, an own-account-transfer voucher + // whose only link is a NULL-cash_account transaction with a contradicting + // sign counts as unmatched here too (that row is the transfer's OTHER leg), + // so both legs of a transfer land in this list symmetrically instead of one + // leg polluting unexplained_difference. // effectiveFrom, NOT the caller's dateFrom: countedLines and countedTx are both // clamped to the IB floor above, and this list has to describe the SAME window // or the card contradicts itself. With the raw dateFrom, a window that opens @@ -1518,8 +1523,10 @@ export async function fetchJunctionLinkedTxIds( /** A match candidate that carries how many transactions already point at it. */ export interface GLLineForMatching extends UnlinkedGLLine { /** Transactions settling this entry ON THE REQUESTED ACCOUNT (plus legacy - * rows with no cash_account_id, which count everywhere). A transaction on - * another cash account, e.g. the outgoing leg of an own-account transfer, + * rows with no cash_account_id, which count everywhere EXCEPT on a + * non-primary account when the voucher is an own-account transfer and the + * row's sign contradicts this account's leg; 20260828220000). A transaction + * on another cash account, e.g. the outgoing leg of an own-account transfer, * does not mark the voucher as matched here (issue #1026). */ linked_transaction_count: number } diff --git a/supabase/migrations/20260828220000_gl_lines_direction_aware_null_links.sql b/supabase/migrations/20260828220000_gl_lines_direction_aware_null_links.sql new file mode 100644 index 00000000..fbb62170 --- /dev/null +++ b/supabase/migrations/20260828220000_gl_lines_direction_aware_null_links.sql @@ -0,0 +1,232 @@ +-- Direction-aware NULL-link settlement for get_account_gl_lines_for_matching. +-- +-- Since 20260723160000 a linked transaction whose cash_accounts row resolves to +-- ANOTHER ledger account does not settle the voucher for p_account_number, +-- while a transaction with no resolvable cash account (cash_account_id NULL, +-- the unbackfilled legacy shape) kept counting for EVERY account. For an +-- own-account transfer voucher that asymmetry produced a false "oforklarat" on +-- the bankavstamning card: the transfer's near leg counted as settled on the +-- far leg's account through a NULL row that provably belongs to the other side +-- (an outflow row cannot settle an inflow leg), while the far-leg voucher was +-- listed. Reported by a user whose momskonto card showed differens 0 kr with +-- oforklarat -2 593,75. +-- +-- New rule for a NULL-attributed link (pointer or junction), per voucher V and +-- requested account L. It counts as settling V for L unless ALL of these hold: +-- 1. the company has a primary cash account whose ledger_account differs +-- from L (so L is a NON-primary card; the primary card and companies +-- without a primary keep the legacy behavior, mirroring +-- scopeTransactionsToAccount / resolveCashAccountScope where only the +-- primary claims NULL bank rows), +-- 2. V has lines on >= 2 of the company's cash-account ledgers (an +-- own-account transfer; single-leg vouchers keep the legacy behavior so +-- unbackfilled rows that genuinely belong to L are never flagged), and +-- 3. the transaction's sign contradicts V's net line on L (sign(t.amount) +-- and sign(net) both non-zero and different: an outflow row cannot be the +-- settlement of an inflow leg). +-- Attributed links are unchanged in both directions. +-- +-- Measured on prod 2026-08-28 before shipping: the naive variant (NULL counts +-- only for the primary account) flipped 64 vouchers across 11 cards in 10 +-- companies and made 4 cards WORSE, including a -37 000 kr false alarm on +-- single-leg vouchers with no user action available. This three-condition rule +-- flips 24 vouchers across 7 cards in 6 companies; simulated per-card with the +-- exact status formula, 5 cards improve (3 to exactly 0,00) and the 2 that +-- move up do so because a real, user-fixable mislink (two bank rows linked to +-- one transfer voucher while its sibling holds none) stops being hidden. +-- +-- get_unlinked_gl_lines is deliberately untouched: it feeds the auto-matcher +-- with vouchers that have NO link at all, and every voucher affected here has +-- one. + +CREATE OR REPLACE FUNCTION public.get_account_gl_lines_for_matching( + p_company_id UUID, + p_account_number TEXT DEFAULT '1930', + p_date_from DATE DEFAULT NULL, + p_date_to DATE DEFAULT NULL, + p_include_matched BOOLEAN DEFAULT false +) +RETURNS TABLE ( + line_id UUID, + journal_entry_id UUID, + debit_amount NUMERIC, + credit_amount NUMERIC, + line_description TEXT, + entry_date DATE, + voucher_number INT, + voucher_series TEXT, + entry_description TEXT, + source_type TEXT, + linked_transaction_count INT +) +LANGUAGE sql +STABLE +SECURITY DEFINER +SET search_path = public +AS $$ + SELECT + jel.id AS line_id, + je.id AS journal_entry_id, + jel.debit_amount, + jel.credit_amount, + jel.line_description, + je.entry_date, + je.voucher_number, + je.voucher_series, + je.description AS entry_description, + je.source_type, + -- Account-scoped: a transaction provably on ANOTHER cash account (its + -- cash_accounts row resolves to a different ledger_account) does not make + -- this voucher "matched" for p_account_number. A NULL / unresolvable cash + -- account keeps counting for every account (conservative legacy behavior) + -- EXCEPT the one shape where it provably cannot be this account's leg: + -- non-primary card + own-account-transfer voucher + contradicting sign + -- (see v.* below). Junction-linked transactions count exactly like + -- pointer-linked ones. + ( + ( + SELECT count(*) + FROM public.transactions t + LEFT JOIN public.cash_accounts ca ON ca.id = t.cash_account_id + WHERE t.journal_entry_id = je.id + AND t.company_id = p_company_id + AND ( + ca.ledger_account = p_account_number + OR ( + ca.ledger_account IS NULL + AND ( + v.legacy_null_ok + OR v.single_bank_leg + OR sign(t.amount) = 0 + OR sign(v.account_net) = 0 + OR sign(t.amount) = sign(v.account_net) + ) + ) + ) + ) + ( + SELECT count(*) + FROM public.transaction_voucher_links l + JOIN public.transactions t ON t.id = l.transaction_id + LEFT JOIN public.cash_accounts ca ON ca.id = t.cash_account_id + WHERE l.journal_entry_id = je.id + AND l.company_id = p_company_id + AND t.journal_entry_id IS DISTINCT FROM je.id + AND ( + ca.ledger_account = p_account_number + OR ( + ca.ledger_account IS NULL + AND ( + v.legacy_null_ok + OR v.single_bank_leg + OR sign(t.amount) = 0 + OR sign(v.account_net) = 0 + OR sign(t.amount) = sign(v.account_net) + ) + ) + ) + ) + )::int AS linked_transaction_count + FROM public.journal_entry_lines jel + JOIN public.journal_entries je ON je.id = jel.journal_entry_id + CROSS JOIN LATERAL ( + SELECT + -- Legacy behavior applies when L is the primary card or the company has + -- no primary at all: only a primary elsewhere disqualifies NULL rows, + -- the same ownership rule the TS bank side applies to NULL rows. + NOT EXISTS ( + SELECT 1 + FROM public.cash_accounts pca + WHERE pca.company_id = p_company_id + AND pca.is_primary + AND pca.ledger_account <> p_account_number + ) AS legacy_null_ok, + -- A voucher touching < 2 of the company's cash-account ledgers is not an + -- own-account transfer; its NULL links keep settling every account. + ( + SELECT count(DISTINCT ca4.ledger_account) + FROM public.journal_entry_lines jel2 + JOIN public.cash_accounts ca4 + ON ca4.company_id = p_company_id + AND ca4.ledger_account = jel2.account_number + WHERE jel2.journal_entry_id = je.id + ) < 2 AS single_bank_leg, + -- The voucher's net movement on L, for the sign test. Summed over the + -- voucher's L-lines, matching the voucher-level settled/unsettled + -- semantics of the NOT EXISTS filter below. + ( + SELECT COALESCE(sum(jel3.debit_amount - jel3.credit_amount), 0) + FROM public.journal_entry_lines jel3 + WHERE jel3.journal_entry_id = je.id + AND jel3.account_number = p_account_number + ) AS account_net + ) v + WHERE jel.account_number = p_account_number + AND je.company_id = p_company_id + AND je.status = 'posted' + AND je.source_type IS DISTINCT FROM 'opening_balance' + AND je.source_type IS DISTINCT FROM 'storno' + AND je.source_type IS DISTINCT FROM 'correction' + AND (p_date_from IS NULL OR je.entry_date >= p_date_from) + AND (p_date_to IS NULL OR je.entry_date <= p_date_to) + AND ( + p_include_matched + OR ( + NOT EXISTS ( + SELECT 1 + FROM public.transactions t + LEFT JOIN public.cash_accounts ca ON ca.id = t.cash_account_id + WHERE t.journal_entry_id = je.id + AND t.company_id = p_company_id + AND ( + ca.ledger_account = p_account_number + OR ( + ca.ledger_account IS NULL + AND ( + v.legacy_null_ok + OR v.single_bank_leg + OR sign(t.amount) = 0 + OR sign(v.account_net) = 0 + OR sign(t.amount) = sign(v.account_net) + ) + ) + ) + ) + AND NOT EXISTS ( + SELECT 1 + FROM public.transaction_voucher_links l + JOIN public.transactions t ON t.id = l.transaction_id + LEFT JOIN public.cash_accounts ca ON ca.id = t.cash_account_id + WHERE l.journal_entry_id = je.id + AND l.company_id = p_company_id + AND ( + ca.ledger_account = p_account_number + OR ( + ca.ledger_account IS NULL + AND ( + v.legacy_null_ok + OR v.single_bank_leg + OR sign(t.amount) = 0 + OR sign(v.account_net) = 0 + OR sign(t.amount) = sign(v.account_net) + ) + ) + ) + ) + ) + ) + -- Tenant guard: anon/authenticated may only read their own companies; + -- service_role and direct/superuser access (no JWT role) bypass. + AND ( + coalesce(nullif(current_setting('request.jwt.claims', true), '')::jsonb ->> 'role', '') + NOT IN ('anon', 'authenticated') + OR je.company_id IN (SELECT public.user_company_ids()) + ) + ORDER BY je.entry_date, je.voucher_number; +$$; + +-- CREATE OR REPLACE preserves the ACL; re-assert least privilege so this +-- migration stands alone on a fresh replay (20260611130000). +REVOKE EXECUTE ON FUNCTION public.get_account_gl_lines_for_matching(uuid, text, date, date, boolean) FROM PUBLIC, anon; +GRANT EXECUTE ON FUNCTION public.get_account_gl_lines_for_matching(uuid, text, date, date, boolean) TO authenticated, service_role; + +NOTIFY pgrst, 'reload schema'; diff --git a/tests/pg/get_account_gl_lines_for_matching.pg.test.ts b/tests/pg/get_account_gl_lines_for_matching.pg.test.ts index ae0a1ab1..10b39507 100644 --- a/tests/pg/get_account_gl_lines_for_matching.pg.test.ts +++ b/tests/pg/get_account_gl_lines_for_matching.pg.test.ts @@ -12,7 +12,9 @@ * account: a transaction provably on ANOTHER cash account does not mark the * voucher as matched for p_account_number. This surfaces the unsettled second * leg of an own-account transfer by default (issue #1026) while transactions - * with no resolvable cash account keep counting for every account. + * with no resolvable cash account keep counting for every account, EXCEPT + * (20260828220000) on a non-primary account when the voucher is an own-account + * transfer and the NULL row's sign contradicts that account's leg. * (The companion mark_entry_as_opening_balance guard from the same migration * is covered in mark-entry-as-opening-balance.pg.test.ts.) */ @@ -234,7 +236,7 @@ describe('get_account_gl_lines_for_matching RPC: account-scoped link count (#102 expect(withMatched.find((r) => r.journal_entry_id === salaryEntry).linked_transaction_count).toBe(1) }) - it('treats transactions without a resolvable cash account as settling every account', async () => { + it('treats transactions without a resolvable cash account as settling every account (no primary elsewhere)', async () => { const userId = await insertAuthUser() const companyId = await insertCompany({ createdBy: userId }) const fiscalPeriodId = await insertFiscalPeriod({ @@ -269,3 +271,213 @@ describe('get_account_gl_lines_for_matching RPC: account-scoped link count (#102 expect(withMatched.find((r) => r.journal_entry_id === legacyEntry).linked_transaction_count).toBe(1) }) }) + +describe('get_account_gl_lines_for_matching RPC: direction-aware NULL links (20260828220000)', () => { + /** 1930 primary + 1931 + 1940, the three-account shape from the field report. */ + async function seedThreeAccounts(withPrimary = true) { + const userId = await insertAuthUser() + const companyId = await insertCompany({ createdBy: userId }) + const fiscalPeriodId = await insertFiscalPeriod({ + userId, companyId, periodStart: '2026-01-01', periodEnd: '2026-12-31', + }) + await insertCashAccount({ companyId, ledgerAccount: '1930', isPrimary: withPrimary }) + await insertCashAccount({ companyId, ledgerAccount: '1931' }) + await insertCashAccount({ companyId, ledgerAccount: '1940' }) + return { userId, companyId, fiscalPeriodId } + } + + /** Transfer voucher moving amount from 1930 into 1931 (debit 1931 / credit 1930). */ + async function insertTransferInto1931(params: { + userId: string + companyId: string + fiscalPeriodId: string + amount: number + }): Promise { + return insertPostedJournalEntry({ + userId: params.userId, + companyId: params.companyId, + fiscalPeriodId: params.fiscalPeriodId, + entryDate: '2026-01-27', + sourceType: 'import', + voucherNumber: 1, + lines: [ + { account: '1931', debit: params.amount, credit: 0 }, + { account: '1930', debit: 0, credit: params.amount }, + ], + }) + } + + it('flags the transfer leg a sign-contradicting NULL row cannot settle (non-primary account)', async () => { + const { userId, companyId, fiscalPeriodId } = await seedThreeAccounts() + + // The field-report shape: money moved 1930 -> 1931; the voucher's only link + // is the 1930-side CSV row (cash_account_id NULL, amount negative). That + // outflow row cannot be the settlement of the +2593.75 leg on 1931. + const transfer = await insertTransferInto1931({ userId, companyId, fiscalPeriodId, amount: 2593.75 }) + await insertTransaction({ + companyId, userId, amount: -2593.75, date: '2026-01-27', + journalEntryId: transfer, cashAccountId: null, + }) + + // On 1931 (non-primary) the voucher must surface as unmatched, so the + // status card lists BOTH transfer legs and unexplained_difference nets to 0. + const { rows: on1931 } = await getPool().query( + `SELECT journal_entry_id, linked_transaction_count + FROM public.get_account_gl_lines_for_matching(p_company_id => $1, p_account_number => '1931')`, + [companyId], + ) + const row1931 = on1931.find((r) => r.journal_entry_id === transfer) + expect(row1931).toBeDefined() + expect(row1931.linked_transaction_count).toBe(0) + + // On 1930 (the primary card) the NULL row keeps counting: not listed. + const { rows: on1930 } = await getPool().query( + `SELECT journal_entry_id + FROM public.get_account_gl_lines_for_matching(p_company_id => $1, p_account_number => '1930')`, + [companyId], + ) + expect(on1930.find((r) => r.journal_entry_id === transfer)).toBeUndefined() + }) + + it('keeps a sign-contradicting NULL row settling the PRIMARY card (legacy_null_ok)', async () => { + const { userId, companyId, fiscalPeriodId } = await seedThreeAccounts() + + // Reverse transfer: money moves 1931 -> 1930 (debit 1930 / credit 1931), + // linked only to the 1931-side outflow row (NULL, negative). The sign + // contradicts 1930's +net, so ONLY the primary-account exemption + // (condition 1) keeps the voucher settled on 1930. Without this case the + // primary assertion above also passes via sign match, leaving the + // exemption untested. + const reverse = await insertPostedJournalEntry({ + userId, companyId, fiscalPeriodId, + entryDate: '2026-01-28', sourceType: 'import', voucherNumber: 3, + lines: [ + { account: '1930', debit: 2593.75, credit: 0 }, + { account: '1931', debit: 0, credit: 2593.75 }, + ], + }) + await insertTransaction({ + companyId, userId, amount: -2593.75, date: '2026-01-28', + journalEntryId: reverse, cashAccountId: null, + }) + + const { rows: on1930 } = await getPool().query( + `SELECT journal_entry_id + FROM public.get_account_gl_lines_for_matching(p_company_id => $1, p_account_number => '1930')`, + [companyId], + ) + expect(on1930.find((r) => r.journal_entry_id === reverse)).toBeUndefined() + + // On 1931 (non-primary) the same row's sign MATCHES the -net leg: settled. + const { rows: on1931 } = await getPool().query( + `SELECT journal_entry_id + FROM public.get_account_gl_lines_for_matching(p_company_id => $1, p_account_number => '1931')`, + [companyId], + ) + expect(on1931.find((r) => r.journal_entry_id === reverse)).toBeUndefined() + }) + + it('keeps a sign-compatible NULL row settling the transfer leg', async () => { + const { userId, companyId, fiscalPeriodId } = await seedThreeAccounts() + + // Same transfer, but the NULL row is an inflow: it plausibly IS the 1931 + // leg, so the voucher stays settled there (conservative). + const transfer = await insertTransferInto1931({ userId, companyId, fiscalPeriodId, amount: 2593.75 }) + await insertTransaction({ + companyId, userId, amount: 2593.75, date: '2026-01-27', + journalEntryId: transfer, cashAccountId: null, + }) + + const { rows: on1931 } = await getPool().query( + `SELECT journal_entry_id + FROM public.get_account_gl_lines_for_matching(p_company_id => $1, p_account_number => '1931')`, + [companyId], + ) + expect(on1931.find((r) => r.journal_entry_id === transfer)).toBeUndefined() + }) + + it('never flags a single-bank-leg voucher over a NULL link, whatever the sign', async () => { + const { userId, companyId, fiscalPeriodId } = await seedThreeAccounts() + + // Unbackfilled legacy shape: an income voucher on non-primary 1931 whose + // NULL row genuinely belongs to 1931 but points the "wrong" way relative + // to nothing: only one bank leg exists, so the sign test must not run. + // Flagging these was the measured -37 000 kr false-alarm regression. + const income = await insertPostedJournalEntry({ + userId, companyId, fiscalPeriodId, + entryDate: '2026-02-10', sourceType: 'import', voucherNumber: 2, + lines: [ + { account: '1931', debit: 0, credit: 1200 }, + { account: '5810', debit: 1200, credit: 0 }, + ], + }) + await insertTransaction({ + companyId, userId, amount: 1200, date: '2026-02-10', + journalEntryId: income, cashAccountId: null, + }) + + const { rows: on1931 } = await getPool().query( + `SELECT journal_entry_id + FROM public.get_account_gl_lines_for_matching(p_company_id => $1, p_account_number => '1931')`, + [companyId], + ) + expect(on1931.find((r) => r.journal_entry_id === income)).toBeUndefined() + }) + + it('keeps full legacy behavior when the company has no primary cash account', async () => { + const { userId, companyId, fiscalPeriodId } = await seedThreeAccounts(false) + + const transfer = await insertTransferInto1931({ userId, companyId, fiscalPeriodId, amount: 2593.75 }) + await insertTransaction({ + companyId, userId, amount: -2593.75, date: '2026-01-27', + journalEntryId: transfer, cashAccountId: null, + }) + + // No primary anywhere: NULL rows count for every account, exactly as before. + const { rows: on1931 } = await getPool().query( + `SELECT journal_entry_id + FROM public.get_account_gl_lines_for_matching(p_company_id => $1, p_account_number => '1931')`, + [companyId], + ) + expect(on1931.find((r) => r.journal_entry_id === transfer)).toBeUndefined() + }) + + it('applies the same sign test to junction links and to the matched link count', async () => { + const { userId, companyId, fiscalPeriodId } = await seedThreeAccounts() + + // Junction-anchored variant of the field-report shape: the NULL outflow row + // is linked through transaction_voucher_links instead of the pointer. + const transfer = await insertTransferInto1931({ userId, companyId, fiscalPeriodId, amount: 2593.75 }) + const txId = await insertTransaction({ + companyId, userId, amount: -2593.75, date: '2026-01-27', + journalEntryId: null, cashAccountId: null, + }) + await getPool().query( + `INSERT INTO public.transaction_voucher_links + (id, user_id, company_id, transaction_id, journal_entry_id, allocated_amount, role) + VALUES (gen_random_uuid(), $1, $2, $3, $4, $5, 'other')`, + [userId, companyId, txId, transfer, 2593.75], + ) + + const { rows: on1931 } = await getPool().query( + `SELECT journal_entry_id, linked_transaction_count + FROM public.get_account_gl_lines_for_matching( + p_company_id => $1, p_account_number => '1931', p_include_matched => true)`, + [companyId], + ) + const row = on1931.find((r) => r.journal_entry_id === transfer) + // Listed (include_matched or not) and the sign-contradicting junction link + // is excluded from the account's link count. + expect(row).toBeDefined() + expect(row.linked_transaction_count).toBe(0) + + // On the primary 1930 card the same junction link still counts. + const { rows: on1930 } = await getPool().query( + `SELECT journal_entry_id, linked_transaction_count + FROM public.get_account_gl_lines_for_matching( + p_company_id => $1, p_account_number => '1930', p_include_matched => true)`, + [companyId], + ) + expect(on1930.find((r) => r.journal_entry_id === transfer).linked_transaction_count).toBe(1) + }) +})