fix(import): preserve customized SIE #KONTO account names (#669)

* feat(import): add syncMappedAccounts helper for account create + rename Single home for the create-missing-accounts logic that exists in three near-identical copies (executeSIEImport, the SIE execute route, and the arcim-migration extension), plus a new rename pass that carries customized SIE #KONTO names into accounts that already exist (e.g. K1-seeded defaults). The file's name applies only to identity mappings (source === target); remapped targets keep their BAS/current name. With updateAccountNames=false the behavior matches the legacy code exactly. Not wired up yet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(import): preserve SIE #KONTO account names; add updateAccountNames option Customer report: account names customized in Fortnox did not follow into Accounted via SIE import. The import always used BAS default names for accounts in the BAS reference and never touched accounts that already existed (the K1-seeded chart), so the file's names were silently dropped. executeSIEImport now routes account creation through syncMappedAccounts, which prefers the file's #KONTO name for identity-mapped accounts and renames existing accounts whose name differs (surfaced as a warning). New option updateAccountNames (default true) restores the old behavior when disabled. The duplicated pre-create blocks in the execute route and the arcim-migration extension are removed — executeSIEImport owns account sync on every path now, including the Fortnox re-sync (idempotent renames). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(mcp): expose update_account_names on gnubok_import_sie Optional boolean on the tool schema, staged into the pending operation and threaded through commitImportSie to executeSIEImport. Defaults to true at both stage and commit time — the commit-side default also covers operations staged before the param existed (Boolean(undefined) would have silently flipped it off). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(api): v1 SIE import generated no account mappings The route passed [] as mappings to executeSIEImport, which the mapping-coverage guard (added in #613) rejects for any real file — and before that guard, every voucher was silently skipped as unmapped. The route has never produced a working import for files with vouchers. Generate mappings server-side from the file's #KONTO records plus stored per-company overrides (same as the dashboard execute route), reject unmappable files with a clean 400 before the operation row is created, and expose options.updateAccountNames (default true). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(import): "Använd kontonamn från filen" toggle in import review step New switch (default on) controlling whether the SIE file's #KONTO names are carried into the chart of accounts. Helper text shows how many identity-mapped accounts carry names that differ from the BAS defaults. The page already serializes the whole options object to the execute route, so no further wiring is needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(import): address PR #669 review — parallel renames, rename audit trail - Rename pass now runs UPDATEs concurrently in bounded batches of 25 (greptile P2): a re-sync with many custom names no longer serializes N round trips, and a pathological full-chart rename cannot stampede the API. Per-rename failures stay non-fatal via Promise.allSettled. - Persist the per-account rename detail (number, from, to) into sie_imports.migration_documentation as accountRenames — the behandlingshistorik record per BFNAR 2013:2 (swedish-compliance review); the result warnings only carry the count. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 20:35:49 +02:00
parent 4a54467599
commit f7cd1b86e7
14 changed files with 1362 additions and 246 deletions
@@ -1,10 +1,8 @@
-import { fetchAllRows } from '@/lib/supabase/fetch-all'
 import { NextResponse } from 'next/server'
 import { parseSIEFile, detectEncoding, decodeBuffer } from '@/lib/import/sie-parser'
 import { suggestMappings } from '@/lib/import/account-mapper'
 import { executeSIEImport, checkDuplicateImport } from '@/lib/import/sie-import'
 import { BAS_REFERENCE } from '@/lib/bookkeeping/bas-data'
-import { getBASReference } from '@/lib/bookkeeping/bas-reference'
 import { withRouteContext } from '@/lib/api/with-route-context'
 import { errorResponseFromCode } from '@/lib/errors/get-structured-error'
 import type { AccountMapping, SIEAccountMappingRecord } from '@/lib/import/types'
@@ -46,6 +44,7 @@ export const POST = withRouteContext(
        importOpeningBalances: true,
        importTransactions: true,
        voucherSeries: companyDefaultSeries,
+        updateAccountNames: true,
      }

      const arrayBuffer = await file.arrayBuffer()
@@ -93,95 +92,9 @@ export const POST = withRouteContext(
        })
      }

