fix(bookkeeping): clear the period IB link when stornoing an opening balance (#1022)
* fix(bookkeeping): clear the period IB link when stornoing an opening balance Reversing a period's opening-balance verifikat left fiscal_periods.opening_balance_entry_id pointing at the reversed entry, and nothing reads that pointer's status. The storno was a no-op where it mattered: - getOpeningBalances() reads the linked entry's lines with no status filter, so the Balansrapport kept showing the cancelled IB. - Year-end blocks while the pointer is non-null and tells the user to "reverse it before re-running year-end": advice the storno could never satisfy. delete_last_voucher and the opening-balance/correct route both refuse an already-reversed entry, so there was no in-app way out. reverseEntry now drops the link, mirroring the bank-transaction unlink directly above it. getOpeningBalances falls through to the duplicate-safe compute_prior_opening_balances RPC, and year-end can re-book the IB. This also closes the documented residual edge in opening-balance/correct (storno succeeded, relink failed) and makes runYearEnd's rollback comment true. Two statements, not one: enforce_opening_balance_immutability rejects a pointer change while opening_balances_set is still true. Covered by a pg-real test, since a mocked client happily accepts the single-statement version that the real trigger rejects. Found via support: a user could not close 2025 because bogus 2026 opening balances from a SIE import would not go away. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: record the storno/IB-link decision in DECISIONS.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
b6332e9ff4
commit
a558c75678
@@ -150,3 +150,4 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. 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.
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<string, unknown> = {}
|
||||
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<string, unknown>[] = []
|
||||
|
||||
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<string, unknown> = {}
|
||||
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<string, unknown> = {}
|
||||
const filters: Record<string, unknown> = {}
|
||||
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<string, unknown> = {}
|
||||
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([])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user