Mcp/template data feedback (#617)
* fix(booking-templates): scope template list to the active company GET /api/settings/booking-templates relied solely on the btl_select RLS policy, which is membership-wide (user_company_ids) and returns templates from every company the user belongs to. A user who owns multiple companies saw all their templates merged regardless of which company was active. Narrow the list in the API layer (mirroring counterparty-templates) to system + the active company + the active company's team. RLS stays the security backstop; this fixes the cross-company merge within a single user's own view (it was never a cross-tenant data leak). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(import): show proper message for duplicate bank file upload The bank file import page mis-parsed the structured error envelope ({ error: { code, message, details } }), so a BANK_FILE_DUPLICATE (409) fell through to the generic "Kunde inte läsa filen" fallback. The upload step also hardcoded that same string as the error heading, so duplicates were doubly misreported as parse failures. - Parse the structured envelope by error.code; surface error.message for all codes instead of rendering the error object. - Add a dedicated BANK_FILE_DUPLICATE message using the importedAt / importedCount details the route already returns. - Add an optional errorTitle prop to BankFileUploadStep (defaults to the previous text) and pass "Filen är redan importerad" for dupes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(tests): add comprehensive tests for recordateEntry, inbox-linking, and external-id handling - Implemented unit tests for recordateEntry in the bookkeeping module to validate various scenarios including date changes, non-posted entries, and fiscal period restrictions. - Created tests for inbox-linking status in pending operations to ensure correct handling of invoice inbox items and supplier invoices, addressing historical bugs related to status updates. - Added tests for external-id utilities to ensure consistent handling of monetary amounts and deduplication keys across different transaction sources. - Introduced new functions in external-id.ts for stable external ID generation and normalization of imported descriptions, enhancing transaction deduplication reliability. feat(migrations): add new database migrations for transaction handling - Created migration to exclude storno and correction vouchers from unmatched GL lines, ensuring accurate reconciliation. - Added a migration to preserve original bank transaction descriptions in a new immutable column, allowing for user edits while maintaining audit trails and deduplication integrity. * feat(migrations): add function to exclude storno/correction vouchers from unmatched GL lines * feat(transactions): enhance transaction handling with improved description normalization and preloaded original entries --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
2c59c3633f
commit
c6c86cded4
@@ -20,7 +20,19 @@ vi.mock('@/lib/auth/require-write', () => ({
|
||||
requireWritePermission: vi.fn().mockResolvedValue({ ok: true }),
|
||||
}))
|
||||
|
||||
import { DELETE } from '../route'
|
||||
// PATCH (edit title) goes through withRouteContext → requireAuth.
|
||||
vi.mock('@/lib/auth/require-auth', () => ({
|
||||
requireAuth: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/sandbox/guard', () => ({
|
||||
guardSandbox: vi.fn(),
|
||||
}))
|
||||
|
||||
import { DELETE, PATCH } from '../route'
|
||||
import { requireAuth } from '@/lib/auth/require-auth'
|
||||
import { guardSandbox } from '@/lib/sandbox/guard'
|
||||
import { NextResponse } from 'next/server'
|
||||
|
||||
describe('DELETE /api/transactions/[id]', () => {
|
||||
const mockUser = { id: 'user-1', email: 'test@test.se' }
|
||||
@@ -117,3 +129,158 @@ describe('DELETE /api/transactions/[id]', () => {
|
||||
expect(body).toEqual({ error: 'Failed to delete transaction' })
|
||||
})
|
||||
})
|
||||
|
||||
describe('PATCH /api/transactions/[id] (edit title)', () => {
|
||||
const mockUser = { id: 'user-1', email: 'test@test.se' }
|
||||
|
||||
function patchReq(body: unknown) {
|
||||
return new Request('http://localhost/api/transactions/tx-1', {
|
||||
method: 'PATCH',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(body),
|
||||
})
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
reset()
|
||||
vi.mocked(requireAuth).mockResolvedValue({
|
||||
user: mockUser as never,
|
||||
supabase: mockSupabase as never,
|
||||
error: null,
|
||||
})
|
||||
vi.mocked(guardSandbox).mockResolvedValue(null)
|
||||
})
|
||||
|
||||
it('returns 401 when not authenticated', async () => {
|
||||
vi.mocked(requireAuth).mockResolvedValue({
|
||||
user: null as never,
|
||||
supabase: mockSupabase as never,
|
||||
error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
|
||||
})
|
||||
|
||||
const res = await PATCH(patchReq({ description: 'Ny titel' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(401)
|
||||
})
|
||||
|
||||
it('returns 400 when the title is empty / whitespace-only', async () => {
|
||||
const res = await PATCH(patchReq({ description: ' ' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(400)
|
||||
})
|
||||
|
||||
it('returns 404 when the transaction is not found', async () => {
|
||||
enqueue({ data: null, error: { message: 'Not found' } })
|
||||
|
||||
const res = await PATCH(patchReq({ description: 'Ny titel' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(404)
|
||||
})
|
||||
|
||||
it('returns 409 when the transaction is booked', async () => {
|
||||
enqueue({
|
||||
data: {
|
||||
id: 'tx-1',
|
||||
description: 'X',
|
||||
original_description: 'X',
|
||||
journal_entry_id: 'je-1',
|
||||
invoice_id: null,
|
||||
supplier_invoice_id: null,
|
||||
},
|
||||
error: null,
|
||||
})
|
||||
|
||||
const res = await PATCH(patchReq({ description: 'Ny titel' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('TRANSACTION_TITLE_LOCKED')
|
||||
})
|
||||
|
||||
it('returns 409 when matched to an invoice even if journal_entry_id is null', async () => {
|
||||
enqueue({
|
||||
data: {
|
||||
id: 'tx-1',
|
||||
description: 'X',
|
||||
original_description: 'X',
|
||||
journal_entry_id: null,
|
||||
invoice_id: 'inv-1',
|
||||
supplier_invoice_id: null,
|
||||
},
|
||||
error: null,
|
||||
})
|
||||
|
||||
const res = await PATCH(patchReq({ description: 'Ny titel' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(409)
|
||||
})
|
||||
|
||||
it('updates the title for an editable (unbooked, unmatched) transaction', async () => {
|
||||
enqueue({
|
||||
data: {
|
||||
id: 'tx-1',
|
||||
description: 'ICA',
|
||||
original_description: 'ICA',
|
||||
journal_entry_id: null,
|
||||
invoice_id: null,
|
||||
supplier_invoice_id: null,
|
||||
},
|
||||
error: null,
|
||||
}) // fetch
|
||||
enqueue({
|
||||
data: { id: 'tx-1', description: 'Lunch med kund', title_edited_at: '2026-06-01T10:00:00Z' },
|
||||
error: null,
|
||||
}) // update
|
||||
|
||||
const res = await PATCH(
|
||||
patchReq({ description: 'Lunch med kund' }),
|
||||
createMockRouteParams({ id: 'tx-1' }),
|
||||
)
|
||||
const { status, body } = await parseJsonResponse<{ data: { description: string } }>(res)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data.description).toBe('Lunch med kund')
|
||||
})
|
||||
|
||||
it('restores the original title (200) when the new title equals original_description', async () => {
|
||||
enqueue({
|
||||
data: {
|
||||
id: 'tx-1',
|
||||
description: 'Lunch med kund',
|
||||
original_description: 'ICA MAXI',
|
||||
journal_entry_id: null,
|
||||
invoice_id: null,
|
||||
supplier_invoice_id: null,
|
||||
},
|
||||
error: null,
|
||||
}) // fetch
|
||||
enqueue({
|
||||
data: { id: 'tx-1', description: 'ICA MAXI', title_edited_at: null },
|
||||
error: null,
|
||||
}) // update
|
||||
|
||||
const res = await PATCH(patchReq({ description: 'ICA MAXI' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ data: { title_edited_at: string | null } }>(res)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data.title_edited_at).toBeNull()
|
||||
})
|
||||
|
||||
it('returns 409 when the row is matched/booked between read and write (optimistic-lock miss)', async () => {
|
||||
enqueue({
|
||||
data: {
|
||||
id: 'tx-1',
|
||||
description: 'ICA',
|
||||
original_description: 'ICA',
|
||||
journal_entry_id: null,
|
||||
invoice_id: null,
|
||||
supplier_invoice_id: null,
|
||||
},
|
||||
error: null,
|
||||
}) // fetch passes the read gate
|
||||
enqueue({ data: null, error: null }) // UPDATE affects 0 rows (gate re-assert failed)
|
||||
|
||||
const res = await PATCH(patchReq({ description: 'Ny titel' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('TRANSACTION_TITLE_LOCKED')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -2,6 +2,12 @@ import { createClient } from '@/lib/supabase/server'
|
||||
import { NextResponse } from 'next/server'
|
||||
import { requireCompanyId } from '@/lib/company/context'
|
||||
import { requireWritePermission } from '@/lib/auth/require-write'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error'
|
||||
import { validateBody } from '@/lib/api/validate'
|
||||
import { UpdateTransactionTitleSchema } from '@/lib/api/schemas'
|
||||
import { guardSandbox } from '@/lib/sandbox/guard'
|
||||
import type { Transaction } from '@/types'
|
||||
|
||||
export async function DELETE(
|
||||
_request: Request,
|
||||
@@ -52,3 +58,101 @@ export async function DELETE(
|
||||
|
||||
return NextResponse.json({ success: true })
|
||||
}
|
||||
|
||||
/**
|
||||
* Edit a bank transaction's title (description).
|
||||
*
|
||||
* Legal under BFL only while the row is a mutable staging label — i.e. NOT yet
|
||||
* booked into a verifikat and NOT confirmed-matched to an invoice. Once booked
|
||||
* the description is räkenskapsinformation and corrections go through storno
|
||||
* (reverseEntry/correctEntry), so this route hard-blocks those rows. The bank's
|
||||
* original title is preserved immutably in original_description (set at ingest)
|
||||
* and is never written here; passing it back restores the "not edited" tag.
|
||||
*/
|
||||
export const PATCH = withRouteContext(
|
||||
'transaction.updateTitle',
|
||||
async (request, ctx, { params }: { params: Promise<{ id: string }> }) => {
|
||||
const { id } = await params
|
||||
const { supabase, companyId, log, requestId, user } = ctx
|
||||
|
||||
const blocked = await guardSandbox(supabase, companyId)
|
||||
if (blocked) return blocked
|
||||
|
||||
const validation = await validateBody(request, UpdateTransactionTitleSchema, {
|
||||
log,
|
||||
operation: 'transaction.updateTitle',
|
||||
})
|
||||
if (!validation.success) return validation.response
|
||||
const { description } = validation.data
|
||||
|
||||
const { data: transaction, error: fetchError } = await supabase
|
||||
.from('transactions')
|
||||
.select('id, description, original_description, journal_entry_id, invoice_id, supplier_invoice_id')
|
||||
.eq('id', id)
|
||||
.eq('company_id', companyId)
|
||||
.single()
|
||||
|
||||
if (fetchError || !transaction) {
|
||||
return errorResponseFromCode('TX_CATEGORIZE_TX_NOT_FOUND', log, { requestId })
|
||||
}
|
||||
|
||||
// Gate: editable only when neither booked nor confirmed-matched. (A
|
||||
// confirmed invoice/supplier-invoice match also sets journal_entry_id, but
|
||||
// we check all three for defense-in-depth.) An unbooked row has no fiscal
|
||||
// period, so the period-lock requirement is satisfied implicitly.
|
||||
if (transaction.journal_entry_id || transaction.invoice_id || transaction.supplier_invoice_id) {
|
||||
return errorResponseFromCode('TRANSACTION_TITLE_LOCKED', log, { requestId })
|
||||
}
|
||||
|
||||
// Restoring to the bank original clears the "edited" tag; any other value
|
||||
// marks the title as user-edited. Compare against the TRIMMED original (the
|
||||
// incoming description is already trimmed by the schema) so a legacy
|
||||
// original carrying surrounding whitespace still restores cleanly.
|
||||
const isRestore =
|
||||
transaction.original_description != null &&
|
||||
description === transaction.original_description.trim()
|
||||
const titleEditedAt = isRestore ? null : new Date().toISOString()
|
||||
|
||||
const { data: updated, error: updateError } = await supabase
|
||||
.from('transactions')
|
||||
.update({ description, title_edited_at: titleEditedAt })
|
||||
.eq('id', id)
|
||||
.eq('company_id', companyId)
|
||||
// Re-assert the FULL editable gate atomically against a concurrent book
|
||||
// or auto-match. Ingest's supplier auto-match can set supplier_invoice_id
|
||||
// WITHOUT journal_entry_id, so guarding journal_entry_id alone leaves a
|
||||
// narrow TOCTOU window — mirror the read-time gate here.
|
||||
.is('journal_entry_id', null)
|
||||
.is('invoice_id', null)
|
||||
.is('supplier_invoice_id', null)
|
||||
// Return only what the client renders (data minimisation — the row also
|
||||
// carries company_id and other internal fields the caller doesn't need).
|
||||
.select('id, description, title_edited_at')
|
||||
.maybeSingle<Pick<Transaction, 'id' | 'description' | 'title_edited_at'>>()
|
||||
|
||||
if (updateError) {
|
||||
return errorResponse(updateError, log, { requestId })
|
||||
}
|
||||
if (!updated) {
|
||||
// 0 rows updated → the row was booked/matched between read and write.
|
||||
return errorResponseFromCode('TRANSACTION_TITLE_LOCKED', log, { requestId })
|
||||
}
|
||||
|
||||
// Behandlingshistorik (BFNAR 2013:2 kap 8) — light-touch for a pre-verifikat
|
||||
// working label; updated_at (trigger) captures "when". We deliberately do
|
||||
// NOT log the description text: a bank label can carry PII (payee names,
|
||||
// reference numbers). The before-value stays recoverable in
|
||||
// original_description and the after-value is the row's current
|
||||
// description, so the log only needs to record that/which way it changed.
|
||||
log.info('transaction title edited', {
|
||||
transactionId: id,
|
||||
actor: user.id,
|
||||
restored: isRestore,
|
||||
previousLength: transaction.description?.length ?? 0,
|
||||
newLength: description.length,
|
||||
})
|
||||
|
||||
return NextResponse.json({ data: updated })
|
||||
},
|
||||
{ requireWrite: true },
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user