From 027734ffc7604f9e8e2cf4d3de9895ef61c098e4 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Thu, 18 Jun 2026 13:49:00 +0200 Subject: [PATCH] =?UTF-8?q?fix(reconciliation):=20scope=20bank=20avst?= =?UTF-8?q?=C3=A4mning=20to=20a=20fiscal=20period=20so=20the=20IB=20stops?= =?UTF-8?q?=20counting=20(#751)=20(#754)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(reconciliation): scope bank reconciliation to a fiscal period so the IB stops counting (#751) The bank reconciliation widget defaulted its date window to "full history" (empty dateFrom). With no lower bound the GL side spans the fiscal-year boundary: a prior period's movements on the account net to exactly the opening balance, and the new period's IB entry adds another copy. The IB *summary* was excluded but the prior-period *detail* stayed in the period movement while the bank feed only covered the current period — a phantom difference equal to the IB (the "räknar med IB fast den säger borträknad" report in #751). - Server: getReconciliationStatus now floors the window at the most recent opening-balance date on the account (effectiveFrom = max(dateFrom, ibDate)) and clamps both the GL movement set and the bank-feed set identically. Derived from the already-fetched lines — no extra query. A no-op when the caller already passes period_start; a safety net otherwise. - UI: BankReconciliationView scopes to a fiscal period via FiscalYearSelector (defaults to the newest period), seeding dateFrom/dateTo and gating the initial fetch so the full-history numbers never flash. - Tests: two regression cases reproducing the cross-period scenario. Proven against prod: full-history -> difference -10 172,94 (matched the screenshot); period-bounded -> 0,00. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(reconciliation): re-fetch on fiscal-period switch; document IB-floor choice Review follow-up on #754: - BankReconciliationView: add selectedPeriodId to the gated fetch effect deps so switching räkenskapsår re-fetches with the new window, and a late period selection (selector signalling ready before the company context hydrates) still triggers the real period-scoped fetch instead of leaving the empty-window result. Manual date edits still stay on the explicit "Filtrera" action. - getReconciliationStatus: comment why ibFloor takes the LATEST opening-balance date (one IB per period invariant; across a multi-year window the most recent IB is the intended floor; same-date duplicates cancel). Co-Authored-By: Claude Opus 4.8 (1M context) * test(reconciliation): cover mid-period window (dateFrom after the IB date) Review follow-up on #754: documents that a per-month reconciliation window starting after the fiscal-year IB correctly excludes the IB and reconciles on the in-window movements alone (gl_1930_opening_balance = 0 by design). Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- components/reports/BankReconciliationView.tsx | 52 +++++++- .../__tests__/bank-reconciliation.test.ts | 113 ++++++++++++++++++ lib/reconciliation/bank-reconciliation.ts | 73 +++++++---- 3 files changed, 213 insertions(+), 25 deletions(-) diff --git a/components/reports/BankReconciliationView.tsx b/components/reports/BankReconciliationView.tsx index 067f7492..854cb6ab 100644 --- a/components/reports/BankReconciliationView.tsx +++ b/components/reports/BankReconciliationView.tsx @@ -13,6 +13,7 @@ import { AlertCircle, ChevronDown, ChevronRight, Link2, Unlink, Play, Eye, EyeOf import { formatCurrency, formatDate } from '@/lib/utils' import { formatVoucher } from '@/lib/bookkeeping/voucher-series-resolver' import { CashAccountSelector } from '@/components/common/CashAccountSelector' +import { FiscalYearSelector } from '@/components/common/FiscalYearSelector' import { MatchVerifikationPicker, type UnlinkedGLLine } from '@/components/reconciliation/MatchVerifikationPicker' import { DropdownMenu, @@ -28,7 +29,7 @@ import { } from '@/components/ui/destructive-confirm-dialog' import { useToast } from '@/components/ui/use-toast' import { ToastAction } from '@/components/ui/toast' -import type { CashAccount } from '@/types' +import type { CashAccount, FiscalPeriod } from '@/types' function formatAmount(amount: number): string { return amount.toLocaleString('sv-SE', { minimumFractionDigits: 2, maximumFractionDigits: 2 }) @@ -129,11 +130,17 @@ export function BankReconciliationView() { const [loading, setLoading] = useState(true) const [error, setError] = useState(null) - // dateFrom stays empty by default (full history) so nothing the user needs to - // reconcile is hidden on first load. dateTo defaults to today so the field - // isn't a blank "åååå-mm-dd" and the upper bound is concrete. + // The window is scoped to a fiscal period (issue #751): a bank reconciliation + // is inherently per-period, and a "full history" window spans the fiscal-year + // boundary — mixing a prior period's movements with the current year's IB and + // manufacturing a phantom difference equal to the IB. dateFrom/dateTo are + // seeded from the selected räkenskapsår below (and stay editable as a manual + // override). The first fetch is gated on `periodReady` so we never flash the + // full-history numbers before the period is known. const [dateFrom, setDateFrom] = useState('') const [dateTo, setDateTo] = useState(() => new Date().toISOString().slice(0, 10)) + const [selectedPeriodId, setSelectedPeriodId] = useState(null) + const [periodReady, setPeriodReady] = useState(false) const [accountNumber, setAccountNumber] = useState('1930') const [cashAccounts, setCashAccounts] = useState([]) // Date filters apply on demand (the "Filtrera" button or an account switch), @@ -299,9 +306,37 @@ export function BankReconciliationView() { // "Filtrera" button calls fetchAll() explicitly for date changes. }, [accountNumber, accountCurrency, includeMatched]) + // Seed the reconciliation window from the selected räkenskapsår. dateTo is + // clamped to today for the current (open) year so we don't claim to reconcile + // into the future; a past year ends at its period_end. periodReady unblocks the + // gated initial fetch below. The dates stay editable via the manual inputs. + const handlePeriodChange = useCallback( + (periodId: string | null, period?: FiscalPeriod | null) => { + setSelectedPeriodId(periodId) + if (period) { + const today = new Date().toISOString().slice(0, 10) + setDateFrom(period.period_start) + setDateTo(period.period_end < today ? period.period_end : today) + } + setPeriodReady(true) + }, + [], + ) + + // Gate the first load until the fiscal period (and therefore the date window) + // is known — otherwise we'd fetch once with the empty full-history window and + // briefly render the phantom-diff numbers before the period seeds the dates. + // The date-seeding setState in handlePeriodChange updates dateFromRef/dateToRef + // (the effects above) before this effect runs on the same render, so fetchAll + // reads the freshly-seeded window. selectedPeriodId is a dependency so that + // switching räkenskapsår re-fetches with the new window, and so a late period + // selection (e.g. if the selector signals ready before the company context has + // hydrated and onChange arrives a tick later) still triggers the real fetch. + // Manual date edits intentionally do NOT auto-fetch — that stays on "Filtrera". useEffect(() => { + if (!periodReady) return fetchAll() - }, [fetchAll]) + }, [fetchAll, periodReady, selectedPeriodId]) // Reset transient per-account UI state when the selected account changes. A // verifikation pick or a dry-run preview computed for the previous account is @@ -644,6 +679,13 @@ export function BankReconciliationView() { value={accountNumber} onChange={setAccountNumber} /> + setPeriodReady(true)} + includeAllOption={false} + hideFuturePeriods + />
{ expect(status.difference).toBe(-500) expect(status.is_reconciled).toBe(false) }) + + it('floors a full-history window at the IB date so a prior period cannot count the IB (issue #751)', async () => { + // Real-world repro: the widget's date filter defaults to "full history" + // (no dateFrom). A company with two fiscal periods has prior-period (2025) + // movements on the account that net to EXACTLY the opening balance, plus the + // new period's IB entry dated on period start. Summing the whole history and + // only subtracting the IB *summary* leaves the prior-period *detail* in the + // movement while the bank feed only covers the current period — a phantom + // difference equal to the IB. The server now floors the window at the most + // recent IB date on the account, clamping BOTH sides to the current period. + const { supabase, enqueue } = createQueueMockSupabase() + + // 1) transactions: a prior-period feed line (6000, 2025) + the current + // period's feed (5000, 2026). The floor must drop the 2025 one. + enqueue({ + data: [ + { date: '2025-03-31', amount: 6000, journal_entry_id: 'je-p', reconciliation_method: 'manual' }, + { date: '2026-02-15', amount: 5000, journal_entry_id: 'je-c', reconciliation_method: 'manual' }, + ], + }) + // 2) GL lines on the account: prior-period movements (6000 + 4000 = 10000, + // dated 2025) that net to the IB, the IB itself (10000, dated 2026-01-01), + // and the current period's movement (5000, dated 2026). + enqueue({ + data: [ + { debit_amount: 6000, credit_amount: 0, journal_entries: { id: 'je-p1', status: 'posted', source_type: 'import', entry_date: '2025-03-31' } }, + { debit_amount: 4000, credit_amount: 0, journal_entries: { id: 'je-p2', status: 'posted', source_type: 'import', entry_date: '2025-09-30' } }, + { debit_amount: 10000, credit_amount: 0, journal_entries: { id: 'je-ib', status: 'posted', source_type: 'opening_balance', entry_date: '2026-01-01' } }, + { debit_amount: 5000, credit_amount: 0, journal_entries: { id: 'je-c1', status: 'posted', source_type: 'bank_transaction', entry_date: '2026-02-15' } }, + ], + }) + // 3) RPC: empty + enqueue({ data: [] }) + + // No dateFrom — the "full history" default that triggers the bug. + const status = await getReconciliationStatus(supabase as never, 'company-1') + + // Both sides clamped to >= 2026-01-01 (the IB date): only the IB + the + // current-period movement remain on the GL side; only the 2026 feed remains + // on the bank side. The prior period (which equalled the IB) is excluded. + expect(status.gl_1930_balance).toBe(15000) // IB 10000 + 5000 (NOT 25000) + expect(status.gl_1930_period_movement).toBe(5000) // IB excluded + expect(status.gl_1930_opening_balance).toBe(10000) // UI "räknas inte" note stays truthful + expect(status.bank_transaction_total).toBe(5000) // 2025 feed floored out + expect(status.difference).toBe(0) + expect(status.is_reconciled).toBe(true) + }) + + it('floors at the IB date even when the caller passes a dateFrom earlier than the IB', async () => { + // Safety net for a manual date override / direct API call: clearing the date + // filter (or setting it before the IB) must not resurrect the phantom diff. + // effectiveFrom = max(dateFrom, ibDate), so an earlier dateFrom is raised. + const { supabase, enqueue } = createQueueMockSupabase() + + enqueue({ + data: [ + { date: '2025-05-01', amount: 8000, journal_entry_id: 'je-p', reconciliation_method: 'manual' }, + { date: '2026-04-10', amount: 3000, journal_entry_id: 'je-c', reconciliation_method: 'manual' }, + ], + }) + enqueue({ + data: [ + { debit_amount: 8000, credit_amount: 0, journal_entries: { id: 'je-p1', status: 'posted', source_type: 'import', entry_date: '2025-05-01' } }, + { debit_amount: 8000, credit_amount: 0, journal_entries: { id: 'je-ib', status: 'posted', source_type: 'opening_balance', entry_date: '2026-01-01' } }, + { debit_amount: 3000, credit_amount: 0, journal_entries: { id: 'je-c1', status: 'posted', source_type: 'bank_transaction', entry_date: '2026-04-10' } }, + ], + }) + enqueue({ data: [] }) + + // dateFrom deliberately before the IB date. + const status = await getReconciliationStatus(supabase as never, 'company-1', '2025-01-01') + + expect(status.gl_1930_period_movement).toBe(3000) // IB + prior detail both excluded + expect(status.gl_1930_opening_balance).toBe(8000) + expect(status.bank_transaction_total).toBe(3000) // 2025 feed floored out + expect(status.difference).toBe(0) + expect(status.is_reconciled).toBe(true) + }) + + it('reconciles a mid-period window (dateFrom after the IB date) on movements alone', async () => { + // Per-month reconciliation: the user scopes to March, after the fiscal-year + // IB (2026-01-01). effectiveFrom = max(dateFrom, ibDate) = the March dateFrom, + // so the IB and Jan/Feb movements are correctly excluded — they belong to the + // opening position of a March window, not its movements. gl_1930_opening_balance + // is 0 here BY DESIGN: a mid-period window contains no fiscal-year IB, so the + // "räknas inte" note is simply absent (not misleadingly zero). The window + // reconciles on March's movements vs March's feed. + const { supabase, enqueue } = createQueueMockSupabase() + + enqueue({ + data: [ + { date: '2026-02-10', amount: 2000, journal_entry_id: 'je-feb', reconciliation_method: 'manual' }, + { date: '2026-03-15', amount: 3000, journal_entry_id: 'je-mar', reconciliation_method: 'manual' }, + ], + }) + enqueue({ + data: [ + { debit_amount: 9000, credit_amount: 0, journal_entries: { id: 'je-ib', status: 'posted', source_type: 'opening_balance', entry_date: '2026-01-01' } }, + { debit_amount: 2000, credit_amount: 0, journal_entries: { id: 'je-feb1', status: 'posted', source_type: 'import', entry_date: '2026-02-10' } }, + { debit_amount: 3000, credit_amount: 0, journal_entries: { id: 'je-mar1', status: 'posted', source_type: 'import', entry_date: '2026-03-15' } }, + ], + }) + enqueue({ data: [] }) + + // dateFrom in March — after the 2026-01-01 IB. + const status = await getReconciliationStatus(supabase as never, 'company-1', '2026-03-01') + + expect(status.gl_1930_opening_balance).toBe(0) // IB not part of a March window + expect(status.gl_1930_period_movement).toBe(3000) // only March movements + expect(status.bank_transaction_total).toBe(3000) // only March feed + expect(status.difference).toBe(0) + expect(status.is_reconciled).toBe(true) + }) }) diff --git a/lib/reconciliation/bank-reconciliation.ts b/lib/reconciliation/bank-reconciliation.ts index 9044c5e3..ed399236 100644 --- a/lib/reconciliation/bank-reconciliation.ts +++ b/lib/reconciliation/bank-reconciliation.ts @@ -355,7 +355,7 @@ export async function getReconciliationStatus( // second same-currency account from inflating bankTotal here. let txQuery = supabase .from('transactions') - .select('amount, journal_entry_id, reconciliation_method, is_ignored') + .select('date, amount, journal_entry_id, reconciliation_method, is_ignored') .eq('company_id', companyId) txQuery = scopeTransactionsToAccount(txQuery, cashAccountId, currency, includeUnassigned) @@ -386,7 +386,12 @@ export async function getReconciliationStatus( const { data: glLines } = await glQuery - type GlEntry = { id?: string | null; status?: string | null; source_type?: string | null } + type GlEntry = { + id?: string | null + status?: string | null + source_type?: string | null + entry_date?: string | null + } type GlLineRow = { debit_amount: number | string | null credit_amount: number | string | null @@ -405,23 +410,53 @@ export async function getReconciliationStatus( // posted + reversed = the ledger balance, exactly as the trial balance counts // it. The .in() filter on the query already excludes draft/cancelled. - const countedLines = (glLines || []) as GlLineRow[] + const fetchedLines = (glLines || []) as GlLineRow[] - // Bank side: every feed transaction in range, full stop. We deliberately do - // NOT special-case rows linked to a reversed entry any more. Because the GL - // side now counts the reversed original, its storno AND the correction - // together (just like the balance sheet), a corrected bank line nets to its - // true amount on both sides and reconciles on its own — whether correctEntry - // re-pointed the transaction to the live corrected entry or a legacy row still - // points at the reversed original, the result is identical. The previous - // "drop reversed-linked transactions" rule paired with the now-removed - // correction subtraction below; together with correctEntry re-pointing the - // transaction they manufactured a difference equal to a corrected amount. - const bankTotal = (transactions || []).reduce( - (sum, tx) => sum + (Number(tx.amount) || 0), - 0 + // Floor the window at the most recent opening-balance date on this account + // (issue #751). Everything dated before that IB is prior history the IB entry + // already summarises; if the window has no lower bound (the "full history" + // default) it spans the fiscal-year boundary and pulls the prior period's real + // movements — which net to exactly the IB — into the period movement, while the + // bank feed only covers the current period. The IB *summary* is excluded below, + // but the prior-period *detail* would otherwise remain, manufacturing a phantom + // difference equal to the IB. effectiveFrom is the later of the caller's + // dateFrom and that IB date; it only ever RAISES the lower bound, so the + // dateFrom SQL pre-filter on both queries above stays valid. In normal use the + // UI passes dateFrom = period_start = the IB date, so this is a no-op there. + const ibDates = fetchedLines + .filter((l) => entryOf(l)?.source_type === 'opening_balance') + .map((l) => entryOf(l)?.entry_date) + .filter((d): d is string => typeof d === 'string' && d.length > 0) + // Take the LATEST IB date. The invariant is one opening_balance entry per + // fiscal period (set_opening_balances / SIE import / year-end rollover all + // create exactly one, dated period_start), so within a single-period window + // there is only one. Across a multi-year window the most recent IB is the + // correct floor — an earlier year's IB and the movements it summarises are + // prior history we deliberately drop. Same-date duplicates are harmless: they + // land in both countedLines and glOpeningBalance and cancel. + const ibFloor = ibDates.length ? ibDates.reduce((a, b) => (a > b ? a : b)) : null + const effectiveFrom = + dateFrom && ibFloor ? (dateFrom > ibFloor ? dateFrom : ibFloor) : dateFrom || ibFloor || null + // ISO yyyy-mm-dd compares lexically; undated rows (e.g. test fixtures) pass. + const onOrAfterFloor = (d: string | null | undefined): boolean => + !effectiveFrom || typeof d !== 'string' ? true : d >= effectiveFrom + + // Clamp BOTH sides to the floor identically so they stay comparable. Lines and + // transactions before the IB belong to a prior period's reconciliation. + const countedLines = fetchedLines.filter((l) => onOrAfterFloor(entryOf(l)?.entry_date)) + const countedTx = (transactions || []).filter((tx) => + onOrAfterFloor((tx as { date?: string | null }).date), ) + // Bank side: every feed transaction in the (floored) window, full stop. We + // deliberately do NOT special-case rows linked to a reversed entry any more. + // Because the GL side now counts the reversed original, its storno AND the + // correction together (just like the balance sheet), a corrected bank line nets + // to its true amount on both sides and reconciles on its own — whether + // correctEntry re-pointed the transaction to the live corrected entry or a + // legacy row still points at the reversed original, the result is identical. + const bankTotal = countedTx.reduce((sum, tx) => sum + (Number(tx.amount) || 0), 0) + // gl_1930_balance: the real ledger balance on this account incl. IB — // byte-for-byte the figure the balansräkning / saldobalans report. const glBalance = countedLines.reduce((sum, line) => sum + lineAmount(line), 0) @@ -445,11 +480,9 @@ export async function getReconciliationStatus( // a bank-feed counterpart, so it stays in. const glPeriodMovement = glBalance - glOpeningBalance - const matchedCount = (transactions || []).filter( - (tx) => tx.journal_entry_id !== null - ).length + const matchedCount = countedTx.filter((tx) => tx.journal_entry_id !== null).length - const unmatchedTransactionCount = (transactions || []).filter( + const unmatchedTransactionCount = countedTx.filter( (tx) => tx.journal_entry_id === null && tx.is_ignored !== true ).length