* fix(transactions): bind manually-fed transactions to a cash account (#1016) create_transactions inserted rows with cash_account_id = null, so ledger accounts fed via MCP/CSV without a PSD2 feed (e.g. 1935 Wise SEK) had no kassakonto: get_reconciliation_status 404'd with "Okänt kassakonto" and the "Matcha mot befintlig verifikation" dialog fell back to 1930. No schema change: cash_accounts.bank_connection_id is already nullable and source='manual' already exists (every company is seeded a manual 1930). This is the creation-side leg of the #985-#987 root cause: the resolution chain was fixed, but manually-fed accounts never got the cash_account_id link. - Add ensureManualCashAccount (lib/cash-accounts/service.ts): find-or-create a manual (source='manual', bank_connection_id=null) cash_accounts row for a ledger slot, tolerating the (company_id, ledger_account) UNIQUE race. - Add an optional ledger_account hint (^19xx) to gnubok_create_transactions; commitCreateTransaction resolves/creates the manual account and sets cash_account_id on the inserted row. Reconciliation and voucher matching then resolve the real account unchanged. Forward-looking; historical cash_account_id=null remediation stays in #1001. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Alexander Reinthal <email@reinthal.me> * fix(cash-accounts): guard ensureManualCashAccount against currency mismatch (CodeRabbit #1017) The existing-row lookup matched only on (company_id, ledger_account) and returned the row id ignoring currency, so a SEK transaction hinting at a ledger already claimed for USD would bind to the wrong-currency cash account. Since that pair is UNIQUE (one currency per ledger), a mismatch is a real conflict: throw instead of silently mis-binding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Alexander Reinthal <email@reinthal.me> --------- Signed-off-by: Alexander Reinthal <email@reinthal.me> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Jakob Wennberg <jakob.wennberg@gmail.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
Jakob Wennberg
parent
aa5edd3aa7
commit
f8033cb32d
@@ -16,6 +16,7 @@ import {
|
||||
defaultLedgerForCurrency,
|
||||
getRevokedConnectionIds,
|
||||
upsertFromPsd2,
|
||||
ensureManualCashAccount,
|
||||
} from '../service'
|
||||
|
||||
type CashRow = { ledger_account: string; bank_connection_id: string | null }
|
||||
@@ -556,3 +557,110 @@ describe('upsertFromPsd2', () => {
|
||||
).rejects.toThrow(/duplicate key/)
|
||||
})
|
||||
})
|
||||
|
||||
// ── ensureManualCashAccount ──────────────────────────────────────────────
|
||||
|
||||
interface ManualStub {
|
||||
lookup: { data: { id: string; currency?: string } | null; error?: { message: string } | null }
|
||||
insert?: { data: { id: string } | null; error?: { message: string; code?: string } | null }
|
||||
reread?: { data: { id: string } | null; error?: { message: string } | null }
|
||||
inserted: Array<Record<string, unknown>>
|
||||
lookupCount: number
|
||||
}
|
||||
|
||||
function makeManualSupabase(stub: ManualStub): SupabaseClient {
|
||||
return {
|
||||
from: vi.fn((table: string) => {
|
||||
expect(table).toBe('cash_accounts')
|
||||
return {
|
||||
// lookup / reread path: select().eq().eq().maybeSingle()
|
||||
select: vi.fn(() => ({
|
||||
eq: vi.fn(() => ({
|
||||
eq: vi.fn(() => ({
|
||||
maybeSingle: vi.fn(() => {
|
||||
stub.lookupCount += 1
|
||||
// First maybeSingle = initial lookup; a second = post-23505 reread.
|
||||
const r = stub.lookupCount === 1 ? stub.lookup : stub.reread ?? { data: null }
|
||||
return Promise.resolve({ data: r.data, error: r.error ?? null })
|
||||
}),
|
||||
})),
|
||||
})),
|
||||
})),
|
||||
// insert().select('id').single()
|
||||
insert: vi.fn((payload: Record<string, unknown>) => {
|
||||
stub.inserted.push(payload)
|
||||
return {
|
||||
select: vi.fn(() => ({
|
||||
single: vi.fn(() =>
|
||||
Promise.resolve({
|
||||
data: stub.insert?.data ?? null,
|
||||
error: stub.insert?.error ?? null,
|
||||
}),
|
||||
),
|
||||
})),
|
||||
}
|
||||
}),
|
||||
}
|
||||
}),
|
||||
} as unknown as SupabaseClient
|
||||
}
|
||||
|
||||
describe('ensureManualCashAccount', () => {
|
||||
it('returns the existing row id without inserting when the currency matches', async () => {
|
||||
const stub: ManualStub = { lookup: { data: { id: 'ca-1', currency: 'SEK' } }, inserted: [], lookupCount: 0 }
|
||||
const id = await ensureManualCashAccount(makeManualSupabase(stub), 'c1', '1935', 'sek')
|
||||
expect(id).toBe('ca-1')
|
||||
expect(stub.inserted).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('throws when the existing row is a different currency (UNIQUE ledger conflict)', async () => {
|
||||
const stub: ManualStub = { lookup: { data: { id: 'ca-usd', currency: 'USD' } }, inserted: [], lookupCount: 0 }
|
||||
await expect(
|
||||
ensureManualCashAccount(makeManualSupabase(stub), 'c1', '1935', 'SEK'),
|
||||
).rejects.toThrow(/denominated in USD, not SEK/)
|
||||
expect(stub.inserted).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('creates a manual row (source=manual, uppercased currency) when none exists', async () => {
|
||||
const stub: ManualStub = {
|
||||
lookup: { data: null },
|
||||
insert: { data: { id: 'ca-new' } },
|
||||
inserted: [],
|
||||
lookupCount: 0,
|
||||
}
|
||||
const id = await ensureManualCashAccount(makeManualSupabase(stub), 'c1', '1935', 'sek')
|
||||
expect(id).toBe('ca-new')
|
||||
expect(stub.inserted[0]).toMatchObject({
|
||||
company_id: 'c1',
|
||||
ledger_account: '1935',
|
||||
currency: 'SEK',
|
||||
source: 'manual',
|
||||
is_primary: false,
|
||||
enabled: true,
|
||||
})
|
||||
})
|
||||
|
||||
it('re-reads the winner on a 23505 race instead of throwing', async () => {
|
||||
const stub: ManualStub = {
|
||||
lookup: { data: null },
|
||||
insert: { data: null, error: { message: 'duplicate key', code: '23505' } },
|
||||
reread: { data: { id: 'ca-winner' } },
|
||||
inserted: [],
|
||||
lookupCount: 0,
|
||||
}
|
||||
const id = await ensureManualCashAccount(makeManualSupabase(stub), 'c1', '1935', 'SEK')
|
||||
expect(id).toBe('ca-winner')
|
||||
})
|
||||
|
||||
it('throws on a non-race insert failure', async () => {
|
||||
const stub: ManualStub = {
|
||||
lookup: { data: null },
|
||||
insert: { data: null, error: { message: 'boom' } },
|
||||
inserted: [],
|
||||
lookupCount: 0,
|
||||
}
|
||||
await expect(
|
||||
ensureManualCashAccount(makeManualSupabase(stub), 'c1', '1935', 'SEK'),
|
||||
).rejects.toThrow(/boom/)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -486,6 +486,89 @@ export async function upsertFromPsd2(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Find (or create) a manual cash account for a BAS ledger slot, so transactions
|
||||
* ingested outside the PSD2 flow (create_transactions / CSV) can carry a real
|
||||
* cash_account_id instead of NULL. Without the link, reconciliation 404s on the
|
||||
* account and the match dialog falls back to 1930 (issue #1016).
|
||||
*
|
||||
* Manual rows (source='manual', bank_connection_id=null) are already first-class:
|
||||
* every company is seeded a manual 1930 the same way, and upsertFromPsd2 promotes
|
||||
* a manual holder in place if a bank later claims the slot. So pre-creating one
|
||||
* here does NOT race the PSD2 sync (the concern noted in lib/transactions/ingest.ts):
|
||||
* the worst case is a later connection promoting this row, which is the intended flow.
|
||||
*
|
||||
* Keyed on the (company_id, ledger_account) UNIQUE constraint: a concurrent
|
||||
* insert surfaces as 23505, which we treat as "someone else won the race" and
|
||||
* re-read. The row's currency follows the first transaction that created it; a
|
||||
* ledger account holds one currency by that same constraint.
|
||||
*/
|
||||
export async function ensureManualCashAccount(
|
||||
supabase: SupabaseClient,
|
||||
companyId: string,
|
||||
ledgerAccount: string,
|
||||
currency: string,
|
||||
name?: string | null,
|
||||
): Promise<string> {
|
||||
const existing = await supabase
|
||||
.from('cash_accounts')
|
||||
.select('id, currency')
|
||||
.eq('company_id', companyId)
|
||||
.eq('ledger_account', ledgerAccount)
|
||||
.maybeSingle()
|
||||
if (existing.error) {
|
||||
throw new Error(`ensureManualCashAccount lookup failed: ${existing.error.message}`)
|
||||
}
|
||||
if (existing.data) {
|
||||
const row = existing.data as { id: string; currency: string | null }
|
||||
// (company_id, ledger_account) is UNIQUE, so a ledger holds exactly one
|
||||
// currency. A different-currency transaction pointing at the same ledger is
|
||||
// a real conflict (e.g. a SEK row landing on a ledger already claimed for
|
||||
// USD): fail loudly instead of binding it to the wrong-currency account.
|
||||
if (row.currency && row.currency.toUpperCase() !== currency.toUpperCase()) {
|
||||
throw new Error(
|
||||
`Cash account ${ledgerAccount} is denominated in ${row.currency}, not ${currency.toUpperCase()}`,
|
||||
)
|
||||
}
|
||||
return row.id
|
||||
}
|
||||
|
||||
const insert = await supabase
|
||||
.from('cash_accounts')
|
||||
.insert({
|
||||
company_id: companyId,
|
||||
ledger_account: ledgerAccount,
|
||||
currency: currency.toUpperCase(),
|
||||
name: name?.trim() || `Bankkonto ${currency.toUpperCase()}`,
|
||||
enabled: true,
|
||||
is_primary: false,
|
||||
source: 'manual' as CashAccountSource,
|
||||
})
|
||||
.select('id')
|
||||
.single()
|
||||
|
||||
if (insert.error) {
|
||||
// Lost the (company_id, ledger_account) race: re-read the winner's row.
|
||||
if (insert.error.code === '23505') {
|
||||
const reread = await supabase
|
||||
.from('cash_accounts')
|
||||
.select('id')
|
||||
.eq('company_id', companyId)
|
||||
.eq('ledger_account', ledgerAccount)
|
||||
.maybeSingle()
|
||||
if (reread.data) return (reread.data as { id: string }).id
|
||||
}
|
||||
log.error('ensureManualCashAccount insert failed', {
|
||||
companyId,
|
||||
ledgerAccount,
|
||||
error: insert.error.message,
|
||||
})
|
||||
throw new Error(`ensureManualCashAccount insert failed: ${insert.error.message}`)
|
||||
}
|
||||
|
||||
return (insert.data as { id: string }).id
|
||||
}
|
||||
|
||||
/**
|
||||
* Toggle a cash account's enabled flag. Used by the AccountPicker when a user
|
||||
* opts in or out of syncing a particular PSD2 account.
|
||||
|
||||
@@ -304,6 +304,53 @@ describe('commitPendingOperation: create_transaction', () => {
|
||||
expect(result.http_status).toBe(409)
|
||||
expect(result.error).toMatch(/already exists/)
|
||||
})
|
||||
|
||||
it('binds cash_account_id when ledger_account is given, creating the manual account', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
|
||||
enqueue({ data: null, error: null }) // ensureManualCashAccount lookup miss
|
||||
enqueue({ data: { id: 'ca-1935' }, error: null }) // ensureManualCashAccount insert
|
||||
enqueue({ data: { id: 'tx-9' }, error: null }) // executor transactions insert
|
||||
enqueue({ data: null, error: null }) // dispatcher's update
|
||||
|
||||
const op = makePendingOp({
|
||||
operation_type: 'create_transaction',
|
||||
params: {
|
||||
date: '2026-05-01',
|
||||
amount: -200,
|
||||
description: 'WISE KORT ST',
|
||||
currency: 'SEK',
|
||||
ledger_account: '1935',
|
||||
},
|
||||
})
|
||||
|
||||
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
|
||||
|
||||
expect(result.status).toBe('committed')
|
||||
expect(result.data).toMatchObject({ transaction_id: 'tx-9' })
|
||||
})
|
||||
|
||||
it('rejects a non-19xx ledger_account with 400', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
|
||||
enqueue({ data: null, error: null }) // dispatcher's reject update
|
||||
|
||||
const op = makePendingOp({
|
||||
operation_type: 'create_transaction',
|
||||
params: {
|
||||
date: '2026-05-01',
|
||||
amount: -200,
|
||||
description: 'WISE KORT ST',
|
||||
ledger_account: '3001',
|
||||
},
|
||||
})
|
||||
|
||||
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
|
||||
|
||||
expect(result.status).toBe('failed')
|
||||
expect(result.http_status).toBe(400)
|
||||
expect(result.error).toMatch(/19xx cash account/)
|
||||
})
|
||||
})
|
||||
|
||||
// ─── import_sie ─────────────────────────────────────────────────────
|
||||
|
||||
@@ -26,6 +26,7 @@ import {
|
||||
createCreditNoteJournalEntry,
|
||||
} from '@/lib/bookkeeping/invoice-entries'
|
||||
import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account'
|
||||
import { ensureManualCashAccount } from '@/lib/cash-accounts/service'
|
||||
import { createJournalEntry, findFiscalPeriod, reverseEntry, validateBalance } from '@/lib/bookkeeping/engine'
|
||||
import { coerceDimensionsBag } from '@/lib/bookkeeping/dimension-resolver'
|
||||
import { cancelOrphanedPaymentEntry } from '@/lib/bookkeeping/cancel-orphaned-entry'
|
||||
@@ -648,12 +649,25 @@ async function commitCreateTransaction(
|
||||
const amount = Number(params.amount)
|
||||
const description = (params.description as string) ?? ''
|
||||
const currency = ((params.currency as string) || 'SEK') as Currency
|
||||
const ledgerAccount = (params.ledger_account as string) || null
|
||||
const bankConnectionId = (params.bank_connection_id as string) || null
|
||||
const externalId = (params.external_id as string) || null
|
||||
|
||||
if (!date || !description.trim() || !Number.isFinite(amount)) {
|
||||
return { error: 'date, description, and amount are required', status: 400 }
|
||||
}
|
||||
if (ledgerAccount && !/^19\d{2}$/.test(ledgerAccount)) {
|
||||
return { error: 'ledger_account must be a BAS 19xx cash account', status: 400 }
|
||||
}
|
||||
|
||||
// Bind the row to a manual kassakonto when a ledger account is given, so
|
||||
// reconciliation and voucher matching resolve the real account instead of
|
||||
// falling back to 1930 (issue #1016). Find-or-create; the row's currency
|
||||
// follows this transaction.
|
||||
let cashAccountId: string | null = null
|
||||
if (ledgerAccount) {
|
||||
cashAccountId = await ensureManualCashAccount(supabase, companyId, ledgerAccount, currency)
|
||||
}
|
||||
|
||||
const { data, error } = await supabase
|
||||
.from('transactions')
|
||||
@@ -661,6 +675,7 @@ async function commitCreateTransaction(
|
||||
user_id: userId,
|
||||
company_id: companyId,
|
||||
bank_connection_id: bankConnectionId,
|
||||
cash_account_id: cashAccountId,
|
||||
external_id: externalId,
|
||||
date,
|
||||
description: description.trim(),
|
||||
|
||||
Reference in New Issue
Block a user