Bug/mcp bas lag (#543)
* fix(mcp): update workflow descriptions for transaction categorization and approval processes * feat: implement account validation for transaction categorization to handle inactive accounts * fix(tests): stub findMissingAccountsMock to ensure no missing accounts during batch-categorize tests * fix(errors): ensure deterministic sorting of account numbers in AccountsNotInChartError
This commit is contained in:
@@ -26,6 +26,53 @@ describe('Typed bookkeeping errors', () => {
|
||||
expect(err).toBeInstanceOf(Error)
|
||||
})
|
||||
|
||||
it('AccountsNotInChartError ordering is deterministic across input permutations', () => {
|
||||
// The user-facing toast lists which accounts to activate in Kontoplan.
|
||||
// If the same set of missing accounts came back in different orders on
|
||||
// each call, the user might mistake an identical error for a different
|
||||
// one. Lock in: same set → same array, regardless of input order.
|
||||
const sets: string[][] = [
|
||||
['5410', '2641'],
|
||||
['2641', '5410'],
|
||||
['5410', '5410', '2641', '2641'],
|
||||
['2641', '2641', '5410'],
|
||||
]
|
||||
const outputs = sets.map((s) => new AccountsNotInChartError(s).accountNumbers)
|
||||
for (const out of outputs) {
|
||||
expect(out).toEqual(['2641', '5410'])
|
||||
}
|
||||
// And the rendered message stays stable too.
|
||||
expect(new AccountsNotInChartError(['5410', '2641']).message).toBe(
|
||||
new AccountsNotInChartError(['2641', '5410']).message,
|
||||
)
|
||||
})
|
||||
|
||||
it('AccountsNotInChartError sorts numerically, not by UTF-16 code units', () => {
|
||||
// Default string sort would order ['245', '1930'] as ['1930', '245']
|
||||
// because '1' < '2'. That's wrong for accounts — a user looking at the
|
||||
// toast and walking down their kontoplan expects numeric order.
|
||||
const err = new AccountsNotInChartError(['1930', '245', '5410'])
|
||||
expect(err.accountNumbers).toEqual(['245', '1930', '5410'])
|
||||
})
|
||||
|
||||
it('AccountsNotInChartError tie-breaks numerically-equal strings deterministically', () => {
|
||||
// "0245" and "245" compare as Number-equal but are distinct strings; the
|
||||
// comparator must not collapse them by returning 0 unstably. We don't
|
||||
// emit zero-padded BAS codes anywhere internally, but the public surface
|
||||
// shouldn't be sensitive to that defensive case.
|
||||
const err = new AccountsNotInChartError(['245', '0245'])
|
||||
expect(err.accountNumbers).toEqual(['0245', '245'])
|
||||
})
|
||||
|
||||
it('AccountsNotInChartError preserves order across multiple calls with same data', () => {
|
||||
// Same inputs ⇒ identical outputs across separate calls (no hidden
|
||||
// randomness, no Set iteration drift, no dependence on call order).
|
||||
const calls = Array.from({ length: 5 }, () => new AccountsNotInChartError(['5410', '1930', '2641']).accountNumbers)
|
||||
for (let i = 1; i < calls.length; i++) {
|
||||
expect(calls[i]).toEqual(calls[0])
|
||||
}
|
||||
})
|
||||
|
||||
it('JournalEntryNotBalancedError preserves amounts and kind', () => {
|
||||
const err = new JournalEntryNotBalancedError(100, 80, 'correction')
|
||||
expect(err.code).toBe('JOURNAL_ENTRY_NOT_BALANCED')
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import type { SupabaseClient } from '@supabase/supabase-js'
|
||||
import type { MappingResult } from '@/types'
|
||||
|
||||
/**
|
||||
* Return the subset of `accountNumbers` that are NOT present-and-active in the
|
||||
* given company's chart_of_accounts. Mirrors the engine's resolveAccountIds
|
||||
* (lib/bookkeeping/engine.ts) so a pre-validation in API routes catches the
|
||||
* same condition (AccountsNotInChartError) before any DB writes happen.
|
||||
*
|
||||
* Empty/duplicate inputs are normalised; preserves first-seen order in output.
|
||||
* On Supabase error: bubbles up. A chart-of-accounts read failure is real
|
||||
* infrastructure trouble and should surface as 500 rather than be silently
|
||||
* masked as "account missing".
|
||||
*/
|
||||
export async function findMissingActiveAccounts(
|
||||
supabase: SupabaseClient,
|
||||
companyId: string,
|
||||
accountNumbers: readonly string[],
|
||||
): Promise<string[]> {
|
||||
const seen = new Set<string>()
|
||||
const unique: string[] = []
|
||||
for (const num of accountNumbers) {
|
||||
if (!num) continue
|
||||
if (seen.has(num)) continue
|
||||
seen.add(num)
|
||||
unique.push(num)
|
||||
}
|
||||
if (unique.length === 0) return []
|
||||
|
||||
const { data, error } = await supabase
|
||||
.from('chart_of_accounts')
|
||||
.select('account_number')
|
||||
.eq('company_id', companyId)
|
||||
.eq('is_active', true)
|
||||
.in('account_number', unique)
|
||||
|
||||
if (error) throw error
|
||||
|
||||
const present = new Set<string>((data ?? []).map((r) => r.account_number as string))
|
||||
return unique.filter((n) => !present.has(n))
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract every chart account number a MappingResult will post to: the headline
|
||||
* debit/credit plus every account_number in vat_lines. Returns the raw list
|
||||
* (duplicates intact); pass through findMissingActiveAccounts to dedupe.
|
||||
*/
|
||||
export function collectMappingResultAccounts(mr: MappingResult): string[] {
|
||||
const out: string[] = []
|
||||
if (mr.debit_account) out.push(mr.debit_account)
|
||||
if (mr.credit_account) out.push(mr.credit_account)
|
||||
for (const line of mr.vat_lines ?? []) {
|
||||
if (line.account_number) out.push(line.account_number)
|
||||
}
|
||||
return out
|
||||
}
|
||||
@@ -25,13 +25,35 @@ export class AccountsNotInChartError extends Error {
|
||||
readonly accountNumbers: string[]
|
||||
|
||||
constructor(accountNumbers: string[]) {
|
||||
const sorted = [...new Set(accountNumbers)].sort()
|
||||
// Numeric-first sort so mixed-length BAS codes (rare but possible) order
|
||||
// by value rather than by UTF-16 code units — otherwise ['245', '1930']
|
||||
// would sort to ['1930', '245'] under the default string comparator,
|
||||
// confusing a user about which accounts to activate in Kontoplan.
|
||||
// Non-numeric tokens fall back to a stable string compare so the order
|
||||
// is fully deterministic for any input.
|
||||
const sorted = [...new Set(accountNumbers)].sort(compareAccountNumbers)
|
||||
super(`Accounts not enabled in chart of accounts: ${sorted.join(', ')}`)
|
||||
this.name = 'AccountsNotInChartError'
|
||||
this.accountNumbers = sorted
|
||||
}
|
||||
}
|
||||
|
||||
function compareAccountNumbers(a: string, b: string): number {
|
||||
const na = Number(a)
|
||||
const nb = Number(b)
|
||||
const aIsNum = Number.isFinite(na)
|
||||
const bIsNum = Number.isFinite(nb)
|
||||
if (aIsNum && bIsNum) {
|
||||
if (na !== nb) return na - nb
|
||||
// Same numeric value but different string (e.g. "0245" vs "245") —
|
||||
// break the tie deterministically by string.
|
||||
return a < b ? -1 : a > b ? 1 : 0
|
||||
}
|
||||
if (aIsNum) return -1
|
||||
if (bIsNum) return 1
|
||||
return a < b ? -1 : a > b ? 1 : 0
|
||||
}
|
||||
|
||||
export function isAccountsNotInChartError(err: unknown): err is AccountsNotInChartError {
|
||||
return err instanceof AccountsNotInChartError
|
||||
}
|
||||
@@ -203,7 +225,12 @@ export function accountsNotInChartResponse(err: AccountsNotInChartError) {
|
||||
error: {
|
||||
code: err.code,
|
||||
message: `Följande konton behöver aktiveras: ${err.accountNumbers.join(', ')}`,
|
||||
// Dual-emit: top-level for legacy frontend callers, nested under
|
||||
// `details` to match the v1 envelope shape so a single client (MCP
|
||||
// or external) can read `error.details.account_numbers` regardless
|
||||
// of which categorize endpoint it hit.
|
||||
account_numbers: err.accountNumbers,
|
||||
details: { account_numbers: err.accountNumbers },
|
||||
},
|
||||
},
|
||||
{ status: 400 }
|
||||
|
||||
Reference in New Issue
Block a user