diff --git a/app/api/reconciliation/bank/run/route.ts b/app/api/reconciliation/bank/run/route.ts index 6af4266e..32a85b45 100644 --- a/app/api/reconciliation/bank/run/route.ts +++ b/app/api/reconciliation/bank/run/route.ts @@ -36,7 +36,7 @@ export async function POST(request: Request) { // endpoint, which is likewise lenient for '1930'. const { data: cashAccount } = await supabase .from('cash_accounts') - .select('id, currency') + .select('id, currency, is_primary') .eq('company_id', companyId) .eq('ledger_account', accountNumber) .maybeSingle() @@ -55,6 +55,9 @@ export async function POST(request: Request) { accountNumber, currency, cashAccountId: cashAccount?.id as string | undefined, + // Only the primary account claims unassigned (NULL cash_account_id) rows — + // a secondary same-currency account must scope strictly to its own id. + includeUnassigned: Boolean(cashAccount?.is_primary), dryRun: dry_run ?? false, }) diff --git a/app/api/reconciliation/bank/status/route.ts b/app/api/reconciliation/bank/status/route.ts index a485d90c..5be29ad7 100644 --- a/app/api/reconciliation/bank/status/route.ts +++ b/app/api/reconciliation/bank/status/route.ts @@ -23,7 +23,7 @@ export async function GET(request: Request) { // produces nonsense. const { data: cashAccount } = await supabase .from('cash_accounts') - .select('id, currency') + .select('id, currency, is_primary') .eq('company_id', companyId) .eq('ledger_account', accountNumber) .maybeSingle() @@ -37,6 +37,10 @@ export async function GET(request: Request) { const currency = (cashAccount?.currency as string | undefined) ?? 'SEK' const cashAccountId = cashAccount?.id as string | undefined + // Only the primary account claims unassigned (NULL cash_account_id) rows. + // A secondary same-currency account (e.g. a 1931 savings account) must not, or + // 1930's unassigned rows inflate its bank total and show a bogus difference. + const includeUnassigned = Boolean(cashAccount?.is_primary) const status = await getReconciliationStatus( supabase, @@ -46,6 +50,7 @@ export async function GET(request: Request) { accountNumber, currency, cashAccountId, + includeUnassigned, ) return NextResponse.json({ data: status }) diff --git a/app/api/reconciliation/bank/unmatched-entries/route.ts b/app/api/reconciliation/bank/unmatched-entries/route.ts index 7074a8df..14f4e7ce 100644 --- a/app/api/reconciliation/bank/unmatched-entries/route.ts +++ b/app/api/reconciliation/bank/unmatched-entries/route.ts @@ -1,6 +1,6 @@ import { createClient } from '@/lib/supabase/server' import { NextResponse } from 'next/server' -import { fetchUnlinkedGLLines, tryReconcileTransaction } from '@/lib/reconciliation/bank-reconciliation' +import { fetchGLLinesForMatching, tryReconcileTransaction } from '@/lib/reconciliation/bank-reconciliation' import { requireCompanyId } from '@/lib/company/context' import type { Transaction } from '@/types' @@ -24,6 +24,11 @@ export async function GET(request: Request) { // lib/reconciliation/bank-reconciliation pulls in server-only deps (event // bus, match-log) and must never reach the client bundle. const transactionId = searchParams.get('transaction_id') || undefined + // When true, also return vouchers already matched to a bank transaction (each + // carries linked_transaction_count) so the user can attach a second/third + // transaction to the same verifikat — the N:1 "lägga på flera" case. Default + // false keeps the list to unmatched candidates only. + const includeMatched = searchParams.get('include_matched') === 'true' // Defense-in-depth: only allow account numbers that the company has actually // registered as a cash account. Without this, a curious caller could probe @@ -45,7 +50,7 @@ export async function GET(request: Request) { ) } - const lines = await fetchUnlinkedGLLines(supabase, companyId, accountNumber, dateFrom, dateTo) + const lines = await fetchGLLinesForMatching(supabase, companyId, accountNumber, dateFrom, dateTo, includeMatched) if (transactionId) { // company-scoped fetch (defense-in-depth). A malformed/foreign id yields no diff --git a/app/api/transactions/route.ts b/app/api/transactions/route.ts index 810dcd8f..858ae2e2 100644 --- a/app/api/transactions/route.ts +++ b/app/api/transactions/route.ts @@ -41,15 +41,20 @@ export async function GET(request: Request) { let derivedCurrency = currency let cashAccountId: string | undefined + // Only the primary account claims unassigned (NULL cash_account_id) rows, so + // a secondary same-currency account's lists match its status card instead of + // pooling the primary's unassigned rows. See scopeTransactionsToAccount. + let includeUnassigned = true if (accountNumberParam) { const { data: cashAccount } = await supabase .from('cash_accounts') - .select('id, currency') + .select('id, currency, is_primary') .eq('company_id', companyId) .eq('ledger_account', accountNumberParam) .maybeSingle() if (cashAccount) { cashAccountId = cashAccount.id as string + includeUnassigned = Boolean(cashAccount.is_primary) if (!derivedCurrency && cashAccount.currency) derivedCurrency = cashAccount.currency as string } } @@ -79,7 +84,7 @@ export async function GET(request: Request) { // Shares one implementation with the reconciliation lib so the filter shape // can't drift between the status card and these lists. if (cashAccountId || derivedCurrency) { - query = scopeTransactionsToAccount(query, cashAccountId, derivedCurrency ?? 'SEK') + query = scopeTransactionsToAccount(query, cashAccountId, derivedCurrency ?? 'SEK', includeUnassigned) } if (dateFrom) query = query.gte('date', dateFrom) if (dateTo) query = query.lte('date', dateTo) diff --git a/app/api/v1/companies/[companyId]/reconciliation/bank/run/route.ts b/app/api/v1/companies/[companyId]/reconciliation/bank/run/route.ts index 29ebe3be..ee41b3e1 100644 --- a/app/api/v1/companies/[companyId]/reconciliation/bank/run/route.ts +++ b/app/api/v1/companies/[companyId]/reconciliation/bank/run/route.ts @@ -123,7 +123,7 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>( const accountNumber = body.account_number ?? '1930' const { data: cashAccount } = await ctx.supabase .from('cash_accounts') - .select('id, currency') + .select('id, currency, is_primary') .eq('company_id', ctx.companyId!) .eq('ledger_account', accountNumber) .maybeSingle() @@ -144,6 +144,8 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>( accountNumber, currency: (cashAccount?.currency as string | undefined) ?? 'SEK', cashAccountId: cashAccount?.id as string | undefined, + // Only the primary account claims unassigned (NULL cash_account_id) rows. + includeUnassigned: Boolean(cashAccount?.is_primary), dryRun: ctx.dryRun, }) } catch (err) { diff --git a/app/api/v1/companies/[companyId]/reconciliation/bank/status/route.ts b/app/api/v1/companies/[companyId]/reconciliation/bank/status/route.ts index 5f834152..0a1a9aaf 100644 --- a/app/api/v1/companies/[companyId]/reconciliation/bank/status/route.ts +++ b/app/api/v1/companies/[companyId]/reconciliation/bank/status/route.ts @@ -89,7 +89,7 @@ export const GET = withApiV1<{ params: Promise<{ companyId: string }> }>( const accountNumber = parsed.data.account_number ?? '1930' const { data: cashAccount } = await ctx.supabase .from('cash_accounts') - .select('id, currency') + .select('id, currency, is_primary') .eq('company_id', ctx.companyId!) .eq('ledger_account', accountNumber) .maybeSingle() @@ -111,6 +111,8 @@ export const GET = withApiV1<{ params: Promise<{ companyId: string }> }>( accountNumber, (cashAccount?.currency as string | undefined) ?? 'SEK', cashAccount?.id as string | undefined, + // Only the primary account claims unassigned (NULL cash_account_id) rows. + Boolean(cashAccount?.is_primary), ) return ok(status, { requestId: ctx.requestId }) } catch (err) { diff --git a/components/reconciliation/MatchVerifikationPicker.tsx b/components/reconciliation/MatchVerifikationPicker.tsx index 672f40e8..ad2f6f7f 100644 --- a/components/reconciliation/MatchVerifikationPicker.tsx +++ b/components/reconciliation/MatchVerifikationPicker.tsx @@ -3,6 +3,7 @@ import { useState, useEffect, useMemo, useRef } from 'react' import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' +import { Badge } from '@/components/ui/badge' import { Search, X } from 'lucide-react' import { formatCurrency, formatDate } from '@/lib/utils' import { formatVoucher } from '@/lib/bookkeeping/voucher-series-resolver' @@ -29,6 +30,10 @@ export interface UnlinkedGLLine { entry_description: string source_type: string confidence?: number + /** How many bank transactions already point at this entry. > 0 means the + * voucher is already matched — surfaced (behind the "visa matchade" opt-in) + * so a second/third transaction can be attached to it (N:1). */ + linked_transaction_count?: number } interface MatchPickerProps { @@ -37,6 +42,15 @@ interface MatchPickerProps { onChange: (journalEntryId: string) => void disabled?: boolean placeholder?: string + /** + * Render the candidate list in normal document flow (always visible below the + * search box) instead of as an absolutely-positioned overlay. Use inside a + * Dialog or any `overflow-y-auto` container: an absolute dropdown is clipped at + * the container's edge (the "klipper i dialogerna" bug — the list got cut off + * and the dialog couldn't scroll to it). The reconciliation view keeps the + * compact overlay (one picker per transaction row); the modal uses inline. + */ + inline?: boolean } /** @@ -56,19 +70,25 @@ export function MatchVerifikationPicker({ onChange, disabled, placeholder = 'Sök ver.nr, datum, belopp eller beskrivning…', + inline = false, }: MatchPickerProps) { + // `open` controls the overlay dropdown only. In inline mode the list is always + // rendered, so the setOpen() writes in the handlers below are harmless no-ops + // there (the inline branch never reads `open`). const [open, setOpen] = useState(false) const [search, setSearch] = useState('') const containerRef = useRef(null) useEffect(() => { - if (!open) return + // Inline mode shows the list permanently, so there's nothing to close on an + // outside click — the overlay-only dismissal handler would be dead weight. + if (inline || !open) return function onDocMouseDown(e: MouseEvent) { if (!containerRef.current?.contains(e.target as Node)) setOpen(false) } document.addEventListener('mousedown', onDocMouseDown) return () => document.removeEventListener('mousedown', onDocMouseDown) - }, [open]) + }, [open, inline]) const selected = glLines.find((l) => l.journal_entry_id === value) || null @@ -97,11 +117,16 @@ export function MatchVerifikationPicker({ {formatDate(selected.entry_date)} {formatCurrency(amount)} {selected.entry_description} + {(selected.linked_transaction_count ?? 0) > 0 && ( + + Redan matchad + + )} + ) + })} + {glLines.length > filtered.length && ( +
+ Visar {filtered.length} av {glLines.length} — sök för att filtrera fler. +
+ )} + + ) + + const searchBox = ( +
+ + { + setSearch(e.target.value) + setOpen(true) + }} + onFocus={() => setOpen(true)} + placeholder={placeholder} + disabled={disabled} + className="pl-9" + /> +
+ ) + + // Inline: list lives in normal flow so it can never be clipped by a scroll + // container (Dialog). The enclosing modal scrolls if the whole thing is tall. + if (inline) { + return ( +
+ {searchBox} +
+ {listContent} +
+
+ ) + } + + // Overlay: compact, opens on focus, dismisses on outside click. Right for the + // reconciliation view's one-picker-per-row layout. return (
-
- - { - setSearch(e.target.value) - setOpen(true) - }} - onFocus={() => setOpen(true)} - placeholder={placeholder} - disabled={disabled} - className="pl-9" - /> -
+ {searchBox} {open && (
- {filtered.length === 0 ? ( -
- Inga verifikationer matchar "{search}" -
- ) : ( -
- {filtered.map((line) => { - const amount = line.debit_amount > 0 ? line.debit_amount : -line.credit_amount - return ( - - ) - })} - {glLines.length > filtered.length && ( -
- Visar {filtered.length} av {glLines.length} — sök för att filtrera fler. -
- )} -
- )} + {listContent}
)}
diff --git a/components/reports/BankReconciliationView.tsx b/components/reports/BankReconciliationView.tsx index a5dbd4ad..64914015 100644 --- a/components/reports/BankReconciliationView.tsx +++ b/components/reports/BankReconciliationView.tsx @@ -5,6 +5,7 @@ import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card' import { Button } from '@/components/ui/button' import { Label } from '@/components/ui/label' import { Badge } from '@/components/ui/badge' +import { Switch } from '@/components/ui/switch' import { AccountNumber } from '@/components/ui/account-number' import { AlertCircle, ChevronDown, ChevronRight, Link2, Unlink, Play, Eye, EyeOff, PiggyBank, MoreHorizontal } from 'lucide-react' import { formatCurrency, formatDate } from '@/lib/utils' @@ -154,6 +155,13 @@ export function BankReconciliationView() { const [unlinkLoading, setUnlinkLoading] = useState(null) const [actionLoading, setActionLoading] = useState(null) + // Opt-in: also surface vouchers already matched to a bank transaction as + // candidates, so a second/third transaction can be attached to the same + // verifikat (N:1 — e.g. a salary run paid out in several transfers). Only + // affects the per-row picker candidates; the "Omatchade verifikationer" table + // below stays unmatched-only (it lists vouchers that still need a transaction). + const [includeMatched, setIncludeMatched] = useState(false) + const [showMatched, setShowMatched] = useState(false) // Default expanded so users discover the undo path. The card itself only // renders when ignoredTx.length > 0 — collapsing it by default hid the @@ -178,6 +186,12 @@ export function BankReconciliationView() { const accountCurrency = cashAccounts.find((a) => a.ledger_account === accountNumber)?.currency ?? 'SEK' + // glLines feeds the per-row picker (which may include already-matched vouchers + // when includeMatched is on). The "Omatchade verifikationer" table below must + // stay unmatched-only — a voucher with a linked transaction isn't something + // that still needs one. + const unmatchedGlLines = glLines.filter((l) => !(l.linked_transaction_count ?? 0)) + useEffect(() => { let cancelled = false fetch('/api/cash-accounts') @@ -213,6 +227,13 @@ export function BankReconciliationView() { params.set('account_number', accountNumber) const qs = `?${params}` + // The candidate-lines fetch optionally includes already-matched vouchers + // (for N:1); the status endpoint must NOT — its movement/diff is computed + // independently — so it keeps the plain qs. + const glParams = new URLSearchParams(params) + if (includeMatched) glParams.set('include_matched', 'true') + const glQs = `?${glParams}` + const txParams = new URLSearchParams() txParams.set('currency', accountCurrency) txParams.set('account_number', accountNumber) @@ -223,7 +244,7 @@ export function BankReconciliationView() { const [statusRes, glRes, unmatchedRes, matchedRes] = await Promise.all([ fetch(`/api/reconciliation/bank/status${qs}`, { signal }), - fetch(`/api/reconciliation/bank/unmatched-entries${qs}`, { signal }), + fetch(`/api/reconciliation/bank/unmatched-entries${glQs}`, { signal }), fetch(`/api/transactions${unmatchedQs}`, { signal }), fetch(`/api/transactions${reconciledQs}`, { signal }), ]) @@ -269,9 +290,10 @@ export function BankReconciliationView() { if (!signal.aborted) setLoading(false) } // Deliberately excludes dateFrom/dateTo: editing a date must NOT auto-fetch - // (it read from refs above). Re-runs only on account / currency change and - // mount; the "Filtrera" button calls fetchAll() explicitly for date changes. - }, [accountNumber, accountCurrency]) + // (it read from refs above). Re-runs on account / currency change, on the + // matched-toggle flip (which changes the candidate set), and on mount; the + // "Filtrera" button calls fetchAll() explicitly for date changes. + }, [accountNumber, accountCurrency, includeMatched]) useEffect(() => { fetchAll() @@ -682,15 +704,25 @@ export function BankReconciliationView() { {/* Unmatched Transactions */} {unmatchedTx.length > 0 && (
-
+

