Add/transaction deletion (#695)

* feat(salary): add remove-employee button to draft salary runs The DELETE /api/salary/runs/{id}/employees/{employeeId} endpoint already existed (draft-only, cascades to the employee's line items) but had no UI trigger, so a mistakenly added employee could only be cleared by deleting the whole draft. Add a trash-icon action column to the "Anställda" table, gated on draft status + write permission to match the endpoint's guard, with a confirm prompt and success/error toast. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(settings): prevent horizontal overflow on mobile The company settings invite form was a non-wrapping fixed-width flex row that overflowed narrow viewports, forcing the full-screen settings modal to scroll on the x-axis. Stack the form vertically on mobile (sm:flex-row at and above the sm breakpoint) and add the missing min-w-0 guard to the modal content pane. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(bookkeeping): move journal entry filters into a filter dialog The ledger toolbar showed every filter inline (fiscal year, sort, series, date range, missing-documents toggle), which felt cluttered. Keep only the search field visible and move the rest into a "Filtrera" dialog with an active-filter count badge. - JournalEntryList now owns the fiscal-year scope, restored from the same localStorage key FiscalYearSelector writes, so the page no longer renders the selector separately. - Filters apply live and the dialog stays open; "Rensa alla filter" clears them. - Export STORAGE_KEY_PREFIX / ALL_YEARS_VALUE from FiscalYearSelector so the list reuses the persisted selection without duplicating the key. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(transactions): implement imported transaction guard for deletion - Added a guard to prevent deletion of transactions that are imported via bank sync or file uploads. - Introduced `isImportedTransaction` utility to determine if a transaction is user-created or imported. - Updated DELETE endpoint to return a 409 status for attempts to delete imported transactions. - Enhanced transaction history and inbox components to reflect the new deletion rules. - Added tests for transaction origin determination and deletion behavior. - Updated UI components to include a confirmation dialog for clearing journal entry forms. - Localized new strings for clearing form functionality in English and Swedish. * feat(transactions): enhance transaction deletion guard and improve fiscal year visibility --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-08 15:25:27 +02:00
parent 679b154ad2
commit 64991eb3c9
21 changed files with 1223 additions and 348 deletions
@@ -140,6 +140,22 @@ describe('GET /api/bookkeeping/journal-entries', () => {
    )
  })

+  it('uses the direct query path (not the RPC) when a search term is set', async () => {
+    enqueue({ data: [], error: null, count: 0 })
+
+    const request = createMockRequest('/api/bookkeeping/journal-entries', {
+      searchParams: { period_id: 'period-1', search: 'luftfyllning' },
+    })
+    const response = await GET(request)
+    const { status } = await parseJsonResponse(response)
+
+    expect(status).toBe(200)
+    // Free-text search needs an ILIKE the include_related RPC can't express, so
+    // the route must fall through to the direct PostgREST query.
+    expect(mockSupabase.from).toHaveBeenCalledWith('journal_entries')
+    expect(mockSupabase.rpc).not.toHaveBeenCalled()
+  })
+
  it('returns 500 on database error', async () => {
    enqueue({ data: null, error: { message: 'DB error' } })

@@ -7,6 +7,7 @@ import { validateBody } from '@/lib/api/validate'
 import { CreateJournalEntrySchema } from '@/lib/api/schemas'
 import { requireCompanyId } from '@/lib/company/context'
 import { requireWritePermission } from '@/lib/auth/require-write'
+import { escapeLikePattern } from '@/lib/invoices/duplicate-payment-guard'

 ensureInitialized()

@@ -32,6 +33,14 @@ export async function GET(request: Request) {
  // other value is passed (defense against trivial injection / typos).
  const seriesRaw = searchParams.get('series')
  const seriesFilter = seriesRaw && /^[A-Z]$/.test(seriesRaw) ? seriesRaw : null
+  // Free-text search over the voucher description (verifikationstext). When set,
+  // we take the direct-query path below (the include_related RPC can't search),
+  // which filters strictly by fiscal_period_id. So search is scoped to the
+  // selected fiscal period / company and — like voucher sort — does NOT surface
+  // cross-period follow-up entries: every result stays inside the selected
+  // year's series (the BFL-compliant per-year view). It narrows the period, it
+  // never widens it.
+  const search = searchParams.get('search')?.trim() || null
  // 'date_desc' (default) | 'date_asc' | 'voucher_asc' | 'voucher_desc'
  // sort_by overrides sort_date when present. sort_date is kept for backwards
  // compatibility with older clients.
@@ -55,7 +64,7 @@ export async function GET(request: Request) {
  // belonging to a different year's series would be misleading. The trade-off
  // is that the visible row count may differ between sort modes for the same
  // period; the strict count is the BFL-compliant view of that year.
-  if (periodId && includeRelated && !isVoucherSort) {
+  if (periodId && includeRelated && !isVoucherSort && !search) {
    const { data, error } = await supabase.rpc('list_fiscal_period_entries_with_related', {
      p_company_id: companyId,
      p_period_id: periodId,
@@ -133,6 +142,15 @@ export async function GET(request: Request) {
    query = query.eq('voucher_series', seriesFilter)
  }

+  if (search) {
+    // Escape LIKE wildcards (\ % _) so they match literally, and cap the needle
+    // length (≤200 chars) — both handled by the shared escapeLikePattern helper.
+    // The cap bounds DB work against oversized/pathological inputs (compliance
+    // A.8.28 / ASVS V1.2.5); escaping prevents silent over-matching on values
+    // like "50%". Supabase parameterises the value, so this is not about SQLi.
+    query = query.ilike('description', `%${escapeLikePattern(search)}%`)
+  }
+
  const { data, error, count } = await query

  if (error) {
@@ -88,22 +88,28 @@ export async function GET(
    })
  }

-  // Build avgifter entry preview
+  // Build avgifter entry preview — skipped for a nollkörning (0 avgifter),
+  // mirroring the vacation/pension guards below. The bookkeeping engine never
+  // posts an all-zero 7510/2731 voucher (see book/route.ts nollkörning path),
+  // so previewing one would falsely imply a verifikat that is never created.
  const totalAvgifter = employees.reduce((sum, e) => sum + e.avgifter_amount, 0)
-  const avgifterLines: CreateJournalEntryLineInput[] = [
-    {
-      account_number: SALARY_ACCOUNTS.AVGIFTER_EXPENSE,
-      debit_amount: Math.round(totalAvgifter * 100) / 100,
-      credit_amount: 0,
-      line_description: `${desc} — Arbetsgivaravgifter`,
-    },
-    {
-      account_number: SALARY_ACCOUNTS.AVGIFTER_LIABILITY,
-      debit_amount: 0,
-      credit_amount: Math.round(totalAvgifter * 100) / 100,
-      line_description: `${desc} — Arbetsgivaravgifter`,
-    },
-  ]
+  const roundedAvgifter = Math.round(totalAvgifter * 100) / 100
+  const avgifterLines: CreateJournalEntryLineInput[] = roundedAvgifter !== 0
+    ? [
+        {
+          account_number: SALARY_ACCOUNTS.AVGIFTER_EXPENSE,
+          debit_amount: roundedAvgifter,
+          credit_amount: 0,
+          line_description: `${desc} — Arbetsgivaravgifter`,
+        },
+        {
+          account_number: SALARY_ACCOUNTS.AVGIFTER_LIABILITY,
+          debit_amount: 0,
+          credit_amount: roundedAvgifter,
+          line_description: `${desc} — Arbetsgivaravgifter`,
+        },
+      ]
+    : []

  // Build vacation entry preview
  const totalVacation = employees.reduce((sum, e) => sum + e.vacation_accrual, 0)
@@ -170,14 +176,17 @@ export async function GET(

  return NextResponse.json({
    data: {
-      salaryEntry: {
+      // Each entry is null when it has no lines — a nollkörning posts nothing,
+      // so the salary and avgifter entries fall away just like vacation/pension
+      // already do, and the UI can simply skip the null ones.
+      salaryEntry: salaryLines.length > 0 ? {
        description: desc,
        lines: salaryLines,
-      },
-      avgifterEntry: {
+      } : null,
+      avgifterEntry: avgifterLines.length > 0 ? {
        description: `${desc} — Arbetsgivaravgifter`,
        lines: avgifterLines,
-      },
+      } : null,
      vacationEntry: vacationLines.length > 0 ? {
        description: `${desc} — Semesteravsättning`,
        lines: vacationLines,
@@ -79,33 +79,46 @@ describe('DELETE /api/transactions/[id]', () => {
    expect(body.error.message).toMatch(/Bankavstämning|storna/)
  })

-  it('allows deleting unbooked bank-synced transactions', async () => {
+  it('blocks deleting an unbooked bank-synced transaction (ignore-only)', async () => {
+    // A live bank connection (PSD2) makes this an imported row — not the user's
+    // to delete, only to ignore.
    const tx = makeTransaction({ bank_connection_id: 'bc-1', journal_entry_id: null, import_source: null })
-    enqueue({ data: tx, error: null }) // fetch
-    enqueue({ data: null, error: null }) // delete
+    enqueue({ data: tx, error: null }) // fetch (no delete should follow)

    const request = new Request('http://localhost/api/transactions/tx-1', { method: 'DELETE' })
    const response = await DELETE(request, createMockRouteParams({ id: 'tx-1' }))
-    const { status, body } = await parseJsonResponse(response)
+    const { status, body } = await parseJsonResponse<{ error: { code: string; message: string } }>(response)

-    expect(status).toBe(200)
-    expect(body).toEqual({ success: true })
+    expect(status).toBe(409)
+    expect(body.error.code).toBe('TRANSACTION_DELETE_IMPORTED')
+    expect(body.error.message).toMatch(/banken|ignorera/i)
  })

-  it('allows deleting unbooked imported transactions', async () => {
+  it('blocks deleting an unbooked CSV-imported transaction (ignore-only)', async () => {
    const tx = makeTransaction({ import_source: 'csv_nordea', journal_entry_id: null, bank_connection_id: null })
-    enqueue({ data: tx, error: null }) // fetch
-    enqueue({ data: null, error: null }) // delete
+    enqueue({ data: tx, error: null }) // fetch (no delete should follow)

    const request = new Request('http://localhost/api/transactions/tx-1', { method: 'DELETE' })
    const response = await DELETE(request, createMockRouteParams({ id: 'tx-1' }))
-    const { status, body } = await parseJsonResponse(response)
+    const { status, body } = await parseJsonResponse<{ error: { code: string } }>(response)

-    expect(status).toBe(200)
-    expect(body).toEqual({ success: true })
+    expect(status).toBe(409)
+    expect(body.error.code).toBe('TRANSACTION_DELETE_IMPORTED')
  })

-  it('deletes a manually added unbooked transaction', async () => {
+  it('blocks deleting an unbooked Enable Banking transaction (ignore-only)', async () => {
+    const tx = makeTransaction({ import_source: 'enable_banking', journal_entry_id: null, bank_connection_id: null })
+    enqueue({ data: tx, error: null }) // fetch (no delete should follow)
+
+    const request = new Request('http://localhost/api/transactions/tx-1', { method: 'DELETE' })
+    const response = await DELETE(request, createMockRouteParams({ id: 'tx-1' }))
+    const { status, body } = await parseJsonResponse<{ error: { code: string } }>(response)
+
+    expect(status).toBe(409)
+    expect(body.error.code).toBe('TRANSACTION_DELETE_IMPORTED')
+  })
+
+  it('deletes a manually added unbooked transaction (null source)', async () => {
    const tx = makeTransaction({ journal_entry_id: null, bank_connection_id: null, import_source: null })
    enqueue({ data: tx, error: null }) // fetch
    enqueue({ data: null, error: null }) // delete
@@ -118,6 +131,22 @@ describe('DELETE /api/transactions/[id]', () => {
    expect(body).toEqual({ success: true })
  })

+  it.each(['manual', 'mcp'])(
+    'deletes an unbooked in-app transaction created via %s',
+    async (source) => {
+      const tx = makeTransaction({ journal_entry_id: null, bank_connection_id: null, import_source: source })
+      enqueue({ data: tx, error: null }) // fetch
+      enqueue({ data: null, error: null }) // delete
+
+      const request = new Request('http://localhost/api/transactions/tx-1', { method: 'DELETE' })
+      const response = await DELETE(request, createMockRouteParams({ id: 'tx-1' }))
+      const { status, body } = await parseJsonResponse(response)
+
+      expect(status).toBe(200)
+      expect(body).toEqual({ success: true })
+    },
+  )
+
  it('returns 500 with a structured code when deletion fails', async () => {
    const tx = makeTransaction({ journal_entry_id: null, bank_connection_id: null, import_source: null })
    enqueue({ data: tx, error: null }) // fetch
@@ -132,9 +161,12 @@ describe('DELETE /api/transactions/[id]', () => {
  })

  it('returns 409 with an audit-trail code when the immutability trigger blocks the delete', async () => {
-    // An unbooked row with payment_match_log rows: the cascade hits the
-    // audit_log_immutable trigger (P0001), not a clean FK error.
-    const tx = makeTransaction({ journal_entry_id: null, bank_connection_id: 'bc-1', import_source: null })
+    // A user-created (deletable) row that still carries payment_match_log rows
+    // — e.g. it was auto-suggested a match. The cascade on delete hits the
+    // audit_log_immutable trigger (P0001), not a clean FK error. Fixture must be
+    // user-created (no bank link / import source) so it passes the imported
+    // guard and actually reaches the delete.
+    const tx = makeTransaction({ journal_entry_id: null, bank_connection_id: null, import_source: 'manual' })
    enqueue({ data: tx, error: null }) // fetch
    enqueue({
      data: null,
@@ -7,6 +7,7 @@ import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structure
 import { validateBody } from '@/lib/api/validate'
 import { UpdateTransactionTitleSchema } from '@/lib/api/schemas'
 import { guardSandbox } from '@/lib/sandbox/guard'
+import { isImportedTransaction } from '@/lib/transactions/origin'
 import type { Transaction } from '@/types'

 export async function DELETE(
@@ -26,10 +27,11 @@ export async function DELETE(

  const companyId = await requireCompanyId(supabase, user.id)

-  // Fetch the transaction with ownership check
+  // Fetch the transaction with ownership check. `bank_connection_id` +
+  // `import_source` tell us where the row came from (see the imported guard below).
  const { data: transaction, error: fetchError } = await supabase
    .from('transactions')
-    .select('id, journal_entry_id')
+    .select('id, journal_entry_id, bank_connection_id, import_source')
    .eq('id', id)
    .eq('company_id', companyId)
    .single()
@@ -66,6 +68,27 @@ export async function DELETE(
    )
  }

+  // Guard: only transactions the user created in the app can be deleted. Rows
+  // fetched via bank sync or uploaded via a bank-file import are an external
+  // record of money that moved — deleting one would silently drop a real bank
+  // line (and a re-sync would just bring it back). The user can *ignore* such a
+  // row (POST /api/transactions/[id]/ignore) to take it off the to-book list,
+  // but never delete it. See lib/transactions/origin.ts for the origin rule.
+  if (isImportedTransaction(transaction)) {
+    return NextResponse.json(
+      {
+        error: {
+          code: 'TRANSACTION_DELETE_IMPORTED',
+          message:
+            'Transaktionen har hämtats från banken eller importerats via fil och kan inte raderas. Du kan ignorera den så att den döljs från listan över transaktioner att bokföra.',
+          message_en:
+            'This transaction was fetched from your bank or imported from a file and cannot be deleted. You can ignore it to hide it from the list of transactions to book.',
+        },
+      },
+      { status: 409 }
+    )
+  }
+
  const { error: deleteError } = await supabase
    .from('transactions')
    .delete()