From 84ba8323b4ea1bfe8f232d7f43861e08a90f9dc1 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Sun, 30 Aug 2026 23:16:34 +0200 Subject: [PATCH] fix(transactions): derive pre-migration marker cutoff from imported voucher dates, not fiscal_year_end (#2047) * fix(transactions): derive pre-migration marker cutoff from imported voucher dates, not fiscal_year_end A SIE file exported mid-year still declares the full fiscal year in #RAR 0, so sie_imports.fiscal_year_end is a future date for mid-year migrators and the 'fran perioden fore din migrering' marker fired on every new bank transaction until New Year. The cutoff now comes from the latest posted source_type='import' entry_date (excluding the M-series omforingsverifikation, which is deliberately dated at fiscal year end), so it tracks where the imported bokforing actually ends and self-corrects on undo/replace. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UU5QbL3p8xNB3tSZDbyrr3 * fix(transactions): arm pre-migration cutoff on completed SIE import, exclude omforing by description Skeptic findings on the frozen commit: (1) source_type='import' is accepted from v1 API clients, so a never-migrated company with an API-labeled backfill would get a false cutoff; the marker is now armed only when a completed sie_imports row exists. (2) Imported vouchers keep the source file's voucher series, so excluding series M dropped genuine M-series vouchers (6 prod companies); the omforingsverifikation is now excluded by its hardcoded description prefix instead. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UU5QbL3p8xNB3tSZDbyrr3 * docs(transactions): state the pre-migration cutoff residuals truthfully The ledger duplicate guard only reaches the completed-year single-skip case (7-day window vs a fiscal-year-end-dated aggregate omforing), so it is not a general backstop for the skip-window gap; the gap is accepted on rarity. Also documents the rattelse-rename fragility of the description-keyed exclusion. Skeptic re-review condition, no code change. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UU5QbL3p8xNB3tSZDbyrr3 --------- Co-authored-by: Claude Fable 5 --- DECISIONS.md | 3 + app/(dashboard)/transactions/page.tsx | 24 ++-- .../__tests__/migration-coverage.test.ts | 108 ++++++++++++++++++ lib/transactions/migration-coverage.ts | 71 ++++++++++++ 4 files changed, 192 insertions(+), 14 deletions(-) create mode 100644 lib/transactions/__tests__/migration-coverage.test.ts create mode 100644 lib/transactions/migration-coverage.ts diff --git a/DECISIONS.md b/DECISIONS.md index aea4f928..0f8e10e2 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1364,3 +1364,6 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-30] book_skattekonto_row(s) tier 'medium' + scope 'transactions:write': rule-driven booking with no caller-supplied lines mirrors book_mileage_period (not create_voucher's 'high'); scope follows reconcile_residual (books an outside row). Commit service gates on SKATTEVERKET_ENABLED for HTTP-dispatcher parity, recoverable so the op stays pending. [2026-08-30] Reminder text overrides (company_settings.reminder_text_overrides, level_1..3 x subject/body): the defaults are expressed as placeholder patterns (REMINDER_EMAIL_DEFAULT_TEXTS) and BOTH the stock mail and overrides render through the same substitution pipeline (applyPlaceholders + escape per output variant), so the settings-UI prefill is byte-for-byte the mail that goes out and cannot drift; this differs from the invoice_email_texts precedent, whose hand-written pattern forms can drift from the coded defaults. The level-3 default body is now an explicit inkassovarning (8 days, fordran till inkasso, costs per lag (1981:739)) but the level TITLE stays 'Slutlig paminnelse': the title is reused as the level name in settings labels and subject prefix, and renaming it everywhere is wording churn beyond the ask. An overridden subject owns the whole line (no automatic ' (inkl. drojsmalsranta)' suffix; {belopp} already includes surcharges), the stock subject keeps the suffix byte-identically. No pg test for the migration: a declarative CHECK (jsonb_typeof object) identical in shape to invoice_email_texts (20260703091000), which also shipped without one. The v1 REST/MCP update_company_settings surface was NOT extended: it is a curated field set with staged operations and its own placeholder refinement, a separate parity slice. typecheck/antipattern baselines deliberately not ratcheted in this diff: both one-count drops predate the branch (main drift), gates only fail on increase. [2026-08-30] PR #2021 round 2 (#546): the relayed Peppol buyer restriction now says the customer's org number must not be a personnummer (prepareParty('buyer') in lib/invoices/peppol-bis-billing.ts refuses it with BUYER_PARTICIPANT_IDENTIFIER_UNSUPPORTED, so an enskild firma CUSTOMER is refused, not only an enskild firma sender), Step 4 of the invoicing-rules workflow points at the Peppol section so a top-down reader never reaches the external-provider fallback first, the mark-sent recovery is scoped to the still-draft invoice in every text (INVOICE_MARK_SENT_REPAIR_REQUIRED leaves the invoice sent with the verifikat posted and a second mark-sent returns 409; the reviewer's proposed repair tool gnubok_link_invoice_to_voucher is the PAYMENT link and requires status sent/overdue/partially_paid, so no tool is named and the repair is left to support), and the verifikat parenthetical says "under faktureringsmetoden" (kontantmetod and defer_invoice_booking companies get none at issue). The guard test now also pins the two v1 route descriptions by reading the route source (apiskill:check only detects generated-vs-source drift, not a truth regression). The atom bump was seeded as a THIRD append-only migration (20260830101500, atom v9) rather than consolidating to one: the Supabase preview branch for the PR (xxnqggttsefleehmarjo) has applied both 20260829000100 and 20260829010000 per its schema_migrations, so deleting either would leave a remote with versions absent from the repo, the orphan class the migration rule forbids; all three seeds are idempotent upserts with the version guard, so prod applying them in sequence ends at v9. The generator's max-plus-one name (20260829010001) was renamed to 20260830101500 for the same reason as round 1 (newer than every file on origin/main and every sibling worktree; skills:check hashes content, the pg replay test globs the seed). +[2026-08-30] Pre-migration inbox marker cutoff derived at query time from max posted source_type='import' entry_date (excl. series M) instead of a stored sie_imports coverage column: no migration/backfill needed and undo/replace self-corrects; series M excluded because the importer's omforingsverifikation is dated at fiscal year end. +[2026-08-30] Skeptic round on PR #2047: cutoff armed by a completed sie_imports row (API clients can post source_type='import', which would false-mark never-migrated companies) and omforingsverifikation excluded by description prefix instead of voucher_series 'M' (imported vouchers keep the source file's series; 6 prod companies use series M for real vouchers). Residual known gap: skip-window covered only by the omforing stays unmarked; DuplicateBookingDialog is the backstop. +[2026-08-30] Correction to the PR #2047 residual-gap note: the DuplicateBookingDialog backstop only covers a single skipped bank movement within 7 days of the omforing's fiscal-year-end date; mid-year and aggregated-skip variants are unmitigated and accepted on rarity (conjunction of skipped voucher + tail date + synced account + fetched history). Exact closure = persist skipped-voucher max date as coverage_end on sie_imports at import time (skippedDetails already has the dates); filed as follow-up. diff --git a/app/(dashboard)/transactions/page.tsx b/app/(dashboard)/transactions/page.tsx index e7ea82f6..8ef8ae1e 100644 --- a/app/(dashboard)/transactions/page.tsx +++ b/app/(dashboard)/transactions/page.tsx @@ -76,6 +76,7 @@ import { cn, formatCurrency, formatDate } from '@/lib/utils' import { roundOre } from '@/lib/money' import type { TransactionCategory, CreateTransactionInput, Invoice, Customer, SupplierInvoice, Supplier, VatTreatment, EntityType, LinePatternEntry, BookingTemplateLibrary } from '@/types' import type { SuggestedTemplate } from '@/lib/transactions/category-suggestions' +import { fetchMigrationCoverageEnd } from '@/lib/transactions/migration-coverage' import { isImportedTransaction } from '@/lib/transactions/origin' import { computeJeUnderlagStatus, type JeUnderlagStatus } from '@/lib/transactions/underlag-status' import { isWithinBounds, resolvePeriodBounds } from '@/lib/transactions/period-filter' @@ -560,10 +561,13 @@ export default function TransactionsPage() { // "Kör matchning igen" in the review surface. const [rerunningMatch, setRerunningMatch] = useState(false) - // End of the company's completed SIE-import coverage (latest - // fiscal_year_end). Drives the quiet "från perioden före din migrering" - // marker on inbox rows: period-based on purpose, it labels which period a - // row belongs to, it never suggests a sync skip date (that was #917). + // End of the company's SIE-migration data coverage (latest imported + // voucher date, see lib/transactions/migration-coverage.ts). Drives the + // quiet "från perioden före din migrering" marker on inbox rows: date-based + // on purpose, it labels rows the imported bokföring should already cover, + // it never suggests a sync skip date (that was #917). Not fiscal_year_end: + // for a mid-year migration that is a future date and the marker fired on + // every new transaction until New Year. const [sieCoverageEnd, setSieCoverageEnd] = useState(null) useEffect(() => { if (!companyId) { @@ -572,17 +576,9 @@ export default function TransactionsPage() { } let cancelled = false ;(async () => { - const { data } = await supabase - .from('sie_imports') - .select('fiscal_year_end') - .eq('company_id', companyId) - .eq('status', 'completed') - .not('fiscal_year_end', 'is', null) - .order('fiscal_year_end', { ascending: false }) - .limit(1) - .maybeSingle() + const coverageEnd = await fetchMigrationCoverageEnd(supabase, companyId) if (!cancelled) { - setSieCoverageEnd((data as { fiscal_year_end?: string } | null)?.fiscal_year_end || null) + setSieCoverageEnd(coverageEnd) } })() return () => { diff --git a/lib/transactions/__tests__/migration-coverage.test.ts b/lib/transactions/__tests__/migration-coverage.test.ts new file mode 100644 index 00000000..99d628a3 --- /dev/null +++ b/lib/transactions/__tests__/migration-coverage.test.ts @@ -0,0 +1,108 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { fetchMigrationCoverageEnd } from '@/lib/transactions/migration-coverage' +import type { SupabaseClient } from '@supabase/supabase-js' + +/** + * Recording chain mock: remembers every chained call so the test can assert + * the exact filters. The generic createMockSupabase proxy swallows arguments. + * Results are dequeued per from() call: first for sie_imports (arm gate), + * second for journal_entries. + */ +function createRecordingSupabase(results: Array<{ data: unknown; error: unknown }>) { + const calls: Array<{ method: string; args: unknown[] }> = [] + const queue = [...results] + const buildChain = () => { + const result = queue.shift() ?? { data: null, error: null } + const chain: Record = {} + const record = + (method: string) => + (...args: unknown[]) => { + calls.push({ method, args }) + return chain + } + for (const method of ['select', 'eq', 'neq', 'not', 'order', 'limit']) { + chain[method] = record(method) + } + chain.maybeSingle = vi.fn().mockImplementation(() => { + calls.push({ method: 'maybeSingle', args: [] }) + return Promise.resolve(result) + }) + return chain + } + const from = vi.fn().mockImplementation((...args: unknown[]) => { + calls.push({ method: 'from', args }) + return buildChain() + }) + return { supabase: { from } as unknown as SupabaseClient, calls } +} + +const armed = { data: { id: 'import-1' }, error: null } + +describe('fetchMigrationCoverageEnd', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('returns the latest imported entry date for a migrated company', async () => { + const { supabase } = createRecordingSupabase([ + armed, + { data: { entry_date: '2026-06-30' }, error: null }, + ]) + await expect(fetchMigrationCoverageEnd(supabase, 'company-1')).resolves.toBe('2026-06-30') + }) + + it('returns null without a completed SIE import, even if import-typed entries exist', async () => { + // Arm gate: source_type='import' is accepted from API clients, so a + // company that never migrated must never get a cutoff from such entries. + const { supabase, calls } = createRecordingSupabase([ + { data: null, error: null }, + { data: { entry_date: '2026-12-31' }, error: null }, + ]) + await expect(fetchMigrationCoverageEnd(supabase, 'company-1')).resolves.toBeNull() + expect(calls.filter((c) => c.method === 'from').map((c) => c.args[0])).toEqual(['sie_imports']) + }) + + it('returns null when a migrated company has no imported entries', async () => { + const { supabase } = createRecordingSupabase([armed, { data: null, error: null }]) + await expect(fetchMigrationCoverageEnd(supabase, 'company-1')).resolves.toBeNull() + }) + + it('arms on completed imports and takes the max over posted import entries excluding the omföringsverifikation', async () => { + const { supabase, calls } = createRecordingSupabase([armed, { data: null, error: null }]) + await fetchMigrationCoverageEnd(supabase, 'company-1') + + expect(calls.filter((c) => c.method === 'from').map((c) => c.args[0])).toEqual([ + 'sie_imports', + 'journal_entries', + ]) + expect(calls).toContainEqual({ method: 'eq', args: ['status', 'completed'] }) + expect(calls).toContainEqual({ method: 'eq', args: ['company_id', 'company-1'] }) + expect(calls).toContainEqual({ method: 'eq', args: ['status', 'posted'] }) + expect(calls).toContainEqual({ method: 'eq', args: ['source_type', 'import'] }) + // The importer's omföringsverifikation is dated at fiscal year end; without + // this exclusion a mid-year migration with skipped vouchers would get a + // future cutoff again (the bug this module exists to fix). Excluded by its + // hardcoded description prefix, NOT by voucher series: imported vouchers + // keep the source file's series, and real files use series M. + expect(calls).toContainEqual({ + method: 'not', + args: ['description', 'like', 'Omföringsverifikation:%'], + }) + expect(calls.filter((c) => c.method === 'neq')).toEqual([]) + expect(calls).toContainEqual({ + method: 'order', + args: ['entry_date', { ascending: false }], + }) + expect(calls).toContainEqual({ method: 'limit', args: [1] }) + }) + + it('never reads sie_imports.fiscal_year_end as the cutoff', async () => { + // Regression guard for the original bug: the cutoff value must come from + // actual imported data, never the #RAR-declared fiscal year. sie_imports + // is consulted only as the arm gate (select id). + const { supabase, calls } = createRecordingSupabase([armed, { data: null, error: null }]) + await fetchMigrationCoverageEnd(supabase, 'company-1') + const selects = calls.filter((c) => c.method === 'select').map((c) => c.args[0]) + expect(selects).toEqual(['id', 'entry_date']) + }) +}) diff --git a/lib/transactions/migration-coverage.ts b/lib/transactions/migration-coverage.ts new file mode 100644 index 00000000..c4794093 --- /dev/null +++ b/lib/transactions/migration-coverage.ts @@ -0,0 +1,71 @@ +import type { SupabaseClient } from '@supabase/supabase-js' + +/** + * End of the company's SIE-migration data coverage: the latest entry_date + * among posted imported verifikat. Drives the quiet "från perioden före din + * migrering" marker on inbox rows. + * + * Derived from journal_entries, NOT from sie_imports.fiscal_year_end: a SIE + * file exported mid-year still declares the full fiscal year in #RAR 0, so + * for a mid-year migrator fiscal_year_end is a FUTURE date and every new + * bank transaction satisfied `date <= cutoff` until New Year (the 2026-08-30 + * user report). The imported vouchers themselves end where the old system's + * data ends, which is the boundary the marker is about. + * + * Armed only when a completed sie_imports row exists: source_type='import' + * is accepted from API clients too (CreateJournalEntrySchema), so without + * the gate a third-party backfill labeled 'import' would paint a false + * pre-migration marker across a company that never migrated. With the gate, + * such entries can still stretch a real migrator's cutoff, but a company + * labeling API entries 'import' post-migration is doing exactly what the + * label says. + * + * The importer's omföringsverifikation (skipped-voucher adjustment) is + * excluded by its hardcoded description prefix: it is deliberately dated at + * fiscal year end (sie-import.ts) and would reintroduce the future-date bug + * for any import with skipped vouchers. Excluding by description rather + * than by its 'M' voucher series keeps genuine series-M vouchers from the + * source file in the max (prod has companies whose files use series M for + * ordinary vouchers, e.g. moms). Two accepted residuals: (1) bank movement + * covered only by the omföring (skipped vouchers dated after the last + * cleanly imported one) falls outside the cutoff and is mostly unmitigated: + * the ledger duplicate guard only catches a single skipped movement within + * 7 days of the omföring's fiscal-year-end date, never the mid-year or + * aggregated-skip variants. Accepted because it needs a conjunction of + * rare conditions, against the systematic all-year over-marking it + * replaces. Exact closure needs skipped-voucher dates persisted at import + * (skippedDetails in sie-import.ts has them; candidate follow-up: a + * coverage_end column on sie_imports). (2) The exclusion keys on + * description, which inline rättelse can edit: renaming the omföring + * re-admits its fiscal-year-end date and degrades that one company to the + * pre-fix over-marking, nothing worse. + * + * Undo/replace of an import deletes or recreates these entries, so the + * cutoff self-corrects with no stored state to maintain. Returns null when + * the company has no completed migration: callers render no marker. + */ +export async function fetchMigrationCoverageEnd( + supabase: SupabaseClient, + companyId: string, +): Promise { + const { data: completedImport } = await supabase + .from('sie_imports') + .select('id') + .eq('company_id', companyId) + .eq('status', 'completed') + .limit(1) + .maybeSingle() + if (!completedImport) return null + + const { data } = await supabase + .from('journal_entries') + .select('entry_date') + .eq('company_id', companyId) + .eq('status', 'posted') + .eq('source_type', 'import') + .not('description', 'like', 'Omföringsverifikation:%') + .order('entry_date', { ascending: false }) + .limit(1) + .maybeSingle() + return (data as { entry_date?: string } | null)?.entry_date ?? null +}