feat(mcp): accept article_id on gnubok_create_invoice lines (#1638)
* feat(mcp): accept article_id on gnubok_create_invoice lines Invoice lines staged via MCP can now reference a catalog article (artikelregister). Staging prefills description, unit, unit_price, vat_rate and revenue account from the article with explicit-wins semantics, mirroring the web line picker. Unknown, foreign-company and deactivated articles are refused at staging, as is a price prefill from an article priced in another currency. The approval executor gains a company-scope gate for staged article_id values: the FK on invoice_items.article_id proves existence, not tenancy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mcp): adopt article VAT rate only from the customer's default set Web line picker parity (InvoiceEditor's applyArticle guard): an article's stored vat_rate is its domestic rate. The prefill previously staged it unconditionally, and the staging/commit gates check the wider permitted set (which includes 25/12/6 for taxed-where-performed supplies), so {article_id, quantity} to a validated EU business staged 25% Swedish VAT onto a reverse-charge invoice. The customer is now fetched before the prefill and the article rate is adopted only when it is in the customer's default rate set; foreign-business lines fall back to the 0% reverse-charge/export default unless the agent sets vat_rate explicitly. Prefill logic extracted into resolveInvoiceLineFromArticle (CodeRabbit). Found by the skeptic review pass (two independent refutations) and the Swedish accounting compliance bot, all converging on the same defect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(invoices): name the article VAT adoption set in vat-rules, not the MCP server The vat-rate-gate-parity guard pins that no invoice write path mentions getAvailableVatRates: gating on the picker default is the bug it exists to prevent. The article-rate adoption in gnubok_create_invoice needs the default set for a different purpose (prefill, not gating), so the semantics move into lib/invoices/vat-rules.ts as getArticleVatRateAdoptionSet(), with tests pinning that adoption is empty for single-rate foreign customers and always a subset of the permitted set. server.ts keeps gating on getPermittedVatRates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Mattsson
co-authored by
Claude Fable 5
Claude Fable 5
parent
f8db38f989
commit
caa0c3b41d
@@ -1,6 +1,7 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import {
|
||||
getAvailableVatRates,
|
||||
getArticleVatRateAdoptionSet,
|
||||
getPermittedVatRates,
|
||||
getVatTreatmentForRate,
|
||||
getVatRules,
|
||||
@@ -355,3 +356,43 @@ describe('getMomsRutaDescription', () => {
|
||||
expect(getMomsRutaDescription('99')).toBe('99')
|
||||
})
|
||||
})
|
||||
|
||||
// ============================================================
|
||||
// getArticleVatRateAdoptionSet
|
||||
// ============================================================
|
||||
|
||||
describe('getArticleVatRateAdoptionSet', () => {
|
||||
it('adopts nothing for a VAT-validated EU business (single locked 0% reverse charge)', () => {
|
||||
expect(getArticleVatRateAdoptionSet('eu_business', true).size).toBe(0)
|
||||
})
|
||||
|
||||
it('adopts nothing for a non-EU business (single locked 0% export)', () => {
|
||||
expect(getArticleVatRateAdoptionSet('non_eu_business', false).size).toBe(0)
|
||||
})
|
||||
|
||||
it('adopts the full domestic set for a Swedish business', () => {
|
||||
const set = getArticleVatRateAdoptionSet('swedish_business', false)
|
||||
expect([...set].sort((a, b) => a - b)).toEqual([0, 6, 12, 25])
|
||||
})
|
||||
|
||||
it('adopts the full domestic set for an EU business WITHOUT validated VAT', () => {
|
||||
const set = getArticleVatRateAdoptionSet('eu_business', false)
|
||||
expect(set.has(25)).toBe(true)
|
||||
})
|
||||
|
||||
it('is always a subset of the permitted set (adoption can never stage an unlawful rate)', () => {
|
||||
const combos: Array<['individual' | 'swedish_business' | 'eu_business' | 'non_eu_business', boolean]> = [
|
||||
['individual', false],
|
||||
['swedish_business', false],
|
||||
['eu_business', false],
|
||||
['eu_business', true],
|
||||
['non_eu_business', false],
|
||||
]
|
||||
for (const [type, validated] of combos) {
|
||||
const permitted = new Set(getPermittedVatRates(type, validated).map((r) => r.rate))
|
||||
for (const rate of getArticleVatRateAdoptionSet(type, validated)) {
|
||||
expect(permitted.has(rate)).toBe(true)
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -48,6 +48,24 @@ export function getAvailableVatRates(
|
||||
]
|
||||
}
|
||||
|
||||
/**
|
||||
* The set of VAT rates an article's stored rate may be ADOPTED from when the
|
||||
* article prefills an invoice line (web line picker parity: the picker only
|
||||
* adopts a rate the customer could have picked themselves). Empty when the
|
||||
* customer is locked to a single rate (foreign business 0% reverse charge /
|
||||
* export): an article's stored rate is its DOMESTIC rate, and adopting it
|
||||
* there would silently put Swedish VAT on a reverse-charge or export invoice
|
||||
* even though the wider permitted set would accept it. This governs PREFILL
|
||||
* only; every validation gate keeps using getPermittedVatRates().
|
||||
*/
|
||||
export function getArticleVatRateAdoptionSet(
|
||||
customerType: CustomerType,
|
||||
vatNumberValidated: boolean = false,
|
||||
): ReadonlySet<number> {
|
||||
const offered = getAvailableVatRates(customerType, vatNumberValidated)
|
||||
return new Set(offered.length > 1 ? offered.map((r) => r.rate) : [])
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the VAT rates that may LEGALLY appear on an invoice line for this
|
||||
* customer type. This is the set validation must gate on.
|
||||
|
||||
@@ -265,6 +265,79 @@ describe('commitPendingOperation: create_invoice: VAT rates for a foreign busine
|
||||
})
|
||||
})
|
||||
|
||||
describe('commitPendingOperation: create_invoice: staged article references', () => {
|
||||
it('writes a company-scoped article_id through to the invoice_items row', async () => {
|
||||
// Queue: CAS claim → customers → company_settings → articles scope check →
|
||||
// invoices insert → invoice_items insert → complete select → update.
|
||||
const { supabase, inserts } = createCapturingSupabase([
|
||||
{ data: { id: 'op-1' } },
|
||||
{ data: customer },
|
||||
{ data: { vat_registered: true } },
|
||||
{ data: [{ id: 'art-1' }] },
|
||||
{ data: { id: 'inv-1', invoice_number: null } },
|
||||
{ data: null },
|
||||
{ data: { id: 'inv-1' } },
|
||||
{ data: null },
|
||||
])
|
||||
|
||||
const op = makePendingOp({
|
||||
params: {
|
||||
customer_id: 'cust-1',
|
||||
items: [
|
||||
{
|
||||
description: 'Konsulttimme',
|
||||
quantity: 2,
|
||||
unit: 'tim',
|
||||
unit_price: 1200,
|
||||
vat_rate: 25,
|
||||
article_id: 'art-1',
|
||||
},
|
||||
],
|
||||
},
|
||||
})
|
||||
|
||||
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
|
||||
|
||||
expect(result.status).toBe('committed')
|
||||
const itemRows = inserts['invoice_items'][0] as Array<Record<string, unknown>>
|
||||
expect(itemRows[0]).toMatchObject({ article_id: 'art-1', line_total: 2400 })
|
||||
})
|
||||
|
||||
it('fails when a staged article_id belongs to another company (drift/tamper gate)', async () => {
|
||||
// The FK on invoice_items.article_id only proves existence, not tenancy:
|
||||
// the executor must refuse an id the scoped select cannot see.
|
||||
const { supabase, inserts } = createCapturingSupabase([
|
||||
{ data: { id: 'op-1' } },
|
||||
{ data: customer },
|
||||
{ data: { vat_registered: true } },
|
||||
{ data: [] },
|
||||
{ data: null },
|
||||
])
|
||||
|
||||
const op = makePendingOp({
|
||||
params: {
|
||||
customer_id: 'cust-1',
|
||||
items: [
|
||||
{
|
||||
description: 'Konsulttimme',
|
||||
quantity: 1,
|
||||
unit: 'tim',
|
||||
unit_price: 1200,
|
||||
vat_rate: 25,
|
||||
article_id: 'art-foreign',
|
||||
},
|
||||
],
|
||||
},
|
||||
})
|
||||
|
||||
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
|
||||
|
||||
expect(result.status).toBe('failed')
|
||||
expect(result.error).toMatch(/Artikel art-foreign finns inte i företaget/)
|
||||
expect(inserts['invoices']).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('commitPendingOperation: create_invoice: dimensions propagation (PR7)', () => {
|
||||
it('staged default_dimensions lands on the invoices row and item bags on invoice_items rows', async () => {
|
||||
const { supabase, inserts } = createCapturingSupabase(queueFor({ vat_registered: true }))
|
||||
|
||||
@@ -1554,6 +1554,26 @@ async function commitCreateInvoice(
|
||||
}
|
||||
}
|
||||
|
||||
// Drift/tamper gate for staged article references: the FK on
|
||||
// invoice_items.article_id only proves the article exists, not that it
|
||||
// belongs to THIS company, so scope-check here like revenue_account above.
|
||||
const stagedArticleIds = Array.from(
|
||||
new Set(items.map((i) => i.article_id).filter((a): a is string => !!a)),
|
||||
)
|
||||
if (stagedArticleIds.length > 0) {
|
||||
const { data: articleRows, error: articleError } = await supabase
|
||||
.from('articles')
|
||||
.select('id')
|
||||
.eq('company_id', companyId)
|
||||
.in('id', stagedArticleIds)
|
||||
if (articleError) return { error: articleError.message, status: 500 }
|
||||
const foundArticleIds = new Set((articleRows ?? []).map((a: { id: string }) => a.id))
|
||||
const missingArticleId = stagedArticleIds.find((a) => !foundArticleIds.has(a))
|
||||
if (missingArticleId) {
|
||||
return { error: `Artikel ${missingArticleId} finns inte i företaget`, status: 400 }
|
||||
}
|
||||
}
|
||||
|
||||
const total = subtotal + vatAmount
|
||||
const currency = ((params.currency as string) || 'SEK') as Currency
|
||||
const invoiceDate = (params.invoice_date as string) || new Date().toISOString().split('T')[0]
|
||||
|
||||
Reference in New Issue
Block a user