diff --git a/DECISIONS.md b/DECISIONS.md index 2666f50f..49cb6690 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -150,3 +150,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-13] employee_opening_balances created_by preserved via read-then-upsert, not a DB trigger: a BEFORE UPDATE trigger would need a new migration for a pure audit concern; the extra select is one indexed query and the lock trigger already backstops races. [2026-07-13] Opening balances are authoritative for pre-cutover YTD: runSalaryCalculation now excludes booked runs before the cutover month from the YTD aggregation for employees with opening balances, instead of blocking pre-cutover backdated runs (backfill of history is a supported flow). [2026-07-13] Superseded the 2026-07-13 decline of the NOT VALID suggestion for migration 20260713100000: Emil asked to resolve the PR findings, and the migration is branch-only (verified absent from prod schema_migrations), so the never-modify-shipped-migrations rule does not apply; staging already recorded the versions, so edits only change what prod runs at merge. Implemented as ADD ... NOT VALID in 20260713100000 + 20260713121000 with VALIDATE split into 20260713123000: VALIDATE in the same transaction as ADD would be a no-op since Postgres holds the ACCESS EXCLUSIVE lock until commit; a separate migration file gets its own transaction and validates under SHARE UPDATE EXCLUSIVE. 20260713123000 applied to staging (no-op VALIDATE) and version recorded. +[2026-07-15] Stornoing an opening_balance entry now clears fiscal_periods.opening_balance_entry_id inside reverseEntry, rather than making the year-end gate skip reversed IB entries: a status-aware gate alone is strictly worse, because the close would then proceed to generateOpeningBalances, whose bare UPDATE of opening_balance_entry_id is rejected by enforce_opening_balance_immutability while the old pointer is still set, trading a clear blocker for an opaque Postgres exception after the period is already locked and closed. Clearing at storno time also lets getOpeningBalances fall through to the duplicate-safe compute_prior_opening_balances RPC (it has no status filter and would otherwise keep rendering a cancelled IB), and mirrors the bank-transaction unlink already in reverseEntry. The write is two statements (flag, then pointer) because the trigger reads OLD.opening_balances_set; same order as replace_period_opening_balance_link. Not fixed by refusing to storno a linked IB (the other candidate): the user's goal was to remove a bogus IB so bokslut could re-book it, and the IB-correct flow can only replace, never remove. diff --git a/lib/bookkeeping/__tests__/engine.pg.test.ts b/lib/bookkeeping/__tests__/engine.pg.test.ts index dcfe5524..f8e107a2 100644 --- a/lib/bookkeeping/__tests__/engine.pg.test.ts +++ b/lib/bookkeeping/__tests__/engine.pg.test.ts @@ -98,4 +98,63 @@ describe('engine.pg: triggers & RPCs that mocks cannot catch', () => { ) expect(seq.rows[0]!.user_id).toBe(userId) }) + + // reverseEntry() clears the period's IB link when it stornos an + // opening_balance entry. enforce_opening_balance_immutability dictates the + // shape of that write, and only a real Postgres can prove the ordering: a + // mocked client accepts the single-statement version that the trigger + // rejects, which is how a "fixed" storno can still leave the period pinned + // to a cancelled IB (blocking year-end forever). + it('enforce_opening_balance_immutability forces a two-step IB unlink', async () => { + const { userId, companyId, fiscalPeriodId } = await seedCompany() + + const ibEntryId = await insertDraftJournalEntry({ + userId, + companyId, + fiscalPeriodId, + status: 'posted', + voucherNumber: 1, + }) + + // Linking is legal: the trigger only guards the pointer once it is set. + await getPool().query( + `UPDATE public.fiscal_periods + SET opening_balance_entry_id = $2::uuid, opening_balances_set = true + WHERE id = $1::uuid`, + [fiscalPeriodId, ibEntryId], + ) + + // Clearing both columns at once still reads OLD.opening_balances_set = + // true, so the trigger rejects it. This is the write reverseEntry must + // never emit. + await expect( + getPool().query( + `UPDATE public.fiscal_periods + SET opening_balance_entry_id = NULL, opening_balances_set = false + WHERE id = $1::uuid`, + [fiscalPeriodId], + ), + ).rejects.toThrow(/opening balances are immutable once set/i) + + // Flag first, pointer second: the order reverseEntry uses. + await getPool().query( + `UPDATE public.fiscal_periods SET opening_balances_set = false WHERE id = $1::uuid`, + [fiscalPeriodId], + ) + await getPool().query( + `UPDATE public.fiscal_periods SET opening_balance_entry_id = NULL WHERE id = $1::uuid`, + [fiscalPeriodId], + ) + + const period = await getPool().query<{ + opening_balance_entry_id: string | null + opening_balances_set: boolean + }>( + `SELECT opening_balance_entry_id, opening_balances_set + FROM public.fiscal_periods WHERE id = $1::uuid`, + [fiscalPeriodId], + ) + expect(period.rows[0]!.opening_balance_entry_id).toBeNull() + expect(period.rows[0]!.opening_balances_set).toBe(false) + }) }) diff --git a/lib/bookkeeping/__tests__/engine.test.ts b/lib/bookkeeping/__tests__/engine.test.ts index d56c91ce..8fb823f8 100644 --- a/lib/bookkeeping/__tests__/engine.test.ts +++ b/lib/bookkeeping/__tests__/engine.test.ts @@ -750,3 +750,122 @@ describe('reverseEntry: bank transaction unlink', () => { expect(txFilters).toMatchObject({ company_id: 'company-1', journal_entry_id: 'entry-1' }) }) }) + +describe('reverseEntry: opening balance unlink', () => { + // Reversing a period's IB verifikat must also drop the period's pointer to + // it. getOpeningBalances() reads the linked entry's lines with no status + // filter, so a still-linked cancelled IB keeps showing in the Balansrapport, + // and year-end refuses to run while the pointer is non-null: the storno its + // own error message asks for could never satisfy it. See engine.pg.test.ts + // for why the flag must fall before the pointer. + function buildSupabase(sourceType: string) { + const original = { + id: 'entry-1', + company_id: 'company-1', + status: 'posted', + fiscal_period_id: 'period-1', + voucher_series: 'A', + voucher_number: 1, + entry_date: '2026-01-01', + description: 'Ingående balanser från SIE-import', + source_type: sourceType, + source_id: null, + lines: [ + { account_number: '1930', debit_amount: 1000, credit_amount: 0 }, + { account_number: '2350', debit_amount: 0, credit_amount: 1000 }, + ], + } + const reversal = { id: 'reversal-1', reverses_id: 'entry-1', source_type: 'storno' } + + let jeCall = 0 + const jeResults = [ + { data: original, error: null }, + { data: reversal, error: null }, + { data: null, error: null }, + { data: [{ id: 'entry-1' }], error: null }, + { data: { ...reversal, lines: [] }, error: null }, + ] + function jeBuilder() { + const b: Record = {} + for (const m of ['select', 'eq', 'in', 'update', 'insert']) { + b[m] = vi.fn().mockReturnValue(b) + } + b.single = vi.fn().mockImplementation(async () => jeResults[jeCall++]) + b.then = (resolve: (v: unknown) => void) => resolve(jeResults[jeCall++]) + return b + } + + const fpUpdates: unknown[] = [] + const fpFilters: Record[] = [] + + const supabase = { + rpc: vi.fn().mockResolvedValue({ data: 2, error: null }), + from: vi.fn().mockImplementation((table: string) => { + if (table === 'journal_entries') return jeBuilder() + if (table === 'chart_of_accounts') { + const b: Record = {} + for (const m of ['select', 'eq', 'in']) b[m] = vi.fn().mockReturnValue(b) + b.then = (resolve: (v: unknown) => void) => + resolve({ + data: [ + { id: 'acc-1930', account_number: '1930' }, + { id: 'acc-2350', account_number: '2350' }, + ], + error: null, + }) + return b + } + if (table === 'journal_entry_lines') { + return { insert: vi.fn().mockResolvedValue({ error: null }) } + } + if (table === 'fiscal_periods') { + const b: Record = {} + const filters: Record = {} + b.update = vi.fn().mockImplementation((payload: unknown) => { + fpUpdates.push(payload) + fpFilters.push(filters) + return b + }) + b.eq = vi.fn().mockImplementation((col: string, val: unknown) => { + filters[col] = val + return b + }) + b.then = (resolve: (v: unknown) => void) => resolve({ error: null }) + return b + } + if (table === 'transactions') { + const b: Record = {} + for (const m of ['update', 'eq']) b[m] = vi.fn().mockReturnValue(b) + b.then = (resolve: (v: unknown) => void) => resolve({ error: null }) + return b + } + return createMockChain() + }), + } + + return { supabase, fpUpdates, fpFilters } + } + + it('clears the period IB link, flag before pointer, for an opening_balance entry', async () => { + const { supabase, fpUpdates, fpFilters } = buildSupabase('opening_balance') + + const result = await reverseEntry(supabase as never, 'company-1', 'user-1', 'entry-1') + + expect(result.id).toBe('reversal-1') + // Order matters: enforce_opening_balance_immutability rejects the pointer + // write while opening_balances_set is still true. + expect(fpUpdates).toEqual([{ opening_balances_set: false }, { opening_balance_entry_id: null }]) + // Scoped to this entry, so a period pointing elsewhere is untouched. + for (const f of fpFilters) { + expect(f).toMatchObject({ company_id: 'company-1', opening_balance_entry_id: 'entry-1' }) + } + }) + + it('leaves fiscal_periods untouched for a non-opening_balance entry', async () => { + const { supabase, fpUpdates } = buildSupabase('manual') + + await reverseEntry(supabase as never, 'company-1', 'user-1', 'entry-1') + + expect(fpUpdates).toEqual([]) + }) +}) diff --git a/lib/bookkeeping/engine.ts b/lib/bookkeeping/engine.ts index 8766aa14..2f495752 100644 --- a/lib/bookkeeping/engine.ts +++ b/lib/bookkeeping/engine.ts @@ -865,6 +865,48 @@ export async function reverseEntry( log.error('failed to unlink transactions from reversed entry', unlinkError, { entryId }) } + // Same hazard one table over: a period whose opening_balance_entry_id still + // points at the entry we just reversed. getOpeningBalances() reads the linked + // entry's lines directly with no status filter, so the Balansrapport would go + // on showing a cancelled IB, and year-end refuses to run while the link is + // non-null ("Next fiscal period already has opening balance entry posted; + // reverse it before re-running year-end"): advice the storno itself could + // never satisfy, leaving no in-app way out. Clearing the link falls + // getOpeningBalances through to the duplicate-safe + // compute_prior_opening_balances RPC, and lets year-end re-book the IB. + // + // Two statements, not one: enforce_opening_balance_immutability rejects any + // UPDATE that changes opening_balance_entry_id while OLD.opening_balances_set + // is still true, so the flag must fall first (same order, and same reason, as + // the replace_period_opening_balance_link RPC). Both are scoped to this + // entryId, so a period already pointing elsewhere is untouched and callers + // that storno an old IB then relink a fresh one (opening-balance/correct) + // still win: they relink after this returns. + if (original.source_type === 'opening_balance') { + const { error: obFlagError } = await supabase + .from('fiscal_periods') + .update({ opening_balances_set: false }) + .eq('company_id', companyId) + .eq('opening_balance_entry_id', entryId) + + if (obFlagError) { + log.error('failed to clear opening_balances_set on reversed IB period', obFlagError, { + entryId, + }) + } else { + const { error: obUnlinkError } = await supabase + .from('fiscal_periods') + .update({ opening_balance_entry_id: null }) + .eq('company_id', companyId) + .eq('opening_balance_entry_id', entryId) + if (obUnlinkError) { + log.error('failed to unlink reversed opening balance entry from period', obUnlinkError, { + entryId, + }) + } + } + } + // If this was a payment entry, sync the linked invoice/supplier-invoice status. // Helper is shared with the DELETE journal entry route so both code paths leave // the invoice in a consistent state (BFL 5 kap 5§ requires GL reversal; this