fix(orders): book webshop orders against 1686 and stop the missing-account dead end (#1697)
Booking an order from the Orders page could fail outright on a fresh company. seed_chart_of_accounts() seeds a deliberately small chart: 3001/3002/3003 and 2611/2621/2631 are in it, but 3004, 3740 and the clearing account are not. All three are reachable from an entirely ordinary order (a 0%-rate line, an ore residual, or simply no payment-method mapping yet), and the engine treats a missing or inactive account as AccountsNotInChartError, so the user's first click on Bokfor returned an error naming accounts they had no reason to know about, with no way forward but to hand-add them. The book route now ensures the closed set of accounts our own prefill can emit exists before drafting. Deliberately narrow: only accounts in WEBSHOP_PREFILL_ACCOUNTS are ever created, and only when a submitted line uses one, so an account the user typed still surfaces as a real error instead of quietly growing the chart. A deactivated row is reactivated rather than duplicated, and every failure is swallowed so the engine's typed error still wins over a chart tidy-up. The unmapped default also moves from 1680 to 1686. 1680 is the generic "Andra kortfristiga fordringar" parent; 1686 "Fordringar for kontokort och kuponger" is what BAS defines for a claim on a payment provider, which is what money sitting at Klarna or Stripe actually is. The Stripe extension already settles against 1686, so a store running both surfaces now shares one clearing account instead of splitting the same receivable across two. Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Fable 5
parent
3a1b842e4a
commit
bb5fafe87b
@@ -5,6 +5,7 @@ import {
|
||||
resolveBookingWarnings,
|
||||
resolvePaymentAccount,
|
||||
DEFAULT_PAYMENT_ACCOUNT,
|
||||
WEBSHOP_PREFILL_ACCOUNTS,
|
||||
} from '../booking-lines'
|
||||
import type { CreateJournalEntryLineInput, WebshopStoreSettings } from '@/types'
|
||||
|
||||
@@ -65,13 +66,46 @@ describe('resolvePaymentAccount', () => {
|
||||
expect(result.invoiceMode).toBe(true)
|
||||
})
|
||||
|
||||
it('falls back to 1680 when unmapped or without settings', () => {
|
||||
it('falls back to the 1686 clearing account when unmapped or without settings', () => {
|
||||
expect(resolvePaymentAccount(makeOrder({ payment_method: 'stripe' }), settings).account).toBe(
|
||||
DEFAULT_PAYMENT_ACCOUNT,
|
||||
)
|
||||
expect(resolvePaymentAccount(makeOrder(), null).account).toBe(DEFAULT_PAYMENT_ACCOUNT)
|
||||
expect(resolvePaymentAccount(makeOrder({ payment_method: null }), settings).mapped).toBe(false)
|
||||
})
|
||||
|
||||
it('defaults to BAS 1686, the card/PSP receivable, not the 1680 parent', () => {
|
||||
expect(DEFAULT_PAYMENT_ACCOUNT).toBe('1686')
|
||||
})
|
||||
})
|
||||
|
||||
describe('WEBSHOP_PREFILL_ACCOUNTS', () => {
|
||||
it('covers every account the builder can emit', () => {
|
||||
// Guards the ensure-accounts contract: an account the prefill emits but
|
||||
// this set omits would resurface as AccountsNotInChartError in booking.
|
||||
const emitted = new Set<string>()
|
||||
for (const rate of [25, 12, 6, 0]) {
|
||||
for (const lineSet of [
|
||||
buildOrderBookingLines({
|
||||
order: makeOrder({
|
||||
total: 100.01,
|
||||
total_tax: rate === 0 ? 0 : 20,
|
||||
vat_breakdown: [{ rate, net: 80, tax: rate === 0 ? 0 : 20 }],
|
||||
}),
|
||||
settings: null,
|
||||
}),
|
||||
]) {
|
||||
for (const line of lineSet) emitted.add(line.account_number)
|
||||
}
|
||||
}
|
||||
for (const account of emitted) {
|
||||
expect(WEBSHOP_PREFILL_ACCOUNTS).toContain(account)
|
||||
}
|
||||
// The residual account is only reachable through rounding drift; assert
|
||||
// it explicitly so the set never silently loses it.
|
||||
expect(WEBSHOP_PREFILL_ACCOUNTS).toContain('3740')
|
||||
expect(WEBSHOP_PREFILL_ACCOUNTS).toContain('3004')
|
||||
})
|
||||
})
|
||||
|
||||
describe('fallbackVatBreakdown', () => {
|
||||
|
||||
@@ -0,0 +1,177 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { ensureWebshopPrefillAccounts } from '../ensure-accounts'
|
||||
import type { SupabaseClient } from '@supabase/supabase-js'
|
||||
import type { Logger } from '@/lib/logger'
|
||||
|
||||
/**
|
||||
* Minimal chart_of_accounts double: records what the helper selected,
|
||||
* updated and upserted so each assertion can check the exact call shape.
|
||||
*/
|
||||
function makeSupabase(options: {
|
||||
existing?: Array<{ id: string; account_number: string; is_active: boolean }>
|
||||
selectError?: { message: string }
|
||||
upsertError?: { message: string }
|
||||
updateError?: { message: string }
|
||||
} = {}) {
|
||||
const calls = {
|
||||
selectedIn: [] as string[][],
|
||||
upserted: [] as Array<Record<string, unknown>>,
|
||||
upsertOptions: [] as unknown[],
|
||||
reactivatedIds: [] as string[][],
|
||||
}
|
||||
|
||||
const from = vi.fn((table: string) => {
|
||||
expect(table).toBe('chart_of_accounts')
|
||||
return {
|
||||
select: () => ({
|
||||
eq: () => ({
|
||||
in: (_col: string, values: string[]) => {
|
||||
calls.selectedIn.push(values)
|
||||
return Promise.resolve({
|
||||
data: options.selectError ? null : (options.existing ?? []),
|
||||
error: options.selectError ?? null,
|
||||
})
|
||||
},
|
||||
}),
|
||||
}),
|
||||
update: (_patch: Record<string, unknown>) => ({
|
||||
in: (_col: string, ids: string[]) => {
|
||||
calls.reactivatedIds.push(ids)
|
||||
return {
|
||||
eq: () => Promise.resolve({ error: options.updateError ?? null }),
|
||||
}
|
||||
},
|
||||
}),
|
||||
upsert: (row: Record<string, unknown>, opts: unknown) => {
|
||||
calls.upserted.push(row)
|
||||
calls.upsertOptions.push(opts)
|
||||
return Promise.resolve({ error: options.upsertError ?? null })
|
||||
},
|
||||
}
|
||||
})
|
||||
|
||||
return { supabase: { from } as unknown as SupabaseClient, calls, from }
|
||||
}
|
||||
|
||||
const log = {
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
child: vi.fn(() => log),
|
||||
} as unknown as Logger
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
describe('ensureWebshopPrefillAccounts', () => {
|
||||
it('creates the prefill accounts that a seeded chart is missing', async () => {
|
||||
const { supabase, calls } = makeSupabase({ existing: [] })
|
||||
|
||||
await ensureWebshopPrefillAccounts(supabase, 'company-1', 'user-1', ['1686', '3740'], log)
|
||||
|
||||
// One literal-payload insert per account, so the phantom-column guard can
|
||||
// actually verify the columns.
|
||||
expect(calls.upserted).toHaveLength(2)
|
||||
expect(calls.upserted.map((r) => r.account_number).sort()).toEqual(['1686', '3740'])
|
||||
// BAS metadata comes from the reference, never invented.
|
||||
const clearing = calls.upserted.find((r) => r.account_number === '1686')!
|
||||
expect(clearing.account_name).toBe('Fordringar för kontokort och kuponger')
|
||||
expect(clearing.account_class).toBe(1)
|
||||
expect(clearing.account_type).toBe('asset')
|
||||
expect(clearing.normal_balance).toBe('debit')
|
||||
expect(clearing.company_id).toBe('company-1')
|
||||
expect(clearing.user_id).toBe('user-1')
|
||||
expect(clearing.is_active).toBe(true)
|
||||
// Concurrent bookings must not 23505 each other.
|
||||
expect(calls.upsertOptions[0]).toMatchObject({
|
||||
onConflict: 'company_id,account_number',
|
||||
ignoreDuplicates: true,
|
||||
})
|
||||
})
|
||||
|
||||
it('never creates an account outside the closed prefill set', async () => {
|
||||
const { supabase, calls } = makeSupabase({ existing: [] })
|
||||
|
||||
// 1930 and 4010 are legitimate accounts a user may have picked, and 9999
|
||||
// is a typo. None of them are ours to create.
|
||||
await ensureWebshopPrefillAccounts(
|
||||
supabase,
|
||||
'company-1',
|
||||
'user-1',
|
||||
['1930', '4010', '9999'],
|
||||
log,
|
||||
)
|
||||
|
||||
expect(calls.selectedIn).toHaveLength(0)
|
||||
expect(calls.upserted).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('does nothing when every prefill account is already active', async () => {
|
||||
const { supabase, calls } = makeSupabase({
|
||||
existing: [
|
||||
{ id: 'a', account_number: '1686', is_active: true },
|
||||
{ id: 'b', account_number: '3001', is_active: true },
|
||||
],
|
||||
})
|
||||
|
||||
await ensureWebshopPrefillAccounts(supabase, 'company-1', 'user-1', ['1686', '3001'], log)
|
||||
|
||||
expect(calls.upserted).toHaveLength(0)
|
||||
expect(calls.reactivatedIds).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('reactivates a deactivated account instead of inserting a duplicate', async () => {
|
||||
// The engine treats is_active = false exactly like missing, so a user who
|
||||
// once hid 3740 would otherwise be stuck with an unbookable order.
|
||||
const { supabase, calls } = makeSupabase({
|
||||
existing: [{ id: 'row-3740', account_number: '3740', is_active: false }],
|
||||
})
|
||||
|
||||
await ensureWebshopPrefillAccounts(supabase, 'company-1', 'user-1', ['3740'], log)
|
||||
|
||||
expect(calls.reactivatedIds).toEqual([['row-3740']])
|
||||
expect(calls.upserted).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('deduplicates repeated account numbers from the submitted lines', async () => {
|
||||
const { supabase, calls } = makeSupabase({ existing: [] })
|
||||
|
||||
await ensureWebshopPrefillAccounts(
|
||||
supabase,
|
||||
'company-1',
|
||||
'user-1',
|
||||
['3001', '3001', '2611'],
|
||||
log,
|
||||
)
|
||||
|
||||
expect(calls.selectedIn[0].sort()).toEqual(['2611', '3001'])
|
||||
})
|
||||
|
||||
it('is a no-op for an empty line set', async () => {
|
||||
const { supabase, from } = makeSupabase()
|
||||
await ensureWebshopPrefillAccounts(supabase, 'company-1', 'user-1', [], log)
|
||||
expect(from).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('swallows a lookup failure so booking still reaches the engine', async () => {
|
||||
const { supabase, calls } = makeSupabase({ selectError: { message: 'rls denied' } })
|
||||
|
||||
await expect(
|
||||
ensureWebshopPrefillAccounts(supabase, 'company-1', 'user-1', ['1686'], log),
|
||||
).resolves.toBeUndefined()
|
||||
|
||||
expect(calls.upserted).toHaveLength(0)
|
||||
expect(log.warn).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('swallows an insert failure so booking still reaches the engine', async () => {
|
||||
const { supabase } = makeSupabase({ existing: [], upsertError: { message: 'boom' } })
|
||||
|
||||
await expect(
|
||||
ensureWebshopPrefillAccounts(supabase, 'company-1', 'user-1', ['1686'], log),
|
||||
).resolves.toBeUndefined()
|
||||
|
||||
expect(log.warn).toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -25,8 +25,20 @@ import type {
|
||||
* this while total_sek is null (booking is blocked until FX resolves).
|
||||
*/
|
||||
|
||||
/** Fallback counter-account when no mapping exists: the old feed's ledger. */
|
||||
export const DEFAULT_PAYMENT_ACCOUNT = '1680'
|
||||
/**
|
||||
* Fallback counter-account when no mapping exists.
|
||||
*
|
||||
* BAS 2026 1686 "Fordringar för kontokort och kuponger": money the payment
|
||||
* provider is holding but has not paid out yet. This is the same ledger the
|
||||
* Stripe extension settles against, so a store that runs both surfaces keeps
|
||||
* one clearing account. 1680 "Andra kortfristiga fordringar" was used before
|
||||
* and is the generic parent bucket, not the card/PSP receivable BAS defines
|
||||
* for this; bas.se moved this receivable off 1580 onto 1686 precisely because
|
||||
* it is a claim on the payment provider, not on the customer.
|
||||
*/
|
||||
export const DEFAULT_PAYMENT_ACCOUNT = '1686'
|
||||
/** BAS 2026 name for DEFAULT_PAYMENT_ACCOUNT; used when adding it to a chart. */
|
||||
export const DEFAULT_PAYMENT_ACCOUNT_NAME = 'Fordringar för kontokort och kuponger'
|
||||
|
||||
/** Revenue account per Swedish VAT rate (BAS 2026). */
|
||||
const REVENUE_ACCOUNT_BY_RATE: Record<number, string> = {
|
||||
@@ -46,6 +58,25 @@ const VAT_ACCOUNT_BY_RATE: Record<number, string> = {
|
||||
/** Öresavrundning. */
|
||||
const ROUNDING_ACCOUNT = '3740'
|
||||
|
||||
/**
|
||||
* Every account this prefill can emit, as a closed set.
|
||||
*
|
||||
* seed_chart_of_accounts() seeds a minimal chart: 3001/3002/3003 and
|
||||
* 2611/2621/2631 are in it, but 3004, 3740 and the clearing account are not.
|
||||
* The engine throws AccountsNotInChartError for an account that is missing or
|
||||
* inactive, so an untouched company hit that error the moment an order had a
|
||||
* rounding residual, a 0%-rate line, or no payment-method mapping. Callers
|
||||
* pass this set to ensureWebshopPrefillAccounts() so the accounts our own
|
||||
* prefill needs are added to the chart on first use, and only ever these:
|
||||
* an account the user typed themselves is never auto-created.
|
||||
*/
|
||||
export const WEBSHOP_PREFILL_ACCOUNTS: readonly string[] = [
|
||||
DEFAULT_PAYMENT_ACCOUNT,
|
||||
...Object.values(REVENUE_ACCOUNT_BY_RATE),
|
||||
...Object.values(VAT_ACCOUNT_BY_RATE),
|
||||
ROUNDING_ACCOUNT,
|
||||
]
|
||||
|
||||
/**
|
||||
* Resolve the prefilled payment counter-account for an order from the
|
||||
* per-store mapping. Returns the account plus whether the store marked this
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
import { getBASReference } from '@/lib/bookkeeping/bas-reference'
|
||||
import { WEBSHOP_PREFILL_ACCOUNTS } from './booking-lines'
|
||||
import type { SupabaseClient } from '@supabase/supabase-js'
|
||||
import type { Logger } from '@/lib/logger'
|
||||
|
||||
/**
|
||||
* Add the BAS accounts our own webshop prefill needs to the company's chart,
|
||||
* so booking an order does not fail on AccountsNotInChartError.
|
||||
*
|
||||
* Why this exists: seed_chart_of_accounts() seeds a deliberately small chart.
|
||||
* 3001/3002/3003 and 2611/2621/2631 are in it; 3004 (momsfri försäljning),
|
||||
* 3740 (öresavrundning) and 1686 (the clearing account) are not. Every one of
|
||||
* those is reachable from a perfectly ordinary order: a 0%-rate line, an öre
|
||||
* residual, or simply no payment-method mapping yet. Before this, the user's
|
||||
* first click on Bokför returned "kontot saknas i kontoplanen" with no way
|
||||
* forward except hand-adding accounts they had no reason to know about.
|
||||
*
|
||||
* Deliberately narrow: only account numbers in WEBSHOP_PREFILL_ACCOUNTS are
|
||||
* ever created, and only when a submitted line actually uses one. An account
|
||||
* the user typed or picked themselves is never auto-created, so a typo still
|
||||
* surfaces as a real error instead of quietly growing the chart.
|
||||
*
|
||||
* Reactivates a soft-deleted (is_active = false) row rather than inserting a
|
||||
* duplicate: the engine treats inactive exactly like missing, and the unique
|
||||
* (company_id, account_number) index would reject the insert anyway.
|
||||
*
|
||||
* Non-fatal by design. If this cannot write, booking proceeds and the engine
|
||||
* raises its normal typed error; we never block a verifikat on a chart tidy-up.
|
||||
*/
|
||||
export async function ensureWebshopPrefillAccounts(
|
||||
supabase: SupabaseClient,
|
||||
companyId: string,
|
||||
userId: string,
|
||||
accountNumbers: string[],
|
||||
log?: Logger,
|
||||
): Promise<void> {
|
||||
const wanted = [...new Set(accountNumbers)].filter((n) =>
|
||||
WEBSHOP_PREFILL_ACCOUNTS.includes(n),
|
||||
)
|
||||
if (wanted.length === 0) return
|
||||
|
||||
try {
|
||||
const { data: existing, error } = await supabase
|
||||
.from('chart_of_accounts')
|
||||
.select('id, account_number, is_active')
|
||||
.eq('company_id', companyId)
|
||||
.in('account_number', wanted)
|
||||
if (error) {
|
||||
log?.warn('webshop chart lookup failed; booking continues', { error: error.message })
|
||||
return
|
||||
}
|
||||
|
||||
const present = new Map<string, { id: string; is_active: boolean }>()
|
||||
for (const row of existing ?? []) {
|
||||
present.set(row.account_number as string, {
|
||||
id: row.id as string,
|
||||
is_active: row.is_active as boolean,
|
||||
})
|
||||
}
|
||||
|
||||
const toReactivate = wanted
|
||||
.map((n) => present.get(n))
|
||||
.filter((row): row is { id: string; is_active: boolean } => !!row && !row.is_active)
|
||||
.map((row) => row.id)
|
||||
if (toReactivate.length > 0) {
|
||||
const { error: reactivateError } = await supabase
|
||||
.from('chart_of_accounts')
|
||||
.update({ is_active: true })
|
||||
.in('id', toReactivate)
|
||||
.eq('company_id', companyId)
|
||||
if (reactivateError) {
|
||||
log?.warn('webshop chart reactivate failed; booking continues', {
|
||||
error: reactivateError.message,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
const missing = wanted.filter((n) => !present.has(n))
|
||||
if (missing.length === 0) return
|
||||
|
||||
// Inserted one row at a time with a literal payload on purpose: the
|
||||
// no-phantom-columns guard can only verify columns it can resolve
|
||||
// statically, and a .map()-built array reads as an opaque expression. At
|
||||
// most eight accounts, only on first use, so the extra round trips are
|
||||
// cheaper than an unverifiable insert.
|
||||
for (const accountNumber of missing) {
|
||||
// Every WEBSHOP_PREFILL_ACCOUNTS member is a real BAS 2026 account, so a
|
||||
// miss here means the reference data drifted: skip rather than invent
|
||||
// metadata for an account we cannot describe.
|
||||
const bas = getBASReference(accountNumber)
|
||||
if (!bas) {
|
||||
log?.warn('no BAS reference for webshop prefill account', { accountNumber })
|
||||
continue
|
||||
}
|
||||
// Concurrent bookings of two orders race here; ignoreDuplicates makes
|
||||
// the loser a no-op instead of a 23505 that would fail a fine entry.
|
||||
const { error: insertError } = await supabase.from('chart_of_accounts').upsert(
|
||||
{
|
||||
user_id: userId,
|
||||
company_id: companyId,
|
||||
account_number: accountNumber,
|
||||
account_name: bas.account_name,
|
||||
account_class: bas.account_class,
|
||||
account_group: bas.account_group,
|
||||
account_type: bas.account_type,
|
||||
normal_balance: bas.normal_balance,
|
||||
sru_code: bas.sru_code ?? null,
|
||||
k2_excluded: bas.k2_excluded ?? false,
|
||||
plan_type: 'full_bas',
|
||||
is_active: true,
|
||||
is_system_account: false,
|
||||
},
|
||||
{ onConflict: 'company_id,account_number', ignoreDuplicates: true },
|
||||
)
|
||||
if (insertError) {
|
||||
log?.warn('webshop chart insert failed; booking continues', {
|
||||
accountNumber,
|
||||
error: insertError.message,
|
||||
})
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
log?.warn('webshop chart ensure threw; booking continues', {
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user