feat(import): undo a bank file import including ignored transactions (#1764)

* feat(import): undo a bank file import including ignored transactions (#1672) A mis-parsed bank CSV could not be cleaned up: re-importing dedup-skips the bad rows, the single-row DELETE refuses imported rows by design (TRANSACTION_DELETE_IMPORTED), and there was no bulk action. Transactions also never recorded which import batch inserted them, so a strictly scoped undo was impossible. - transactions.bank_file_import_id: batch link stamped at ingest by both bank-file import paths (dashboard execute route, v1 REST route). PSD2/ manual/MCP rows stay NULL. No retroactive backfill: fuzzy attribution could delete rows belonging to a different import. - undo_bank_file_import RPC: owner/admin-only bulk delete of the batch's unbooked rows, ignored INCLUDED. Booked rows (journal link, payment rows, voucher links) and rows with append-only payment_match_log history are skipped and reported, mirroring the single-row route's guards. Marks the import 'undone' (re-import reuses the row via the company_id+file_hash upsert), writes one audit_log summary row, and hardens the actor gate like undo_sie_import: p_user_id honored only for service_role callers, 42501 otherwise, no anon EXECUTE. - DELETE /api/import/bank-file/[id]/undo returns the deletion report; RPC 42501 maps to BANK_FILE_UNDO_FORBIDDEN (403). Closes #1672 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Emil <emilmattsson14@gmail.com> * fix(import): return 404 when the bank-file undo target does not exist An unknown or out-of-company import id answered 400 BANK_FILE_UNDO_FAILED, hiding the not-found semantics the SIE import routes already expose ('Import not found', 404). Flag the case in undoBankFileImport (notFound) and map it to a new BANK_FILE_UNDO_NOT_FOUND structured error (404); status-refusals and RPC failures keep the 400 envelope. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Emil <emilmattsson14@gmail.com> * feat(import): show bank file import history with undo on the import tab The undo shipped for issue #1672 was API-only: no surface listed a company's bank_file_imports, so neither users nor founders could reach DELETE /api/import/bank-file/[id]/undo, and the deletion report existed only in JSON. Mirror the SIE pattern (SIEImportHistory, #1574): - GET /api/import/bank-file: list the company's imports newest-first, same { data, count, limit, offset } shape as GET /api/import/sie. - BankFileImportHistory: fold-open 'Tidigare bankfilsimporter' row on the Importera tab with filename, date, format, imported count and status per import, plus an undo action on completed rows behind a DestructiveConfirmDialog. The undo stays owner/admin-only via the undo_bank_file_import RPC's actor gate, like the SIE one. - After undo the toast shows the full report: transactions removed, booked rows skipped, rows with match history skipped, so nothing disappears silently from the ledger's surroundings. - i18n strings in messages/sv.json and messages/en.json following the sie_history_* key style; list-route test mirroring the SIE list test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Emil <emilmattsson14@gmail.com> * chore(migrations): move undo_bank_file_import after main's 2026-08-19 migrations Signed-off-by: Emil <emilmattsson14@gmail.com> * fix(import): validate bank-file list params, fail closed on undo lookup, log lost batch attribution Review findings on #1764 (CodeRabbit): - GET /api/import/bank-file rejects non-integer/negative/oversized limit and offset and unknown status with a mapped 400 (BANK_FILE_LIST_INVALID_QUERY), limit capped at 100; boundary and invalid-input tests added. - undoBankFileImport distinguishes PGRST116 (zero rows -> notFound/404) from other lookup failures, which now return an error instead of masquerading as a permanent 404. - The v1 import route no longer discards the bank_file_imports upsert error: kept non-fatal by design (an unattributed batch imports fine and never appears in undo history), but the failure is now logged loudly. - Route test beforeEach clears the event bus (repo convention). Signed-off-by: Emil <emilmattsson14@gmail.com> --------- Signed-off-by: Emil <emilmattsson14@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-20 21:25:18 +02:00
parent ff4425d10e
commit 47c039453c
23 changed files with 1665 additions and 7 deletions
@@ -278,6 +278,27 @@ describe('POST /api/v1/companies/:companyId/imports/bank', () => {
    }
  })

+  it('passes the bank_file_imports batch id to ingest so undo can scope its delete', async () => {
+    supabase = makeSupabase({
+      company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
+      bank_file_imports: { data: { id: 'import-1' }, error: null },
+    })
+    mockServiceClient.mockReturnValue(supabase)
+
+    await callRoute()
+
+    expect(ingestMock).toHaveBeenCalledTimes(1)
+    expect(ingestMock.mock.calls[0][4]).toEqual({ bankFileImportId: 'import-1' })
+  })
+
+  it('still ingests (rows unlinked) when the upsert returns no batch id', async () => {
+    // Default beforeEach supabase: bank_file_imports resolves { data: null }.
+    await callRoute()
+
+    expect(ingestMock).toHaveBeenCalledTimes(1)
+    expect(ingestMock.mock.calls[0][4]).toBeUndefined()
+  })
+
  it('stamps ids and provenance with the fallback format when an explicit override parses nothing', async () => {
    // Swedbank file forced as `seb`: the parser falls back to the detected
    // format, and external ids / import_source must follow the format the
@@ -228,7 +228,15 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>(
      // upsert gives duplicate-rerun protection within one company. The
      // old (user_id, file_hash) key and its cross-company pre-check
      // (BANK_IMPORT_DUPLICATE_OTHER_COMPANY) are gone.
-      await ctx.supabase
+      // `.select('id')` so the inserted transactions can be stamped with the
+      // batch id (transactions.bank_file_import_id): the scope key for the
+      // owner/admin "undo this import" action. A missing id (upsert error) is
+      // non-fatal BY DESIGN: the import proceeds, its rows just stay
+      // unlinked, exactly like a pre-20260820071500 import. Without the row
+      // the batch never appears in the undo history, so nothing falsely
+      // advertises undo for it — but the failure is logged loudly, since an
+      // unattributed batch is permanently exempt from bulk undo.
+      const { data: importRow, error: importRowError } = await ctx.supabase
        .from('bank_file_imports')
        .upsert(
          {
@@ -244,6 +252,16 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>(
          },
          { onConflict: 'company_id,file_hash' },
        )
+        .select('id')
+        .maybeSingle()
+
+      if (importRowError || !importRow?.id) {
+        ctx.log.warn('bank_file_imports upsert failed: batch will import without undo attribution', {
+          filename: file.name,
+          fileHash,
+          error: importRowError?.message ?? 'no row returned',
+        })
+      }

      // Convert parsed transactions to the RawTransaction shape that
      // ingestTransactions expects. external_id stays stable so re-imports
@@ -290,6 +308,7 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string }> }>(
        ctx.companyId!,
        ctx.userId,
        raw,
+        importRow?.id ? { bankFileImportId: importRow.id as string } : undefined,
      )

      // Mark the bank_file_imports row complete. The unique constraint is