-      const mappedAccountNumbers = [
-        ...new Set(mappings.filter((m) => m.targetAccount).map((m) => m.targetAccount)),
-      ]
-
-      const allCompanyAccounts = await fetchAllRows(({ from, to }) =>
-        supabase
-          .from('chart_of_accounts')
-          .select('account_number')
-          .eq('company_id', companyId)
-          .range(from, to),
-      )
-      const mappedSet = new Set(mappedAccountNumbers)
-      const existingAccounts = allCompanyAccounts.filter((a) => mappedSet.has(a.account_number))
-
-      const mappingNameLookup = new Map<string, string>()
-      for (const m of mappings) {
-        if (m.targetAccount) {
-          mappingNameLookup.set(m.targetAccount, m.targetName || m.sourceName)
-        }
-      }
-
-      const existingNumbers = new Set(existingAccounts.map((a) => a.account_number))
-      const accountsToActivate = mappedAccountNumbers
-        .filter((num) => !existingNumbers.has(num))
-        .map((num) => {
-          const ref = getBASReference(num)
-          if (ref) {
-            return {
-              user_id: user.id,
-              company_id: companyId,
-              account_number: ref.account_number,
-              account_name: ref.account_name,
-              account_class: ref.account_class,
-              account_group: ref.account_group,
-              account_type: ref.account_type,
-              normal_balance: ref.normal_balance,
-              plan_type: 'full_bas' as const,
-              is_active: true,
-              is_system_account: false,
-              description: ref.description,
-              sru_code: ref.sru_code,
-              sort_order: parseInt(ref.account_number),
-            }
-          }
-
-          // Sub-account not in BAS reference (e.g. 1241 Personbilar). Derive
-          // metadata from the account number.
-          const accountClass = parseInt(num.charAt(0), 10)
-          const accountGroup = num.substring(0, 2)
-          const accountName = mappingNameLookup.get(num) || `Konto ${num}`
-          const accountType =
-            accountClass === 1 ? 'asset'
-              : accountClass === 2 ? 'liability'
-                : accountClass === 3 ? 'revenue'
-                  : 'expense'
-          const normalBalance = accountClass <= 1 || accountClass >= 4 ? 'debit' : 'credit'
-
-          return {
-            user_id: user.id,
-            company_id: companyId,
-            account_number: num,
-            account_name: accountName,
-            account_class: accountClass,
-            account_group: accountGroup,
-            account_type: accountType,
-            normal_balance: normalBalance,
-            plan_type: 'full_bas' as const,
-            is_active: true,
-            is_system_account: false,
-            description: accountName,
-            sru_code: null,
-            sort_order: parseInt(num),
-          }
-        })
-
-      if (accountsToActivate.length > 0) {
-        const { error: activateError } = await supabase
-          .from('chart_of_accounts')
-          .insert(accountsToActivate)
-
-        if (activateError) {
-          opLog.error('sie account activation failed', activateError)
-          return errorResponseFromCode('SIE_IMPORT_ACCOUNT_ACTIVATION_FAILED', opLog, {
-            requestId,
-            details: { reason: activateError.message },
-          })
-        }
-      }
-
+      // Account creation (and #KONTO renames) happen inside executeSIEImport
+      // via syncMappedAccounts — the pre-create block that used to live here
+      // was a duplicate of that logic.
      const result = await executeSIEImport(
        supabase,
        companyId!,
@@ -195,6 +108,7 @@ export const POST = withRouteContext(
          importOpeningBalances: options.importOpeningBalances,
          importTransactions: options.importTransactions,
          voucherSeries: options.voucherSeries || companyDefaultSeries,
+          updateAccountNames: options.updateAccountNames ?? true,
        },
      )

