feat(transactions): move an unbooked transaction to another cash account (#1570)
A bank transaction that ingested under the wrong cash account (or with no
account at all: legacy connections, own-account transfers the backfills
deliberately skipped) surfaces under the primary account's reconciliation
and can never be matched on the account it belongs to, because
cross-account matching is deliberately blocked. There was no first-party
way to fix the binding.
New PATCH /api/transactions/[id]/cash-account moves a movable staging row
(not booked, not invoice/supplier-invoice matched, not anchored via
transaction_voucher_links) to another of the company's cash accounts,
addressed by its BAS 19xx ledger account. Cross-currency moves are
hard-rejected (the row would vanish from every report's currency scope),
and the movable gate is re-asserted atomically in the UPDATE filter
against a concurrent book/auto-match, mirroring the title route. The tvl
check runs as a pre-check query since PostgREST cannot express NOT EXISTS
in an update filter; a tvl row appearing concurrently implies the booking
flow, which sets its own transaction state.
UI: 'Flytta till annat konto' in the transaction inbox row menu (opens a
radio-list dialog of the enabled cash accounts, current one preselected
and disabled) and direct 'Flytta till {name}' items in the bank
reconciliation unmatched-row menu that PATCH and refetch the view.
New structured error codes: TRANSACTION_MOVE_BOOKED,
TRANSACTION_MOVE_UNKNOWN_ACCOUNT, TRANSACTION_MOVE_CURRENCY_MISMATCH.
Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Fable 5
parent
c420fd2aa1
commit
1eebb75269
@@ -0,0 +1,195 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import {
|
||||
parseJsonResponse,
|
||||
createMockRouteParams,
|
||||
createQueuedMockSupabase,
|
||||
makeTransaction,
|
||||
} from '@/tests/helpers'
|
||||
|
||||
const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase()
|
||||
vi.mock('@/lib/supabase/server', () => ({
|
||||
createClient: () => Promise.resolve(mockSupabase),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/company/context', () => ({
|
||||
requireCompanyId: vi.fn().mockResolvedValue('company-1'),
|
||||
getActiveCompanyId: vi.fn().mockResolvedValue('company-1'),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/auth/require-write', () => ({
|
||||
requireWritePermission: vi.fn().mockResolvedValue({ ok: true }),
|
||||
}))
|
||||
|
||||
// PATCH goes through withRouteContext → requireAuth.
|
||||
vi.mock('@/lib/auth/require-auth', () => ({
|
||||
requireAuth: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/sandbox/guard', () => ({
|
||||
guardSandbox: vi.fn(),
|
||||
}))
|
||||
|
||||
import { PATCH } from '../route'
|
||||
import { requireAuth } from '@/lib/auth/require-auth'
|
||||
import { guardSandbox } from '@/lib/sandbox/guard'
|
||||
import { NextResponse } from 'next/server'
|
||||
|
||||
describe('PATCH /api/transactions/[id]/cash-account (move cash account)', () => {
|
||||
const mockUser = { id: 'user-1', email: 'test@test.se' }
|
||||
|
||||
function patchReq(body: unknown) {
|
||||
return new Request('http://localhost/api/transactions/tx-1/cash-account', {
|
||||
method: 'PATCH',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(body),
|
||||
})
|
||||
}
|
||||
|
||||
/** A movable staging row: unbooked, unmatched. */
|
||||
function movableTx(overrides: Record<string, unknown> = {}) {
|
||||
return makeTransaction({
|
||||
id: 'tx-1',
|
||||
journal_entry_id: null,
|
||||
invoice_id: null,
|
||||
supplier_invoice_id: null,
|
||||
cash_account_id: 'ca-1',
|
||||
currency: 'SEK',
|
||||
...overrides,
|
||||
})
|
||||
}
|
||||
|
||||
const targetAccount = { id: 'ca-2', ledger_account: '1931', currency: 'SEK' }
|
||||
|
||||
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({ account_number: '1931' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(401)
|
||||
})
|
||||
|
||||
it.each(['4000', '193', '19301', 'abcd', ''])(
|
||||
'returns 400 for a non-19xx account_number (%s)',
|
||||
async (accountNumber) => {
|
||||
const res = await PATCH(
|
||||
patchReq({ account_number: accountNumber }),
|
||||
createMockRouteParams({ id: 'tx-1' }),
|
||||
)
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(400)
|
||||
},
|
||||
)
|
||||
|
||||
it('returns 400 when account_number is missing', async () => {
|
||||
const res = await PATCH(patchReq({}), 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' } }) // tx fetch
|
||||
|
||||
const res = await PATCH(patchReq({ account_number: '1931' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(404)
|
||||
expect(body.error.code).toBe('TX_CATEGORIZE_TX_NOT_FOUND')
|
||||
})
|
||||
|
||||
it('returns 409 when the transaction is booked (journal_entry_id set)', async () => {
|
||||
enqueue({ data: movableTx({ journal_entry_id: 'je-1' }), error: null }) // tx fetch
|
||||
|
||||
const res = await PATCH(patchReq({ account_number: '1931' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('TRANSACTION_MOVE_BOOKED')
|
||||
})
|
||||
|
||||
it('returns 409 when matched to an invoice even if journal_entry_id is null', async () => {
|
||||
enqueue({ data: movableTx({ invoice_id: 'inv-1' }), error: null }) // tx fetch
|
||||
|
||||
const res = await PATCH(patchReq({ account_number: '1931' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('TRANSACTION_MOVE_BOOKED')
|
||||
})
|
||||
|
||||
it('returns 409 when matched to a supplier invoice even if journal_entry_id is null', async () => {
|
||||
enqueue({ data: movableTx({ supplier_invoice_id: 'si-1' }), error: null }) // tx fetch
|
||||
|
||||
const res = await PATCH(patchReq({ account_number: '1931' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('TRANSACTION_MOVE_BOOKED')
|
||||
})
|
||||
|
||||
it('returns 409 when anchored via transaction_voucher_links (bulk-book N>1)', async () => {
|
||||
enqueue({ data: movableTx(), error: null }) // tx fetch passes the field gate
|
||||
enqueue({ data: [{ transaction_id: 'tx-1' }], error: null }) // tvl pre-check hits
|
||||
|
||||
const res = await PATCH(patchReq({ account_number: '1931' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('TRANSACTION_MOVE_BOOKED')
|
||||
})
|
||||
|
||||
it('returns 404 when the account is not one of the company cash accounts', async () => {
|
||||
enqueue({ data: movableTx(), error: null }) // tx fetch
|
||||
enqueue({ data: [], error: null }) // tvl pre-check clean
|
||||
enqueue({ data: null, error: null }) // cash account lookup misses
|
||||
|
||||
const res = await PATCH(patchReq({ account_number: '1959' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(404)
|
||||
expect(body.error.code).toBe('TRANSACTION_MOVE_UNKNOWN_ACCOUNT')
|
||||
})
|
||||
|
||||
it('returns 400 when the transaction currency does not match the target account', async () => {
|
||||
enqueue({ data: movableTx({ currency: 'EUR' }), error: null }) // tx fetch
|
||||
enqueue({ data: [], error: null }) // tvl pre-check clean
|
||||
enqueue({ data: targetAccount, error: null }) // SEK account
|
||||
|
||||
const res = await PATCH(patchReq({ account_number: '1931' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(400)
|
||||
expect(body.error.code).toBe('TRANSACTION_MOVE_CURRENCY_MISMATCH')
|
||||
})
|
||||
|
||||
it('moves a movable transaction to the target account', async () => {
|
||||
enqueue({ data: movableTx(), error: null }) // tx fetch
|
||||
enqueue({ data: [], error: null }) // tvl pre-check clean
|
||||
enqueue({ data: targetAccount, error: null }) // account lookup
|
||||
enqueue({ data: { id: 'tx-1', cash_account_id: 'ca-2' }, error: null }) // update
|
||||
|
||||
const res = await PATCH(patchReq({ account_number: '1931' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ data: { id: string; cash_account_id: string } }>(res)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data).toEqual({ id: 'tx-1', cash_account_id: 'ca-2' })
|
||||
})
|
||||
|
||||
it('returns 409 when the row is booked between read and write (optimistic-lock miss)', async () => {
|
||||
enqueue({ data: movableTx(), error: null }) // tx fetch passes the read gate
|
||||
enqueue({ data: [], error: null }) // tvl pre-check clean
|
||||
enqueue({ data: targetAccount, error: null }) // account lookup
|
||||
enqueue({ data: null, error: null }) // UPDATE affects 0 rows (gate re-assert failed)
|
||||
|
||||
const res = await PATCH(patchReq({ account_number: '1931' }), createMockRouteParams({ id: 'tx-1' }))
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('TRANSACTION_MOVE_BOOKED')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,139 @@
|
||||
import { NextResponse } from 'next/server'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error'
|
||||
import { validateBody } from '@/lib/api/validate'
|
||||
import { MoveTransactionCashAccountSchema } from '@/lib/api/schemas'
|
||||
import { guardSandbox } from '@/lib/sandbox/guard'
|
||||
|
||||
/**
|
||||
* PATCH /api/transactions/[id]/cash-account
|
||||
*
|
||||
* Move an unbooked bank transaction to another of the company's cash accounts
|
||||
* (cash_accounts row, addressed by its BAS 19xx ledger account). This is the
|
||||
* escape hatch for rows that ingested under the wrong account or with no
|
||||
* account at all (legacy connections, own-account transfers the backfills
|
||||
* deliberately skipped): such a row surfaces under the primary account's
|
||||
* reconciliation and can never be matched on the account it belongs to.
|
||||
*
|
||||
* Only a mutable staging row may move: NOT booked (journal_entry_id), NOT
|
||||
* confirmed-matched (invoice_id / supplier_invoice_id), and NOT anchored via
|
||||
* transaction_voucher_links (bulk-book N>1 links transactions to a verifikat
|
||||
* WITHOUT setting journal_entry_id). Once anchored, the voucher's own 19xx
|
||||
* line is ground truth for which account the money moved on (see the repair
|
||||
* backfill 20260609120000), so the binding must not be editable.
|
||||
*/
|
||||
export const PATCH = withRouteContext(
|
||||
'transaction.moveCashAccount',
|
||||
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, MoveTransactionCashAccountSchema, {
|
||||
log,
|
||||
operation: 'transaction.moveCashAccount',
|
||||
})
|
||||
if (!validation.success) return validation.response
|
||||
const { account_number: accountNumber } = validation.data
|
||||
|
||||
const { data: transaction, error: fetchError } = await supabase
|
||||
.from('transactions')
|
||||
.select('id, currency, cash_account_id, 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: movable only when neither booked nor confirmed-matched (a confirmed
|
||||
// invoice match also sets journal_entry_id, but check all three for
|
||||
// defense-in-depth, mirroring the title route).
|
||||
if (transaction.journal_entry_id || transaction.invoice_id || transaction.supplier_invoice_id) {
|
||||
return errorResponseFromCode('TRANSACTION_MOVE_BOOKED', log, { requestId })
|
||||
}
|
||||
|
||||
// Bulk-book (N>1) anchors a transaction to a verifikat via
|
||||
// transaction_voucher_links WITHOUT setting journal_entry_id, so the gate
|
||||
// above misses it. PostgREST cannot express NOT EXISTS in an update filter,
|
||||
// so this runs as a pre-check query instead of being re-asserted in the
|
||||
// UPDATE below. That leaves no extra TOCTOU risk: a tvl row appearing
|
||||
// concurrently implies the booking flow ran, and that flow sets its own
|
||||
// transaction state as part of the same operation.
|
||||
const { data: voucherLinks, error: tvlError } = await supabase
|
||||
.from('transaction_voucher_links')
|
||||
.select('transaction_id')
|
||||
.eq('company_id', companyId)
|
||||
.eq('transaction_id', id)
|
||||
.limit(1)
|
||||
|
||||
if (tvlError) {
|
||||
return errorResponse(tvlError, log, { requestId })
|
||||
}
|
||||
if ((voucherLinks ?? []).length > 0) {
|
||||
return errorResponseFromCode('TRANSACTION_MOVE_BOOKED', log, { requestId })
|
||||
}
|
||||
|
||||
const { data: targetAccount, error: accountError } = await supabase
|
||||
.from('cash_accounts')
|
||||
.select('id, ledger_account, currency')
|
||||
.eq('company_id', companyId)
|
||||
.eq('ledger_account', accountNumber)
|
||||
.maybeSingle<{ id: string; ledger_account: string; currency: string }>()
|
||||
|
||||
if (accountError) {
|
||||
return errorResponse(accountError, log, { requestId })
|
||||
}
|
||||
if (!targetAccount) {
|
||||
return errorResponseFromCode('TRANSACTION_MOVE_UNKNOWN_ACCOUNT', log, { requestId })
|
||||
}
|
||||
|
||||
// A cross-currency move would strand the row: every report scope pins
|
||||
// .eq('currency', accountCurrency), so the row would vanish from BOTH the
|
||||
// old and the new account's reconciliation. Hard-reject.
|
||||
if (transaction.currency.toUpperCase() !== targetAccount.currency.toUpperCase()) {
|
||||
return errorResponseFromCode('TRANSACTION_MOVE_CURRENCY_MISMATCH', log, { requestId })
|
||||
}
|
||||
|
||||
const { data: updated, error: updateError } = await supabase
|
||||
.from('transactions')
|
||||
.update({ cash_account_id: targetAccount.id })
|
||||
.eq('id', id)
|
||||
.eq('company_id', companyId)
|
||||
// Re-assert the movable gate atomically against a concurrent book or
|
||||
// auto-match (ingest's supplier auto-match can set supplier_invoice_id
|
||||
// WITHOUT journal_entry_id), mirroring PATCH /api/transactions/[id].
|
||||
// The tvl part of the gate lives in the pre-check above; see the comment
|
||||
// there for why that is safe.
|
||||
.is('journal_entry_id', null)
|
||||
.is('invoice_id', null)
|
||||
.is('supplier_invoice_id', null)
|
||||
.select('id, cash_account_id')
|
||||
.maybeSingle<{ id: string; cash_account_id: string }>()
|
||||
|
||||
if (updateError) {
|
||||
return errorResponse(updateError, log, { requestId })
|
||||
}
|
||||
if (!updated) {
|
||||
// 0 rows updated: the row was booked/matched between read and write.
|
||||
return errorResponseFromCode('TRANSACTION_MOVE_BOOKED', log, { requestId })
|
||||
}
|
||||
|
||||
// Behandlingshistorik (BFNAR 2013:2 kap 8): light-touch for a pre-verifikat
|
||||
// staging binding, same weight as the title route. updated_at (trigger)
|
||||
// captures "when"; from/to ids record which way the row moved.
|
||||
log.info('transaction moved to another cash account', {
|
||||
transactionId: id,
|
||||
actor: user.id,
|
||||
fromCashAccountId: transaction.cash_account_id,
|
||||
toCashAccountId: targetAccount.id,
|
||||
toLedgerAccount: targetAccount.ledger_account,
|
||||
})
|
||||
|
||||
return NextResponse.json({ data: updated })
|
||||
},
|
||||
{ requireWrite: true },
|
||||
)
|
||||
Reference in New Issue
Block a user