Omatchade transaktioner ({unmatchedTx.length})

- {glLines.length > 0 && ( -

- {glLines.length} verifikation{glLines.length === 1 ? '' : 'er'} att matcha mot -

- )} +
+ {unmatchedGlLines.length > 0 && ( +

+ {unmatchedGlLines.length} verifikation{unmatchedGlLines.length === 1 ? '' : 'er'} att matcha mot +

+ )} + +
{unmatchedTruncated && (

@@ -843,11 +875,11 @@ export function BankReconciliationView() { )} {/* Unmatched GL Lines */} - {glLines.length > 0 && ( + {unmatchedGlLines.length > 0 && ( - Omatchade verifikationer på ({glLines.length}) + Omatchade verifikationer på ({unmatchedGlLines.length}) @@ -862,7 +894,7 @@ export function BankReconciliationView() { - {glLines.map((line) => { + {unmatchedGlLines.map((line) => { const amount = line.debit_amount > 0 ? line.debit_amount : -line.credit_amount return ( diff --git a/components/transactions/MatchVoucherDialog.tsx b/components/transactions/MatchVoucherDialog.tsx index 16f2f100..6a66f166 100644 --- a/components/transactions/MatchVoucherDialog.tsx +++ b/components/transactions/MatchVoucherDialog.tsx @@ -11,6 +11,7 @@ import { } from '@/components/ui/dialog' import { Button } from '@/components/ui/button' import { Badge } from '@/components/ui/badge' +import { Switch } from '@/components/ui/switch' import { MatchVerifikationPicker, type UnlinkedGLLine, @@ -74,9 +75,13 @@ export function MatchVoucherDialog({ const [loading, setLoading] = useState(false) const [submitting, setSubmitting] = useState(false) const [wideRange, setWideRange] = useState(false) + // Opt-in: also surface vouchers already matched to another bank transaction, + // so several transactions can settle one verifikat (N:1 — a salary run paid in + // multiple transfers, an invoice paid in instalments). + const [includeMatched, setIncludeMatched] = useState(false) const loadCandidates = useCallback( - async (tx: TransactionWithInvoice, wide: boolean) => { + async (tx: TransactionWithInvoice, wide: boolean, matched: boolean) => { setLoading(true) try { // Resolve the settlement account from the company's cash accounts. @@ -98,6 +103,7 @@ export function MatchVoucherDialog({ const params = new URLSearchParams() params.set('account_number', account) params.set('transaction_id', tx.id) + if (matched) params.set('include_matched', 'true') if (!wide) { params.set('date_from', shiftDate(tx.date, -WINDOW_DAYS)) params.set('date_to', shiftDate(tx.date, WINDOW_DAYS)) @@ -112,9 +118,15 @@ export function MatchVoucherDialog({ // Auto-select a strong match only when nothing is chosen yet. Toggling // "Visa alla datum" reloads with a wider set — it must NOT discard a // voucher the user already picked. (selected resets to '' on close.) + // Never auto-select an already-matched voucher — N:1 must be a + // deliberate choice, not the default when "visa matchade" is on. const top = lines[0] setSelected((prev) => - prev ? prev : top && (top.confidence ?? 0) >= 0.85 ? top.journal_entry_id : '', + prev + ? prev + : top && (top.confidence ?? 0) >= 0.85 && !(top.linked_transaction_count ?? 0) + ? top.journal_entry_id + : '', ) } finally { setLoading(false) @@ -123,11 +135,12 @@ export function MatchVoucherDialog({ [], ) - // (Re)load whenever the dialog opens for a transaction, or the range widens. + // (Re)load whenever the dialog opens for a transaction, the range widens, or + // the user toggles already-matched vouchers in/out. useEffect(() => { if (!open || !transaction) return - void loadCandidates(transaction, wideRange) - }, [open, transaction, wideRange, loadCandidates]) + void loadCandidates(transaction, wideRange, includeMatched) + }, [open, transaction, wideRange, includeMatched, loadCandidates]) // Reset transient state when the dialog closes so the next open starts clean. useEffect(() => { @@ -135,6 +148,7 @@ export function MatchVoucherDialog({ setGlLines([]) setSelected('') setWideRange(false) + setIncludeMatched(false) setAccountFallback(false) }, [open]) @@ -223,35 +237,52 @@ export function MatchVoucherDialog({

) : glLines.length === 0 ? (
-

Inga omatchade verifikationer på {accountNumber} i perioden.

- {!wideRange && ( - - )} +

+ {includeMatched + ? `Inga verifikationer på ${accountNumber} i perioden.` + : `Inga omatchade verifikationer på ${accountNumber} i perioden.`} +

) : ( <> - {selectedLine && (selectedLine.confidence ?? 0) >= 0.85 && ( - Föreslagen träff - )} - - {!wideRange && ( - + {selectedLine && + (selectedLine.confidence ?? 0) >= 0.85 && + !(selectedLine.linked_transaction_count ?? 0) && ( + Föreslagen träff + )} + + {(selectedLine?.linked_transaction_count ?? 0) > 0 && ( +

+ Verifikationen är redan matchad mot {selectedLine?.linked_transaction_count}{' '} + transaktion{(selectedLine?.linked_transaction_count ?? 0) === 1 ? '' : 'er'}. + Kopplingen lägger till den här transaktionen också — t.ex. en lön utbetald i + flera överföringar. +

)} )} + + {/* Discovery affordances — widen the date window, and surface vouchers + already matched so another transaction can be attached (N:1). */} +
+ + {!wideRange && ( + + )} +
diff --git a/lib/reconciliation/__tests__/bank-reconciliation.test.ts b/lib/reconciliation/__tests__/bank-reconciliation.test.ts index 48fcc582..46f75b44 100644 --- a/lib/reconciliation/__tests__/bank-reconciliation.test.ts +++ b/lib/reconciliation/__tests__/bank-reconciliation.test.ts @@ -79,6 +79,25 @@ describe('scopeTransactionsToAccount', () => { expect(String(orCall?.args[0])).not.toContain('and(') }) + it('scopes strictly to the account (no NULL fallback) when includeUnassigned is false', () => { + const { self, calls } = makeQueryStub() + const id = '22222222-2222-2222-2222-222222222222' + + // includeUnassigned=false is the non-primary account case: a secondary + // same-currency account (e.g. a 1931 savings account) must NOT pull in the + // company's unassigned NULL rows — those belong to the primary account. + // Double-counting them inflated the secondary account's bank total and + // showed a large bogus difference ("1930 works, the other accounts go wonky"). + scopeTransactionsToAccount(self as never, id, 'SEK', false) + + expect(calls).toEqual([ + { method: 'eq', args: ['currency', 'SEK'] }, + { method: 'eq', args: ['cash_account_id', id] }, + ]) + // No OR — the IS NULL fallback must not appear for a non-primary account. + expect(calls.find((c) => c.method === 'or')).toBeUndefined() + }) + it('falls back to a pure currency filter when no cash account id is given', () => { const { self, calls } = makeQueryStub() @@ -550,8 +569,6 @@ describe('manualLink', () => { enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) // Line exists on the selected account enqueue({ data: [{ debit_amount: 1000, credit_amount: 0, account_number: '1930' }] }) - // No existing link - enqueue({ data: null, error: null }) // Update succeeds enqueue({ data: null, error: null }) @@ -574,8 +591,6 @@ describe('manualLink', () => { enqueue({ data: { ledger_account: '1930' } }) // Line exists on 1930 enqueue({ data: [{ debit_amount: 1000, credit_amount: 0, account_number: '1930' }] }) - // No existing link - enqueue({ data: null, error: null }) // Update succeeds enqueue({ data: null, error: null }) @@ -583,6 +598,26 @@ describe('manualLink', () => { expect(result.success).toBe(true) }) + + it('allows N:1 — does not reject when the verifikat already has a linked transaction', async () => { + const { supabase, enqueue } = createQueueMockSupabase() + // This transaction is itself unlinked; the TARGET entry already has another + // transaction pointing at it. manualLink no longer queries for / rejects + // that — several bank transactions may settle one verifikat (a salary run + // paid in multiple transfers). The only per-transaction guard is that THIS + // transaction isn't already linked (tx.journal_entry_id), still enforced. + const tx = makeTransaction({ id: 'tx-2', journal_entry_id: null }) + + enqueue({ data: tx }) + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) + enqueue({ data: [{ debit_amount: 1000, credit_amount: 0, account_number: '1930' }] }) + // Update succeeds — note there is NO existing-link lookup in the sequence. + enqueue({ data: null, error: null }) + + const result = await manualLink(supabase as never, 'company-1', 'tx-2', 'je-1', 'user-1', '1930') + + expect(result.success).toBe(true) + }) }) // ============================================================ diff --git a/lib/reconciliation/bank-reconciliation.ts b/lib/reconciliation/bank-reconciliation.ts index eb0040f4..c1bea221 100644 --- a/lib/reconciliation/bank-reconciliation.ts +++ b/lib/reconciliation/bank-reconciliation.ts @@ -19,6 +19,10 @@ export interface UnlinkedGLLine { voucher_series: string entry_description: string source_type: string + /** How many bank transactions already point at this entry. Present only on + * rows from get_account_gl_lines_for_matching (the N:1 candidate fetch); + * undefined on the unmatched-only path, where it is always implicitly 0. */ + linked_transaction_count?: number } export interface ReconciliationMatch { @@ -90,31 +94,46 @@ export interface ReconciliationOptions { * currency-only behaviour. */ cashAccountId?: string + /** + * Whether this account claims rows with a NULL cash_account_id (legacy / + * unassigned). Only the company's primary cash account should — see + * scopeTransactionsToAccount. Defaults to true for back-compat with the + * currency-only callers (where cashAccountId is omitted and this is moot). + */ + includeUnassigned?: boolean } /** * Scope a transactions query builder to a single cash account, tolerating - * legacy rows that predate the cash_account_id backfill. A bound row shows only - * on its own account; an unbound (NULL) row falls back to currency so nothing - * disappears mid-backfill. When cashAccountId is omitted we keep the pure - * currency filter (back-compat). + * legacy rows that predate the cash_account_id backfill. * - * The applied filter is: - * currency = cur AND (cash_account_id = X OR cash_account_id IS NULL) + * The applied filter is one of: + * includeUnassigned=true: currency = cur AND (cash_account_id = X OR cash_account_id IS NULL) + * includeUnassigned=false: currency = cur AND cash_account_id = X + * no cashAccountId: currency = cur (legacy currency-only path) * - * Earlier this used a single nested `or(cash_account_id.eq.X,and(cash_account_id.is.null,currency.eq.cur))`. - * That nested `and()` form is fragile — it silently returned ZERO rows for - * companies whose transactions were NULL/mis-assigned mid-backfill (issue: bank - * transactions vanished from Bankavstämning while the 1930 GL movement still - * showed). A cash account has exactly one currency (the `cash_accounts` - * (company_id, ledger_account) uniqueness assumption), so constraining the bound - * branch to that currency too loses nothing and lets us use the flat, reliable - * two-term `or` instead. + * Why `includeUnassigned` exists: a NULL cash_account_id row belongs to exactly + * ONE account, but the query can't tell which — these are unbooked rows in + * companies with ≥2 same-currency accounts (the backfill refuses to guess + * between checking + savings) and booked own-account transfers the backfill + * deliberately skips (>1 bank-class line). Attributing them to EVERY + * same-currency account double-counts them: a 1931 savings account would pull in + * 1930's unassigned rows, so Bankavstämning reported a large bogus difference + * for 1931 while 1930 itself still reconciled. The fix: only the company's + * PRIMARY cash account (cash_accounts.is_primary — exactly one per company) + * claims NULL rows; every other account scopes strictly to its own id. Callers + * pass `includeUnassigned = `. When cashAccountId is + * omitted (single-account companies with no row, the '1930' fallback) the pure + * currency filter is used and includeUnassigned is moot. + * + * The earlier nested `or(cash_account_id.eq.X,and(cash_account_id.is.null,currency.eq.cur))` + * form is intentionally avoided — it silently returned ZERO rows mid-backfill. + * A cash account has exactly one currency, so the flat two-term `or` is reliable. */ export function scopeTransactionsToAccount(query: Q, cashAccountId: string | undefined, currency: string): Q { +}>(query: Q, cashAccountId: string | undefined, currency: string, includeUnassigned = true): Q { // Both values are interpolated into a raw PostgREST filter string below. They // are DB-derived in every caller (cash_accounts.id / .currency, or the 'SEK' // default), never raw user input — but assert their shape anyway so a future @@ -126,9 +145,13 @@ export function scopeTransactionsToAccount { // Get all transactions in range, scoped to the selected cash account. Ignored // rows are pulled too so the totals card still reflects what the bank @@ -331,7 +356,7 @@ export async function getReconciliationStatus( .from('transactions') .select('amount, journal_entry_id, reconciliation_method, is_ignored') .eq('company_id', companyId) - txQuery = scopeTransactionsToAccount(txQuery, cashAccountId, currency) + txQuery = scopeTransactionsToAccount(txQuery, cashAccountId, currency, includeUnassigned) if (dateFrom) txQuery = txQuery.gte('date', dateFrom) if (dateTo) txQuery = txQuery.lte('date', dateTo) @@ -518,17 +543,16 @@ export async function manualLink( return { success: false, error: `Verifikationen saknar rad på ${accountNumber}` } } - // Check that no other transaction is already linked to this entry - const { data: existingLink } = await supabase - .from('transactions') - .select('id') - .eq('journal_entry_id', journalEntryId) - .eq('company_id', companyId) - .single() - - if (existingLink) { - return { success: false, error: 'En annan transaktion är redan kopplad till den här verifikationen.' } - } + // N:1 is intentionally allowed: several bank transactions may settle ONE + // verifikat (a salary run paid out in multiple transfers, a supplier invoice + // paid in instalments). The voucher's bank line is counted once in the period + // movement while each transaction sums on the bank side, so correctly-summing + // links net to zero and any mis-link surfaces as a non-zero difference on the + // status card — there's no need to forbid a second link here. (A given + // transaction still can't be double-linked: the tx.journal_entry_id guard + // above already blocks that.) The candidate list only surfaces an + // already-matched voucher when the user opts in via "Visa även matchade + // verifikationer", so this can't happen by accident. // Apply link const { error: updateError } = await supabase @@ -644,6 +668,44 @@ export async function fetchUnlinkedGLLines( return data as UnlinkedGLLine[] } +/** A match candidate that carries how many transactions already point at it. */ +export interface GLLineForMatching extends UnlinkedGLLine { + linked_transaction_count: number +} + +/** + * Fetch GL lines on a settlement account as match candidates. With + * `includeMatched=false` this is parity with fetchUnlinkedGLLines (unmatched + * only); with `includeMatched=true` it also returns already-matched vouchers, + * each carrying `linked_transaction_count`, so a second/third bank transaction + * can be attached to the same verifikat (N:1 — a salary run paid in several + * transfers, a supplier invoice paid in instalments). Server-only: like the rest + * of this module it must never reach the client bundle. + */ +export async function fetchGLLinesForMatching( + supabase: SupabaseClient, + companyId: string, + accountNumber: string = '1930', + dateFrom?: string, + dateTo?: string, + includeMatched: boolean = false, +): Promise { + const { data, error } = await supabase.rpc('get_account_gl_lines_for_matching', { + p_company_id: companyId, + p_account_number: accountNumber, + p_date_from: dateFrom || null, + p_date_to: dateTo || null, + p_include_matched: includeMatched, + }) + + if (error || !data) return [] + // count(*) can arrive as a bigint string over the wire — coerce defensively. + return (data as GLLineForMatching[]).map((line) => ({ + ...line, + linked_transaction_count: Number(line.linked_transaction_count) || 0, + })) +} + /** Get the net amount from a GL line (positive for debit, negative for credit) */ function getDirectionalAmount(line: UnlinkedGLLine): number { if (line.debit_amount > 0) return line.debit_amount diff --git a/supabase/migrations/20260610120000_gl_lines_for_matching.sql b/supabase/migrations/20260610120000_gl_lines_for_matching.sql new file mode 100644 index 00000000..329880b8 --- /dev/null +++ b/supabase/migrations/20260610120000_gl_lines_for_matching.sql @@ -0,0 +1,101 @@ +-- Migration: get_account_gl_lines_for_matching — GL lines on a settlement +-- account as MATCH CANDIDATES, optionally including already-matched ones. +-- +-- Why: get_unlinked_gl_lines (20260605130000) returns only lines with NO bank +-- transaction linked. That is correct for the reconciliation status count and +-- the default picker, but it makes a legitimate N:1 case impossible: a single +-- verifikat settled by SEVERAL bank transactions — e.g. a salary run booked as +-- one voucher (1930 credit = total net pay) but paid out as multiple transfers, +-- or a supplier invoice paid in instalments. Once the first transaction links, +-- the voucher vanishes from the candidate list and the user can't attach the +-- rest ("kan inte välja matchade verifikat → kan inte lägga på flera"). +-- +-- This RPC mirrors get_unlinked_gl_lines exactly (same posted-only filter, same +-- opening_balance / storno / correction exclusions, same date window) and adds: +-- * linked_transaction_count — how many transactions already point at the +-- entry, so the UI can mark a candidate "Redan matchad" and the user opts in +-- consciously. +-- * p_include_matched — when false (default) the result is IDENTICAL to +-- get_unlinked_gl_lines (count = 0 only); when true, already-matched lines +-- are included too. +-- +-- The aggregate reconciliation math stays correct under N:1: the GL line is +-- counted ONCE in the period movement regardless of how many transactions point +-- at it, while each transaction sums on the bank side — so linking transactions +-- whose amounts sum to the voucher's bank line nets to zero difference, and any +-- mis-link surfaces immediately as a non-zero difference on the status card. +-- +-- A separate function (not a parameter added to get_unlinked_gl_lines) so the +-- status-count path and its existing coverage are untouched, and so the extra +-- column never leaks into callers that don't expect it. + +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, + ( + SELECT count(*) + FROM public.transactions t + WHERE t.journal_entry_id = je.id + AND t.company_id = p_company_id + )::int AS linked_transaction_count + FROM public.journal_entry_lines jel + JOIN public.journal_entries je ON je.id = jel.journal_entry_id + WHERE jel.account_number = p_account_number + AND je.company_id = p_company_id + AND je.status = 'posted' + -- Same no-bank-counterpart exclusions as get_unlinked_gl_lines: IB, and the + -- book-only storno/correction vouchers can never be a bank-transaction target. + 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) + -- Default: only unmatched lines (parity with get_unlinked_gl_lines). When + -- p_include_matched is true, already-matched lines are returned too so a + -- second/third transaction can be attached to the same verifikat. + AND ( + p_include_matched + OR NOT EXISTS ( + SELECT 1 + FROM public.transactions t + WHERE t.journal_entry_id = je.id + AND t.company_id = p_company_id + ) + ) + ORDER BY je.entry_date, je.voucher_number; +$$; + +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 new file mode 100644 index 00000000..635fdb58 --- /dev/null +++ b/tests/pg/get_account_gl_lines_for_matching.pg.test.ts @@ -0,0 +1,134 @@ +/** + * pg-real test for get_account_gl_lines_for_matching + * (20260610120000_gl_lines_for_matching.sql). + * + * This RPC backs the N:1 "lägga på flera" feature: it mirrors get_unlinked_gl_lines + * but can ALSO surface already-matched vouchers (so a second/third bank + * transaction can be attached to one verifikat), each carrying how many + * transactions already point at it. + */ +import { describe, it, expect } from 'vitest' +import { randomUUID } from 'node:crypto' +import { getPool } from './setup' +import { insertAuthUser, insertCompany, insertFiscalPeriod, insertTransaction } from './fixtures' + +async function insertPostedJournalEntry(params: { + userId: string + companyId: string + fiscalPeriodId: string + entryDate: string + sourceType: 'opening_balance' | 'manual' | 'bank_transaction' | 'import' | 'storno' | 'correction' + voucherNumber: number + amount?: number +}): Promise { + const id = randomUUID() + const amount = params.amount ?? 1000 + await getPool().query( + `INSERT INTO public.journal_entries + (id, user_id, company_id, fiscal_period_id, voucher_number, voucher_series, + entry_date, description, source_type, status) + VALUES ($1, $2, $3, $4, $5, 'A', $6, $7, $8, 'posted')`, + [ + id, + params.userId, + params.companyId, + params.fiscalPeriodId, + params.voucherNumber, + params.entryDate, + `Test ${params.sourceType}`, + params.sourceType, + ], + ) + await getPool().query( + `INSERT INTO public.journal_entry_lines + (journal_entry_id, account_number, debit_amount, credit_amount) + VALUES ($1, '1930', $2, 0), + ($1, '2091', 0, $2)`, + [id, amount], + ) + return id +} + +describe('get_account_gl_lines_for_matching RPC — N:1 candidates', () => { + it('returns already-matched vouchers (with link count) only when p_include_matched is true', async () => { + const userId = await insertAuthUser() + const companyId = await insertCompany({ createdBy: userId }) + const fiscalPeriodId = await insertFiscalPeriod({ + userId, companyId, periodStart: '2026-01-01', periodEnd: '2026-12-31', + }) + + // One unmatched voucher, one voucher already settled by TWO transactions + // (the salary-run-paid-in-two-transfers shape). + const unmatchedEntry = await insertPostedJournalEntry({ + userId, companyId, fiscalPeriodId, + entryDate: '2026-03-15', sourceType: 'bank_transaction', voucherNumber: 1, amount: 1500, + }) + const matchedEntry = await insertPostedJournalEntry({ + userId, companyId, fiscalPeriodId, + entryDate: '2026-03-20', sourceType: 'manual', voucherNumber: 2, amount: 30000, + }) + await insertTransaction({ companyId, userId, currency: 'SEK', journalEntryId: matchedEntry }) + await insertTransaction({ companyId, userId, currency: 'SEK', journalEntryId: matchedEntry }) + + // Default (p_include_matched=false): parity with get_unlinked_gl_lines — only + // the unmatched voucher, count 0. + const { rows: unmatchedOnly } = await getPool().query( + `SELECT journal_entry_id, linked_transaction_count + FROM public.get_account_gl_lines_for_matching(p_company_id => $1)`, + [companyId], + ) + const unmatchedIds = new Set(unmatchedOnly.map((r) => r.journal_entry_id)) + expect(unmatchedIds.has(unmatchedEntry)).toBe(true) + expect(unmatchedIds.has(matchedEntry)).toBe(false) + expect(unmatchedOnly.find((r) => r.journal_entry_id === unmatchedEntry).linked_transaction_count).toBe(0) + + // p_include_matched=true: the matched voucher appears too, reporting both links. + const { rows: withMatched } = await getPool().query( + `SELECT journal_entry_id, linked_transaction_count + FROM public.get_account_gl_lines_for_matching(p_company_id => $1, p_include_matched => true)`, + [companyId], + ) + const byId = new Map(withMatched.map((r) => [r.journal_entry_id, r.linked_transaction_count])) + expect(byId.get(unmatchedEntry)).toBe(0) + expect(byId.get(matchedEntry)).toBe(2) + }) + + it('still excludes opening_balance / storno / correction even with p_include_matched', async () => { + const userId = await insertAuthUser() + const companyId = await insertCompany({ createdBy: userId }) + const fiscalPeriodId = await insertFiscalPeriod({ + userId, companyId, periodStart: '2026-01-01', periodEnd: '2026-12-31', + }) + + // These book-only / IB vouchers have no bank-feed counterpart and can never + // be a match target — the include_matched opt-in must not resurrect them. + await insertPostedJournalEntry({ + userId, companyId, fiscalPeriodId, + entryDate: '2026-01-01', sourceType: 'opening_balance', voucherNumber: 1, amount: 50000, + }) + await insertPostedJournalEntry({ + userId, companyId, fiscalPeriodId, + entryDate: '2026-05-02', sourceType: 'storno', voucherNumber: 2, amount: 25000, + }) + await insertPostedJournalEntry({ + userId, companyId, fiscalPeriodId, + entryDate: '2026-05-02', sourceType: 'correction', voucherNumber: 3, amount: 25000, + }) + const bankEntry = await insertPostedJournalEntry({ + userId, companyId, fiscalPeriodId, + entryDate: '2026-05-03', sourceType: 'bank_transaction', voucherNumber: 4, amount: 1500, + }) + + const { rows } = await getPool().query( + `SELECT journal_entry_id, source_type + FROM public.get_account_gl_lines_for_matching(p_company_id => $1, p_include_matched => true)`, + [companyId], + ) + + const returnedIds = new Set(rows.map((r) => r.journal_entry_id)) + expect(returnedIds.has(bankEntry)).toBe(true) + expect(rows.find((r) => r.source_type === 'opening_balance')).toBeUndefined() + expect(rows.find((r) => r.source_type === 'storno')).toBeUndefined() + expect(rows.find((r) => r.source_type === 'correction')).toBeUndefined() + }) +}) diff --git a/tests/pg/transactions-cash-account-id.pg.test.ts b/tests/pg/transactions-cash-account-id.pg.test.ts index 81319cca..0b97305f 100644 --- a/tests/pg/transactions-cash-account-id.pg.test.ts +++ b/tests/pg/transactions-cash-account-id.pg.test.ts @@ -207,7 +207,7 @@ describe('transactions.cash_account_id — backfill pass (c) single-account-of-c }) describe('transactions.cash_account_id — account-scoped query isolation', () => { - it('scopes to one account with a NULL→currency fallback, never leaking same-currency rows', async () => { + it('only the primary account claims NULL rows; a secondary same-currency account stays strict', async () => { const { userId, companyId } = await seedCompany() const ca1930 = await insertCashAccount({ companyId, ledgerAccount: '1930', currency: 'SEK' }) const ca1931 = await insertCashAccount({ companyId, ledgerAccount: '1931', currency: 'SEK' }) @@ -217,27 +217,41 @@ describe('transactions.cash_account_id — account-scoped query isolation', () = const txNullSek = await insertTransaction({ companyId, userId, currency: 'SEK' }) const txNullEur = await insertTransaction({ companyId, userId, currency: 'EUR' }) - // Mirror the runtime predicate: - // cash_account_id = X OR (cash_account_id IS NULL AND currency = cur) - const scoped = async (cashAccountId: string, currency: string): Promise => { + // Mirror the runtime predicate from scopeTransactionsToAccount(). The + // `includeUnassigned` flag is the account's cash_accounts.is_primary in the + // real code: ONLY the primary account claims unassigned (NULL) rows. + // primary: cash_account_id = X OR (cash_account_id IS NULL AND currency = cur) + // secondary: cash_account_id = X AND currency = cur + const scoped = async ( + cashAccountId: string, + currency: string, + includeUnassigned: boolean, + ): Promise => { + const predicate = includeUnassigned + ? `(cash_account_id = $2 OR (cash_account_id IS NULL AND currency = $3))` + : `(cash_account_id = $2 AND currency = $3)` const { rows } = await getPool().query( `SELECT id FROM public.transactions - WHERE company_id = $1 - AND (cash_account_id = $2 OR (cash_account_id IS NULL AND currency = $3))`, + WHERE company_id = $1 AND ${predicate}`, [companyId, cashAccountId, currency], ) return rows.map((r) => r.id) } - const for1930 = await scoped(ca1930, 'SEK') + // Primary (1930) claims the legacy NULL SEK row via the fallback. + const for1930 = await scoped(ca1930, 'SEK', true) expect(for1930).toContain(tx1930) expect(for1930).toContain(txNullSek) // legacy NULL row visible via fallback expect(for1930).not.toContain(tx1931) // the other account never leaks expect(for1930).not.toContain(txNullEur) // wrong-currency NULL excluded - const for1931 = await scoped(ca1931, 'SEK') + // Secondary (1931) is strict. Pulling in the NULL row would double-count + // what belongs to 1930 — the user-reported "1930 works but the other + // accounts go wonky" bug, where 1930's unassigned rows inflated 1931's bank + // total and produced a large bogus difference. + const for1931 = await scoped(ca1931, 'SEK', false) expect(for1931).toContain(tx1931) - expect(for1931).toContain(txNullSek) + expect(for1931).not.toContain(txNullSek) // the fix: no double-count onto 1931 expect(for1931).not.toContain(tx1930) }) })