@@ -0,0 +1,187 @@
+/**
+ * Integration tests for POST /api/v1/companies/:companyId/imports/sie.
+ *
+ * Regression: the route used to pass [] as account mappings, which
+ * executeSIEImport's mapping-coverage guard rejects for any real file
+ * (before that guard existed, every voucher was silently skipped). The
+ * route must generate mappings server-side from the file's #KONTO records,
+ * like the dashboard execute route does.
+ */
+import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
+
+beforeAll(() => {
+  if (process.env.NODE_ENV !== 'test') throw new Error('NODE_ENV=test required')
+  process.env.NEXT_PUBLIC_SUPABASE_URL ||= 'http://localhost:54321'
+  process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY ||= 'test-anon-key'
+})
+
+vi.mock('@/lib/auth/api-keys', async () => {
+  const actual = await vi.importActual<typeof import('@/lib/auth/api-keys')>('@/lib/auth/api-keys')
+  return { ...actual, validateApiKey: vi.fn(), createServiceClientNoCookies: vi.fn() }
+})
+vi.mock('@supabase/supabase-js', async () => {
+  const actual = await vi.importActual<typeof import('@supabase/supabase-js')>('@supabase/supabase-js')
+  return { ...actual, createClient: vi.fn().mockReturnValue({}) }
+})
+
+const { executeSIEImportMock, checkDuplicateImportMock, startOperationMock } = vi.hoisted(() => ({
+  executeSIEImportMock: vi.fn(),
+  checkDuplicateImportMock: vi.fn().mockResolvedValue(null),
+  startOperationMock: vi.fn().mockResolvedValue({ id: 'op-1' }),
+}))
+
+vi.mock('@/lib/import/sie-import', async () => {
+  const actual = await vi.importActual<typeof import('@/lib/import/sie-import')>(
+    '@/lib/import/sie-import',
+  )
+  return {
+    ...actual,
+    executeSIEImport: executeSIEImportMock,
+    checkDuplicateImport: checkDuplicateImportMock,
+  }
+})
+vi.mock('@/lib/api/v1/operations', () => ({
+  startOperation: startOperationMock,
+  completeOperation: vi.fn().mockResolvedValue(undefined),
+  failOperation: vi.fn().mockResolvedValue(undefined),
+}))
+
+import { validateApiKey, createServiceClientNoCookies } from '@/lib/auth/api-keys'
+import { POST } from '../route'
+
+const mockValidate = validateApiKey as ReturnType<typeof vi.fn>
+const mockServiceClient = createServiceClientNoCookies as ReturnType<typeof vi.fn>
+
+type MockResult = { data?: unknown; error?: unknown }
+function makeFlexibleSupabase(byTable: Record<string, MockResult | MockResult[]>) {
+  const queues = new Map<string, MockResult[]>()
+  for (const [t, val] of Object.entries(byTable)) {
+    queues.set(t, Array.isArray(val) ? [...val] : [val])
+  }
+  const buildChain = (table: string): unknown => {
+    const handler: ProxyHandler<object> = {
+      get(_target, prop) {
+        if (prop === 'then') {
+          return (resolve: (v: unknown) => void) => {
+            const q = queues.get(table)
+            const next = q && q.length > 1 ? q.shift()! : (q?.[0] ?? { data: null, error: null })
+            resolve(next)
+          }
+        }
+        return (..._args: unknown[]) => buildChain(table)
+      },
+    }
+    return new Proxy({}, handler)
+  }
+  return { from: vi.fn((table: string) => buildChain(table)) }
+}
+
+const COMPANY_ID = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa'
+
+const VALID_SIE = [
+  '#FLAGGA 0',
+  '#SIETYP 4',
+  '#FNAMN "Import AB"',
+  '#ORGNR 5566778899',
+  '#RAR 0 20240101 20241231',
+  '#KONTO 1930 "Företagskonto Swedbank"',
+  '#KONTO 2081 "Aktiekapital"',
+  '#KONTO 6110 "Kontorsmaterial"',
+  '#IB 0 1930 50000.00',
+  '#IB 0 2081 -50000.00',
+  '#VER A 1 20240115 "Inköp"',
+  '{',
+  '#TRANS 6110 {} 1000.00',
+  '#TRANS 1930 {} -1000.00',
+  '}',
+].join('\n')
+
+function makeRequest(options?: Record<string, unknown>): Request {
+  const fd = new FormData()
+  fd.append('file', new File([VALID_SIE], 'bok.se', { type: 'application/octet-stream' }))
+  if (options) fd.append('options', JSON.stringify(options))
+  return new Request(`https://x.test/api/v1/companies/${COMPANY_ID}/imports/sie`, {
+    method: 'POST',
+    headers: { Authorization: 'Bearer test-fixture-not-a-real-key' },
+    body: fd,
+  })
+}
+
+function callRoute(options?: Record<string, unknown>) {
+  return POST(makeRequest(options), {
+    params: Promise.resolve({ companyId: COMPANY_ID }),
+  })
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  mockValidate.mockResolvedValue({
+    userId: 'user-1',
+    companyId: COMPANY_ID,
+    apiKeyId: 'ak_1',
+    scopes: ['bookkeeping:write'],
+    mode: 'live',
+  })
+  checkDuplicateImportMock.mockResolvedValue(null)
+  startOperationMock.mockResolvedValue({ id: 'op-1' })
+  executeSIEImportMock.mockResolvedValue({
+    success: true,
+    importId: 'imp-1',
+    fiscalPeriodId: 'fp-1',
+    openingBalanceEntryId: 'ob-1',
+    journalEntriesCreated: 1,
+    journalEntryIds: ['je-1'],
+    errors: [],
+    warnings: [],
+    replacedPriorImport: null,
+  })
+  mockServiceClient.mockReturnValue(
+    makeFlexibleSupabase({
+      company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
+      sie_account_mappings: { data: [], error: null },
+    }),
+  )
+})
+
+describe('POST /imports/sie', () => {
+  it('generates account mappings from #KONTO records instead of passing []', async () => {
+    const res = await callRoute()
+
+    expect(res.status).toBe(202)
+    const body = await res.json()
+    expect(body.data.operation_id).toBe('op-1')
+
+    expect(executeSIEImportMock).toHaveBeenCalledTimes(1)
+    const mappings = executeSIEImportMock.mock.calls[0][4] as Array<{
+      sourceAccount: string
+      sourceName: string
+      targetAccount: string
+    }>
+    expect(mappings).toHaveLength(3)
+    // Identity mappings carrying the file's #KONTO names.
+    const m1930 = mappings.find((m) => m.sourceAccount === '1930')!
+    expect(m1930.targetAccount).toBe('1930')
+    expect(m1930.sourceName).toBe('Företagskonto Swedbank')
+  })
+
+  it('defaults updateAccountNames to true', async () => {
+    await callRoute()
+
+    const options = executeSIEImportMock.mock.calls[0][5] as Record<string, unknown>
+    expect(options.updateAccountNames).toBe(true)
+  })
+
+  it('passes updateAccountNames: false through from the options JSON', async () => {
+    await callRoute({ updateAccountNames: false })
+
+    const options = executeSIEImportMock.mock.calls[0][5] as Record<string, unknown>
+    expect(options.updateAccountNames).toBe(false)
+  })
+
+  it('rejects unknown options keys (schema stays strict)', async () => {
+    const res = await callRoute({ updateAccountNamez: true })
+
+    expect(res.status).toBe(400)
+    expect(executeSIEImportMock).not.toHaveBeenCalled()
+  })
+})
@@ -42,6 +42,9 @@ import {
  executeSIEImport,
  checkDuplicateImport,
 } from '@/lib/import/sie-import'
