fix(arcim): offer the company's own accounts as mapping targets (#2164)
The Fortnox migration's account mapping step builds its target dropdown from BAS_REFERENCE alone: the 1290 standard accounts. Any account a company created outside the standard cannot be selected as a target. Seen on a live Fortnox migration: 3005 "Provisioner inom Sverige" was active in the chart, returned by /api/bookkeeping/accounts, visible everywhere else in the app, and missing from this one list, because BAS defines 3000-3004 and stops there. The plain SIE import at app/(dashboard)/import already used the company's own chart (fetchAccounts(false)), so the two routes into the same AccountMappingStep disagreed about what could be mapped onto. Targets are now the company chart unioned with BAS, deduplicated by account number with the company row winning: its name is whatever the user renamed the account to, and that is the label they look for. BAS stays for standard accounts a first migration is about to create, and a failed chart read degrades to BAS rather than throwing, since an incomplete list still lets the migration proceed while an exception stops it.
This commit is contained in:
@@ -0,0 +1,82 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { SupabaseClient } from '@supabase/supabase-js'
|
||||
|
||||
import { buildMappingTargets } from '../lib/mapping-targets'
|
||||
|
||||
/**
|
||||
* A chart_of_accounts read that returns the given rows for any range, which is
|
||||
* all fetchAllRows needs: one page, then an empty one.
|
||||
*/
|
||||
function supabaseWith(rows: Array<Record<string, unknown>> | Error): SupabaseClient {
|
||||
let served = false
|
||||
const range = () =>
|
||||
rows instanceof Error
|
||||
? Promise.resolve({ data: null, error: { message: rows.message } })
|
||||
: Promise.resolve({ data: served ? [] : ((served = true), rows), error: null })
|
||||
const builder = {
|
||||
select: () => builder,
|
||||
eq: () => builder,
|
||||
order: () => builder,
|
||||
range,
|
||||
}
|
||||
return { from: () => builder } as unknown as SupabaseClient
|
||||
}
|
||||
|
||||
describe('buildMappingTargets', () => {
|
||||
// The bug this file exists for: BAS defines 3000-3004 and stops, so a
|
||||
// company account like 3005 was active in the chart, visible everywhere else
|
||||
// in the app, and still absent from the migration's mapping dropdown.
|
||||
it('offers a company account that BAS does not define', async () => {
|
||||
const targets = await buildMappingTargets(
|
||||
supabaseWith([
|
||||
{ account_number: '3005', account_name: 'Provisioner inom Sverige', account_class: 3 },
|
||||
]),
|
||||
'company-1',
|
||||
)
|
||||
const found = targets.find((t) => t.account_number === '3005')
|
||||
expect(found?.account_name).toBe('Provisioner inom Sverige')
|
||||
})
|
||||
|
||||
it('still offers standard accounts the company has not created', async () => {
|
||||
const targets = await buildMappingTargets(supabaseWith([]), 'company-1')
|
||||
expect(targets.find((t) => t.account_number === '1930')).toBeTruthy()
|
||||
expect(targets.length).toBeGreaterThan(1000)
|
||||
})
|
||||
|
||||
// The user renamed it; that is the label they will look for.
|
||||
it('prefers the company name over the BAS name on a collision', async () => {
|
||||
const targets = await buildMappingTargets(
|
||||
supabaseWith([
|
||||
{ account_number: '3001', account_name: 'Försäljning konsulttjänster', account_class: 3 },
|
||||
]),
|
||||
'company-1',
|
||||
)
|
||||
const matches = targets.filter((t) => t.account_number === '3001')
|
||||
expect(matches).toHaveLength(1)
|
||||
expect(matches[0].account_name).toBe('Försäljning konsulttjänster')
|
||||
})
|
||||
|
||||
it('derives the class from the number when the row has none', async () => {
|
||||
const targets = await buildMappingTargets(
|
||||
supabaseWith([{ account_number: '3005', account_name: 'X', account_class: null }]),
|
||||
'company-1',
|
||||
)
|
||||
expect(targets.find((t) => t.account_number === '3005')?.account_class).toBe(3)
|
||||
})
|
||||
|
||||
it('sorts by account number so the dropdown groups read in order', async () => {
|
||||
const targets = await buildMappingTargets(
|
||||
supabaseWith([{ account_number: '3005', account_name: 'X', account_class: 3 }]),
|
||||
'company-1',
|
||||
)
|
||||
const numbers = targets.map((t) => t.account_number)
|
||||
expect(numbers).toEqual([...numbers].sort((a, b) => a.localeCompare(b)))
|
||||
})
|
||||
|
||||
// An incomplete list still lets the migration run; an exception stops it.
|
||||
it('falls back to BAS when the chart cannot be read', async () => {
|
||||
const targets = await buildMappingTargets(supabaseWith(new Error('boom')), 'company-1')
|
||||
expect(targets.length).toBeGreaterThan(1000)
|
||||
expect(targets.find((t) => t.account_number === '3005')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -34,7 +34,7 @@ import { mergeParsedSIEFiles } from '@/lib/import/sie-merge'
|
||||
import { scanSieForCp1252Artifacts, formatSieArtifactWarning } from '@/lib/import/sie-artifact-scan'
|
||||
import { suggestMappings, getMappingStats, isSystemAccount } from '@/lib/import/account-mapper'
|
||||
import { loadMappings, generateImportPreview, executeSIEImport, findOverlappingPeriodImports } from '@/lib/import/sie-import'
|
||||
import { BAS_REFERENCE } from '@/lib/bookkeeping/bas-reference'
|
||||
import { buildMappingTargets } from './lib/mapping-targets'
|
||||
import type { ProviderName } from '@/lib/providers/types'
|
||||
import { FORTNOX_DOCUMENT_SCOPES_APPROVED } from '@/lib/providers/fortnox/oauth'
|
||||
import { errorResponseFromCode } from '@/lib/errors/get-structured-error'
|
||||
@@ -1023,12 +1023,13 @@ export const arcimMigrationExtension: Extension = {
|
||||
updated_at: '',
|
||||
}))
|
||||
|
||||
// Suggest mappings
|
||||
const basAccounts = BAS_REFERENCE.map(b => ({
|
||||
account_number: b.account_number,
|
||||
account_name: b.account_name,
|
||||
}))
|
||||
const mappings = suggestMappings(allAccounts, basAccounts, existingRecords)
|
||||
// Mapping targets are the company's OWN chart of accounts first,
|
||||
// then BAS for anything it does not have yet. BAS alone cannot
|
||||
// express an account the company added outside the standard, so
|
||||
// such an account was impossible to map onto. See
|
||||
// ./lib/mapping-targets.
|
||||
const mappingTargets = await buildMappingTargets(supabase, companyId)
|
||||
const mappings = suggestMappings(allAccounts, mappingTargets, existingRecords)
|
||||
const mappingStats = getMappingStats(mappings)
|
||||
|
||||
log.info(`Account mapping: ${allAccounts.length} unique accounts across ${sieFiles.length} files, ${mappingStats.unmapped} unmapped`)
|
||||
@@ -1111,7 +1112,7 @@ export const arcimMigrationExtension: Extension = {
|
||||
// Allowed years whose provider export failed: the wizard warns
|
||||
// the user before proceeding so an IB/UB gap cannot slip through.
|
||||
failedYears,
|
||||
basAccounts: BAS_REFERENCE,
|
||||
basAccounts: mappingTargets,
|
||||
})
|
||||
} catch (error) {
|
||||
log.error('arcim sie-data fetch failed', error as Error)
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
import type { SupabaseClient } from '@supabase/supabase-js'
|
||||
|
||||
import { BAS_REFERENCE } from '@/lib/bookkeeping/bas-reference'
|
||||
import { fetchAllRows } from '@/lib/supabase/fetch-all'
|
||||
|
||||
/** What the mapping step offers as a target, and what it renders per option. */
|
||||
export interface MappingTarget {
|
||||
account_number: string
|
||||
account_name: string
|
||||
/** Grouping in the dropdown. Derived from the number when a row lacks it. */
|
||||
account_class?: number
|
||||
}
|
||||
|
||||
/** BAS numbers are 4 digits and the first is the class: 3005 is class 3. */
|
||||
function classOf(accountNumber: string): number | undefined {
|
||||
const first = Number.parseInt(accountNumber.slice(0, 1), 10)
|
||||
return Number.isFinite(first) ? first : undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* The accounts a Fortnox source account may be mapped onto: the company's own
|
||||
* chart first, then the BAS catalogue for standard accounts it has not created
|
||||
* yet.
|
||||
*
|
||||
* Why both. BAS alone hides every account a company added outside the
|
||||
* standard, and there are plenty: BAS defines 3000-3004 and stops, so an
|
||||
* account like 3005 "Provisioner inom Sverige" can be active in the chart,
|
||||
* listed everywhere else in the app, and still impossible to select as a
|
||||
* mapping target. The company
|
||||
* chart alone would be wrong in the other direction: on a first migration it
|
||||
* can be nearly empty, and the user is mapping onto standard accounts the
|
||||
* import is about to create.
|
||||
*
|
||||
* On a collision the company row wins. Its name is whatever the user renamed
|
||||
* the account to, and that is the label they are looking for in the list.
|
||||
*
|
||||
* A failed read degrades to BAS rather than throwing: an incomplete list still
|
||||
* lets the migration proceed, an error stops it dead.
|
||||
*/
|
||||
export async function buildMappingTargets(
|
||||
supabase: SupabaseClient,
|
||||
companyId: string,
|
||||
): Promise<MappingTarget[]> {
|
||||
let own: MappingTarget[] = []
|
||||
try {
|
||||
const rows = await fetchAllRows(({ from, to }) =>
|
||||
supabase
|
||||
.from('chart_of_accounts')
|
||||
.select('account_number, account_name, account_class')
|
||||
.eq('company_id', companyId)
|
||||
.order('account_number')
|
||||
.range(from, to),
|
||||
)
|
||||
own = (rows as Array<Record<string, unknown>>).map((r) => ({
|
||||
account_number: String(r.account_number),
|
||||
account_name: String(r.account_name ?? ''),
|
||||
account_class:
|
||||
typeof r.account_class === 'number' ? r.account_class : classOf(String(r.account_number)),
|
||||
}))
|
||||
} catch {
|
||||
own = []
|
||||
}
|
||||
|
||||
const seen = new Set(own.map((a) => a.account_number))
|
||||
const fromBas = BAS_REFERENCE.filter((b) => !seen.has(b.account_number)).map((b) => ({
|
||||
account_number: b.account_number,
|
||||
account_name: b.account_name,
|
||||
account_class: classOf(b.account_number),
|
||||
}))
|
||||
|
||||
// Sorted by number so the dropdown's per-class groups read in order
|
||||
// regardless of which source a given account came from.
|
||||
return [...own, ...fromBas].sort((a, b) => a.account_number.localeCompare(b.account_number))
|
||||
}
|
||||
Reference in New Issue
Block a user