feat(invoice): handle duplicate supplier invoice number conflict with… (#685)

* feat(invoice): handle duplicate supplier invoice number conflict with detailed error response

* fix(invoice): enhance error response for duplicate invoice number to ensure data minimization

app/(dashboard)/supplier-invoices/new/page.tsx

@@ -37,6 +37,30 @@ interface LineItem {
   reverse_charge_rate?: number
 }
 
+// The existing invoice surfaced on a duplicate-number conflict, used to drive
+// the resolution dialog (open it / uncredit-and-retry).
+interface ExistingSupplierInvoice {
+  id: string
+  supplier_invoice_number: string
+  status: string
+  credit_note_id: string | null
+}
+
+// Canonical create/convert response. On failure `error` is the structured
+// envelope's inner object ({ code, message, details }); a few legacy convert
+// paths still return a flat string, so accept both.
+interface CreateResult {
+  data?: { id: string; arrival_number: number }
+  error?:
+    | string
+    | {
+        code?: string
+        message?: string
+        message_en?: string
+        details?: { existing?: ExistingSupplierInvoice | null }
+      }
+}
+
 interface FormData {
   supplier_id: string
   supplier_invoice_number: string
@@ -271,7 +295,7 @@ export default function NewSupplierInvoicePage() {
   // Conflict state for duplicate-supplier-invoice-number
   const [conflict, setConflict] = useState<{
     message: string
-    existing: { id: string; supplier_invoice_number: string; status: string; credit_note_id: string | null } | null
+    existing: ExistingSupplierInvoice | null
   } | null>(null)
   const [isResolvingConflict, setIsResolvingConflict] = useState(false)
   const invoiceNumberInputRef = useRef<HTMLInputElement | null>(null)
@@ -733,16 +757,13 @@ export default function NewSupplierInvoicePage() {
   }
 
   // Single submit endpoint chooser — convert when we came from inbox, plain
-  // POST otherwise. Both endpoints validate the same CreateSupplierInvoiceSchema.
+  // POST otherwise. Both endpoints validate the same CreateSupplierInvoiceSchema
+  // and return the same canonical error envelope ({ error: { code, message,
+  // details } }) — including the recoverable duplicate-number 409.
   async function postCreate(data: FormData): Promise<{
     ok: boolean
     status: number
-    result: {
-      data?: { id: string; arrival_number: number }
-      error?: string
-      message?: string
-      existing?: { id: string; supplier_invoice_number: string; status: string; credit_note_id: string | null }
-    }
+    result: CreateResult
   }> {
     const url = inboxItemId
       ? `/api/extensions/ext/invoice-inbox/items/${inboxItemId}/convert`
@@ -809,7 +830,12 @@ export default function NewSupplierInvoicePage() {
     const { ok, status, result } = await postCreate(data)
 
     if (!ok) {
-      handleCreateError(status, result)
+      // EF/direct path also hits the duplicate-number 409 (e.g. converting an
+      // inbox receipt whose number was already registered) — offer recovery
+      // instead of a dead-end toast.
+      if (!tryHandleDuplicateConflict(status, result)) {
+        handleCreateError(status, result)
+      }
       setIsSubmitting(false)
       return
     }
@@ -892,24 +918,39 @@ export default function NewSupplierInvoicePage() {
       router.push(afterCreate(invoiceId))
     } else {
       // Treat duplicate-number as a recoverable conflict; everything else as a hard error.
-      if (status === 409 && result.error === 'duplicate_supplier_invoice_number') {
-        setShowReview(false)
-        setConflict({
-          message: result.message || t('duplicate_default_message'),
-          existing: result.existing ?? null,
-        })
-      } else {
+      if (!tryHandleDuplicateConflict(status, result)) {
         handleCreateError(status, result)
       }
     }
     setIsSubmitting(false)
   }
 
+  // Detect the recoverable duplicate-supplier-invoice-number conflict and open
+  // the resolution dialog. Both the inbox `convert` route and the plain create
+  // route return the same structured 409 envelope, so this works for every
+  // submit path. Returns true when handled (caller should skip the error toast).
+  function tryHandleDuplicateConflict(status: number, result: CreateResult): boolean {
+    const err = result.error
+    if (
+      status !== 409 ||
+      typeof err !== 'object' ||
+      err === null ||
+      err.code !== 'SI_CREATE_DUPLICATE_INVOICE_NUMBER'
+    ) {
+      return false
+    }
+    // Close the review dialog if it was the path that triggered the conflict;
+    // a no-op for the EF/direct paths where it was never opened.
+    setShowReview(false)
+    setConflict({
+      message: err.message || t('duplicate_default_message'),
+      existing: err.details?.existing ?? null,
+    })
+    return true
+  }
+
   // Shared error toast for non-conflict failures.
-  function handleCreateError(
-    status: number,
-    result: { error?: string; message?: string },
-  ) {
+  function handleCreateError(status: number, result: CreateResult) {
     toast({
       title: t('register_invoice_failed_title'),
       description: getErrorMessage(result, { context: 'supplier_invoice', statusCode: status }),
@@ -992,12 +1033,7 @@ export default function NewSupplierInvoicePage() {
     const { ok, status, result } = await postCreate(pendingData)
 
     if (!ok || !result.data) {
-      if (status === 409 && result.error === 'duplicate_supplier_invoice_number') {
-        setConflict({
-          message: result.message || t('duplicate_default_message'),
-          existing: result.existing ?? null,
-        })
-      } else {
+      if (!tryHandleDuplicateConflict(status, result)) {
         handleCreateError(status, result)
       }
       setIsSubmitting(false)

extensions/general/invoice-inbox/__tests__/convert-route.test.ts

@@ -131,6 +131,45 @@ describe('POST /items/:id/convert', () => {
     expect(status).toBe(404)
   })
 
+  it('returns 409 (not 500) when the supplier invoice number already exists', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: makeInvoiceInboxItem({ status: 'received' }) })
+    enqueue({ data: makeSupplier({ id: SUPPLIER_UUID }) })
+    enqueue({ data: 42 })
+    // Insert collides with idx_supplier_invoices_company_supplier_number.
+    enqueue({
+      data: null,
+      error: {
+        code: '23505',
+        message:
+          'duplicate key value violates unique constraint "idx_supplier_invoices_company_supplier_number"',
+      },
+    })
+    // Lookup of the existing (non-credited) invoice for the conflict payload.
+    enqueue({
+      data: { id: 'existing-1', supplier_invoice_number: 'F-2024-001', status: 'approved' },
+    })
+
+    const ctx = buildCtx(supabase)
+    const request = createMockRequest('/items/item-1/convert', {
+      method: 'POST',
+      body: VALID_CONVERT_BODY,
+      searchParams: { _id: 'item-1' },
+    })
+    const res = await route.handler(request, ctx)
+    const { status, body } = await parseJsonResponse<{
+      error: { code: string; details?: Record<string, unknown> & { existing?: { id: string } } }
+    }>(res)
+
+    expect(status).toBe(409)
+    expect(body.error.code).toBe('SI_CREATE_DUPLICATE_INVOICE_NUMBER')
+    expect(body.error.details?.existing?.id).toBe('existing-1')
+    // Data minimisation: the raw request body must NOT be echoed back into the
+    // error envelope — only the server-authoritative `existing` row.
+    expect(body.error.details).not.toHaveProperty('supplierId')
+    expect(body.error.details).not.toHaveProperty('supplierInvoiceNumber')
+  })
+
   it('successfully converts inbox item to supplier invoice', async () => {
     const { supabase, enqueue } = createQueuedMockSupabase()
     const inboxItem = makeInvoiceInboxItem({ status: 'received', document_id: 'doc-1' })

extensions/general/invoice-inbox/index.ts

@@ -1718,6 +1718,68 @@ export const invoiceInboxExtension: Extension = {
           .single()
 
         if (invoiceError || !invoice) {
+          // A unique-index hit on (company_id, supplier_id,
+          // supplier_invoice_number) is a recoverable conflict — the user
+          // already registered this invoice (often manually, then tried to
+          // convert the same inbox document). Mirror the main
+          // /api/supplier-invoices route and return a friendly 409 with the
+          // existing invoice, instead of letting the raw Postgres message
+          // surface as a generic 500 ("Ett oväntat serverfel uppstod").
+          const pgErr = invoiceError as { code?: string; message?: string } | null
+          const isDuplicateNumber =
+            pgErr?.code === '23505' &&
+            (pgErr.message || '').includes('idx_supplier_invoices_company_supplier_number')
+
+          if (isDuplicateNumber) {
+            // Tenancy: ctx.supabase is the cookie-scoped RLS client and the
+            // supplier_invoices SELECT policy is
+            // `company_id IN (SELECT user_company_ids())`. Combined with the
+            // explicit company_id filter below, this lookup can only ever
+            // resolve an invoice the caller's own company owns — the returned
+            // details are never cross-tenant (OWASP ASVS V8.2.1; ISO 27001
+            // A.8.3; GDPR art.25(2)).
+            const { data: existing } = await ctx.supabase
+              .from('supplier_invoices')
+              .select('id, supplier_invoice_number, status')
+              .eq('company_id', ctx.companyId)
+              .eq('supplier_id', body.supplier_id)
+              .eq('supplier_invoice_number', body.supplier_invoice_number)
+              .maybeSingle()
+
+            let creditNoteId: string | null = null
+            if (existing?.status === 'credited') {
+              const { data: creditNote } = await ctx.supabase
+                .from('supplier_invoices')
+                .select('id')
+                .eq('company_id', ctx.companyId)
+                .eq('credited_invoice_id', existing.id)
+                .eq('is_credit_note', true)
+                .maybeSingle()
+              creditNoteId = creditNote?.id ?? null
+            }
+
+            // Return ONLY server-authoritative fields the recovery dialog needs
+            // (the existing row, read under RLS). The raw request body
+            // (supplier_id / supplier_invoice_number) is deliberately not
+            // echoed back: the client already holds it from its own form state,
+            // and reflecting user-supplied values widens the response surface
+            // for no benefit (GDPR art.5(1)(c) data minimisation; OWASP ASVS
+            // V4.5). The Postgres constraint name is used only to classify the
+            // error above and is never placed in the response.
+            return errorResponseFromCode('SI_CREATE_DUPLICATE_INVOICE_NUMBER', ctx.log, {
+              details: {
+                existing: existing
+                  ? {
+                      id: existing.id,
+                      supplier_invoice_number: existing.supplier_invoice_number,
+                      status: existing.status,
+                      credit_note_id: creditNoteId,
+                    }
+                  : null,
+              },
+            })
+          }
+
           return NextResponse.json({ error: invoiceError?.message || 'Failed to create invoice' }, { status: 500 })
         }