fix(supplier-invoices): freeze verifikat-critical fields once the registration entry is posted (#1249)
* fix(supplier-invoices): freeze verifikat-critical fields once the registration entry is posted invoice_date becomes the registration verifikat's entry_date and supplier_invoice_number goes into its description, but both stayed freely writable through the shared UpdateSupplierInvoiceSchema. Editing either on a booked invoice moved the invoice row while the posted entry kept its original values: the two disagreed silently, nothing landed in journal_entry_rattelse_log, and the change bypassed both sanctioned rättelse paths (BFL 5 kap 5-7 §). Adds findLockedVerifikatFields() next to the other supplier-invoice lifecycle predicates and calls it from both writers (dashboard PUT and v1 PATCH, which also covers the API-key/MCP path). Only a differing value is refused, so a full-form resend of the stored value still succeeds, and due_date, payment_reference and notes stay editable for the aged-invoice flow (#1206). Fixes #1230 * fix(supplier-invoices): make the verifikat-field lock atomic with the write Review follow-up on #1230: the lock check read the row a moment before the update ran, so a registration entry posted in between let exactly the drift the guard exists to prevent slip through. When an update moves a verifikat-critical field on a row that read as unbooked, the write is now pinned with `registration_journal_entry_id is null`. A concurrent posting therefore matches zero rows: the dashboard route returns its existing SI_EDIT_CONFLICT ("reload and try again", and the retry hits the lock with the right message), and the v1 route re-reads to answer with SI_EDIT_VERIFIKAT_LOCKED plus reason=race rather than a guess. The pin is conditional on the update actually moving one of those fields, so metadata-only edits and full-form resends of unchanged values on a booked invoice keep working (#1206).
This commit is contained in:
@@ -20,6 +20,10 @@ import { registerEndpoint, dataEnvelope } from '@/lib/api/v1/registry'
|
||||
import { withApiV1 } from '@/lib/api/v1/with-api-v1'
|
||||
import { v1ErrorResponse, v1ErrorResponseFromCode } from '@/lib/api/v1/errors'
|
||||
import { UpdateSupplierInvoiceSchema } from '@/lib/api/schemas'
|
||||
import {
|
||||
findChangedVerifikatFields,
|
||||
findLockedVerifikatFields,
|
||||
} from '@/lib/supplier-invoices/lifecycle'
|
||||
|
||||
// V1-only strict variant. The shared `UpdateSupplierInvoiceSchema` is also
|
||||
// consumed by the dashboard, where unknown keys are silently stripped, fine
|
||||
@@ -177,12 +181,13 @@ registerEndpoint({
|
||||
description:
|
||||
'Patches a supplier invoice with the supplied fields. Only allowed on `registered` status: once approved, paid, or credited, the record is effectively immutable from the API\'s perspective. Idempotent (mandatory Idempotency-Key). Dry-runnable.',
|
||||
useWhen:
|
||||
'You need to fix a typo in supplier_invoice_number, adjust dates, or attach a payment reference / notes to a registered SI before approval. Use dry-run to confirm the merged state first.',
|
||||
'You need to adjust due_date, or attach a payment reference / notes to a registered SI before approval. Use dry-run to confirm the merged state first.',
|
||||
doNotUseFor:
|
||||
'Editing line items (immutable: credit the SI and register a new one). Changing status (use action verbs). Approved/paid/credited SIs (returns 400 SI_NOT_DRAFT).',
|
||||
'Editing line items (immutable: credit the SI and register a new one). Changing status (use action verbs). Approved/paid/credited SIs (returns 400 SI_NOT_DRAFT). invoice_date / supplier_invoice_number on an SI that already has a registration verifikat (returns 400 SI_EDIT_VERIFIKAT_LOCKED).',
|
||||
pitfalls: [
|
||||
'Returns 400 SI_NOT_DRAFT when current status !== "registered".',
|
||||
'invoice_date / due_date changes do not re-post the registration JE; if the entry date needs to change, credit the SI and re-register.',
|
||||
'invoice_date and supplier_invoice_number are on the posted registration verifikat (entry_date and description). Once registration_journal_entry_id is set, patching them returns 400 SI_EDIT_VERIFIKAT_LOCKED: correct the entry via a rättelse (gnubok_correct_entry) or credit the SI and re-register. Resending the unchanged value is accepted.',
|
||||
'Patching a field never re-posts the registration JE.',
|
||||
],
|
||||
example: {
|
||||
request: { payment_reference: 'OCR-1234567890' },
|
||||
@@ -276,11 +281,46 @@ export const PATCH = withApiV1<{ params: Promise<{ companyId: string; id: string
|
||||
})
|
||||
}
|
||||
|
||||
// For a company without defer_invoice_booking the registration verifikat is
|
||||
// posted at create time, so a 'registered' SI is normally already booked.
|
||||
// invoice_date is that entry's entry_date and supplier_invoice_number is in
|
||||
// its description: patching either here would desync the verifikat outside
|
||||
// both sanctioned rättelse paths (#1230). Dry-run is gated too, so the
|
||||
// preview never promises a write the real call refuses.
|
||||
const lockedFields = findLockedVerifikatFields(
|
||||
body,
|
||||
existing as {
|
||||
registration_journal_entry_id: string | null
|
||||
invoice_date: string | null
|
||||
supplier_invoice_number: string | null
|
||||
},
|
||||
)
|
||||
if (lockedFields.length > 0) {
|
||||
return v1ErrorResponseFromCode('SI_EDIT_VERIFIKAT_LOCKED', ctx.log, {
|
||||
requestId: ctx.requestId,
|
||||
details: {
|
||||
fields: lockedFields,
|
||||
journal_entry_id: (existing as { registration_journal_entry_id: string | null })
|
||||
.registration_journal_entry_id,
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
if (ctx.dryRun) {
|
||||
return dryRunPreview({ ...existing, ...updateData }, { requestId: ctx.requestId, log: ctx.log })
|
||||
}
|
||||
|
||||
const { data, error } = await ctx.supabase
|
||||
// The lock check read a row that was still unbooked. If this patch moves a
|
||||
// verifikat-critical field, the write stays conditional on that: a
|
||||
// registration entry posted in between must lose the race rather than end
|
||||
// up disagreeing with the invoice it was built from.
|
||||
const movesVerifikatFields =
|
||||
findChangedVerifikatFields(
|
||||
body,
|
||||
existing as { invoice_date: string | null; supplier_invoice_number: string | null },
|
||||
).length > 0
|
||||
|
||||
let write = ctx.supabase
|
||||
.from('supplier_invoices')
|
||||
.update(updateData)
|
||||
.eq('company_id', ctx.companyId!)
|
||||
@@ -288,13 +328,46 @@ export const PATCH = withApiV1<{ params: Promise<{ companyId: string; id: string
|
||||
// Race guard: another request may have approved / paid between the
|
||||
// pre-flight status check and this update.
|
||||
.eq('status', 'registered')
|
||||
.select(SI_DETAIL_COLUMNS)
|
||||
.maybeSingle()
|
||||
if (movesVerifikatFields) {
|
||||
write = write.is('registration_journal_entry_id', null)
|
||||
}
|
||||
|
||||
const { data, error } = await write.select(SI_DETAIL_COLUMNS).maybeSingle()
|
||||
|
||||
if (error) {
|
||||
return v1ErrorResponse(error, ctx.log, { requestId: ctx.requestId })
|
||||
}
|
||||
if (!data) {
|
||||
// Either the status moved or a registration entry landed. Re-read so the
|
||||
// caller gets the reason that actually applies instead of a guess.
|
||||
const { data: current } = await ctx.supabase
|
||||
.from('supplier_invoices')
|
||||
.select('status, registration_journal_entry_id, invoice_date, supplier_invoice_number')
|
||||
.eq('company_id', ctx.companyId!)
|
||||
.eq('id', invoiceId)
|
||||
.maybeSingle()
|
||||
|
||||
const nowLocked = current
|
||||
? findLockedVerifikatFields(
|
||||
body,
|
||||
current as {
|
||||
registration_journal_entry_id: string | null
|
||||
invoice_date: string | null
|
||||
supplier_invoice_number: string | null
|
||||
},
|
||||
)
|
||||
: []
|
||||
if (nowLocked.length > 0) {
|
||||
return v1ErrorResponseFromCode('SI_EDIT_VERIFIKAT_LOCKED', ctx.log, {
|
||||
requestId: ctx.requestId,
|
||||
details: {
|
||||
fields: nowLocked,
|
||||
journal_entry_id: (current as { registration_journal_entry_id: string | null })
|
||||
.registration_journal_entry_id,
|
||||
reason: 'race',
|
||||
},
|
||||
})
|
||||
}
|
||||
return v1ErrorResponseFromCode('SI_NOT_DRAFT', ctx.log, {
|
||||
requestId: ctx.requestId,
|
||||
details: { reason: 'race' },
|
||||
|
||||
@@ -136,6 +136,7 @@ function makeFlexibleSupabase(
|
||||
const COMPANY_ID = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa'
|
||||
const SUPPLIER_ID = 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb'
|
||||
const SI_ID = 'cccccccc-cccc-4ccc-8ccc-cccccccccccc'
|
||||
const JE_ID = 'dddddddd-dddd-4ddd-8ddd-dddddddddddd'
|
||||
const USER_ID = 'user-1'
|
||||
|
||||
function makeRequest(url: string, init?: RequestInit): Request {
|
||||
@@ -1051,6 +1052,85 @@ describe('PATCH /api/v1/companies/:companyId/supplier-invoices/:id', () => {
|
||||
expect(body.error.code).toBe('SI_NOT_DRAFT')
|
||||
})
|
||||
|
||||
// #1230: the shared guard has to hold on the API-key path too, not just in
|
||||
// the dashboard route: invoice_date is the posted entry's entry_date and
|
||||
// supplier_invoice_number is in its description.
|
||||
it('refuses to move invoice_date on an SI that already has a registration verifikat', async () => {
|
||||
mockServiceClient.mockReturnValue(
|
||||
makeFlexibleSupabase({
|
||||
company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
|
||||
supplier_invoices: {
|
||||
data: { ...SAMPLE_SI, registration_journal_entry_id: JE_ID },
|
||||
error: null,
|
||||
},
|
||||
idempotency_keys: { data: null, error: null },
|
||||
}),
|
||||
)
|
||||
const res = await updateSI(
|
||||
makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/supplier-invoices/${SI_ID}`, {
|
||||
method: 'PATCH',
|
||||
body: JSON.stringify({ invoice_date: '2026-06-01' }),
|
||||
}),
|
||||
detailParams(COMPANY_ID, SI_ID),
|
||||
)
|
||||
expect(res.status).toBe(400)
|
||||
const body = await res.json()
|
||||
expect(body.error.code).toBe('SI_EDIT_VERIFIKAT_LOCKED')
|
||||
expect(body.error.details.fields).toEqual(['invoice_date'])
|
||||
})
|
||||
|
||||
it('still patches due_date on a booked SI, and accepts unchanged verifikat fields', async () => {
|
||||
const booked = { ...SAMPLE_SI, registration_journal_entry_id: JE_ID }
|
||||
mockServiceClient.mockReturnValue(
|
||||
makeFlexibleSupabase({
|
||||
company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
|
||||
supplier_invoices: { data: { ...booked, due_date: '2026-07-31' }, error: null },
|
||||
idempotency_keys: { data: null, error: null },
|
||||
}),
|
||||
)
|
||||
const res = await updateSI(
|
||||
makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/supplier-invoices/${SI_ID}`, {
|
||||
method: 'PATCH',
|
||||
body: JSON.stringify({
|
||||
due_date: '2026-07-31',
|
||||
invoice_date: SAMPLE_SI.invoice_date,
|
||||
supplier_invoice_number: SAMPLE_SI.supplier_invoice_number,
|
||||
}),
|
||||
}),
|
||||
detailParams(COMPANY_ID, SI_ID),
|
||||
)
|
||||
expect(res.status).toBe(200)
|
||||
const body = await res.json()
|
||||
expect(body.data.due_date).toBe('2026-07-31')
|
||||
})
|
||||
|
||||
it('reports the lock, not a generic race, when a registration entry lands mid-flight', async () => {
|
||||
// Queue: pre-flight read (unbooked) -> pinned update matches nothing ->
|
||||
// re-read shows the entry that landed in between.
|
||||
mockServiceClient.mockReturnValue(
|
||||
makeFlexibleSupabase({
|
||||
company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
|
||||
supplier_invoices: [
|
||||
{ data: SAMPLE_SI, error: null },
|
||||
{ data: null, error: null },
|
||||
{ data: { ...SAMPLE_SI, registration_journal_entry_id: JE_ID }, error: null },
|
||||
],
|
||||
idempotency_keys: { data: null, error: null },
|
||||
}),
|
||||
)
|
||||
const res = await updateSI(
|
||||
makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/supplier-invoices/${SI_ID}`, {
|
||||
method: 'PATCH',
|
||||
body: JSON.stringify({ invoice_date: '2026-06-01' }),
|
||||
}),
|
||||
detailParams(COMPANY_ID, SI_ID),
|
||||
)
|
||||
expect(res.status).toBe(400)
|
||||
const body = await res.json()
|
||||
expect(body.error.code).toBe('SI_EDIT_VERIFIKAT_LOCKED')
|
||||
expect(body.error.details.reason).toBe('race')
|
||||
})
|
||||
|
||||
it('rejects unknown body keys (V4.5 strict schema)', async () => {
|
||||
mockServiceClient.mockReturnValue(
|
||||
makeFlexibleSupabase({
|
||||
|
||||
Reference in New Issue
Block a user