From a49d75db77195d7f054f06bfd36ffeb88a7cd0c3 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Sat, 8 Aug 2026 10:45:02 +0200 Subject: [PATCH] fix(migration): Visma pagination + chunk-insert resilience (the '300 misslyckades' case) (#1455) * fix(providers): paginate Visma eAccounting with $page/$pagesize eAccounting silently ignores OData $top/$skip, so every request returned page 1 and getPaginated appended the first page TotalNumberOfPages times: customers were imported in triplicate and invoice chunks hit unique violations. Also stop on an empty page so a stale Meta can never loop or duplicate. Co-Authored-By: Claude Fable 5 * fix(migration): survive bad rows in entity imports instead of failing whole chunks One PostgREST insert per 500-row chunk is all-or-nothing, so a single duplicate reported every row as failed ('300 misslyckades') with no cause shown. Now: dedupe repeats within the fetched data (paging faults, source duplicates), fall back to per-row inserts when a chunk is rejected, store empty invoice numbers as NULL instead of colliding '', surface the first DB error in the result UI, and mark all-failed steps with an error icon. Sales invoices also carry remaining_amount so open invoices no longer land as settled. Co-Authored-By: Claude Fable 5 * fix(migration): never per-row retry after a successful bulk insert with short read-back A succeeded statement whose .select() returns fewer rows than sent means the rows ARE in the table; retrying them one by one would duplicate every unreturned row. Pair what came back and report the tail instead. Co-Authored-By: Claude Fable 5 * fix(migration): count stub-insert casualties as failed and sample enrichment errors Review follow-ups: invoices dropped because their customer/supplier stub insert errored are DB failures, not matching misses; classifying them as noMatch rendered a green result row with the database error hidden. Enrichment failures now also feed errorSample. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- .../general/ArcimMigrationWorkspace.tsx | 47 ++- .../entity-mapper-invoice-numbers.test.ts | 99 ++++++ .../lib/__tests__/insert-fallback.test.ts | 83 ++++++ .../arcim-migration/lib/entity-mapper.ts | 14 +- .../arcim-migration/lib/insert-fallback.ts | 77 +++++ .../lib/migration-orchestrator.ts | 282 +++++++++++------- extensions/general/arcim-migration/types.ts | 8 +- lib/providers/visma/__tests__/client.test.ts | 86 ++++++ lib/providers/visma/client.ts | 13 +- 9 files changed, 577 insertions(+), 132 deletions(-) create mode 100644 extensions/general/arcim-migration/lib/__tests__/entity-mapper-invoice-numbers.test.ts create mode 100644 extensions/general/arcim-migration/lib/__tests__/insert-fallback.test.ts create mode 100644 extensions/general/arcim-migration/lib/insert-fallback.ts create mode 100644 lib/providers/visma/__tests__/client.test.ts diff --git a/components/extensions/general/ArcimMigrationWorkspace.tsx b/components/extensions/general/ArcimMigrationWorkspace.tsx index c2a3222b..122bdf62 100644 --- a/components/extensions/general/ArcimMigrationWorkspace.tsx +++ b/components/extensions/general/ArcimMigrationWorkspace.tsx @@ -120,10 +120,10 @@ interface SkipReasons { interface MigrationResults { companyInfo?: { imported: boolean } - customers?: { total: number; imported: number; updated?: number; skipped: number; skipReasons?: SkipReasons } - suppliers?: { total: number; imported: number; skipped: number; skipReasons?: SkipReasons } - salesInvoices?: { total: number; imported: number; skipped: number; skipReasons?: SkipReasons } - supplierInvoices?: { total: number; imported: number; skipped: number; skipReasons?: SkipReasons } + customers?: { total: number; imported: number; updated?: number; skipped: number; skipReasons?: SkipReasons; errorSample?: string } + suppliers?: { total: number; imported: number; skipped: number; skipReasons?: SkipReasons; errorSample?: string } + salesInvoices?: { total: number; imported: number; skipped: number; skipReasons?: SkipReasons; errorSample?: string } + supplierInvoices?: { total: number; imported: number; skipped: number; skipReasons?: SkipReasons; errorSample?: string } } import AccountMappingStep from '@/components/import/AccountMappingStep' import type { AccountMapping, ImportResult, ParsedSIEFile } from '@/lib/import/types' @@ -1643,38 +1643,38 @@ function ResultStep({ } label="Kunder" - status="success" + status={entityRowStatus(results.customers!.imported, results.customers!.skipReasons)} statusText={results.customers!.updated ? `${results.customers!.imported} importerade, ${results.customers!.updated} kompletterade` : `${results.customers!.imported} importerade`} - detail={results.customers!.skipped > 0 ? formatSkipReasons(results.customers!.skipReasons, 'customer') ?? `${results.customers!.skipped} hoppades över` : undefined} + detail={results.customers!.skipped > 0 ? formatSkipReasons(results.customers!.skipReasons, 'customer', results.customers!.errorSample) ?? `${results.customers!.skipped} hoppades över` : undefined} /> )} {hasSuppliers && ( } label="Leverantörer" - status="success" + status={entityRowStatus(results.suppliers!.imported, results.suppliers!.skipReasons)} statusText={`${results.suppliers!.imported} importerade`} - detail={results.suppliers!.skipped > 0 ? formatSkipReasons(results.suppliers!.skipReasons, 'supplier') ?? `${results.suppliers!.skipped} hoppades över` : undefined} + detail={results.suppliers!.skipped > 0 ? formatSkipReasons(results.suppliers!.skipReasons, 'supplier', results.suppliers!.errorSample) ?? `${results.suppliers!.skipped} hoppades över` : undefined} /> )} {hasSalesInvoices && ( } label="Kundfakturor" - status="success" + status={entityRowStatus(results.salesInvoices!.imported, results.salesInvoices!.skipReasons)} statusText={`${results.salesInvoices!.imported} importerade`} - detail={results.salesInvoices!.skipped > 0 ? formatSkipReasons(results.salesInvoices!.skipReasons, 'invoice') ?? `${results.salesInvoices!.skipped} hoppades över` : undefined} + detail={results.salesInvoices!.skipped > 0 ? formatSkipReasons(results.salesInvoices!.skipReasons, 'invoice', results.salesInvoices!.errorSample) ?? `${results.salesInvoices!.skipped} hoppades över` : undefined} /> )} {hasSupplierInvoices && ( } label="Leverantörsfakturor" - status="success" + status={entityRowStatus(results.supplierInvoices!.imported, results.supplierInvoices!.skipReasons)} statusText={`${results.supplierInvoices!.imported} importerade`} - detail={results.supplierInvoices!.skipped > 0 ? formatSkipReasons(results.supplierInvoices!.skipReasons, 'invoice') ?? `${results.supplierInvoices!.skipped} hoppades över` : undefined} + detail={results.supplierInvoices!.skipped > 0 ? formatSkipReasons(results.supplierInvoices!.skipReasons, 'invoice', results.supplierInvoices!.errorSample) ?? `${results.supplierInvoices!.skipped} hoppades över` : undefined} /> )} @@ -1742,7 +1742,11 @@ function ResultStep({ ) } -function formatSkipReasons(reasons?: SkipReasons, entityType?: 'customer' | 'supplier' | 'invoice'): string | undefined { +function formatSkipReasons( + reasons?: SkipReasons, + entityType?: 'customer' | 'supplier' | 'invoice', + errorSample?: string, +): string | undefined { if (!reasons) return undefined const parts: string[] = [] if (reasons.duplicate) parts.push(`${reasons.duplicate} fanns redan`) @@ -1751,10 +1755,21 @@ function formatSkipReasons(reasons?: SkipReasons, entityType?: 'customer' | 'sup const matchLabel = entityType === 'invoice' ? 'utan matchning' : 'utan matchning' parts.push(`${reasons.noMatch} ${matchLabel}`) } - if (reasons.failed) parts.push(`${reasons.failed} misslyckades`) + if (reasons.failed) { + parts.push( + errorSample + ? `${reasons.failed} misslyckades: ${errorSample.slice(0, 140)}` + : `${reasons.failed} misslyckades` + ) + } return parts.length > 0 ? parts.join(', ') : undefined } +/** A step that failed everything it tried is an error, not a green checkmark. */ +function entityRowStatus(imported: number, reasons?: SkipReasons): 'success' | 'error' { + return imported === 0 && (reasons?.failed ?? 0) > 0 ? 'error' : 'success' +} + /** Simple row for non-SIE entity results (customers, invoices, etc.) */ function EntityResultRow({ icon, @@ -1765,7 +1780,7 @@ function EntityResultRow({ }: { icon: React.ReactNode label: string - status: 'success' | 'skipped' + status: 'success' | 'skipped' | 'error' statusText: string detail?: string }) { @@ -1777,7 +1792,7 @@ function EntityResultRow({

{statusText}

{detail &&

{detail}

} - + ) } diff --git a/extensions/general/arcim-migration/lib/__tests__/entity-mapper-invoice-numbers.test.ts b/extensions/general/arcim-migration/lib/__tests__/entity-mapper-invoice-numbers.test.ts new file mode 100644 index 00000000..50dd5943 --- /dev/null +++ b/extensions/general/arcim-migration/lib/__tests__/entity-mapper-invoice-numbers.test.ts @@ -0,0 +1,99 @@ +import { describe, it, expect } from 'vitest' +import { mapSalesInvoice, mapSupplierInvoice } from '../entity-mapper' +import type { SalesInvoiceDto, SupplierInvoiceDto, PartyDto } from '@/lib/providers/dto' + +/** + * Guards two migration hardenings: + * - empty invoice numbers from a provider payload must be stored as NULL, + * never '': the unique indexes on (company_id, invoice_number) and + * (company_id, supplier_id, supplier_invoice_number) treat NULLs as + * distinct but collide on repeated empty strings; + * - sales invoices must carry remaining_amount (NOT NULL DEFAULT 0), or + * every migrated open invoice looks fully settled in AR aging. + */ + +const party: PartyDto = { name: 'Motpart AB', identifications: [] } + +function makeSalesDto(over: { invoiceNumber?: string; paid?: boolean; balance?: number; total?: number }): SalesInvoiceDto { + const total = over.total ?? 1000 + return { + id: 'inv-1', + invoiceNumber: over.invoiceNumber ?? 'F-100', + issueDate: '2026-01-10', + dueDate: '2026-02-10', + currencyCode: 'SEK', + status: 'sent', + supplier: party, + customer: party, + lines: [], + legalMonetaryTotal: { + lineExtensionAmount: { value: total, currencyCode: 'SEK' }, + payableAmount: { value: total, currencyCode: 'SEK' }, + }, + paymentStatus: { + paid: over.paid ?? false, + balance: { value: over.balance ?? total, currencyCode: 'SEK' }, + }, + } +} + +function makeSupplierDto(invoiceNumber: string): SupplierInvoiceDto { + return { + id: 'sinv-1', + invoiceNumber, + issueDate: '2026-01-10', + currencyCode: 'SEK', + status: 'booked', + supplier: party, + buyer: party, + lines: [], + legalMonetaryTotal: { + lineExtensionAmount: { value: 500, currencyCode: 'SEK' }, + payableAmount: { value: 500, currencyCode: 'SEK' }, + }, + paymentStatus: { paid: false, balance: { value: 500, currencyCode: 'SEK' } }, + } +} + +describe('invoice number nulling', () => { + it('sales invoice: empty invoiceNumber becomes NULL, real one is kept', () => { + const empty = mapSalesInvoice(makeSalesDto({ invoiceNumber: '' }), 'u', 'c', 'cust').invoice + expect(empty.invoice_number).toBeNull() + + const real = mapSalesInvoice(makeSalesDto({ invoiceNumber: 'F-7' }), 'u', 'c', 'cust').invoice + expect(real.invoice_number).toBe('F-7') + }) + + it('supplier invoice: empty invoiceNumber becomes NULL', () => { + const empty = mapSupplierInvoice(makeSupplierDto(''), 'u', 'c', 'sup').invoice + expect(empty.supplier_invoice_number).toBeNull() + + const real = mapSupplierInvoice(makeSupplierDto('LF-9'), 'u', 'c', 'sup').invoice + expect(real.supplier_invoice_number).toBe('LF-9') + }) +}) + +describe('sales invoice remaining_amount', () => { + it('open invoice keeps its full balance as remaining', () => { + const inv = mapSalesInvoice(makeSalesDto({ paid: false, balance: 1000, total: 1000 }), 'u', 'c', 'cust').invoice + expect(inv.remaining_amount).toBe(1000) + expect(inv.paid_amount).toBe(0) + }) + + it('partially paid invoice: remaining mirrors the provider balance', () => { + const inv = mapSalesInvoice(makeSalesDto({ paid: false, balance: 250.5, total: 1000 }), 'u', 'c', 'cust').invoice + expect(inv.remaining_amount).toBe(250.5) + expect(inv.paid_amount).toBe(749.5) + }) + + it('paid invoice: remaining is 0', () => { + const inv = mapSalesInvoice(makeSalesDto({ paid: true, balance: 0, total: 1000 }), 'u', 'c', 'cust').invoice + expect(inv.remaining_amount).toBe(0) + expect(inv.paid_amount).toBe(1000) + }) + + it('negative provider balance never yields negative remaining', () => { + const inv = mapSalesInvoice(makeSalesDto({ paid: false, balance: -3, total: 1000 }), 'u', 'c', 'cust').invoice + expect(inv.remaining_amount).toBe(0) + }) +}) diff --git a/extensions/general/arcim-migration/lib/__tests__/insert-fallback.test.ts b/extensions/general/arcim-migration/lib/__tests__/insert-fallback.test.ts new file mode 100644 index 00000000..867dc018 --- /dev/null +++ b/extensions/general/arcim-migration/lib/__tests__/insert-fallback.test.ts @@ -0,0 +1,83 @@ +import { describe, it, expect, vi } from 'vitest' +import type { SupabaseClient } from '@supabase/supabase-js' +import { insertWithPerRowFallback } from '../insert-fallback' + +/** + * The chunked migration inserts are one PostgREST statement per chunk, so a + * single bad row used to reject the whole chunk and report every row as + * failed ("300 misslyckades"). The fallback must keep the bulk fast path, + * retry per row only after a bulk failure, and attribute results by index. + */ + +type InsertResult = { data: Record[] | null; error: { message: string } | null } + +/** Minimal supabase stand-in: scripted responses per insert call, in order. */ +function makeSupabase(script: InsertResult[]) { + let call = 0 + const inserted: unknown[] = [] + const from = vi.fn(() => ({ + insert: (rows: unknown) => { + inserted.push(rows) + const result = script[Math.min(call, script.length - 1)] + call++ + return { select: () => Promise.resolve(result) } + }, + })) + return { supabase: { from } as unknown as SupabaseClient, inserted, calls: () => call } +} + +const row = (n: number) => ({ invoice_number: `F-${n}` }) + +describe('insertWithPerRowFallback', () => { + it('bulk success: one statement, results paired by index', async () => { + const { supabase, calls } = makeSupabase([ + { data: [{ id: 'a' }, { id: 'b' }], error: null }, + ]) + + const outcome = await insertWithPerRowFallback(supabase, 'invoices', [row(1), row(2)], 'id') + + expect(calls()).toBe(1) + expect(outcome.failedCount).toBe(0) + expect(outcome.firstError).toBeNull() + expect(outcome.returned).toEqual([{ id: 'a' }, { id: 'b' }]) + }) + + it('bulk failure: retries per row, healthy rows land, offender is counted with its message', async () => { + const dup = 'duplicate key value violates unique constraint "idx_invoices_company_invoice_number"' + const { supabase, calls } = makeSupabase([ + { data: null, error: { message: dup } }, // bulk statement + { data: [{ id: 'a' }], error: null }, // row 0 + { data: null, error: { message: dup } }, // row 1 (the offender) + { data: [{ id: 'c' }], error: null }, // row 2 + ]) + + const outcome = await insertWithPerRowFallback(supabase, 'invoices', [row(1), row(2), row(3)], 'id') + + expect(calls()).toBe(4) + expect(outcome.returned).toEqual([{ id: 'a' }, null, { id: 'c' }]) + expect(outcome.failedCount).toBe(1) + expect(outcome.firstError).toBe(dup) + }) + + it('bulk success with short read-back: never retries (rows are already in the table)', async () => { + const { supabase, calls } = makeSupabase([ + { data: [{ id: 'a' }], error: null }, // succeeded, but returned 1 of 2 + ]) + + const outcome = await insertWithPerRowFallback(supabase, 'customers', [row(1), row(2)], 'id') + + expect(calls()).toBe(1) + expect(outcome.returned).toEqual([{ id: 'a' }, null]) + expect(outcome.failedCount).toBe(1) + expect(outcome.firstError).toMatch(/could not be paired/) + }) + + it('empty input: no statements at all', async () => { + const { supabase, calls } = makeSupabase([{ data: [], error: null }]) + + const outcome = await insertWithPerRowFallback(supabase, 'invoices', [], 'id') + + expect(calls()).toBe(0) + expect(outcome).toEqual({ returned: [], failedCount: 0, firstError: null }) + }) +}) diff --git a/extensions/general/arcim-migration/lib/entity-mapper.ts b/extensions/general/arcim-migration/lib/entity-mapper.ts index 1af6cc16..0b469fc7 100644 --- a/extensions/general/arcim-migration/lib/entity-mapper.ts +++ b/extensions/general/arcim-migration/lib/entity-mapper.ts @@ -517,7 +517,10 @@ export function mapSalesInvoice( user_id: userId, company_id: companyId, customer_id: customerId, - invoice_number: dto.invoiceNumber, + // Empty string must become NULL: the UNIQUE (company_id, invoice_number) + // index is partial on NOT NULL, so '' from a provider payload missing the + // field would collide on the second invoice and reject the insert. + invoice_number: dto.invoiceNumber || null, invoice_date: dto.issueDate, due_date: dto.dueDate || dto.issueDate, status: statusMap[dto.status] || 'sent', @@ -540,6 +543,9 @@ export function mapSalesInvoice( document_type: isCreditNote ? 'credit_note' : 'invoice', paid_at: dto.paymentStatus.paid ? dto.paymentStatus.lastPaymentDate || dto.issueDate : null, paid_amount: dto.paymentStatus.paid ? total : round2(total - dto.paymentStatus.balance.value), + // remaining_amount is NOT NULL DEFAULT 0, so omitting it makes every + // migrated open invoice look fully settled in AR aging. + remaining_amount: dto.paymentStatus.paid ? 0 : Math.max(0, round2(dto.paymentStatus.balance.value)), } const items = dto.lines.map((line, idx) => mapSalesInvoiceLine(line, idx)) @@ -625,7 +631,11 @@ export function mapSupplierInvoice( user_id: userId, company_id: companyId, supplier_id: supplierId, - supplier_invoice_number: dto.invoiceNumber, + // Empty string must become NULL: with '' every number-less invoice from + // the same supplier collides on the UNIQUE + // (company_id, supplier_id, supplier_invoice_number) index, while NULLs + // are treated as distinct. + supplier_invoice_number: dto.invoiceNumber || null, invoice_date: dto.issueDate, due_date: dto.dueDate || dto.issueDate, received_date: dto.issueDate, diff --git a/extensions/general/arcim-migration/lib/insert-fallback.ts b/extensions/general/arcim-migration/lib/insert-fallback.ts new file mode 100644 index 00000000..819506f3 --- /dev/null +++ b/extensions/general/arcim-migration/lib/insert-fallback.ts @@ -0,0 +1,77 @@ +/** + * Chunked inserts during migration are one PostgREST statement per chunk, + * which Postgres treats as all-or-nothing: a single bad row (usually a + * 23505 unique violation) rejects every row in the chunk. Historically that + * surfaced as "300 misslyckades" with zero explanation of which row broke. + * + * This helper keeps the fast path (one bulk insert) and, only when the bulk + * statement fails, retries the same rows one at a time so healthy rows still + * land and the real per-row error becomes visible. + */ + +import type { SupabaseClient } from '@supabase/supabase-js' + +export interface PerRowInsertOutcome { + /** + * Selected columns of each inserted row, index-aligned with the input. + * null = that row failed to insert. + */ + returned: (Record | null)[] + failedCount: number + /** First database error message, for result summaries and logs. */ + firstError: string | null +} + +export async function insertWithPerRowFallback( + supabase: SupabaseClient, + table: string, + rows: Record[], + select: string, +): Promise { + if (rows.length === 0) { + return { returned: [], failedCount: 0, firstError: null } + } + + const bulk = await supabase.from(table).insert(rows).select(select) + if (!bulk.error) { + const data = (bulk.data ?? []) as unknown as Record[] + // PostgREST returns inserted rows in input order, so a full-length result + // pairs 1:1 with the input. + if (data.length === rows.length) { + return { returned: data, failedCount: 0, firstError: null } + } + // The statement SUCCEEDED but returned fewer rows than sent (an RLS + // read-back gap). The rows ARE in the table, so retrying per row would + // duplicate them: pair what came back and report the tail unpaired. + const returned: (Record | null)[] = new Array(rows.length).fill(null) + for (let i = 0; i < data.length; i++) returned[i] = data[i] + return { + returned, + failedCount: rows.length - data.length, + firstError: `insert returned ${data.length} of ${rows.length} rows; unreturned rows were inserted but could not be paired`, + } + } + + const returned: (Record | null)[] = new Array(rows.length).fill(null) + let failedCount = 0 + let firstError: string | null = bulk.error?.message ?? null + + for (let i = 0; i < rows.length; i++) { + const single = await supabase.from(table).insert(rows[i]).select(select) + if (single.error) { + failedCount++ + firstError ??= single.error.message + // Keep the first PER-ROW error too: the bulk message is often the same, + // but when it isn't, the row-level one names the actual offender. + if (failedCount === 1 && single.error.message) { + firstError = single.error.message + } + continue + } + const data = (single.data ?? []) as unknown as Record[] + returned[i] = data[0] ?? null + if (!returned[i]) failedCount++ + } + + return { returned, failedCount, firstError } +} diff --git a/extensions/general/arcim-migration/lib/migration-orchestrator.ts b/extensions/general/arcim-migration/lib/migration-orchestrator.ts index 2b43534e..3b2416ad 100644 --- a/extensions/general/arcim-migration/lib/migration-orchestrator.ts +++ b/extensions/general/arcim-migration/lib/migration-orchestrator.ts @@ -38,6 +38,7 @@ import { type CustomerMetadataEnrichment, type ExistingCustomerMetadata, } from './customer-metadata' +import { insertWithPerRowFallback } from './insert-fallback' import { mapCustomer, mapSupplier, @@ -217,6 +218,7 @@ export async function executeMigration(options: MigrationOptions): Promise() for (const customer of customers) { if (!customer.active) { @@ -258,32 +265,40 @@ export async function executeMigration(options: MigrationOptions): Promise p.row) - const { data: inserted, error } = await supabase - .from('customers') - .insert(rows) - .select('id, org_number, name') + const outcome = await insertWithPerRowFallback( + supabase, 'customers', batch.map((p) => p.row), 'id, org_number, name' + ) - if (error) { - console.error(`[migration] Customer batch insert failed (${batch.length} rows):`, error.message) - skipReasons.failed = (skipReasons.failed ?? 0) + batch.length - skipped += batch.length - continue + if (outcome.failedCount > 0) { + console.error( + `[migration] Customer insert failed for ${outcome.failedCount} of ${batch.length} rows:`, + outcome.firstError + ) + skipReasons.failed = (skipReasons.failed ?? 0) + outcome.failedCount + skipped += outcome.failedCount + errorSample ??= outcome.firstError } - // PostgREST returns inserted rows in the same order as supplied, - // so we can pair them up by index to recover the provider id. - const insertedRows = inserted ?? [] - for (let i = 0; i < batch.length && i < insertedRows.length; i++) { + for (let i = 0; i < batch.length; i++) { + const insertedRow = outcome.returned[i] + if (!insertedRow) continue const providerId = batch[i].dto.id - const newId = insertedRows[i].id + const newId = insertedRow.id as string customerIdMap.set(providerId, newId) - if (insertedRows[i].org_number) orgNumberToCustomerId.set(insertedRows[i].org_number!, newId) - if (insertedRows[i].name) nameToCustomerId.set(insertedRows[i].name!, newId) + if (insertedRow.org_number) orgNumberToCustomerId.set(insertedRow.org_number as string, newId) + if (insertedRow.name) nameToCustomerId.set(insertedRow.name as string, newId) imported++ } } @@ -313,6 +328,7 @@ export async function executeMigration(options: MigrationOptions): Promise } const pending: PendingSupplier[] = [] + // Same in-run repeat guard as customers. + const pendingSupplierKeys = new Set() for (const supplier of suppliers) { if (!supplier.active) { @@ -380,35 +399,45 @@ export async function executeMigration(options: MigrationOptions): Promise p.row) - const { data: inserted, error } = await supabase - .from('suppliers') - .insert(rows) - .select('id, org_number, name') + const outcome = await insertWithPerRowFallback( + supabase, 'suppliers', batch.map((p) => p.row), 'id, org_number, name' + ) - if (error) { - console.error(`[migration] Supplier batch insert failed (${batch.length} rows):`, error.message) - skipReasons.failed = (skipReasons.failed ?? 0) + batch.length - skipped += batch.length - continue + if (outcome.failedCount > 0) { + console.error( + `[migration] Supplier insert failed for ${outcome.failedCount} of ${batch.length} rows:`, + outcome.firstError + ) + skipReasons.failed = (skipReasons.failed ?? 0) + outcome.failedCount + skipped += outcome.failedCount + errorSample ??= outcome.firstError } - const insertedRows = inserted ?? [] - for (let i = 0; i < batch.length && i < insertedRows.length; i++) { + for (let i = 0; i < batch.length; i++) { + const insertedRow = outcome.returned[i] + if (!insertedRow) continue const providerId = batch[i].dto.id - const newId = insertedRows[i].id + const newId = insertedRow.id as string supplierIdMap.set(providerId, newId) - if (insertedRows[i].org_number) orgNumberToSupplierId.set(insertedRows[i].org_number!, newId) - if (insertedRows[i].name) nameToSupplierId.set(insertedRows[i].name!, newId) + if (insertedRow.org_number) orgNumberToSupplierId.set(insertedRow.org_number as string, newId) + if (insertedRow.name) nameToSupplierId.set(insertedRow.name as string, newId) imported++ } } - results.suppliers = { total: suppliers.length, imported, skipped, skipReasons } + results.suppliers = { total: suppliers.length, imported, skipped, skipReasons, errorSample: errorSample ?? undefined } } catch (err) { console.error('Failed to import suppliers:', err) } @@ -434,6 +463,13 @@ export async function executeMigration(options: MigrationOptions): Promise() // Phase A: resolve customer for each invoice; collect those that // need a minimal customer record to be created on-the-fly. @@ -455,6 +491,14 @@ export async function executeMigration(options: MigrationOptions): Promise 0) { const stubList = [...stubByKey.values()] for (const batch of chunk(stubList, INSERT_CHUNK_SIZE)) { - const { data: inserted, error } = await supabase - .from('customers') - .insert(batch.map((s) => s.row)) - .select('id, org_number, name') + const outcome = await insertWithPerRowFallback( + supabase, 'customers', batch.map((s) => s.row), 'id, org_number, name' + ) - if (error) { + if (outcome.failedCount > 0) { console.error( - `[migration] Sales invoice customer stub insert failed (${batch.length} rows):`, - error.message + `[migration] Sales invoice customer stub insert failed for ${outcome.failedCount} of ${batch.length} rows:`, + outcome.firstError ) - // Mark invoices waiting on failed stubs as no-match - for (const s of batch) { - for (const idx of s.waitingInvoiceIndices) { - resolved[idx] = { ...resolved[idx], customerId: '__FAILED__' } - } - } - continue + errorSample ??= outcome.firstError } - const insertedRows = inserted ?? [] - for (let i = 0; i < batch.length && i < insertedRows.length; i++) { - const newId = insertedRows[i].id - if (insertedRows[i].org_number) orgNumberToCustomerId.set(insertedRows[i].org_number!, newId) - if (insertedRows[i].name) nameToCustomerId.set(insertedRows[i].name!, newId) + for (let i = 0; i < batch.length; i++) { + const insertedRow = outcome.returned[i] + if (!insertedRow) { + // Mark invoices waiting on this failed stub as no-match + for (const idx of batch[i].waitingInvoiceIndices) { + resolved[idx] = { ...resolved[idx], customerId: '__FAILED__' } + } + continue + } + const newId = insertedRow.id as string + if (insertedRow.org_number) orgNumberToCustomerId.set(insertedRow.org_number as string, newId) + if (insertedRow.name) nameToCustomerId.set(insertedRow.name as string, newId) for (const idx of batch[i].waitingInvoiceIndices) { resolved[idx] = { ...resolved[idx], customerId: newId } } @@ -534,10 +578,13 @@ export async function executeMigration(options: MigrationOptions): Promise { if (r.customerId === '__FAILED__') { - skipReasons.noMatch = (skipReasons.noMatch ?? 0) + 1 + skipReasons.failed = (skipReasons.failed ?? 0) + 1 skipped++ return false } @@ -562,22 +609,25 @@ export async function executeMigration(options: MigrationOptions): Promise m.invoice)) - .select('id') + const outcome = await insertWithPerRowFallback( + supabase, 'invoices', mappedBatch.map((m) => m.invoice), 'id' + ) - if (invErr) { - console.error(`[migration] Sales invoice batch insert failed (${batch.length}):`, invErr.message) - skipReasons.failed = (skipReasons.failed ?? 0) + batch.length - skipped += batch.length - continue + if (outcome.failedCount > 0) { + console.error( + `[migration] Sales invoice insert failed for ${outcome.failedCount} of ${batch.length} rows:`, + outcome.firstError + ) + skipReasons.failed = (skipReasons.failed ?? 0) + outcome.failedCount + skipped += outcome.failedCount + errorSample ??= outcome.firstError } - const invoiceRows = insertedInvoices ?? [] const allItems: Record[] = [] - for (let i = 0; i < mappedBatch.length && i < invoiceRows.length; i++) { - const invoiceId = invoiceRows[i].id + for (let i = 0; i < mappedBatch.length; i++) { + const insertedRow = outcome.returned[i] + if (!insertedRow) continue + const invoiceId = insertedRow.id for (const item of mappedBatch[i].items) { allItems.push({ ...item, invoice_id: invoiceId }) } @@ -599,7 +649,7 @@ export async function executeMigration(options: MigrationOptions): Promise 0) { const stubList = [...stubByKey.values()] for (const batch of chunk(stubList, INSERT_CHUNK_SIZE)) { - const { data: inserted, error } = await supabase - .from('suppliers') - .insert(batch.map((s) => s.row)) - .select('id, org_number, name') + const outcome = await insertWithPerRowFallback( + supabase, 'suppliers', batch.map((s) => s.row), 'id, org_number, name' + ) - if (error) { + if (outcome.failedCount > 0) { console.error( - `[migration] Supplier invoice supplier stub insert failed (${batch.length}):`, - error.message + `[migration] Supplier invoice supplier stub insert failed for ${outcome.failedCount} of ${batch.length} rows:`, + outcome.firstError ) - for (const s of batch) { - for (const idx of s.waitingInvoiceIndices) { - resolved[idx] = { ...resolved[idx], supplierId: '__FAILED__' } - } - } - continue + errorSample ??= outcome.firstError } - const insertedRows = inserted ?? [] - for (let i = 0; i < batch.length && i < insertedRows.length; i++) { - const newId = insertedRows[i].id - if (insertedRows[i].org_number) orgNumberToSupplierId.set(insertedRows[i].org_number!, newId) - if (insertedRows[i].name) nameToSupplierId.set(insertedRows[i].name!, newId) + for (let i = 0; i < batch.length; i++) { + const insertedRow = outcome.returned[i] + if (!insertedRow) { + for (const idx of batch[i].waitingInvoiceIndices) { + resolved[idx] = { ...resolved[idx], supplierId: '__FAILED__' } + } + continue + } + const newId = insertedRow.id as string + if (insertedRow.org_number) orgNumberToSupplierId.set(insertedRow.org_number as string, newId) + if (insertedRow.name) nameToSupplierId.set(insertedRow.name as string, newId) for (const idx of batch[i].waitingInvoiceIndices) { resolved[idx] = { ...resolved[idx], supplierId: newId } } @@ -735,20 +786,30 @@ export async function executeMigration(options: MigrationOptions): Promise() const ready = resolved.filter((r) => { if (r.supplierId === '__FAILED__' || !r.supplierId) { + // Failed stub insert = DB failure with errorSample set, so count + // it as failed; noMatch would hide the error in the result row. if (r.supplierId === '__FAILED__') { - skipReasons.noMatch = (skipReasons.noMatch ?? 0) + 1 + skipReasons.failed = (skipReasons.failed ?? 0) + 1 skipped++ } return false } - const dupKey = `${r.supplierId}::${r.dto.invoiceNumber}` - if (existingSuppInvKeys.has(dupKey)) { - skipReasons.duplicate = (skipReasons.duplicate ?? 0) + 1 - skipped++ - return false + if (r.dto.invoiceNumber) { + const dupKey = `${r.supplierId}::${r.dto.invoiceNumber}` + if (existingSuppInvKeys.has(dupKey) || seenSuppInvKeys.has(dupKey)) { + skipReasons.duplicate = (skipReasons.duplicate ?? 0) + 1 + skipped++ + return false + } + seenSuppInvKeys.add(dupKey) } return true }) @@ -770,24 +831,29 @@ export async function executeMigration(options: MigrationOptions): Promise m.invoice)) - .select('id') + const outcome = await insertWithPerRowFallback( + supabase, 'supplier_invoices', mappedBatch.map((m) => m.invoice), 'id' + ) - if (invErr) { - console.error(`[migration] Supplier invoice batch insert failed (${batch.length}):`, invErr.message) - skipReasons.failed = (skipReasons.failed ?? 0) + batch.length - skipped += batch.length - // Roll the counter back so we don't leave a huge gap on retry. - nextArrivalNumber -= batch.length - continue + if (outcome.failedCount > 0) { + console.error( + `[migration] Supplier invoice insert failed for ${outcome.failedCount} of ${batch.length} rows:`, + outcome.firstError + ) + skipReasons.failed = (skipReasons.failed ?? 0) + outcome.failedCount + skipped += outcome.failedCount + errorSample ??= outcome.firstError + // A failed row leaves a hole in the arrival numbering. That is + // acceptable: ankomstnummer is an internal sequence, not a + // verifikationsnummer, and rewinding the counter after a PARTIAL + // success would hand out numbers that already landed. } - const invoiceRows = insertedInvoices ?? [] const allItems: Record[] = [] - for (let i = 0; i < mappedBatch.length && i < invoiceRows.length; i++) { - const invoiceId = invoiceRows[i].id + for (let i = 0; i < mappedBatch.length; i++) { + const insertedRow = outcome.returned[i] + if (!insertedRow) continue + const invoiceId = insertedRow.id for (const item of mappedBatch[i].items) { allItems.push({ ...item, supplier_invoice_id: invoiceId }) } @@ -809,7 +875,7 @@ export async function executeMigration(options: MigrationOptions): Promise { + let fetchSpy: ReturnType; + + beforeEach(() => { + fetchSpy = vi.spyOn(globalThis, 'fetch'); + }); + + afterEach(() => { + fetchSpy.mockRestore(); + }); + + function requestedUrl(callIndex: number): URL { + const [input] = fetchSpy.mock.calls[callIndex]; + return new URL(String(input)); + } + + it('getPage sends $page/$pagesize, never $top/$skip', async () => { + fetchSpy.mockResolvedValueOnce(page([{ Id: 'a' }], 1)); + + const client = new VismaClient(); + await client.getPage('token', '/customers', { page: 2, pageSize: 100 }); + + const url = requestedUrl(0); + expect(url.searchParams.get('$page')).toBe('2'); + expect(url.searchParams.get('$pagesize')).toBe('100'); + expect(url.searchParams.has('$top')).toBe(false); + expect(url.searchParams.has('$skip')).toBe(false); + }); + + it('getPaginated walks every page once and concatenates in order', async () => { + fetchSpy + .mockResolvedValueOnce(page([{ Id: 'a' }, { Id: 'b' }], 3, 5)) + .mockResolvedValueOnce(page([{ Id: 'c' }, { Id: 'd' }], 3, 5)) + .mockResolvedValueOnce(page([{ Id: 'e' }], 3, 5)); + + const client = new VismaClient(); + const items = await client.getPaginated<{ Id: string }>('token', '/customerinvoices'); + + expect(items.map((i) => i.Id)).toEqual(['a', 'b', 'c', 'd', 'e']); + expect(fetchSpy).toHaveBeenCalledTimes(3); + expect(requestedUrl(0).searchParams.get('$page')).toBe('1'); + expect(requestedUrl(1).searchParams.get('$page')).toBe('2'); + expect(requestedUrl(2).searchParams.get('$page')).toBe('3'); + }); + + it('getPaginated stops on an empty page even if Meta promises more', async () => { + fetchSpy + .mockResolvedValueOnce(page([{ Id: 'a' }], 99)) + .mockResolvedValueOnce(page([], 99)); + + const client = new VismaClient(); + const items = await client.getPaginated<{ Id: string }>('token', '/customers'); + + expect(items).toHaveLength(1); + expect(fetchSpy).toHaveBeenCalledTimes(2); + }); +}); diff --git a/lib/providers/visma/client.ts b/lib/providers/visma/client.ts index e82a455e..1aabca9e 100644 --- a/lib/providers/visma/client.ts +++ b/lib/providers/visma/client.ts @@ -88,8 +88,12 @@ export class VismaClient { const page = options?.page ?? 1; const params = new URLSearchParams(); - params.set('$top', String(pageSize)); - params.set('$skip', String((page - 1) * pageSize)); + // eAccounting paginates with $page/$pagesize (default 50, max 1000). + // OData-style $top/$skip are silently IGNORED by the API: every request + // returns page 1, so a multi-page fetch yields the first page N times + // and never reaches the rest of the collection. + params.set('$page', String(page)); + params.set('$pagesize', String(pageSize)); if (options?.modifiedSince && options?.modifiedField) { params.set( @@ -135,6 +139,11 @@ export class VismaClient { modifiedField: options?.modifiedField, }); + // An empty page means the collection is exhausted regardless of what + // Meta claims; trusting a stale/buggy TotalNumberOfPages here would + // re-append duplicate rows or loop for nothing. + if (result.items.length === 0) break; + allItems.push(...result.items); totalPages = result.totalPages; page++;