fix(customers): make country ISO-2 everywhere and check it against the customer type (#2241)

* fix(customers): make country ISO-2 everywhere and check it against the customer type (#2025, #2028) customers.country and suppliers.country were read as ISO codes by the periodisk sammanstallning (SKV 5740), Peppol and the provider importers but written as English names by the customer form and the v1 API, so a correct German customer produced GERMANY811234567 in the SKV file plus two false warnings, and an EU customer saved with land Sverige got reverse charge with nothing objecting until after the invoice was sent. - lib/vat/country-codes.ts: one helper that normalises codes and the Swedish/English names the writers used to store, the country-vs-type rule (swedish_business = SE, eu_business = EU member other than SE that matches the VAT prefix, non_eu_business = outside the EU), and the reverse-charge country gate. - Writers: customer form and supplier form get a country select; internal REST, v1 REST, bulk-create, MCP create/update, CSV/Excel import and the provider migration mapper normalise to a code and refuse unknown text; the consistency rule is a form error and an API 400 (CUSTOMER_COUNTRY_MISMATCH on update). An omitted country is SE for Swedish types, derived from the VAT prefix for eu_business, required for non_eu_business. - vat-rules.ts: getVatRules and friends take the country as a third argument and grant reverse charge only for an EU country other than SE; every invoice/sales-order/MCP call site passes customer.country. - periodisk sammanstallning reads legacy names through the same helper. - Migration 20260903170000: normalize_country_code() SQL twin, country_raw rollback column on both tables, backfill of every non-code row; unknown text is left as-is. pg-real test for the function. Closes #2025, closes #2028 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D5EmmndLyDCmY5NHYAvYkE * fix(customers): keep reverse charge for defaulted-SE EU rows, gate the country rule on the fields it reads, fix build Skeptic and CI findings on #2241, one pass: - Migration step 4: eu_business rows whose country was null or only the old writer default (SE) while the VAT number names another EU member take the country from the prefix. The pre-2026-09 rules granted reverse charge on type + VIES validation alone, so these rows invoiced at 0% and would have flipped to 25% on the next invoice. country_raw = '' marks a null origin; rollback uses nullif(country_raw, ''). - countryPermitsReverseCharge refuses SE only: a VIES-validated number outweighs a non-EU address (Swiss company registered in DE, Monaco with a FR number, Northern Ireland XI). - checkCountryConsistency: an eu_business outside the EU VAT area is accepted when the VAT prefix is an EU-trade registration (incl. XI); Monaco maps to the FR prefix. - Internal PATCH, MCP update and the commit executor judge the country rule only when customer_type, country or vat_number is part of the update, so a contradictory legacy row can still change its email (v1 already did). - Webshop-order customers get the order's billing country; spreadsheet import derives a missing country from the type and flags contradictions (parser row error + execute schema refine). - Build: v1 [id] route typed the existing row through a narrowed alias (never) and passed messageSv/messageEn the v1 error context lacks; the self-billed customer projection lacked country. - Checks: regenerated skills/accounted-api (customer example country SE). - New parity test holds the migration's SQL name table to the TS table. - DECISIONS.md: correct migration version and the revised rule. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D5EmmndLyDCmY5NHYAvYkE --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-03 18:09:46 +02:00
parent 80b87c55fc
commit 3918ff6620
48 changed files with 2288 additions and 97 deletions
@@ -9,6 +9,7 @@ import type { SupabaseClient } from '@supabase/supabase-js'
 import { fetchExchangeRate } from '@/lib/currency/riksbanken'
 import { encryptCustomerPersonalNumber } from '@/lib/customers/protect-personal-number'
 import { normalizeVatRateToFraction } from '@/lib/vat/vat-rate-unit'
+import { normalizeCountryCode } from '@/lib/vat/country-codes'
 import { sumLineVat, lineVatFromPercent } from '@/lib/providers/amounts'
 import type { Currency, CustomerType, ExchangeRate, SupplierType, VatTreatment } from '@/types'
 import type {
@@ -46,7 +47,10 @@ function formatAddress(addr?: PostalAddress): {
    address_line2: addr.additionalStreetName || null,
    postal_code: addr.postalZone || null,
    city: addr.cityName || null,
-    country: addr.countryCode || null,
+    // customers.country is ISO 3166-1 alpha-2; some providers hand over a
+    // name (Fortnox's Country is "Sverige"), which is mapped when known and
+    // kept as-is otherwise so the periodisk report can point at it.
+    country: normalizeCountryCode(addr.countryCode) ?? addr.countryCode ?? null,
  }
 }

@@ -0,0 +1,215 @@
+/**
+ * customers.country through the MCP tools: ISO 3166-1 alpha-2, derived from
+ * the VAT prefix when an EU business omits it, and checked against the
+ * customer type at staging (#2025, #2028).
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { createQueuedMockSupabase } from '@/tests/helpers'
+import { eventBus } from '@/lib/events/bus'
+import { tools } from '../server'
+
+const CUSTOMER_ID = '11111111-1111-4111-8111-111111111111'
+const createTool = () => tools.find((candidate) => candidate.name === 'gnubok_create_customer')!
+const updateTool = () => tools.find((candidate) => candidate.name === 'gnubok_update_customer')!
+
+function currentCustomer(overrides: Record<string, unknown> = {}) {
+  return {
+    id: CUSTOMER_ID,
+    name: 'Test Customer AB',
+    customer_type: 'swedish_business',
+    customer_number: '1001',
+    email: 'billing@example.test',
+    phone: '',
+    address_line1: 'Testgatan 1',
+    address_line2: null,
+    postal_code: '12345',
+    city: 'Teststad',
+    country: 'SE',
+    org_number: '556000-0000',
+    vat_number: null,
+    vat_number_validated: false,
+    language: 'sv',
+    default_payment_terms: 30,
+    notes: null,
+    ...overrides,
+  }
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  eventBus.clear()
+})
+
+describe('gnubok_create_customer: country', () => {
+  it('documents the code shape and the type rule in the input schema', () => {
+    const properties = createTool().inputSchema.properties as Record<string, Record<string, unknown>>
+    expect(String(properties.country.description)).toMatch(/ISO 3166-1 alpha-2/)
+    expect(String(properties.country.description)).toMatch(/default SE/)
+  })
+
+  it('previews a country name as its code', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: null }) // company_settings read (payment-terms default)
+
+    const result = (await createTool().execute(
+      {
+        name: 'Muster Handels GmbH',
+        customer_type: 'eu_business',
+        country: 'Tyskland',
+        vat_number: 'DE811234567',
+        dry_run: true,
+      },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as { preview: Record<string, unknown> }
+
+    expect(result.preview.country).toBe('DE')
+  })
+
+  it('derives the country from the VAT prefix when an EU business omits it', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: null })
+
+    const result = (await createTool().execute(
+      { name: 'Muster Handels GmbH', customer_type: 'eu_business', vat_number: 'DE811234567', dry_run: true },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as { preview: Record<string, unknown> }
+
+    expect(result.preview.country).toBe('DE')
+  })
+
+  it('defaults a Swedish business to SE, never to the name Sweden', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: null })
+
+    const result = (await createTool().execute(
+      { name: 'Kund AB', customer_type: 'swedish_business', dry_run: true },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as { preview: Record<string, unknown> }
+
+    expect(result.preview.country).toBe('SE')
+  })
+
+  it('refuses an EU business with land Sverige before staging (#2025)', async () => {
+    const { supabase } = createQueuedMockSupabase()
+
+    await expect(
+      createTool().execute(
+        { name: 'Muster Handels GmbH', customer_type: 'eu_business', country: 'SE', vat_number: 'DE811234567' },
+        'company-1',
+        'user-1',
+        supabase as never,
+      ),
+    ).rejects.toThrow(/country/i)
+    expect(supabase.from).not.toHaveBeenCalled()
+  })
+
+  it('refuses an EU business with neither a country nor a usable VAT prefix', async () => {
+    const { supabase } = createQueuedMockSupabase()
+
+    await expect(
+      createTool().execute(
+        { name: 'Muster Handels GmbH', customer_type: 'eu_business', vat_number: '811234567' },
+        'company-1',
+        'user-1',
+        supabase as never,
+      ),
+    ).rejects.toThrow(/country is required/i)
+  })
+
+  it('refuses a country it cannot read as a code', async () => {
+    const { supabase } = createQueuedMockSupabase()
+
+    await expect(
+      createTool().execute(
+        { name: 'Kund AB', customer_type: 'swedish_business', country: 'Atlantis' },
+        'company-1',
+        'user-1',
+        supabase as never,
+      ),
+    ).rejects.toThrow(/not an ISO 3166-1 alpha-2 code/i)
+  })
+})
+
+describe('gnubok_update_customer: country', () => {
+  it('previews a country name as its code', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: currentCustomer() })
+
+    const result = (await updateTool().execute(
+      {
+        customer_id: CUSTOMER_ID,
+        customer_type: 'eu_business',
+        country: 'Tyskland',
+        vat_number: 'DE811234567',
+        dry_run: true,
+      },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as { preview: { proposed?: Record<string, unknown> } }
+
+    expect(result.preview.proposed).toMatchObject({ customer_type: 'eu_business', country: 'DE' })
+  })
+
+  it('refuses a type change that contradicts the stored country', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: currentCustomer() })
+
+    await expect(
+      updateTool().execute(
+        { customer_id: CUSTOMER_ID, customer_type: 'eu_business', dry_run: true },
+        'company-1',
+        'user-1',
+        supabase as never,
+      ),
+    ).rejects.toThrow(/country/i)
+  })
+
+  it('refuses a country change that contradicts the stored VAT prefix', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: currentCustomer({ customer_type: 'eu_business', country: 'DE', vat_number: 'DE811234567' }) })
+
+    await expect(
+      updateTool().execute(
+        { customer_id: CUSTOMER_ID, country: 'FR', dry_run: true },
+        'company-1',
+        'user-1',
+        supabase as never,
+      ),
+    ).rejects.toThrow(/prefix/i)
+  })
+
+  it('lets a contradictory legacy row change unrelated fields', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: currentCustomer({ customer_type: 'eu_business', country: 'SE', vat_number: 'DE811234567' }) })
+
+    const result = (await updateTool().execute(
+      { customer_id: CUSTOMER_ID, email: 'new@example.test', dry_run: true },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as { preview: { proposed?: Record<string, unknown> } }
+
+    expect(result.preview.proposed).toMatchObject({ email: 'new@example.test', country: 'SE' })
+  })
+
+  it('refuses a country it cannot read as a code before touching the database', async () => {
+    const { supabase } = createQueuedMockSupabase()
+
+    await expect(
+      updateTool().execute(
+        { customer_id: CUSTOMER_ID, country: 'Atlantis', dry_run: true },
+        'company-1',
+        'user-1',
+        supabase as never,
+      ),
+    ).rejects.toThrow(/country/i)
+    expect(supabase.from).not.toHaveBeenCalled()
+  })
+})
@@ -139,6 +139,12 @@ import {
 } from './tool-namespace'
 import { getRiskLevel } from '@/lib/pending-operations/risk-tiers'
 import { normalizeVatRateToDecimal } from '@/lib/vat/supplier-invoice-line-checks'
+import {
+  COUNTRY_CONSISTENCY_MESSAGES,
+  checkCountryConsistency,
+  defaultCountryForParty,
+  normalizeCountryCode,
+} from '@/lib/vat/country-codes'
 import {
  ACCOUNT_VAT_TREATMENTS,
  defaultRateForVatTreatment,
@@ -5949,7 +5955,7 @@ export const tools: McpTool[] = [
        address: { type: 'string', description: 'Street address' },
        postal_code: { type: 'string' },
        city: { type: 'string' },
-        country: { type: 'string', description: 'Country (default Sweden)' },
+        country: { type: 'string', description: 'ISO 3166-1 alpha-2 (default SE; a name is normalised). Must agree with customer_type and the VAT prefix.' },
        dry_run: {
          type: 'boolean',
          description: 'If true, validate inputs and return the would-be preview without staging or creating. No DB writes, no side-effects.',
@@ -6023,6 +6029,35 @@ export const tools: McpTool[] = [
        throw new Error('personal_number must be a Swedish personnummer: YYYYMMDD-XXXX, YYMMDD-XXXX or the digits alone.')
      }

+      // Country: stored as ISO 3166-1 alpha-2 (a name is accepted and
+      // normalised), and it must agree with the customer type and the VAT
+      // prefix. An EU customer saved with country SE used to get reverse
+      // charge with nothing objecting until the periodisk sammanställning
+      // (#2025). Checked at staging so the user never approves an operation
+      // that then fails at commit.
+      const countryArg = typeof args.country === 'string' ? args.country.trim() : ''
+      const vatNumberArg = typeof args.vat_number === 'string' ? args.vat_number : null
+      const country = countryArg
+        ? normalizeCountryCode(countryArg)
+        : defaultCountryForParty(customerType, vatNumberArg)
+      if (!country) {
+        throw new Error(
+          countryArg
+            ? `country "${countryArg}" is not an ISO 3166-1 alpha-2 code or a known country name. Use a code such as SE, DE or NO.`
+            : customerType === 'eu_business'
+              ? 'country is required for an EU business unless vat_number carries an EU country prefix (e.g. DE811234567).'
+              : 'country is required for a non-EU business.',
+        )
+      }
+      const countryIssue = checkCountryConsistency({
+        partyType: customerType,
+        country,
+        vatNumber: vatNumberArg,
+      })
+      if (countryIssue) {
+        throw new Error(`country: ${COUNTRY_CONSISTENCY_MESSAGES[countryIssue].en}`)
+      }
+
      // Resolved at staging, not at commit, so the approval preview shows the
      // terms the row will actually get: the caller's value, else the
      // company's invoice_default_days, else 30. (Staging `|| 30` here is why
@@ -6045,7 +6080,7 @@ export const tools: McpTool[] = [
        address: (args.address as string) || null,
        postal_code: (args.postal_code as string) || null,
        city: (args.city as string) || null,
-        country: (args.country as string) || 'Sweden',
+        country,
        // The preview (and the approval UI that renders it) only ever sees
        // the masked form.
        personal_number_masked: personalNumber ? maskCustomerPersonalNumber(personalNumber) : null,
@@ -6103,7 +6138,7 @@ export const tools: McpTool[] = [
        address_line2: { type: 'string' },
        postal_code: { type: 'string' },
        city: { type: 'string' },
-        country: { type: 'string' },
+        country: { type: 'string', description: 'ISO 3166-1 alpha-2 (a name is normalised). Must agree with customer_type and the VAT prefix.' },
        org_number: { type: 'string' },
        personal_number: {
          type: ['string', 'null'],
@@ -6199,6 +6234,24 @@ export const tools: McpTool[] = [
        throw new Error('personal_number is only allowed for customer_type "individual".')
      }

+      // Country vs type vs VAT prefix, judged on the row as it will END UP:
+      // a type change alone can make a stored country wrong, and a country
+      // change alone can contradict the stored VAT number (#2025). Judged
+      // only when one of the three is in the update: a legacy row that is
+      // already contradictory must still be able to change its email.
+      const { customer_type: newType, country: newCountry, vat_number: newVat } = parsed.data.changes
+      const countryIssue =
+        newType !== undefined || newCountry !== undefined || newVat !== undefined
+          ? checkCountryConsistency({
+              partyType: effectiveCustomerType,
+              country: newCountry ?? current.country,
+              vatNumber: newVat ?? current.vat_number,
+            })
+          : null
+      if (countryIssue) {
+        throw new Error(`country: ${COUNTRY_CONSISTENCY_MESSAGES[countryIssue].en}`)
+      }
+
      const currentPreview = {
        customer_id: current.id,
        name: current.name,
@@ -6845,13 +6898,13 @@ export const tools: McpTool[] = [
      }

      // VAT rules from customer type (same logic as web UI)
-      const vatRules = getVatRules(customer.customer_type, customer.vat_number_validated)
+      const vatRules = getVatRules(customer.customer_type, customer.vat_number_validated, customer.country)
      // The DEFAULT set governs article-rate adoption (web parity: the picker
      // only adopts a rate the customer could have picked themselves); a
      // customer locked to a single rate (foreign business 0%) adopts nothing.
      // Gating below stays on the PERMITTED set: adoption and validation are
      // deliberately different sets.
-      const adoptableVatRates = getArticleVatRateAdoptionSet(customer.customer_type, customer.vat_number_validated)
+      const adoptableVatRates = getArticleVatRateAdoptionSet(customer.customer_type, customer.vat_number_validated, customer.country)

      // Article prefill (web line picker parity): the line's own values win,
      // the referenced article fills whatever the agent left out.
@@ -6922,7 +6975,7 @@ export const tools: McpTool[] = [
      // The default is still 0% (vatRules.rate is the fallback below), so a
      // Swedish rate only reaches the staged operation when the agent set it on
      // that line explicitly.
-      const permittedRates = getPermittedVatRates(customer.customer_type, customer.vat_number_validated)
+      const permittedRates = getPermittedVatRates(customer.customer_type, customer.vat_number_validated, customer.country)
      const allowedRates = new Set(permittedRates.map((r) => r.rate))

      // Calculate per-item VAT (line totals net of any per-line discount)
@@ -7240,7 +7293,7 @@ export const tools: McpTool[] = [
      // Article prefill with the same rules as gnubok_create_invoice: the
      // line's own values win, the article fills the rest, and its VAT rate
      // is adopted only inside the customer's default rate set.
-      const adoptableVatRates = getArticleVatRateAdoptionSet(customer.customer_type, customer.vat_number_validated)
+      const adoptableVatRates = getArticleVatRateAdoptionSet(customer.customer_type, customer.vat_number_validated, customer.country)
      const articleIds = Array.from(new Set(rawItems.map((i) => i.article_id).filter((a): a is string => !!a)))
      const articlesById = new Map<string, InvoiceLineArticle>()
      if (articleIds.length > 0) {
@@ -18076,7 +18129,7 @@ export const tools: McpTool[] = [
        // for individuals); never decrypted, staged, or returned here.
        const { data: customer, error: custError } = await supabase
          .from('customers')
-          .select('customer_type, vat_number_validated, personal_number')
+          .select('customer_type, vat_number_validated, country, personal_number')
          .eq('id', invoice.customer_id)
          .eq('company_id', companyId)
          .single()
@@ -18084,9 +18137,9 @@ export const tools: McpTool[] = [
          throw new Error('Customer not found: they may have been deleted. The draft cannot be edited without its customer.')
        }

-        const vatRules = getVatRules(customer.customer_type, customer.vat_number_validated)
+        const vatRules = getVatRules(customer.customer_type, customer.vat_number_validated, customer.country)
        defaultVatRate = vatRules.rate
-        const adoptableVatRates = getArticleVatRateAdoptionSet(customer.customer_type, customer.vat_number_validated)
+        const adoptableVatRates = getArticleVatRateAdoptionSet(customer.customer_type, customer.vat_number_validated, customer.country)

        const articleIds = Array.from(new Set(rawItems.map((i) => i.article_id).filter((a): a is string => !!a)))
        const articlesById = new Map<string, InvoiceLineArticle>()
@@ -18114,7 +18167,7 @@ export const tools: McpTool[] = [
        // carry Swedish VAT even to a foreign business); the default stays
        // vatRules.rate, so a Swedish rate only lands here when set on the
        // line or adopted from an article within the default set.
-        const permittedRates = getPermittedVatRates(customer.customer_type, customer.vat_number_validated)
+        const permittedRates = getPermittedVatRates(customer.customer_type, customer.vat_number_validated, customer.country)
        const allowedRates = new Set(permittedRates.map((r) => r.rate))
        for (const item of items) {
          // Text rows carry no amounts and never book: exclude them from the