fix(webshop-orders): shared effective-rate helper and order-context refusal for the rate-0 slot (#1912) (#2008)
* fix(webshop): share rate classification and check rate-0 order context in bulk book (#1912) The bulk revenue template's guard copied fetchDynamicVatAccounts' effective-rate precedence (explicit momssats > treatment > class-3 number+name inference), so the two could drift. Both now call one exported helper, resolveEffectiveVatRate, and a sibling resolveRevenueVatBox resolves the momsdeklaration box for a revenue account (treatment ruta first, then the static BAS map). The rate-0 slot also ignored order context: a domestic 0% order could be routed to an export account (ruta 36) and vice versa, misstating rutor 35-42 with no VAT amount to catch it. The sweep now refuses, per order, a 0% bucket whose billing country contradicts the chosen account's box: ruta 36 vs SE or an EU country, ruta 40 vs SE, ruta 35/38/39 vs SE or a non-EU country. Unknown country (Shopify), domestic boxes (42/41/07) and unclassified accounts are unchanged; the domestic-account + foreign- country direction stays advisory in the dialog. Item 1 of the issue (require a positive momsfri/export/EU classification for the slot) is deferred: most such accounts are unconfigured today and the strict rule needs a configure path first (DECISIONS.md). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna * fix(webshop): address review findings (#1912) - Finding 1: the rate-0 context guard keys on customer_country, which the WooCommerce sync stores from the billing address; the goods boxes 35/36/38 follow the delivery destination, so a Swedish-billed order shipped outside the EU is a legitimate ruta 36 export the sweep refuses. Soften the WEBSHOP_ORDER_ZERO_RATE_CONTEXT_MISMATCH copy (sv/en) to say the check is based on the billing country and the account may still be right for the delivery address, and ask the user to confirm rather than change the account. Document the limitation in the route comment; storing shipping country in the sync is a follow-up. Test pins the new wording. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna --------- 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:
Jakob Wennberg
co-authored by
Claude Fable 5
Jakob Wennberg
Claude Fable 5
Jakob Wennberg
parent
22f0647d6c
commit
33a58bec51
@@ -919,4 +919,259 @@ describe('POST /api/webshop-orders/bulk-book', () => {
|
||||
expect(body.data.results).toHaveLength(1)
|
||||
expect(mockCreateDraftEntry).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
describe('rate-0 slot order context (#1912)', () => {
|
||||
const ZERO_RATE_ORDER = {
|
||||
total: 500,
|
||||
total_sek: 500,
|
||||
total_tax: 0,
|
||||
vat_breakdown: [{ rate: 0, net: 500, tax: 0 }],
|
||||
}
|
||||
const zeroRateOrder = (
|
||||
id: string,
|
||||
customer_country: string | null,
|
||||
extra: Record<string, unknown> = {},
|
||||
) => makeOrderRow({ id, ...ZERO_RATE_ORDER, customer_country, ...extra })
|
||||
|
||||
const chartRow = (
|
||||
account_number: string,
|
||||
account_name: string,
|
||||
default_vat_treatment: string | null = null,
|
||||
) => ({
|
||||
account_number,
|
||||
account_name,
|
||||
is_active: true,
|
||||
default_vat_rate: null,
|
||||
default_vat_treatment,
|
||||
})
|
||||
const EXPORT_GOODS = chartRow('3105', 'Försäljning varor till land utanför EU')
|
||||
const EU_GOODS = chartRow('3108', 'Försäljning varor till annat EU-land')
|
||||
const EXPORT_SERVICES = chartRow('3305', 'Försäljning tjänster utanför EU')
|
||||
|
||||
async function runOne(
|
||||
order: Record<string, unknown>,
|
||||
chart: Record<string, unknown>,
|
||||
account: string,
|
||||
claim: boolean,
|
||||
) {
|
||||
enqueue({ data: [order] })
|
||||
enqueue({ data: [] }) // store settings
|
||||
enqueue({ data: [chart] }) // chart check
|
||||
if (claim) enqueue({ data: [{ id: order.id }] })
|
||||
return parseJsonResponse<BulkResponse>(
|
||||
await postBulk({ order_ids: [order.id], revenue_accounts: { '0': account } }),
|
||||
)
|
||||
}
|
||||
|
||||
it('refuses an export goods account (ruta 36) for a Swedish billing country', async () => {
|
||||
const { status, body } = await runOne(
|
||||
zeroRateOrder(ORDER_1, 'SE'),
|
||||
EXPORT_GOODS,
|
||||
'3105',
|
||||
false,
|
||||
)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data.failed_count).toBe(1)
|
||||
expect(body.data.results[0].error?.code).toBe(
|
||||
'WEBSHOP_ORDER_ZERO_RATE_CONTEXT_MISMATCH',
|
||||
)
|
||||
expect(body.data.results[0].error?.details).toEqual({
|
||||
account: '3105',
|
||||
box: '36',
|
||||
customer_country: 'SE',
|
||||
})
|
||||
expect(body.data.results[0].error?.message).toContain('ruta 35-42')
|
||||
// The check keys on the billing country only; the copy must say so and
|
||||
// must not assert that the account is wrong (a Swedish-billed order
|
||||
// shipped outside the EU is a legitimate ruta 36 export).
|
||||
expect(body.data.results[0].error?.message).toContain('faktureringslandet')
|
||||
expect(body.data.results[0].error?.message).toContain('leveransadressen')
|
||||
expect(body.data.results[0].error?.message).not.toContain('välj rätt konto')
|
||||
expect(mockCreateDraftEntry).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('refuses an export goods account (ruta 36) for an EU billing country', async () => {
|
||||
// Varuförsäljning to another EU country is ruta 35, never ruta 36.
|
||||
const { body } = await runOne(zeroRateOrder(ORDER_1, 'de'), EXPORT_GOODS, '3105', false)
|
||||
expect(body.data.results[0].error?.code).toBe(
|
||||
'WEBSHOP_ORDER_ZERO_RATE_CONTEXT_MISMATCH',
|
||||
)
|
||||
expect(body.data.results[0].error?.details?.customer_country).toBe('DE')
|
||||
expect(mockCreateDraftEntry).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('books an export goods account for a non-EU billing country', async () => {
|
||||
const { status, body } = await runOne(
|
||||
zeroRateOrder(ORDER_1, 'US'),
|
||||
EXPORT_GOODS,
|
||||
'3105',
|
||||
true,
|
||||
)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data.booked_count).toBe(1)
|
||||
const lines = (
|
||||
mockCreateDraftEntry.mock.calls[0][3] as {
|
||||
lines: { account_number: string; credit_amount: number }[]
|
||||
}
|
||||
).lines
|
||||
expect(lines.find((l) => l.account_number === '3105')?.credit_amount).toBe(500)
|
||||
})
|
||||
|
||||
it('refuses an EU goods account (ruta 35) for a non-EU or Swedish billing country', async () => {
|
||||
const us = await runOne(zeroRateOrder(ORDER_1, 'US'), EU_GOODS, '3108', false)
|
||||
expect(us.body.data.results[0].error?.code).toBe(
|
||||
'WEBSHOP_ORDER_ZERO_RATE_CONTEXT_MISMATCH',
|
||||
)
|
||||
expect(us.body.data.results[0].error?.details?.box).toBe('35')
|
||||
|
||||
reset()
|
||||
const se = await runOne(zeroRateOrder(ORDER_1, 'SE'), EU_GOODS, '3108', false)
|
||||
expect(se.body.data.results[0].error?.code).toBe(
|
||||
'WEBSHOP_ORDER_ZERO_RATE_CONTEXT_MISMATCH',
|
||||
)
|
||||
expect(mockCreateDraftEntry).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('books an EU goods account for another EU billing country', async () => {
|
||||
const { body } = await runOne(zeroRateOrder(ORDER_1, 'DE'), EU_GOODS, '3108', true)
|
||||
expect(body.data.booked_count).toBe(1)
|
||||
})
|
||||
|
||||
it('refuses a services-abroad account (ruta 40) only for Sweden, not for EU', async () => {
|
||||
// Ruta 40 is "omsatta utom landet": outside Sweden, which includes
|
||||
// EU countries for services outside huvudregeln. Only SE contradicts.
|
||||
const se = await runOne(zeroRateOrder(ORDER_1, 'SE'), EXPORT_SERVICES, '3305', false)
|
||||
expect(se.body.data.results[0].error?.code).toBe(
|
||||
'WEBSHOP_ORDER_ZERO_RATE_CONTEXT_MISMATCH',
|
||||
)
|
||||
expect(se.body.data.results[0].error?.details?.box).toBe('40')
|
||||
expect(mockCreateDraftEntry).not.toHaveBeenCalled()
|
||||
|
||||
reset()
|
||||
const de = await runOne(zeroRateOrder(ORDER_1, 'DE'), EXPORT_SERVICES, '3305', true)
|
||||
expect(de.body.data.booked_count).toBe(1)
|
||||
})
|
||||
|
||||
it('resolves the box from a configured treatment on a custom account', async () => {
|
||||
// A company-specific 3060 with treatment export_goods is ruta 36
|
||||
// even though the static BAS map knows nothing about it.
|
||||
const { body } = await runOne(
|
||||
zeroRateOrder(ORDER_1, 'SE'),
|
||||
chartRow('3060', 'Konsultarvode utland', 'export_goods'),
|
||||
'3060',
|
||||
false,
|
||||
)
|
||||
expect(body.data.results[0].error?.code).toBe(
|
||||
'WEBSHOP_ORDER_ZERO_RATE_CONTEXT_MISMATCH',
|
||||
)
|
||||
expect(body.data.results[0].error?.details?.box).toBe('36')
|
||||
expect(mockCreateDraftEntry).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('lets a configured treatment override the static box', async () => {
|
||||
// 3105 is statically ruta 36, but the company configured it as
|
||||
// exempt (ruta 42): a domestic order is then fine.
|
||||
const { body } = await runOne(
|
||||
zeroRateOrder(ORDER_1, 'SE'),
|
||||
chartRow('3105', 'Momsfri försäljning', 'exempt'),
|
||||
'3105',
|
||||
true,
|
||||
)
|
||||
expect(body.data.booked_count).toBe(1)
|
||||
})
|
||||
|
||||
it('books when the billing country is unknown (Shopify stores null)', async () => {
|
||||
const { body } = await runOne(zeroRateOrder(ORDER_1, null), EXPORT_GOODS, '3105', true)
|
||||
expect(body.data.booked_count).toBe(1)
|
||||
})
|
||||
|
||||
it('ignores the rate-0 template when the order has no 0% bucket', async () => {
|
||||
// 25%-only order: the 3105 slot is never used, so no context to check.
|
||||
const { body } = await runOne(
|
||||
makeOrderRow({ customer_country: 'SE' }),
|
||||
EXPORT_GOODS,
|
||||
'3105',
|
||||
true,
|
||||
)
|
||||
expect(body.data.booked_count).toBe(1)
|
||||
})
|
||||
|
||||
it('ignores a 0% bucket with zero net', async () => {
|
||||
const { body } = await runOne(
|
||||
zeroRateOrder(ORDER_1, 'SE', {
|
||||
total: 500,
|
||||
total_sek: 500,
|
||||
total_tax: 100,
|
||||
vat_breakdown: [
|
||||
{ rate: 25, net: 400, tax: 100 },
|
||||
{ rate: 0, net: 0, tax: 0 },
|
||||
],
|
||||
}),
|
||||
EXPORT_GOODS,
|
||||
'3105',
|
||||
true,
|
||||
)
|
||||
expect(body.data.booked_count).toBe(1)
|
||||
})
|
||||
|
||||
it('keeps the domestic direction advisory: exempt account + foreign country books', async () => {
|
||||
// The dialog already warns (zero_rate_foreign); refusing here would
|
||||
// regress every untemplated sweep, so 3004/ruta 42 is never blocked.
|
||||
const { body } = await runOne(
|
||||
zeroRateOrder(ORDER_1, 'DE'),
|
||||
chartRow('3060', 'Momsfri försäljning', 'exempt'),
|
||||
'3060',
|
||||
true,
|
||||
)
|
||||
expect(body.data.booked_count).toBe(1)
|
||||
})
|
||||
|
||||
it('never blocks the default 3004 slot on billing country', async () => {
|
||||
enqueue({ data: [zeroRateOrder(ORDER_1, 'US')] })
|
||||
enqueue({ data: [] }) // store settings
|
||||
// 3004 is in the prefill set: no chart check query.
|
||||
enqueue({ data: [{ id: ORDER_1 }] }) // claim
|
||||
const { body } = await parseJsonResponse<BulkResponse>(
|
||||
await postBulk({ order_ids: [ORDER_1], revenue_accounts: { '0': '3004' } }),
|
||||
)
|
||||
expect(body.data.booked_count).toBe(1)
|
||||
})
|
||||
|
||||
it('leaves an unclassified custom account unchecked (no box)', async () => {
|
||||
// Neither treatment nor the static BAS map classifies 3060; there is
|
||||
// nothing to contradict, so the sweep books (deferred item 1).
|
||||
const { body } = await runOne(
|
||||
zeroRateOrder(ORDER_1, 'SE'),
|
||||
chartRow('3060', 'Konsultarvode utland'),
|
||||
'3060',
|
||||
true,
|
||||
)
|
||||
expect(body.data.booked_count).toBe(1)
|
||||
})
|
||||
|
||||
it('refuses per order inside a mixed batch and books the rest', async () => {
|
||||
enqueue({
|
||||
data: [zeroRateOrder(ORDER_1, 'SE'), zeroRateOrder(ORDER_2, 'US', { order_number: '1002' })],
|
||||
})
|
||||
enqueue({ data: [] }) // store settings
|
||||
enqueue({ data: [EXPORT_GOODS] }) // chart check
|
||||
enqueue({ data: [{ id: ORDER_2 }] }) // claim order 2
|
||||
const { status, body } = await parseJsonResponse<BulkResponse>(
|
||||
await postBulk({
|
||||
order_ids: [ORDER_1, ORDER_2],
|
||||
revenue_accounts: { '0': '3105' },
|
||||
}),
|
||||
)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data.booked_count).toBe(1)
|
||||
expect(body.data.failed_count).toBe(1)
|
||||
expect(body.data.results[0]).toMatchObject({
|
||||
order_id: ORDER_1,
|
||||
success: false,
|
||||
error: { code: 'WEBSHOP_ORDER_ZERO_RATE_CONTEXT_MISMATCH' },
|
||||
})
|
||||
expect(body.data.results[1]).toMatchObject({ order_id: ORDER_2, success: true })
|
||||
expect(mockCreateDraftEntry).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -16,11 +16,12 @@ import {
|
||||
ROUNDING_ACCOUNT,
|
||||
WEBSHOP_PREFILL_ACCOUNTS,
|
||||
} from '@/lib/webshop-orders/booking-lines'
|
||||
import { inferDomesticSalesRate } from '@/lib/reports/vat-revenue-accounts'
|
||||
import {
|
||||
defaultRateForVatTreatment,
|
||||
isAccountVatTreatment,
|
||||
} from '@/lib/vat/account-vat-treatment'
|
||||
resolveEffectiveVatRate,
|
||||
resolveRevenueVatBox,
|
||||
} from '@/lib/reports/vat-revenue-accounts'
|
||||
import { getBoxForAccount, type MomsBox } from '@/lib/vat/moms-box-mapping'
|
||||
import { isEuMemberCountry } from '@/lib/vat/eu-countries'
|
||||
import {
|
||||
assertOrderBookable,
|
||||
bookOrderThroughEngine,
|
||||
@@ -245,17 +246,18 @@ export const POST = withRouteContext(
|
||||
// finding): output VAT books on 2611/2621/2631 per rate regardless of
|
||||
// the template, and the momsdeklaration counts a custom account toward
|
||||
// ruta 05 only when the account resolves to that rate. The effective
|
||||
// rate mirrors fetchDynamicVatAccounts EXACTLY, precedence included: an
|
||||
// explicit momssats always wins, then a rate-mapped treatment, and
|
||||
// number+name inference only when nothing is configured, so an account
|
||||
// explicitly set to 6% can never pass a 25% slot on its name alone
|
||||
// (review finding). A mismatched choice would silently drop the sale's
|
||||
// base out of ruta 05 while its VAT lands in ruta 10-12, so the sweep
|
||||
// refuses it and points at the fix. Rate 0 buckets carry no output VAT
|
||||
// and span legitimate momsfri/export/EU accounts (usually unconfigured),
|
||||
// so they only refuse an account whose resolved rate CONTRADICTS 0%
|
||||
// (review finding). Accounts from our own default set are checked
|
||||
// statically: each is valid only for the rate it is the default for.
|
||||
// rate comes from resolveEffectiveVatRate, the SAME helper the ruta 05
|
||||
// report arithmetic uses (#1912), so the two cannot drift: an explicit
|
||||
// momssats always wins, then a rate-mapped treatment, and number+name
|
||||
// inference only when nothing is configured, so an account explicitly
|
||||
// set to 6% can never pass a 25% slot on its name alone (review
|
||||
// finding). A mismatched choice would silently drop the sale's base out
|
||||
// of ruta 05 while its VAT lands in ruta 10-12, so the sweep refuses it
|
||||
// and points at the fix. Rate 0 buckets carry no output VAT and span
|
||||
// legitimate momsfri/export/EU accounts (usually unconfigured), so they
|
||||
// only refuse an account whose resolved rate CONTRADICTS 0% (review
|
||||
// finding). Accounts from our own default set are checked statically:
|
||||
// each is valid only for the rate it is the default for.
|
||||
const mismatchedAccounts: { rate: number; account: string }[] = []
|
||||
for (const { rate, account } of revenueTemplatePairs) {
|
||||
if (WEBSHOP_PREFILL_ACCOUNTS.includes(account)) {
|
||||
@@ -267,11 +269,11 @@ export const POST = withRouteContext(
|
||||
const row = chartRowByAccount.get(account)
|
||||
if (!row) continue // unreachable: the existence guard above returned
|
||||
const expected = rate / 100
|
||||
const configured =
|
||||
row.default_vat_rate === null ? null : Number(row.default_vat_rate)
|
||||
const effective = isAccountVatTreatment(row.default_vat_treatment)
|
||||
? (configured ?? defaultRateForVatTreatment(row.default_vat_treatment, 3))
|
||||
: (configured ?? inferDomesticSalesRate(account, row.account_name))
|
||||
const effective = resolveEffectiveVatRate({
|
||||
account_number: account,
|
||||
account_class: 3,
|
||||
...row,
|
||||
})
|
||||
const mismatch =
|
||||
rate === 0
|
||||
? effective !== null && effective !== 0
|
||||
@@ -286,6 +288,49 @@ export const POST = withRouteContext(
|
||||
)
|
||||
}
|
||||
|
||||
// Rate-0 order-context guard (#1912): VAT amounts are unaffected by the
|
||||
// 0% slot, but the momsdeklaration is not. Ruta 36 (varuförsäljning
|
||||
// utanför EU) is wrong for a Swedish or EU billing country, ruta 40
|
||||
// (tjänster omsatta utom landet) is wrong for Sweden, and the EU boxes
|
||||
// 35/38/39 are wrong for Sweden or a non-EU country. The box comes from
|
||||
// the chosen account's configured treatment, else the static BAS map;
|
||||
// an account neither classifies (the common unconfigured momsfri case)
|
||||
// gets no context check. The opposite direction, a DOMESTIC 0% account
|
||||
// (3004, ruta 42) receiving a foreign order, stays advisory: the dialog
|
||||
// already warns (zero_rate_foreign) and refusing it would regress every
|
||||
// untemplated sweep. Checked per order below, because the billing
|
||||
// country is per order and the doctrine is partial failure per row.
|
||||
// Known limitation: customer_country is the BILLING country (the
|
||||
// WooCommerce sync never stores the shipping address), while the goods
|
||||
// boxes 35/36/38 follow where the goods are transported. A Swedish-billed
|
||||
// order shipped to Norway is a legitimate ruta 36 export that this guard
|
||||
// refuses; the error copy therefore says the check is billing-based and
|
||||
// sends the user to the single dialog to confirm rather than to change
|
||||
// the account. Storing shipping country is a follow-up.
|
||||
const zeroRateAccount = revenueAccountByRate[0]
|
||||
const zeroRateChartRow = zeroRateAccount
|
||||
? chartRowByAccount.get(zeroRateAccount)
|
||||
: undefined
|
||||
const zeroRateBox: MomsBox | null = !zeroRateAccount
|
||||
? null
|
||||
: WEBSHOP_PREFILL_ACCOUNTS.includes(zeroRateAccount)
|
||||
? (getBoxForAccount(zeroRateAccount) ?? null)
|
||||
: zeroRateChartRow
|
||||
? resolveRevenueVatBox({
|
||||
account_number: zeroRateAccount,
|
||||
account_class: 3,
|
||||
...zeroRateChartRow,
|
||||
})
|
||||
: null // unreachable: the existence guard above returned
|
||||
const zeroRateContextContradicted = (country: string): boolean => {
|
||||
if (zeroRateBox === '36') return isEuMemberCountry(country)
|
||||
if (zeroRateBox === '40') return country === 'SE'
|
||||
if (zeroRateBox === '35' || zeroRateBox === '38' || zeroRateBox === '39') {
|
||||
return country === 'SE' || !isEuMemberCountry(country)
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// Sequential on purpose: each order is its own draft -> claim -> commit
|
||||
// round trip through the engine, and voucher numbers are assigned
|
||||
// atomically per commit. Parallelizing would only contend on the same
|
||||
@@ -349,6 +394,26 @@ export const POST = withRouteContext(
|
||||
continue
|
||||
}
|
||||
|
||||
if (zeroRateAccount && order.customer_country) {
|
||||
const country = order.customer_country.toUpperCase()
|
||||
const hasZeroRateAmount = order.vat_breakdown.some(
|
||||
(b) => b.rate === 0 && b.net !== 0,
|
||||
)
|
||||
if (hasZeroRateAmount && zeroRateContextContradicted(country)) {
|
||||
results.push({
|
||||
order_id: id,
|
||||
order_number: order.order_number,
|
||||
success: false,
|
||||
error: failureFromCode('WEBSHOP_ORDER_ZERO_RATE_CONTEXT_MISMATCH', {
|
||||
account: zeroRateAccount,
|
||||
box: zeroRateBox,
|
||||
customer_country: country,
|
||||
}),
|
||||
})
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
const settings = settingsFor(order)
|
||||
// The store's own mapping routes this payment method through the
|
||||
// invoice flow. Booking it directly would both post a wrong clearing
|
||||
|
||||
Reference in New Issue
Block a user