+import { suggestMappings } from '@/lib/import/account-mapper'
+import { BAS_REFERENCE } from '@/lib/bookkeeping/bas-data'
+import type { SIEAccountMappingRecord } from '@/lib/import/types'

 const SieImportAccepted = z.object({
  operation_id: z.string().uuid(),
@@ -71,6 +74,7 @@ registerEndpoint({
    'Duplicate-file detection is by SHA-256 hash — re-importing the same file returns 409 SIE_IMPORT_DUPLICATE without re-running the import.',
    'The operation can take 1–5 minutes for multi-year files. The HTTP response returns immediately with operation_id; poll /operations/{id} every ~2s for status.',
    'BFL 7 kap räkenskapsinformation: once a SIE import completes, the resulting verifikationer are immutable. Cancellation midway is not supported.',
+    'Account mappings are generated server-side from the file\'s #KONTO records (plus stored per-company overrides). By default the file\'s account names are carried into the chart, renaming existing accounts whose names differ — pass options.updateAccountNames=false to keep BAS default names.',
  ],
  example: {
    response: {
@@ -148,6 +152,7 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>(
        importOpeningBalances: z.boolean().optional().default(true),
        importTransactions: z.boolean().optional().default(true),
        voucherSeries: z.string().min(1).max(2).optional().default('A'),
+        updateAccountNames: z.boolean().optional().default(true),
      })
      // OWASP V4.5: reject unknown keys so a future schema-extension
      // (or a careless edit) doesn't silently pass mass-assigned fields
@@ -223,6 +228,38 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>(
      })
    }

+    // Build account mappings server-side from the file's #KONTO records and
+    // any stored per-company overrides — same as the dashboard execute route.
+    // (This route used to pass [] as mappings, which executeSIEImport's
+    // mapping-coverage guard rejects for any real file.)
+    const { data: storedMappings } = await ctx.supabase
+      .from('sie_account_mappings')
+      .select('*')
+      .eq('company_id', ctx.companyId)
+    const mappings = suggestMappings(
+      parsed.accounts,
+      BAS_REFERENCE,
+      (storedMappings as SIEAccountMappingRecord[]) || undefined,
+    )
+
+    // Reject unmappable files with a clean 400 before starting the operation
+    // row, mirroring the dashboard route — the alternative is a permanently
+    // failed operation from executeSIEImport's coverage guard.
+    const unmapped = mappings.filter((m) => !m.targetAccount)
+    if (unmapped.length > 0) {
+      return v1ErrorResponseFromCode('VALIDATION_ERROR', ctx.log, {
+        requestId: ctx.requestId,
+        details: {
+          field: 'file',
+          message: `${unmapped.length} account(s) in the SIE file could not be mapped to BAS accounts.`,
+          unmapped_accounts: unmapped.slice(0, 5).map((m) => ({
+            account: m.sourceAccount,
+            name: m.sourceName,
+          })),
+        },
+      })
+    }
+
    // Start the operation row — caller polls /operations/{id} for status.
    const op = await startOperation(
      ctx.supabase,
@@ -248,7 +285,7 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>(
        ctx.companyId!,
        ctx.userId,
        parsed,
-        [],
+        mappings,
        {
          filename: file.name,
          fileContent: content,
@@ -256,6 +293,7 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>(
          importOpeningBalances: options.importOpeningBalances,
          importTransactions: options.importTransactions,
          voucherSeries: options.voucherSeries,
+          updateAccountNames: options.updateAccountNames,
        },
      )
      await completeOperation(ctx.supabase, { id: op.id, result }, ctx.log)