fix(arcim-migration): require a completed SIE import for Fortnox too (#2000) (#2006)

* fix(arcim-migration): require a completed SIE import for Fortnox too (#2000) POST /migrate refused entity import without a completed SIE import for every provider except Fortnox, on the assumption that the wizard always runs SIE-over-API first. The wizard lets the user uncheck "Bokföringsdata (SIE)" while keeping customers, suppliers and invoices checked, so a Fortnox company could end up with subledgers but no chart of accounts and no opening balances: the incomplete bokföring the guard exists to prevent. The guard now applies to all providers as "a completed SIE import must exist for the company", not "must be part of this run", so an entities-only re-run after an earlier full migration still works. For providers that serve SIE over the API (Fortnox, Briox, Björn Lundén, WINT) the 409 message points at the wizard checkbox instead of "ladda upp en SIE-fil". The options step shows one attn line and disables "Starta migrering" when SIE is unchecked, never imported, and any API import is selected, so the user sees the rule before the run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna * fix(arcim-migration): address review findings (#2000) - Findings 1, 3, 4 (stale hasSieData on retry): the options step now derives "SIE already imported" from the preview OR this session's successful /import-sie results, so an entities-only retry after a phase-2 failure is no longer blocked with a false "not imported yet" hint and a forced replace-mode ledger re-import. - Finding 2 (company-info-only run gated): importCompanyInfo is dropped from the wizard gate and POST /migrate skips the SIE guard when every entity flag is false. Company info writes no accounts, balances or subledger rows, so the BFL rationale does not apply. Tests pin both the company-info-only pass and the block when any entity flag is set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-28 16:52:12 +02:00
parent 7993700589
commit b4b11c33ca
6 changed files with 174 additions and 33 deletions
@@ -1322,3 +1322,5 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
 [2026-08-28] Per-company hiding of standardmallar via new booking_template_hidden table (insert=hide, delete=unhide), not is_active or a library column: system template rows are shared globally, so per-company state must live beside them; hiding is opt-in per company and restorable in settings (user request).
 [2026-08-28] Same-bank warning limited to observed one-session banks (SEB only): prod shows Handelsbanken tolerates 4 concurrent sessions, and the generic warning made a user abandon a legitimate renewal. Planned sync-death visibility work was dropped: already shipped via #1271 (health probe), #1727 (stale state), #1969 (cron unstarve).
 [2026-08-28] Same-bank warning revised to three tiers after skeptic refutation: hard warn SEB, silent/calm only for verified multi-session banks (Handelsbanken, 4 distinct session_ids observed), legacy hedged warning for unknown banks (fail closed), shared-session siblings exempt (fan-out carries them).
+[2026-08-28] /migrate SIE guard extended to every provider (Fortnox exemption removed) as "a completed SIE import must exist for the company", not "must be part of this run", plus a wizard hint that disables Start when SIE is unchecked and never imported; chose this over forcing the checkbox on because the route is the only seam a direct API call or a stale client cannot bypass, and "must exist" keeps entities-only re-runs after a full migration working (#2000).
+[2026-08-28] /migrate SIE guard skips company-info-only runs (all entity flags false) and the wizard derives "SIE already imported" from the preview OR this session's successful /import-sie results: company info writes no accounts, balances or subledger rows, so the BFL rationale does not apply; and the one-shot preview went stale after phase 1 succeeded and phase 2 failed, falsely blocking an entities-only retry (#2000 review).
@@ -1060,6 +1060,7 @@ function OptionsStep({
  options,
  sieAvailable,
  sieData,
+  hasSieData,
  provider,
  onChange,
  onStart,
@@ -1068,6 +1069,8 @@ function OptionsStep({
  options: MigrationOptions
  sieAvailable: boolean
  sieData: SIEData | null
+  /** The company already has a completed SIE import (any origin). */
+  hasSieData: boolean
  provider: ArcimProvider | null
  onChange: (options: MigrationOptions) => void
  onStart: () => void
@@ -1096,6 +1099,18 @@ function OptionsStep({
  if (options.importSalesInvoices) selectedItems.push('Kundfakturor')
  if (options.importSupplierInvoices) selectedItems.push('Leverantörsfakturor')

+  // Entities without the SIE-derived ledger leave an incomplete bokföring:
+  // POST /migrate refuses with PROVIDER_SIE_IMPORT_REQUIRED unless a completed
+  // SIE import exists. Say so here, before the run, when the user has
+  // unchecked SIE for a company that has never imported it (#2000).
+  // Company info (name, org number, VAT number) writes no ledger data and is
+  // not gated, matching the route.
+  const hasApiImport = options.importCustomers ||
+    options.importSuppliers ||
+    options.importSalesInvoices ||
+    options.importSupplierInvoices
+  const sieRequiredButUnchecked = sieAvailable && !options.importSIEData && !hasSieData && hasApiImport
+
  return (
    <div className="stagger-enter space-y-8">
      <StepHeading
@@ -1104,8 +1119,10 @@ function OptionsStep({
      />

      {/* Years whose provider export failed: must be visible before the user
-          proceeds, otherwise an IB/UB gap slips through. One ochre sentence. */}
-      {sieAvailable && failedYears.length > 0 && (
+          proceeds, otherwise an IB/UB gap slips through. One ochre sentence.
+          Yields to the SIE-required line below: with SIE unchecked no year is
+          imported, and the page carries at most one attn line. */}
+      {sieAvailable && failedYears.length > 0 && !sieRequiredButUnchecked && (
        <AttnLine>
          {failedYears.length === 1
            ? `Räkenskapsår ${failedYears[0].year} kunde inte hämtas från källsystemet: om du fortsätter importeras övriga år, men ingående och utgående balanser kan sakna kontinuitet. Försök igen senare eller ladda upp en SIE-fil för det saknade året manuellt.`
@@ -1202,12 +1219,16 @@ function OptionsStep({
        />
      </div>

+      {sieRequiredButUnchecked && (
+        <AttnLine>{t('ext_arcim_option_sie_required_hint')}</AttnLine>
+      )}
+
      <div className="flex flex-col-reverse gap-3 border-t border-border pt-6 sm:flex-row sm:justify-between">
        <Button variant="outline" className="min-h-11" onClick={onBack}>
          <ArrowLeft className="mr-2 h-4 w-4" />
          Tillbaka
        </Button>
-        <Button className="min-h-11" onClick={() => setShowConfirm(true)} disabled={selectedItems.length === 0}>
+        <Button className="min-h-11" onClick={() => setShowConfirm(true)} disabled={selectedItems.length === 0 || sieRequiredButUnchecked}>
          Starta migrering
          <ArrowRight className="ml-2 h-4 w-4" />
        </Button>
@@ -2975,6 +2996,7 @@ export default function ArcimMigrationWorkspace({
          options={migrationOptions}
          sieAvailable={preview?.sieAvailable ?? false}
          sieData={sieData}
+          hasSieData={(preview?.hasSieData ?? false) || sieImportResults.some(r => r.success)}
          provider={preview?.consent.provider ?? null}
          onChange={setMigrationOptions}
          onStart={handleStartMigration}
@@ -10,8 +10,11 @@ import type { ExtensionContext } from '@/lib/extensions/types'
 * invoices): it never posts to the general ledger. The GL (kontoplan, ingående
 * balanser, verifikationer) arrives via SIE. Importing entities without the
 * SIE-derived ledger leaves an incomplete bokföring under BFL, so the route MUST
- * refuse to run for non-Fortnox providers until a completed SIE import exists.
- * Fortnox is exempt because it pulls SIE itself via API.
+ * refuse to run for EVERY provider until a completed SIE import exists for the
+ * company. Fortnox used to be exempt (it pulls SIE itself via API), but the
+ * wizard lets the user uncheck "Bokföringsdata (SIE)" while keeping entities
+ * checked (#2000), so the exemption is gone. The rule is "must exist", not "must
+ * be part of this run": an entities-only re-run after a full migration passes.
 *
 * Previously this was only an advisory banner + step-gating in the React wizard,
 * which a direct API call or a stale client could bypass. This test locks the
@@ -63,26 +66,51 @@ function buildCtx(count: number | null): ExtensionContext {
  return { supabase, companyId: 'company-1' } as unknown as ExtensionContext
 }

-function migrateRequest() {
+function migrateRequest(body: Record<string, unknown> = { consentId: 'consent-1' }) {
  return createMockRequest('http://localhost/api/extensions/ext/arcim-migration/migrate', {
    method: 'POST',
-    body: { consentId: 'consent-1' },
+    body,
  })
 }

+type GuardErrorBody = { error: { code: string; message: string; message_en?: string } }
+
 describe('POST /migrate: SIE-import-required guard', () => {
  beforeEach(() => {
    vi.clearAllMocks()
  })

+  it('returns 401 when there is no authenticated user', async () => {
+    const ctx = buildCtx(0)
+    ;(ctx.supabase as unknown as { auth: { getUser: Mock } }).auth.getUser
+      .mockResolvedValue({ data: { user: null } })
+
+    const res = await handler(migrateRequest(), ctx)
+
+    expect(res.status).toBe(401)
+    expect(getConsent).not.toHaveBeenCalled()
+    expect(executeMigration).not.toHaveBeenCalled()
+  })
+
+  it('returns 400 when consentId is missing', async () => {
+    const res = await handler(migrateRequest({}), buildCtx(1))
+
+    expect(res.status).toBe(400)
+    expect(getConsent).not.toHaveBeenCalled()
+    expect(executeMigration).not.toHaveBeenCalled()
+  })
+
  it('blocks a non-Fortnox provider when no completed SIE import exists', async () => {
    ;(getConsent as Mock).mockResolvedValue({ id: 'consent-1', status: 1, provider: 'visma' })

    const res = await handler(migrateRequest(), buildCtx(0))
-    const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
+    const { status, body } = await parseJsonResponse<GuardErrorBody>(res)

    expect(status).toBe(409)
    expect(body.error.code).toBe('PROVIDER_SIE_IMPORT_REQUIRED')
+    // Visma has no SIE-over-API: the static registry text ("ladda upp en
+    // SIE-fil") is the right instruction and must stay.
+    expect(body.error.message).toMatch(/ladda upp en SIE-fil/i)
    expect(executeMigration).not.toHaveBeenCalled()
  })

@@ -95,10 +123,71 @@ describe('POST /migrate: SIE-import-required guard', () => {
    expect(executeMigration).toHaveBeenCalledTimes(1)
  })

-  it('exempts Fortnox: entity import runs even with no SIE import (SIE comes via API)', async () => {
+  it('blocks Fortnox when no completed SIE import exists (SIE step unchecked in the wizard)', async () => {
    ;(getConsent as Mock).mockResolvedValue({ id: 'consent-1', status: 1, provider: 'fortnox' })

    const res = await handler(migrateRequest(), buildCtx(0))
+    const { status, body } = await parseJsonResponse<GuardErrorBody>(res)
+
+    expect(status).toBe(409)
+    expect(body.error.code).toBe('PROVIDER_SIE_IMPORT_REQUIRED')
+    expect(executeMigration).not.toHaveBeenCalled()
+  })
+
+  it('tells SIE-over-API providers to tick the wizard checkbox, not to upload a file', async () => {
+    ;(getConsent as Mock).mockResolvedValue({ id: 'consent-1', status: 1, provider: 'fortnox' })
+
+    const res = await handler(migrateRequest(), buildCtx(0))
+    const { body } = await parseJsonResponse<GuardErrorBody>(res)
+
+    expect(body.error.message).toContain('Bokföringsdata (SIE)')
+    expect(body.error.message).not.toMatch(/ladda upp/i)
+    expect(body.error.message_en).toContain('Bokföringsdata (SIE)')
+    expect(body.error.message_en).not.toMatch(/upload/i)
+  })
+
+  it('allows a company-info-only run with no SIE import (writes no ledger data)', async () => {
+    ;(getConsent as Mock).mockResolvedValue({ id: 'consent-1', status: 1, provider: 'fortnox' })
+
+    const res = await handler(
+      migrateRequest({
+        consentId: 'consent-1',
+        importCompanyInfo: true,
+        importCustomers: false,
+        importSuppliers: false,
+        importSalesInvoices: false,
+        importSupplierInvoices: false,
+      }),
+      buildCtx(0),
+    )
+
+    expect(res.status).toBe(200)
+    expect(executeMigration).toHaveBeenCalledTimes(1)
+  })
+
+  it('still blocks when company info is combined with any entity flag', async () => {
+    ;(getConsent as Mock).mockResolvedValue({ id: 'consent-1', status: 1, provider: 'fortnox' })
+
+    const res = await handler(
+      migrateRequest({
+        consentId: 'consent-1',
+        importCompanyInfo: true,
+        importCustomers: false,
+        importSuppliers: true,
+        importSalesInvoices: false,
+        importSupplierInvoices: false,
+      }),
+      buildCtx(0),
+    )
+
+    expect(res.status).toBe(409)
+    expect(executeMigration).not.toHaveBeenCalled()
+  })
+
+  it('allows Fortnox once a completed SIE import exists (entities-only re-run)', async () => {
+    ;(getConsent as Mock).mockResolvedValue({ id: 'consent-1', status: 1, provider: 'fortnox' })
+
+    const res = await handler(migrateRequest(), buildCtx(1))

    expect(res.status).toBe(200)
    expect(executeMigration).toHaveBeenCalledTimes(1)
@@ -1174,31 +1174,57 @@ export const arcimMigrationExtension: Extension = {
          }

          // ── Guard: a completed SIE import is required before entity import ──
-          // Most providers expose ONLY entity data (customers, suppliers,
-          // invoices) via API: never the general ledger. Fortnox pulls the GL
-          // itself via SIE-over-API and is exempt. Briox and Björn Lundén also
-          // serve SIE over the API, but the wizard runs /import-sie before
-          // /migrate, so this guard stays satisfied, and keeps protecting
-          // against a skipped SIE step. Importing entities without the
-          // SIE-derived ledger (kontoplan,
-          // ingående balanser, verifikationer) would leave an incomplete
-          // bokföring under BFL: a subledger with no chart of accounts and no
-          // opening balances, so every subsequent posting and balance is wrong.
-          // The wizard surfaces this as an advisory banner, but it must be
-          // enforced here so the rule cannot be bypassed by a direct API call,
-          // a skipped wizard step, or a stale client.
-          if (consent.provider !== 'fortnox') {
-            const { count: completedSieImports } = await supabase
-              .from('sie_imports')
-              .select('id', { count: 'exact', head: true })
-              .eq('company_id', companyId)
-              .eq('status', 'completed')
+          // Provider APIs expose ONLY entity data (customers, suppliers,
+          // invoices): never the general ledger. The GL (kontoplan, ingående
+          // balanser, verifikationer) arrives via SIE, either uploaded by the
+          // user or, for Fortnox, Briox, Björn Lundén and WINT, pulled over the
+          // API by the wizard's /import-sie phase. Importing entities without
+          // that ledger would leave an incomplete bokföring under BFL: a
+          // subledger with no chart of accounts and no opening balances, so
+          // every subsequent posting and balance is wrong.
+          //
+          // The rule is "a completed SIE import must EXIST for the company",
+          // not "must be part of this run": an entities-only re-run after an
+          // earlier full migration passes. No provider is exempt. Fortnox used
+          // to be, on the assumption that the wizard always runs SIE-over-API
+          // first, but the wizard lets the user uncheck "Bokföringsdata (SIE)"
+          // while keeping entities checked (#2000), and a direct API call or a
+          // stale client can skip it regardless.
+          //
+          // Company info (name, org number, VAT number) writes no accounts,
+          // balances or subledger rows, so a run that imports only that is
+          // not gated.
+          const importsEntities = importCustomers ||
+            importSuppliers ||
+            importSalesInvoices ||
+            importSupplierInvoices
+          const { count: completedSieImports } = importsEntities
+            ? await supabase
+                .from('sie_imports')
+                .select('id', { count: 'exact', head: true })
+                .eq('company_id', companyId)
+                .eq('status', 'completed')
+            : { count: null }

-            if (!completedSieImports || completedSieImports < 1) {
-              return errorResponseFromCode('PROVIDER_SIE_IMPORT_REQUIRED', moduleLog, {
-                details: { provider: consent.provider },
-              })
-            }
+          if (importsEntities && (!completedSieImports || completedSieImports < 1)) {
+            // Providers that serve SIE over the API have no file to upload:
+            // point the user at the wizard checkbox instead of the static
+            // "ladda upp en SIE-fil" text. Same code and status either way so
+            // the wizard's error path renders it unchanged.
+            const sieViaApi = ARCIM_PROVIDERS.some(
+              (p) => p.id === consent.provider && p.sieViaApi,
+            )
+            return errorResponseFromCode('PROVIDER_SIE_IMPORT_REQUIRED', moduleLog, {
+              details: { provider: consent.provider },
+              ...(sieViaApi
+                ? {
+                    messageSv:
+                      'Bokföringsdata (SIE) måste importeras först. Kryssa i "Bokföringsdata (SIE)" i guiden så att kontoplan, ingående balanser och verifikationer hämtas innan kunder, leverantörer och fakturor importeras.',
+                    messageEn:
+                      'A completed SIE import is required first. Tick "Bokföringsdata (SIE)" in the wizard so the chart of accounts, opening balances and verifications are fetched before customers, suppliers and invoices are imported.',
+                  }
+                : {}),
+            })
          }

          log.info(`Starting migration for user ${user.id} from ${consent.provider}`)
@@ -5689,6 +5689,7 @@
    "ext_arcim_documents_error_reference": "Error reference: {requestId}",
    "ext_arcim_documents_provider_message": "Source system response: {message}",
    "ext_arcim_option_series_help": "Vouchers keep their series from the source system (A, D, E ...). The series here is only used for vouchers without one.",
+    "ext_arcim_option_sie_required_hint": "The ledger (chart of accounts, opening balances and verifications) has not been imported yet. Tick Bokföringsdata (SIE) to fetch it in the same run: customers, suppliers and invoices cannot be imported without it.",
    "ext_arcim_documents_result_description": "The document import is complete.",
    "ext_arcim_documents_imported": "Imported",
    "ext_arcim_documents_skipped": "Already present",
@@ -5689,6 +5689,7 @@
    "ext_arcim_documents_error_reference": "Felreferens: {requestId}",
    "ext_arcim_documents_provider_message": "Svar från källsystemet: {message}",
    "ext_arcim_option_series_help": "Verifikat behåller sin serie från källsystemet (A, D, E ...). Serien här används bara för verifikat som saknar serie.",
+    "ext_arcim_option_sie_required_hint": "Bokföringen (kontoplan, ingående balanser och verifikationer) har inte importerats än. Kryssa i Bokföringsdata (SIE) för att hämta den i samma körning: kunder, leverantörer och fakturor kan inte importeras utan den.",
    "ext_arcim_documents_result_description": "Underlagsimporten är klar.",
    "ext_arcim_documents_imported": "Importerade",
    "ext_arcim_documents_skipped": "Fanns redan",