feat(enable-banking): per-account ledger mapping for multicurrency setups (#443) — rebase of #487 (#488)
* feat(enable-banking): per-account ledger mapping for multicurrency setups (#443) Today every bank account routes to BAS 1930. Multicurrency setups (Wise, SEB foreign-currency sub-accounts) get pooled into a single SEK ledger account, making year-end FX revaluation a mess. This change lets each bank account under a PSD2 consent map to its own BAS account (SEK→1930, EUR→1932, USD→1933, etc.). The mapping engine already honors IngestOptions.settlementAccount (lib/bookkeeping/mapping-engine.ts:55-57) so the wiring is small: - StoredAccount gains an optional ledger_account field (no migration — bank_connections.accounts_data is already JSONB). - syncAccountTransactions passes account.ledger_account through as settlementAccount so the bank-side leg routes to the right BAS account. - PATCH /accounts accepts account_mappings, validates 4-digit BAS format, and verifies each ledger_account exists in chart_of_accounts before persisting. Selection edits without account_mappings preserve existing values for back-compat. - AccountPickerDialog shows a per-account "Bokför till konto" combobox populated from the company's 19xx accounts, with currency-based defaults (SEK→1930, EUR→1932, USD→1933, GBP→1934) and a non-blocking warning when two enabled accounts route to the same BAS account with different currencies. Reconciliation still scans 1930 only — foreign-currency accounts skip auto-matching until follow-up PR 4 (filed in the plan). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(enable-banking): address review feedback on PR #487 Two real bugs flagged in review: 1. AccountPickerDialog row wrapped a Radix <Checkbox> (which renders as its own <button role="checkbox">) inside <button onClick={toggle}>. Browsers silently flatten nested interactive elements, the Checkbox lost its onCheckedChange handler, and the toggle interaction was effectively broken. Reverted to <label> + <Checkbox onCheckedChange> with the <Select> as a sibling outside the label so clicking it doesn't also toggle. 2. BAS_ACCOUNT_PATTERN was /^[0-9]{4}$/ — accepted 3001 (revenue), 2640 (input VAT), or any 4-digit account that happens to exist in the chart. Direct API calls would bypass the UI's 19% picker filter and silently misroute every bank-side journal-entry leg into the wrong BAS class, corrupting both the ledger and momsdeklaration. Tightened to /^19[0-9]{2}$/ (kassa/bank only). Tests updated to assert non-19xx rejection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(enable-banking): validate account_mappings UIDs against accounts_data A typo'd UID in account_mappings was silently dropped — the entry never landed in the resulting accounts_data while the response was still 200, leaving the client to believe the mapping was applied. Mirror the existing enabled_uids guard: reject unknown UIDs with 400 + unknown_uids in the body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(enable-banking): catch syncPromise rejections to prevent process crash When the 60s timeout wins the Promise.race, the underlying Promise.all keeps running. A subsequent bank-API rejection has no registered handler, which surfaces as an unhandledRejection — Node 22 (the self-hosted Docker runtime) terminates the process by default on those. The cron self-heals via initial_sync_completed_at IS NULL, so a no-op catch is the right policy: drop the late rejection, let the cron retry. Caught by Greptile review on PR #488. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
04f902fe8f
commit
722b22972d
@@ -37,6 +37,8 @@ interface SupabaseStub {
|
||||
* Falls back to updateError when the index isn't present.
|
||||
*/
|
||||
updateErrorByCall?: Array<{ message: string } | null>
|
||||
/** BAS account numbers that exist in the company's chart_of_accounts (PR 2 ledger validation). */
|
||||
chartAccountNumbers?: string[]
|
||||
/** Last update payload (may be overwritten by a follow-up metadata update). */
|
||||
capturedUpdate?: Record<string, unknown>
|
||||
/** All update payloads in order — first is the status flip, second the initial-sync metadata. */
|
||||
@@ -49,26 +51,44 @@ function buildSupabase(stub: SupabaseStub) {
|
||||
auth: {
|
||||
getUser: vi.fn().mockResolvedValue({ data: { user: stub.authUser }, error: null }),
|
||||
},
|
||||
from: vi.fn(() => ({
|
||||
select: vi.fn().mockReturnThis(),
|
||||
eq: vi.fn().mockReturnThis(),
|
||||
single: vi.fn().mockResolvedValue({
|
||||
data: stub.connectionRow,
|
||||
error: stub.connectionError ?? null,
|
||||
}),
|
||||
update: vi.fn((payload: Record<string, unknown>) => {
|
||||
const callIndex = updateCallCount++
|
||||
stub.capturedUpdate = payload
|
||||
;(stub.capturedUpdates ??= []).push(payload)
|
||||
const error =
|
||||
stub.updateErrorByCall && callIndex < stub.updateErrorByCall.length
|
||||
? stub.updateErrorByCall[callIndex]
|
||||
: stub.updateError ?? null
|
||||
from: vi.fn((table: string) => {
|
||||
// The chart_of_accounts query is used for ledger_account validation
|
||||
// (PR 487). It chains select().eq().in() and is awaited as a thenable.
|
||||
if (table === 'chart_of_accounts') {
|
||||
const numbers = stub.chartAccountNumbers ?? []
|
||||
return {
|
||||
eq: vi.fn().mockResolvedValue({ error }),
|
||||
select: vi.fn().mockReturnThis(),
|
||||
eq: vi.fn().mockReturnThis(),
|
||||
in: vi.fn((_col: string, vals: string[]) => {
|
||||
const data = vals
|
||||
.filter(v => numbers.includes(v))
|
||||
.map(v => ({ account_number: v }))
|
||||
return Promise.resolve({ data, error: null })
|
||||
}),
|
||||
}
|
||||
}),
|
||||
})),
|
||||
}
|
||||
return {
|
||||
select: vi.fn().mockReturnThis(),
|
||||
eq: vi.fn().mockReturnThis(),
|
||||
single: vi.fn().mockResolvedValue({
|
||||
data: stub.connectionRow,
|
||||
error: stub.connectionError ?? null,
|
||||
}),
|
||||
update: vi.fn((payload: Record<string, unknown>) => {
|
||||
const callIndex = updateCallCount++
|
||||
stub.capturedUpdate = payload
|
||||
;(stub.capturedUpdates ??= []).push(payload)
|
||||
// Per-call error overrides win when set; fall back to updateError otherwise.
|
||||
const error =
|
||||
stub.updateErrorByCall && callIndex < stub.updateErrorByCall.length
|
||||
? stub.updateErrorByCall[callIndex]
|
||||
: stub.updateError ?? null
|
||||
return {
|
||||
eq: vi.fn().mockResolvedValue({ error }),
|
||||
}
|
||||
}),
|
||||
}
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -584,4 +604,250 @@ describe('PATCH /accounts (enable-banking)', () => {
|
||||
expect(stub.capturedUpdates).toHaveLength(2)
|
||||
})
|
||||
})
|
||||
|
||||
describe('per-account ledger mapping (account_mappings)', () => {
|
||||
it('persists ledger_account from account_mappings into accounts_data JSONB', async () => {
|
||||
mockedSync.mockResolvedValue({ imported: 0, duplicates: 0, errors: 0 })
|
||||
|
||||
const stub: SupabaseStub = {
|
||||
authUser: { id: 'user-1' },
|
||||
chartAccountNumbers: ['1930', '1932', '1933'],
|
||||
connectionRow: {
|
||||
id: 'conn-1',
|
||||
status: 'pending_selection',
|
||||
accounts_data: [
|
||||
{ uid: 'acc-sek', currency: 'SEK', enabled: true },
|
||||
{ uid: 'acc-eur', currency: 'EUR', enabled: true },
|
||||
{ uid: 'acc-usd', currency: 'USD', enabled: true },
|
||||
],
|
||||
},
|
||||
}
|
||||
const supabase = buildSupabase(stub)
|
||||
const ctx = makeContext(supabase)
|
||||
|
||||
const res = await accountsRoute.handler(
|
||||
makeRequest({
|
||||
connection_id: 'conn-1',
|
||||
enabled_uids: ['acc-sek', 'acc-eur', 'acc-usd'],
|
||||
account_mappings: [
|
||||
{ uid: 'acc-sek', ledger_account: '1930' },
|
||||
{ uid: 'acc-eur', ledger_account: '1932' },
|
||||
{ uid: 'acc-usd', ledger_account: '1933' },
|
||||
],
|
||||
}),
|
||||
ctx
|
||||
)
|
||||
|
||||
expect(res.status).toBe(200)
|
||||
const written = stub.capturedUpdates?.[0]?.accounts_data as StoredAccount[]
|
||||
expect(written.find(a => a.uid === 'acc-sek')?.ledger_account).toBe('1930')
|
||||
expect(written.find(a => a.uid === 'acc-eur')?.ledger_account).toBe('1932')
|
||||
expect(written.find(a => a.uid === 'acc-usd')?.ledger_account).toBe('1933')
|
||||
})
|
||||
|
||||
it('rejects ledger_account not in BAS class 19 (e.g. 3001 revenue)', async () => {
|
||||
// Even though 3001 might exist in the chart, routing the bank-side leg
|
||||
// there would silently misroute every transaction into a revenue account.
|
||||
// The class-19 restriction must be enforced at the API layer regardless of
|
||||
// whether the chart contains the supplied account number.
|
||||
const stub: SupabaseStub = {
|
||||
authUser: { id: 'user-1' },
|
||||
chartAccountNumbers: ['1930', '3001'],
|
||||
connectionRow: {
|
||||
id: 'conn-1',
|
||||
status: 'pending_selection',
|
||||
accounts_data: [{ uid: 'acc-1', currency: 'SEK', enabled: true }],
|
||||
},
|
||||
}
|
||||
const supabase = buildSupabase(stub)
|
||||
const ctx = makeContext(supabase)
|
||||
|
||||
const res = await accountsRoute.handler(
|
||||
makeRequest({
|
||||
connection_id: 'conn-1',
|
||||
enabled_uids: ['acc-1'],
|
||||
account_mappings: [{ uid: 'acc-1', ledger_account: '3001' }],
|
||||
}),
|
||||
ctx
|
||||
)
|
||||
|
||||
expect(res.status).toBe(400)
|
||||
const body = await res.json()
|
||||
expect(body.error).toMatch(/klass 19/)
|
||||
})
|
||||
|
||||
it('rejects ledger_account that is malformed (not 4 digits)', async () => {
|
||||
const stub: SupabaseStub = {
|
||||
authUser: { id: 'user-1' },
|
||||
chartAccountNumbers: ['1930'],
|
||||
connectionRow: {
|
||||
id: 'conn-1',
|
||||
status: 'pending_selection',
|
||||
accounts_data: [{ uid: 'acc-1', currency: 'SEK', enabled: true }],
|
||||
},
|
||||
}
|
||||
const supabase = buildSupabase(stub)
|
||||
const ctx = makeContext(supabase)
|
||||
|
||||
const res = await accountsRoute.handler(
|
||||
makeRequest({
|
||||
connection_id: 'conn-1',
|
||||
enabled_uids: ['acc-1'],
|
||||
account_mappings: [{ uid: 'acc-1', ledger_account: '19' }],
|
||||
}),
|
||||
ctx
|
||||
)
|
||||
|
||||
expect(res.status).toBe(400)
|
||||
const body = await res.json()
|
||||
expect(body.error).toMatch(/klass 19/)
|
||||
})
|
||||
|
||||
it('rejects ledger_account that does not exist in chart_of_accounts', async () => {
|
||||
const stub: SupabaseStub = {
|
||||
authUser: { id: 'user-1' },
|
||||
chartAccountNumbers: ['1930'], // 1932 not in chart
|
||||
connectionRow: {
|
||||
id: 'conn-1',
|
||||
status: 'pending_selection',
|
||||
accounts_data: [{ uid: 'acc-eur', currency: 'EUR', enabled: true }],
|
||||
},
|
||||
}
|
||||
const supabase = buildSupabase(stub)
|
||||
const ctx = makeContext(supabase)
|
||||
|
||||
const res = await accountsRoute.handler(
|
||||
makeRequest({
|
||||
connection_id: 'conn-1',
|
||||
enabled_uids: ['acc-eur'],
|
||||
account_mappings: [{ uid: 'acc-eur', ledger_account: '1932' }],
|
||||
}),
|
||||
ctx
|
||||
)
|
||||
|
||||
expect(res.status).toBe(400)
|
||||
const body = await res.json()
|
||||
expect(body.error).toMatch(/finns inte i kontoplanen/)
|
||||
expect(body.invalid_accounts).toEqual(['1932'])
|
||||
})
|
||||
|
||||
it('preserves existing ledger_account when account_mappings is omitted (selection edit)', async () => {
|
||||
const stub: SupabaseStub = {
|
||||
authUser: { id: 'user-1' },
|
||||
connectionRow: {
|
||||
id: 'conn-1',
|
||||
status: 'active',
|
||||
accounts_data: [
|
||||
{ uid: 'acc-1', currency: 'SEK', enabled: true, ledger_account: '1930' },
|
||||
{ uid: 'acc-2', currency: 'EUR', enabled: true, ledger_account: '1932' },
|
||||
],
|
||||
},
|
||||
}
|
||||
const supabase = buildSupabase(stub)
|
||||
const ctx = makeContext(supabase)
|
||||
|
||||
const res = await accountsRoute.handler(
|
||||
// No account_mappings — pure selection edit (disable acc-2)
|
||||
makeRequest({ connection_id: 'conn-1', enabled_uids: ['acc-1'] }),
|
||||
ctx
|
||||
)
|
||||
|
||||
expect(res.status).toBe(200)
|
||||
const written = stub.capturedUpdates?.[0]?.accounts_data as StoredAccount[]
|
||||
// Both ledger_account values stay intact even though acc-2 is now disabled.
|
||||
expect(written.find(a => a.uid === 'acc-1')?.ledger_account).toBe('1930')
|
||||
expect(written.find(a => a.uid === 'acc-2')?.ledger_account).toBe('1932')
|
||||
})
|
||||
|
||||
it('clears ledger_account when account_mappings entry sets it to null', async () => {
|
||||
mockedSync.mockResolvedValue({ imported: 0, duplicates: 0, errors: 0 })
|
||||
|
||||
const stub: SupabaseStub = {
|
||||
authUser: { id: 'user-1' },
|
||||
chartAccountNumbers: ['1930'],
|
||||
connectionRow: {
|
||||
id: 'conn-1',
|
||||
status: 'active',
|
||||
accounts_data: [
|
||||
{ uid: 'acc-1', currency: 'SEK', enabled: true, ledger_account: '1930' },
|
||||
],
|
||||
},
|
||||
}
|
||||
const supabase = buildSupabase(stub)
|
||||
const ctx = makeContext(supabase)
|
||||
|
||||
const res = await accountsRoute.handler(
|
||||
makeRequest({
|
||||
connection_id: 'conn-1',
|
||||
enabled_uids: ['acc-1'],
|
||||
account_mappings: [{ uid: 'acc-1', ledger_account: null }],
|
||||
}),
|
||||
ctx
|
||||
)
|
||||
|
||||
expect(res.status).toBe(200)
|
||||
const written = stub.capturedUpdates?.[0]?.accounts_data as StoredAccount[]
|
||||
expect(written.find(a => a.uid === 'acc-1')?.ledger_account).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects account_mappings that is not an array', async () => {
|
||||
const stub: SupabaseStub = {
|
||||
authUser: { id: 'user-1' },
|
||||
connectionRow: {
|
||||
id: 'conn-1',
|
||||
status: 'pending_selection',
|
||||
accounts_data: [{ uid: 'acc-1', currency: 'SEK', enabled: true }],
|
||||
},
|
||||
}
|
||||
const supabase = buildSupabase(stub)
|
||||
const ctx = makeContext(supabase)
|
||||
|
||||
const res = await accountsRoute.handler(
|
||||
makeRequest({
|
||||
connection_id: 'conn-1',
|
||||
enabled_uids: ['acc-1'],
|
||||
account_mappings: 'not-an-array',
|
||||
}),
|
||||
ctx
|
||||
)
|
||||
|
||||
expect(res.status).toBe(400)
|
||||
const body = await res.json()
|
||||
expect(body.error).toMatch(/account_mappings/)
|
||||
})
|
||||
|
||||
it('rejects account_mappings with UIDs not in the connection accounts_data', async () => {
|
||||
// Mirrors the enabled_uids guard. Without this, a typo'd UID is silently
|
||||
// dropped (the entry never lands in accounts_data) while the response is
|
||||
// still 200, leaving the client to believe the mapping was applied.
|
||||
const stub: SupabaseStub = {
|
||||
authUser: { id: 'user-1' },
|
||||
chartAccountNumbers: ['1930', '1932'],
|
||||
connectionRow: {
|
||||
id: 'conn-1',
|
||||
status: 'pending_selection',
|
||||
accounts_data: [{ uid: 'acc-1', currency: 'SEK', enabled: true }],
|
||||
},
|
||||
}
|
||||
const supabase = buildSupabase(stub)
|
||||
const ctx = makeContext(supabase)
|
||||
|
||||
const res = await accountsRoute.handler(
|
||||
makeRequest({
|
||||
connection_id: 'conn-1',
|
||||
enabled_uids: ['acc-1'],
|
||||
account_mappings: [
|
||||
{ uid: 'acc-1', ledger_account: '1930' },
|
||||
{ uid: 'acc-typo', ledger_account: '1932' }, // not in accounts_data
|
||||
],
|
||||
}),
|
||||
ctx
|
||||
)
|
||||
|
||||
expect(res.status).toBe(400)
|
||||
const body = await res.json()
|
||||
expect(body.error).toMatch(/account_mappings/)
|
||||
expect(body.unknown_uids).toEqual(['acc-typo'])
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user