fix(supplier-invoices): make the 'overdue' label two-way and stop it locking an invoice (#1227)
* fix(supplier-invoices): make the 'overdue' label two-way and stop it locking an invoice The daily cron flips unbooked payables past their due date to 'overdue' but nothing ever flipped them back, so aging alone pushed an invoice out of every workflow that gated on 'registered': it could not be edited (not even to extend the due date that made it overdue) and it could not be attested. Deletion was already unblocked in #1204; this closes the rest of #1206. - update_overdue_supplier_invoices() gains the inverse branch: a payable whose due date is no longer in the past returns to its resting status. Because the flip collapses 'registered' and 'approved', the un-flip needs a separate attest marker: new supplier_invoices.approved_at, backfilled from updated_at for rows currently sitting in 'approved'. - PUT /api/supplier-invoices/[id] accepts every unsettled status and recomputes the label from the due date it writes, in both directions, instead of leaving it up to a day stale. The update body carries metadata only (numbers, dates, reference, notes), never amounts or accounts, so a posted registration verifikat cannot be desynced by money. - Approve (web route, v1 API, MCP staging tool, staged commit executor) keys off approved_at instead of status === 'registered', so an aged invoice can still be attested. A still-late invoice keeps the 'overdue' label after attest: approving is not a reason to hide that the money is late. - One shared predicate in lib/supplier-invoices/lifecycle.ts for all five call sites, mirroring the SQL; new SI_EDIT_INVALID_STATUS replaces the raw Swedish string the edit gate used to return. Tests: 12 pg-real cases on the cron (5 new, covering both directions and the credit-note/fully-paid boundaries), plus route tests asserting the exact written payload for PUT and approve, and unit tests pinning the shared predicate against the SQL. npm test (11385), lint, check:guards clean. Closes #1206 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(migration): mark backfilled approved_at values as derived, not audit facts Compliance review on #1227 flagged that approved_at = updated_at could later be mistaken for an observed attestation moment (BFNAR 2013:2 kap 8 behandlingshistorik). The column comment and the migration now state plainly that pre-migration values are derived and that audit_log, written by the audit_supplier_invoices trigger, remains the record of what happened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(supplier-invoices): guard the derived status writes with compare-and-swap Review findings on #1227. The status these paths write is derived from facts read a moment earlier, so an unconditional write could overwrite a concurrent cron flip, edit or approval with a label computed from what those changed. - PUT pins status, due_date and approved_at when (and only when) it derives a new status; zero matched rows is now a retryable 409 SI_EDIT_CONFLICT instead of a silently stale label. Metadata-only updates keep writing unconditionally: they never touch status, so they cannot clobber it. - The web approve route and the staged-commit executor gain the same pre-approval guard the v1 route already had (status in registered/overdue, approved_at IS NULL) plus a !data race check, so two concurrent approvals can no longer both stamp approved_at and both emit supplier_invoice.approved. - The v1 guard additionally pins due_date, since nextStatus is derived from it. - The list page no longer invents status/approved_at when the approve response is incomplete: it re-reads instead. An operator about to pay must not be shown a fabricated lifecycle state. - route.overdue.test.ts clears the module-level event bus like its sibling. Tests: new conflict cases for both paths (409 on PUT, refusal without an event emission on approve). npm test 11387 passed, lint 0 errors, check:guards clean, 12 pg-real cases green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 5
Claude Opus 5
parent
f5697cfc2f
commit
df29817826
@@ -1,14 +1,19 @@
|
||||
/**
|
||||
* POST /api/v1/companies/{companyId}/supplier-invoices/{id}/approve
|
||||
*
|
||||
* Transitions a `registered` supplier invoice to `approved`. No journal entry
|
||||
* is involved in this transition: the registration JE has already been posted
|
||||
* Attests a `registered` or `overdue` supplier invoice. No journal entry is
|
||||
* involved in this transition: the registration JE has already been posted
|
||||
* (under accrual) or is deferred to :mark-paid (under cash). Idempotent
|
||||
* (mandatory Idempotency-Key). Dry-runnable.
|
||||
*
|
||||
* Strict-mode: the optimistic-lock UPDATE filters on status='registered' so
|
||||
* concurrent calls (or a same-key replay racing the first) yield a clean 409
|
||||
* rather than a silent no-op.
|
||||
* The resulting status is `approved`, or `overdue` when the invoice is still
|
||||
* past its due date: 'overdue' is derived state the daily cron owns, and
|
||||
* attesting a late payable does not make it on time (#1206).
|
||||
*
|
||||
* Strict-mode: the optimistic-lock UPDATE filters on the pre-approval state
|
||||
* (status in registered/overdue, approved_at IS NULL) so concurrent calls (or
|
||||
* a same-key replay racing the first) yield a clean 409 rather than a silent
|
||||
* no-op.
|
||||
*/
|
||||
|
||||
import { z } from 'zod'
|
||||
@@ -18,14 +23,20 @@ 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 { eventBus } from '@/lib/events'
|
||||
import { getSwedishLocalDate } from '@/lib/bookkeeping/engine'
|
||||
import {
|
||||
canApproveSupplierInvoice,
|
||||
resolveUnsettledStatus,
|
||||
} from '@/lib/supplier-invoices/lifecycle'
|
||||
import type { SupplierInvoice } from '@/types'
|
||||
|
||||
const SI_RESPONSE_COLUMNS =
|
||||
'id, supplier_id, arrival_number, supplier_invoice_number, invoice_date, due_date, status, currency, subtotal, vat_amount, total, paid_amount, remaining_amount, is_credit_note, registration_journal_entry_id, payment_journal_entry_id, created_at, updated_at'
|
||||
'id, supplier_id, arrival_number, supplier_invoice_number, invoice_date, due_date, status, approved_at, currency, subtotal, vat_amount, total, paid_amount, remaining_amount, is_credit_note, registration_journal_entry_id, payment_journal_entry_id, created_at, updated_at'
|
||||
|
||||
const SupplierInvoiceApproved = z.object({
|
||||
id: z.string().uuid(),
|
||||
status: z.literal('approved'),
|
||||
// 'overdue' when the attested invoice is still past its due date.
|
||||
status: z.enum(['approved', 'overdue']),
|
||||
arrival_number: z.number().int(),
|
||||
supplier_invoice_number: z.string(),
|
||||
})
|
||||
@@ -34,16 +45,17 @@ registerEndpoint({
|
||||
operation: 'supplier-invoices.approve',
|
||||
method: 'POST',
|
||||
path: '/api/v1/companies/:companyId/supplier-invoices/:id/approve',
|
||||
summary: 'Approve a registered supplier invoice.',
|
||||
summary: 'Approve a registered or overdue supplier invoice.',
|
||||
description:
|
||||
'Flips a supplier invoice from `registered` to `approved`. No journal entry is posted here: the registration JE was already booked at :create under accrual, or is deferred to :mark-paid under cash. Idempotent. Dry-runnable.',
|
||||
'Attests a supplier invoice that has not been approved yet (status `registered` or `overdue`). The resulting status is `approved`, or `overdue` when the invoice is still past its due date. No journal entry is posted here: the registration JE was already booked at :create under accrual, or is deferred to :mark-paid under cash. Idempotent. Dry-runnable.',
|
||||
useWhen:
|
||||
'A registered SI has been reviewed and you want to mark it ready for payment. Many AP workflows gate :mark-paid behind an explicit approval step.',
|
||||
doNotUseFor:
|
||||
'Posting a journal entry (already done at :create under accrual). Paying the SI (use :mark-paid). Re-approving an already-approved SI (returns 400 SI_APPROVE_NOT_REGISTERED).',
|
||||
pitfalls: [
|
||||
'Idempotency-Key is mandatory.',
|
||||
'Returns 400 SI_APPROVE_NOT_REGISTERED when current status !== "registered". Use the detail endpoint to inspect status first if unsure.',
|
||||
'Returns 400 SI_APPROVE_NOT_REGISTERED when the invoice is already approved (approved_at set) or sits in a settled status. Use the detail endpoint to inspect status first if unsure.',
|
||||
'A still-past-due invoice comes back with status "overdue", not "approved": approved_at is the attest marker, the status is derived from the due date.',
|
||||
],
|
||||
example: {
|
||||
response: {
|
||||
@@ -85,26 +97,48 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string
|
||||
if (!existing) {
|
||||
return v1ErrorResponseFromCode('SI_NOT_FOUND', ctx.log, { requestId: ctx.requestId })
|
||||
}
|
||||
if ((existing as { status: string }).status !== 'registered') {
|
||||
// 'overdue' is approvable: the daily cron flips unbooked invoices there
|
||||
// just by aging (#1206). approved_at is what makes approval idempotent.
|
||||
const invoice = existing as {
|
||||
status: string
|
||||
approved_at?: string | null
|
||||
due_date: string
|
||||
remaining_amount: number
|
||||
is_credit_note?: boolean | null
|
||||
}
|
||||
if (!canApproveSupplierInvoice(invoice)) {
|
||||
return v1ErrorResponseFromCode('SI_APPROVE_NOT_REGISTERED', ctx.log, {
|
||||
requestId: ctx.requestId,
|
||||
details: { current_status: (existing as { status: string }).status },
|
||||
details: { current_status: invoice.status },
|
||||
})
|
||||
}
|
||||
|
||||
// An attested invoice that is still past due keeps the 'overdue' label:
|
||||
// approving is not a reason to hide that the money is late.
|
||||
const approvedAt = new Date().toISOString()
|
||||
const nextStatus = resolveUnsettledStatus(
|
||||
{ ...invoice, approved_at: approvedAt },
|
||||
getSwedishLocalDate(),
|
||||
)
|
||||
|
||||
if (ctx.dryRun) {
|
||||
return dryRunPreview(
|
||||
{ ...(existing as object), status: 'approved' },
|
||||
{ ...(existing as object), status: nextStatus },
|
||||
{ requestId: ctx.requestId, log: ctx.log },
|
||||
)
|
||||
}
|
||||
|
||||
const { data, error } = await ctx.supabase
|
||||
.from('supplier_invoices')
|
||||
.update({ status: 'approved' })
|
||||
.update({ status: nextStatus, approved_at: approvedAt })
|
||||
.eq('company_id', ctx.companyId!)
|
||||
.eq('id', invoiceId)
|
||||
.eq('status', 'registered')
|
||||
.in('status', ['registered', 'overdue'])
|
||||
.is('approved_at', null)
|
||||
// nextStatus was derived from the due date read above, so pin that too:
|
||||
// a concurrent due-date edit must not be papered over with a label
|
||||
// computed from the pre-edit date.
|
||||
.eq('due_date', invoice.due_date)
|
||||
.select(SI_RESPONSE_COLUMNS)
|
||||
.maybeSingle()
|
||||
|
||||
|
||||
@@ -1099,11 +1099,40 @@ describe('POST /api/v1/companies/:companyId/supplier-invoices/:id/approve', () =
|
||||
expect(body.data.status).toBe('approved')
|
||||
})
|
||||
|
||||
it('refuses on already-approved SI (400 SI_APPROVE_NOT_REGISTERED)', async () => {
|
||||
it('attests an overdue SI, which stays overdue while it is still late (#1206)', async () => {
|
||||
// The daily cron flips unbooked payables past due_date to 'overdue'. Attest
|
||||
// must stay reachable there, and it does not make late money on time.
|
||||
const overdue = { ...SAMPLE_SI, status: 'overdue', due_date: '2000-01-01', approved_at: null }
|
||||
mockServiceClient.mockReturnValue(
|
||||
makeFlexibleSupabase({
|
||||
company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
|
||||
supplier_invoices: { data: { ...SAMPLE_SI, status: 'approved' }, error: null },
|
||||
supplier_invoices: [
|
||||
{ data: overdue, error: null },
|
||||
{ data: { ...overdue, approved_at: '2026-07-27T08:00:00Z' }, error: null },
|
||||
],
|
||||
idempotency_keys: { data: null, error: null },
|
||||
}),
|
||||
)
|
||||
const res = await approveSI(
|
||||
makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/supplier-invoices/${SI_ID}/approve`, {
|
||||
method: 'POST',
|
||||
}),
|
||||
detailParams(COMPANY_ID, SI_ID),
|
||||
)
|
||||
expect(res.status).toBe(200)
|
||||
const body = await res.json()
|
||||
expect(body.data.status).toBe('overdue')
|
||||
expect(body.data.approved_at).toBe('2026-07-27T08:00:00Z')
|
||||
})
|
||||
|
||||
it('refuses on an already-attested SI (400 SI_APPROVE_NOT_REGISTERED)', async () => {
|
||||
mockServiceClient.mockReturnValue(
|
||||
makeFlexibleSupabase({
|
||||
company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
|
||||
supplier_invoices: {
|
||||
data: { ...SAMPLE_SI, status: 'overdue', approved_at: '2026-07-01T08:00:00Z' },
|
||||
error: null,
|
||||
},
|
||||
idempotency_keys: { data: null, error: null },
|
||||
}),
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user