diff --git a/core/api/fragments/ap.yaml b/core/api/fragments/ap.yaml index 8b1db8e1..4486dde6 100644 --- a/core/api/fragments/ap.yaml +++ b/core/api/fragments/ap.yaml @@ -174,18 +174,52 @@ paths: posting failure fails the act (a closed period is a 409 blocker period_closed; a line with no account is a 409 blocker line_needs_account; a required chart of accounts code that is missing - is a 409 blocker account_missing naming it). To voided: from pending - (nothing posted) or - approved (the entry is reversed whole; a bill that prices to zero - posted nothing and voids with nothing to reverse); a reason is + is a 409 blocker account_missing naming it; a pending bill that + already carries a paid amount, a legacy or hand edited row no payment + of its own could write, is a 409 blocker has_paid_amount naming the + cents, and the row is left for the operator to mend by hand; a + pending bill that already holds a live journal entry, a legacy row + whose posting ran before the status moved with its entry linked by + hand or not at all, is a 409 blocker entry_not_linked naming the live + entry by its number and id, nothing written). To + voided: from pending or + approved; a bill's credit is live exactly when the POSTED entries of + its VENDOR_INVOICE entry and that entry's whole reversal chain leave + a net amount on Accounts Payable (a reversal of a reversal puts the + credit back, whatever depth the chain has reached; the net is summed + per currency, never across two, as the trial balance sums it, so a + chain holding two currencies whose whole sums cancel still reads + live in the currency that holds the amount, however the chain + arose; the net may stand as a credit or as a debit, and the + refusals name a net amount standing on Accounts Payable, never a + credit alone), and the void + decides on that net, never on one step of the chain: a bill whose + entry holds no live credit (one that prices to zero, a legacy bill + whose posting was swallowed, or an entry the GL routes have already + undone, linked or not) voids with nothing to reverse, a linked live + entry with no reversal row is reversed whole in the entry's own + currency, and a linked live entry whose one reversal row is already + taken (the row is not posted, or the chain was reinstated behind it, + so no new reversal can post) is refused with the + reversal_not_posted + blocker naming the shape; a + reason is required, a bill with money applied is refused with the has_payments - blocker, an approved or partial bill that owes but has no linked - journal entry (a legacy bill whose posting was swallowed, or one - holding the removed sync's one leg credit) with the entry_not_linked + blocker, a bill that holds a live journal entry it is not linked to + (a legacy bill whose posting was swallowed, one holding the removed + sync's one leg credit, one whose total was hand edited after its + approve, or one posted twice, the duplicate named) with the + entry_not_linked blocker, and a bill whose linked entry does not balance with the - entry_unbalanced blocker; nothing is written on either refusal. No - route links an entry to a bill yet: such a bill stays approved and - payable until a later repair item (a stated limit). + entry_unbalanced blocker; nothing is written on any refusal, and a + reversal insert that loses a race with a concurrent GL reverse on the + same entry answers the same reversal_not_posted 409, never a 500, and + so does a GL void of the linked entry that commits while the void + runs (the entry is no longer POSTED, there is nothing left to + reverse, nothing is written and the retry voids with nothing to + reverse). No + route links an entry to a bill yet: such a bill keeps its status + until a later repair item (a stated limit). parameters: - name: id in: path @@ -282,8 +316,11 @@ paths: is a 409 blocker invoice_not_approved; a bill of another vendor is a 400 naming invoice_ids[i]); a repeated id in invoice_ids is a 400 naming its index (appears twice); more than the named bills still owe - is a 409 blocker exceeds_open. The entry posts through gl.PostEntry inside - the act's transaction (DR Accounts Payable, CR Cash), and a posting + is a 409 blocker exceeds_open. The named bills must share one + currency: a payment spanning two is a 409 blocker currency_mismatch + naming the bills that differ. The entry posts through gl.PostEntry + inside the act's transaction in the bills' currency + (DR Accounts Payable, CR Cash), and a posting failure fails the act (a closed period is a 409 blocker period_closed; a missing chart of accounts code is a 409 blocker account_missing naming it). diff --git a/core/api/fragments/gl.yaml b/core/api/fragments/gl.yaml index c5b82a55..d78d7009 100644 --- a/core/api/fragments/gl.yaml +++ b/core/api/fragments/gl.yaml @@ -259,8 +259,11 @@ paths: dated today and linked by reverses_entry_id. The optional body reason is appended to the memo; an undecodable or absent body is ignored. Only a POSTED entry that has not already been reversed and has at - least 2 lines can be reversed. Every refusal, including an unknown - id, is a 400. + least 2 lines can be reversed. The reversal posts in the original + entry's own currency (the entry priced its legs into that currency, + so a reversal in the currency.default setting's code would leave the + control account holding both halves of one undo in different + currencies). Every refusal, including an unknown id, is a 400. parameters: - name: id in: path diff --git a/core/api/fragments/invoice.yaml b/core/api/fragments/invoice.yaml index 6df6a816..7c261794 100644 --- a/core/api/fragments/invoice.yaml +++ b/core/api/fragments/invoice.yaml @@ -461,9 +461,10 @@ paths: description: 'Roles admin, owner, finance; a reason is required. Writes an amount of an unpaid or partial invoice off as bad debt: DR 5040 / CR 1020, a WRITE_OFF application and a WRITE_OFF subledger row. A write off that takes the open amount to zero closes the invoice as written_off, else it stays partial. The precondition - is the invoice''s revision. Refused with 409 exceeds_open_amount, invoice_not_open (invalid_state_transition) - or period_closed. Writes invoice.written_off when it closes the invoice, else invoice.partial, and customer.updated. - Reversing the application (POST /ar/applications/{id}/reverse) reopens the invoice.' + is the invoice''s revision. Refused with 409 exceeds_open_amount, invalid_state_transition (the invoice is + neither unpaid nor partial) or period_closed. Writes invoice.written_off when it closes the invoice, else + invoice.partial, and customer.updated. Reversing the application (POST /ar/applications/{id}/reverse) reopens + the invoice.' parameters: - name: id in: path diff --git a/core/api/openapi.yaml b/core/api/openapi.yaml index 1ba988fc..e03b93c9 100644 --- a/core/api/openapi.yaml +++ b/core/api/openapi.yaml @@ -1317,7 +1317,7 @@ paths: tags: [ap] summary: Approve or void a vendor invoice description: >- - Role guard admin, owner or finance, held to the caller's branch wall. Replaces the old /approve route. The body names the target status and carries the revision (or If-Match); a write with neither is 428, a mismatch 409 stale_revision. To approved: only from pending; the entry posts through gl.PostEntry inside the transition's transaction (a debit leg per line on the line's account, the tax spread across the line accounts pro rata, the credit to Accounts Payable), and a posting failure fails the act (a closed period is a 409 blocker period_closed; a line with no account is a 409 blocker line_needs_account; a required chart of accounts code that is missing is a 409 blocker account_missing naming it). To voided: from pending (nothing posted) or approved (the entry is reversed whole; a bill that prices to zero posted nothing and voids with nothing to reverse); a reason is required, a bill with money applied is refused with the has_payments blocker, an approved or partial bill that owes but has no linked journal entry (a legacy bill whose posting was swallowed, or one holding the removed sync's one leg credit) with the entry_not_linked blocker, and a bill whose linked entry does not balance with the entry_unbalanced blocker; nothing is written on either refusal. No route links an entry to a bill yet: such a bill stays approved and payable until a later repair item (a stated limit). + Role guard admin, owner or finance, held to the caller's branch wall. Replaces the old /approve route. The body names the target status and carries the revision (or If-Match); a write with neither is 428, a mismatch 409 stale_revision. To approved: only from pending; the entry posts through gl.PostEntry inside the transition's transaction (a debit leg per line on the line's account, the tax spread across the line accounts pro rata, the credit to Accounts Payable), and a posting failure fails the act (a closed period is a 409 blocker period_closed; a line with no account is a 409 blocker line_needs_account; a required chart of accounts code that is missing is a 409 blocker account_missing naming it; a pending bill that already carries a paid amount, a legacy or hand edited row no payment of its own could write, is a 409 blocker has_paid_amount naming the cents, and the row is left for the operator to mend by hand; a pending bill that already holds a live journal entry, a legacy row whose posting ran before the status moved with its entry linked by hand or not at all, is a 409 blocker entry_not_linked naming the live entry by its number and id, nothing written). To voided: from pending or approved; a bill's credit is live exactly when the POSTED entries of its VENDOR_INVOICE entry and that entry's whole reversal chain leave a net amount on Accounts Payable (a reversal of a reversal puts the credit back, whatever depth the chain has reached; the net is summed per currency, never across two, as the trial balance sums it, so a chain holding two currencies whose whole sums cancel still reads live in the currency that holds the amount, however the chain arose; the net may stand as a credit or as a debit, and the refusals name a net amount standing on Accounts Payable, never a credit alone), and the void decides on that net, never on one step of the chain: a bill whose entry holds no live credit (one that prices to zero, a legacy bill whose posting was swallowed, or an entry the GL routes have already undone, linked or not) voids with nothing to reverse, a linked live entry with no reversal row is reversed whole in the entry's own currency, and a linked live entry whose one reversal row is already taken (the row is not posted, or the chain was reinstated behind it, so no new reversal can post) is refused with the reversal_not_posted blocker naming the shape; a reason is required, a bill with money applied is refused with the has_payments blocker, a bill that holds a live journal entry it is not linked to (a legacy bill whose posting was swallowed, one holding the removed sync's one leg credit, one whose total was hand edited after its approve, or one posted twice, the duplicate named) with the entry_not_linked blocker, and a bill whose linked entry does not balance with the entry_unbalanced blocker; nothing is written on any refusal, and a reversal insert that loses a race with a concurrent GL reverse on the same entry answers the same reversal_not_posted 409, never a 500, and so does a GL void of the linked entry that commits while the void runs (the entry is no longer POSTED, there is nothing left to reverse, nothing is written and the retry voids with nothing to reverse). No route links an entry to a bill yet: such a bill keeps its status until a later repair item (a stated limit). parameters: - name: id in: path @@ -1400,7 +1400,7 @@ paths: tags: [ap] summary: Pay a vendor description: >- - Role guard admin, owner or finance. The request's money is int64 amount_cents (a decimal amount is a 400 naming it; the old float dollars amount is gone); the rest of the shape keeps its today form (uppercase methods) until the payment routes' own conversion. The payment locks the named bills in id order and applies only to bills in approved or partial (a pending, paid or voided bill is a 409 blocker invoice_not_approved; a bill of another vendor is a 400 naming invoice_ids[i]); a repeated id in invoice_ids is a 400 naming its index (appears twice); more than the named bills still owe is a 409 blocker exceeds_open. The entry posts through gl.PostEntry inside the act's transaction (DR Accounts Payable, CR Cash), and a posting failure fails the act (a closed period is a 409 blocker period_closed; a missing chart of accounts code is a 409 blocker account_missing naming it). + Role guard admin, owner or finance. The request's money is int64 amount_cents (a decimal amount is a 400 naming it; the old float dollars amount is gone); the rest of the shape keeps its today form (uppercase methods) until the payment routes' own conversion. The payment locks the named bills in id order and applies only to bills in approved or partial (a pending, paid or voided bill is a 409 blocker invoice_not_approved; a bill of another vendor is a 400 naming invoice_ids[i]); a repeated id in invoice_ids is a 400 naming its index (appears twice); more than the named bills still owe is a 409 blocker exceeds_open. The named bills must share one currency: a payment spanning two is a 409 blocker currency_mismatch naming the bills that differ. The entry posts through gl.PostEntry inside the act's transaction in the bills' currency (DR Accounts Payable, CR Cash), and a posting failure fails the act (a closed period is a 409 blocker period_closed; a missing chart of accounts code is a 409 blocker account_missing naming it). parameters: - $ref: '#/components/parameters/IdempotencyKey' requestBody: @@ -5465,7 +5465,7 @@ paths: tags: [gl] summary: Reverse a posted journal entry description: >- - Role guard admin or owner. Books a posted, net-zero reversing entry dated today and linked by reverses_entry_id. The optional body reason is appended to the memo; an undecodable or absent body is ignored. Only a POSTED entry that has not already been reversed and has at least 2 lines can be reversed. Every refusal, including an unknown id, is a 400. + Role guard admin or owner. Books a posted, net-zero reversing entry dated today and linked by reverses_entry_id. The optional body reason is appended to the memo; an undecodable or absent body is ignored. Only a POSTED entry that has not already been reversed and has at least 2 lines can be reversed. The reversal posts in the original entry's own currency (the entry priced its legs into that currency, so a reversal in the currency.default setting's code would leave the control account holding both halves of one undo in different currencies). Every refusal, including an unknown id, is a 400. parameters: - name: id in: path @@ -6857,7 +6857,7 @@ paths: tags: - invoice summary: Write off part or all of an invoice's open amount - description: 'Roles admin, owner, finance; a reason is required. Writes an amount of an unpaid or partial invoice off as bad debt: DR 5040 / CR 1020, a WRITE_OFF application and a WRITE_OFF subledger row. A write off that takes the open amount to zero closes the invoice as written_off, else it stays partial. The precondition is the invoice''s revision. Refused with 409 exceeds_open_amount, invoice_not_open (invalid_state_transition) or period_closed. Writes invoice.written_off when it closes the invoice, else invoice.partial, and customer.updated. Reversing the application (POST /ar/applications/{id}/reverse) reopens the invoice.' + description: 'Roles admin, owner, finance; a reason is required. Writes an amount of an unpaid or partial invoice off as bad debt: DR 5040 / CR 1020, a WRITE_OFF application and a WRITE_OFF subledger row. A write off that takes the open amount to zero closes the invoice as written_off, else it stays partial. The precondition is the invoice''s revision. Refused with 409 exceeds_open_amount, invalid_state_transition (the invoice is neither unpaid nor partial) or period_closed. Writes invoice.written_off when it closes the invoice, else invoice.partial, and customer.updated. Reversing the application (POST /ar/applications/{id}/reverse) reopens the invoice.' parameters: - name: id in: path diff --git a/core/internal/ap/fixture_test.go b/core/internal/ap/fixture_test.go index 8fbe5348..00a12da9 100644 --- a/core/internal/ap/fixture_test.go +++ b/core/internal/ap/fixture_test.go @@ -83,6 +83,10 @@ func newFixture(t *testing.T, db *database.DB) *fixture { WithAuditLog(audit.NewLogger(db)).WithBranchGuard(middleware.NewBranchGuard(db)) mux := http.NewServeMux() ap.NewHandler(f.svc).RegisterRoutes(mux) + // The GL routes ride the same server (unauthenticated, as the desk's stand + // in serves them to the fixture's administrator): the void tests reach the + // ledger through the public reverse and void routes, not hand SQL. + gl.NewHandler(f.glSvc).RegisterRoutes(mux) wallMux := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { claims := &middleware.UserClaims{Role: "finance", Roles: []string{"finance"}} claims.Subject = r.Header.Get("X-Test-Sub") @@ -114,17 +118,26 @@ func newFixture(t *testing.T, db *database.DB) *fixture { return f } +// fixtureEntries is every journal entry the fixture's acts wrote for its +// vendor: the entries whose source reference is one of the vendor's bills or +// payments, plus each such entry's whole reversal chain (a reversal of a +// reversal chains two levels down reverses_entry_id, so a one level walk would +// miss it). +const fixtureEntries = `WITH RECURSIVE mine(id) AS ( + SELECT id FROM gl_journal_entries + WHERE source_ref_id IN (SELECT id FROM vendor_invoices WHERE vendor_id = $1) + OR source_ref_id IN (SELECT id FROM ap_payments WHERE vendor_id = $1) + UNION + SELECT e.id FROM gl_journal_entries e JOIN mine ON e.reverses_entry_id = mine.id +) SELECT id FROM mine` + // cleanup removes everything the fixture's vendor produced, children first. func (f *fixture) cleanup() { ctx := context.Background() exec := func(sql string, args ...any) { _, _ = f.db.Pool.Exec(ctx, sql, args...) } inv := `SELECT id FROM vendor_invoices WHERE vendor_id = $1` - entries := `SELECT id FROM gl_journal_entries WHERE source_ref_id IN (` + inv + `) - OR source_ref_id IN (SELECT id FROM ap_payments WHERE vendor_id = $1)` - exec(`DELETE FROM gl_journal_lines WHERE journal_entry_id IN (SELECT id FROM gl_journal_entries WHERE reverses_entry_id IN `+entries+`)`, f.vendor) - exec(`DELETE FROM gl_journal_entries WHERE reverses_entry_id IN `+entries, f.vendor) - exec(`DELETE FROM gl_journal_lines WHERE journal_entry_id IN `+entries, f.vendor) - exec(`DELETE FROM gl_journal_entries WHERE id IN `+entries, f.vendor) + exec(`DELETE FROM gl_journal_lines WHERE journal_entry_id IN (`+fixtureEntries+`)`, f.vendor) + exec(`DELETE FROM gl_journal_entries WHERE id IN (`+fixtureEntries+`)`, f.vendor) exec(`DELETE FROM events_outbox WHERE entity_type = 'vendor_invoice' AND entity_id IN `+inv, f.vendor) exec(`DELETE FROM audit_log WHERE entity_type = 'vendor_invoice' AND entity_id IN `+inv, f.vendor) exec(`DELETE FROM ap_payment_applications WHERE invoice_id IN `+inv, f.vendor) @@ -293,6 +306,55 @@ func (f *fixture) payHeaders(headers []string, cents int64, invoiceIDs ...string }, headers...) } +// --------------------------------------------------------------------------- +// Reaching the ledger through the public GL routes. +// --------------------------------------------------------------------------- + +// glReverse runs the GL reverse route on an entry. +func (f *fixture) glReverse(entry string) resp { + f.t.Helper() + return f.do("POST", "/api/v1/gl/journal-entries/"+entry+"/reverse", map[string]any{"reason": "fixture reversal"}) +} + +// glVoidEntry runs the GL void route on an entry. +func (f *fixture) glVoidEntry(entry string) resp { + f.t.Helper() + return f.do("POST", "/api/v1/gl/journal-entries/"+entry+"/void", nil) +} + +// glEntryOf is the bill's linked entry id. +func (f *fixture) glEntryOf(invoiceID string) string { + f.t.Helper() + var entry string + if err := f.db.Pool.QueryRow(context.Background(), + `SELECT gl_entry_id::text FROM vendor_invoices WHERE id = $1`, invoiceID).Scan(&entry); err != nil { + f.t.Fatal(err) + } + return entry +} + +// glReversalOf is the id of the one reversal pointing at an entry. +func (f *fixture) glReversalOf(entry string) string { + f.t.Helper() + var rev string + if err := f.db.Pool.QueryRow(context.Background(), + `SELECT id::text FROM gl_journal_entries WHERE reverses_entry_id = $1`, entry).Scan(&rev); err != nil { + f.t.Fatal(err) + } + return rev +} + +// number is the bill's own number (AP-). +func (f *fixture) number(invoiceID string) string { + f.t.Helper() + var n string + if err := f.db.Pool.QueryRow(context.Background(), + `SELECT number FROM vendor_invoices WHERE id = $1`, invoiceID).Scan(&n); err != nil { + f.t.Fatal(err) + } + return n +} + // --------------------------------------------------------------------------- // Reading the ledger, the events and the control account. // --------------------------------------------------------------------------- @@ -334,9 +396,19 @@ func (f *fixture) entryLegsWhere(t *testing.T, db *database.DB, where string, ar } // controlBalance is the AP control account's balance in cents, from the -// entries this fixture's acts wrote (approves credit it, payments and -// reversals debit it), scoped to the fixture's own bills and payments. +// POSTED entries this fixture's acts wrote (approves credit it, payments and +// reversals debit it; the trial balance sums POSTED entries alone, so an +// entry the GL void route voided counts nowhere), scoped to the fixture's +// own bills and payments. func (f *fixture) controlBalance() int64 { + f.t.Helper() + return f.controlBalanceIn("") +} + +// controlBalanceIn is controlBalance for one currency (every currency when +// the code is empty): the tie the trial balance states per currency, never +// across two. +func (f *fixture) controlBalanceIn(currency string) int64 { f.t.Helper() var balance int64 err := f.db.Pool.QueryRow(context.Background(), ` @@ -344,11 +416,8 @@ func (f *fixture) controlBalance() int64 { FROM gl_journal_lines l JOIN gl_journal_entries e ON e.id = l.journal_entry_id JOIN gl_accounts a ON a.id = l.account_id - WHERE a.code = '2010' AND e.id IN ( - SELECT id FROM gl_journal_entries - WHERE source_ref_id IN (SELECT id FROM vendor_invoices WHERE vendor_id = $1) - OR source_ref_id IN (SELECT id FROM ap_payments WHERE vendor_id = $1) - OR reverses_entry_id IN (SELECT gl_entry_id FROM vendor_invoices WHERE vendor_id = $1))`, f.vendor).Scan(&balance) + WHERE a.code = '2010' AND e.status = 'POSTED' AND ($2 = '' OR e.currency = $2) AND e.id IN ( + `+fixtureEntries+`)`, f.vendor, currency).Scan(&balance) if err != nil { f.t.Fatal(err) } @@ -357,11 +426,19 @@ func (f *fixture) controlBalance() int64 { // openSum is what the vendor's open bills owe, in cents. func (f *fixture) openSum() int64 { + f.t.Helper() + return f.openSumIn("") +} + +// openSumIn is openSum for one currency (every currency when the code is +// empty). +func (f *fixture) openSumIn(currency string) int64 { f.t.Helper() var open int64 if err := f.db.Pool.QueryRow(context.Background(), - `SELECT ROUND(COALESCE(SUM(amount_open), 0) * 100)::bigint FROM vendor_invoices WHERE vendor_id = $1 AND status IN ('APPROVED', 'PARTIAL')`, - f.vendor).Scan(&open); err != nil { + `SELECT ROUND(COALESCE(SUM(amount_open), 0) * 100)::bigint FROM vendor_invoices + WHERE vendor_id = $1 AND status IN ('APPROVED', 'PARTIAL') AND ($2 = '' OR currency = $2)`, + f.vendor, currency).Scan(&open); err != nil { f.t.Fatal(err) } return open @@ -376,11 +453,7 @@ func (f *fixture) balanced() bool { SELECT count(*) FROM gl_journal_entries e JOIN (SELECT journal_entry_id, SUM(debit) d, SUM(credit) c FROM gl_journal_lines GROUP BY journal_entry_id) s ON s.journal_entry_id = e.id - WHERE e.id IN ( - SELECT id FROM gl_journal_entries - WHERE source_ref_id IN (SELECT id FROM vendor_invoices WHERE vendor_id = $1) - OR source_ref_id IN (SELECT id FROM ap_payments WHERE vendor_id = $1) - OR reverses_entry_id IN (SELECT gl_entry_id FROM vendor_invoices WHERE vendor_id = $1)) + WHERE e.id IN (`+fixtureEntries+`) AND s.d <> s.c`, f.vendor).Scan(&bad) if err != nil { f.t.Fatal(err) diff --git a/core/internal/ap/input.go b/core/internal/ap/input.go index 1499d8f5..330e4309 100644 --- a/core/internal/ap/input.go +++ b/core/internal/ap/input.go @@ -115,11 +115,12 @@ func (req *CreateRequest) Parse() (*Input, error) { } } currency := trim(req.Currency) - if currency == "" { - currency = "USD" // the service replaces this with the setting's default + if currency != "" { + v.Check(len(currency) == 3 && currency == strings.ToUpper(currency), "currency", "must be an ISO 4217 code of three capital letters") + in.Currency = currency } - v.Check(len(currency) == 3 && currency == strings.ToUpper(currency), "currency", "must be an ISO 4217 code of three capital letters") - in.Currency = currency + // An omitted currency is left empty: the service's resolve reads the + // currency.default setting for it (USD where the setting is absent). if n, ok := v.Int("tax_cents", req.TaxCents, false); ok { v.Check(n >= 0, "tax_cents", "must not be negative") v.Check(n <= maxBillCents, "tax_cents", "is more than a bill can hold") @@ -162,7 +163,7 @@ func (req *CreateRequest) Parse() (*Input, error) { line.ProductID = &id } } - if line.GLAccountID != nil && line.Quantity != 0 && line.UnitPrice >= 0 && line.UnitPrice <= maxUnitPriceTenThousandths { + if line.GLAccountID != nil && line.Quantity != 0 && line.UnitPrice >= 0 { total, err := httpx.Extend(line.Quantity, 1, 1, line.UnitPrice) switch { case err != nil: diff --git a/core/internal/ap/repository.go b/core/internal/ap/repository.go index 33645eed..626d6da4 100644 --- a/core/internal/ap/repository.go +++ b/core/internal/ap/repository.go @@ -301,6 +301,100 @@ func (r *Repository) Lock(ctx context.Context, id uuid.UUID) (*LockedInvoice, er return &locked, nil } +// chainLive is the liveness test every AP act keys on (review r4 P1-1): the +// entry and its whole reversal chain, walked down reverses_entry_id through +// entries of any status, net a nonzero amount on the control account when +// only their POSTED entries are summed, exactly as the trial balance sums +// 2010: per currency, never across two (review r6 P2-1), so a chain holding +// two currencies whose whole sums cancel still reads live in the currency +// that holds the amount, however the chain arose (legacy rows included). A +// reversal row of one level says nothing by itself: a reversal that a later +// reversal put back leaves the credit live at any depth, and a chain whose +// POSTED entries cancel holds none. The %s is the root entries' predicate; +// its argument placeholders are the query's own. +const chainLive = `WITH RECURSIVE chain(root, id) AS ( + SELECT e.id, e.id FROM gl_journal_entries e WHERE %s + UNION + SELECT c.root, n.id FROM gl_journal_entries n JOIN chain c ON n.reverses_entry_id = c.id +) +SELECT c.root FROM chain c +JOIN gl_journal_entries e ON e.id = c.id AND e.status = 'POSTED' +JOIN gl_journal_lines l ON l.journal_entry_id = e.id +JOIN gl_accounts a ON a.id = l.account_id AND a.code = '2010' +GROUP BY c.root, e.currency +HAVING SUM(l.credit - l.debit) <> 0` + +// HasPostedInvoiceEntry reports whether a VENDOR_INVOICE journal entry of the +// bill stands live in the control account, linked or not: the entry's whole +// reversal chain nets a nonzero amount on 2010 over its POSTED entries (the +// predicate migration 106 links by, made chain wide in review r4 P1-1), so a +// void can tell a bill whose credit is in the ledger from one that posted +// nothing, whatever mix of reversals and voids the chain holds. +func (r *Repository) HasPostedInvoiceEntry(ctx context.Context, id uuid.UUID) (bool, error) { + sql := fmt.Sprintf(`SELECT count(*) FROM (%s) live_roots`, fmt.Sprintf(chainLive, + `e.source = 'VENDOR_INVOICE' AND e.source_ref_id = $1 AND e.reverses_entry_id IS NULL`)) + var n int + if err := r.db.GetExecutor(ctx).QueryRow(ctx, sql, id).Scan(&n); err != nil { + return false, fmt.Errorf("failed to read the bill's journal entries: %w", err) + } + return n > 0, nil +} + +// LiveInvoiceEntryOtherThan names the bill's newest live VENDOR_INVOICE entry +// other than the one excluded (found is false when the excluded entry is the +// only live one; a nil exclude names no entry, so every live entry of the +// bill matches): the double posted shape, whose duplicate credit a void +// would strand under a VOIDED bill (review r2 P3-1). Live is the chain net +// of HasPostedInvoiceEntry, never one reversal level (review r4 P1-1). +func (r *Repository) LiveInvoiceEntryOtherThan(ctx context.Context, invoiceID, exclude uuid.UUID) (entryNumber int64, entryID uuid.UUID, found bool, err error) { + sql := fmt.Sprintf(`SELECT root.entry_number, root.id FROM (%s) live_roots + JOIN gl_journal_entries root ON root.id = live_roots.root + ORDER BY root.created_at DESC, root.id DESC LIMIT 1`, fmt.Sprintf(chainLive, + `e.source = 'VENDOR_INVOICE' AND e.source_ref_id = $1 AND e.reverses_entry_id IS NULL AND e.id <> $2`)) + err = r.db.GetExecutor(ctx).QueryRow(ctx, sql, invoiceID, exclude).Scan(&entryNumber, &entryID) + if errors.Is(err, pgx.ErrNoRows) { + return 0, uuid.Nil, false, nil + } + if err != nil { + return 0, uuid.Nil, false, fmt.Errorf("failed to read the bill's journal entries: %w", err) + } + return entryNumber, entryID, true, nil +} + +// LinkedEntryLedger reads the linked entry's place in the ledger. live says +// its credit stands in the control account: the entry's whole reversal chain +// nets a nonzero amount on 2010 over its POSTED entries, the trial balance's +// own lens (review r4 P1-1: a reversal of a reversal puts the credit back, +// so one level down the chain decides nothing), summed per currency, never +// across two (review r6 P2-1): live when any one currency's net is nonzero, +// so a mixed currency chain whose whole sums cancel still reads live in the +// currency that holds the amount. reversalTaken says a reversal row of any +// status points at the entry, so the one reversal an entry may hold is +// already taken and no new reversal can post while that row stands. An entry +// deleted by hand holds no credit: live is false (review r2 P2-1). +func (r *Repository) LinkedEntryLedger(ctx context.Context, id uuid.UUID) (live, reversalTaken bool, err error) { + err = r.db.GetExecutor(ctx).QueryRow(ctx, ` + WITH RECURSIVE chain(id) AS ( + SELECT id FROM gl_journal_entries WHERE id = $1 + UNION + SELECT n.id FROM gl_journal_entries n JOIN chain c ON n.reverses_entry_id = c.id + ) + SELECT COALESCE(( + SELECT count(*) FROM ( + SELECT e.currency FROM gl_journal_lines l + JOIN gl_journal_entries e ON e.id = l.journal_entry_id AND e.status = 'POSTED' + JOIN gl_accounts a ON a.id = l.account_id AND a.code = '2010' + WHERE e.id IN (SELECT id FROM chain) + GROUP BY e.currency + HAVING SUM(l.credit - l.debit) <> 0) nets), 0) <> 0, + EXISTS(SELECT 1 FROM gl_journal_entries r WHERE r.reverses_entry_id = $1)`, id). + Scan(&live, &reversalTaken) + if err != nil { + return false, false, fmt.Errorf("failed to read the linked journal entry: %w", err) + } + return live, reversalTaken, nil +} + // LockForPayment takes one invoice of the named vendor FOR UPDATE, in the // caller's id order (ADR 0008 section 9 step 5a). No row means no such // invoice for this vendor. diff --git a/core/internal/ap/service.go b/core/internal/ap/service.go index 7392dfb5..263b9658 100644 --- a/core/internal/ap/service.go +++ b/core/internal/ap/service.go @@ -372,6 +372,31 @@ func (s *Service) approve(ctx context.Context, id uuid.UUID, pre Precondition, w return httpx.InvalidStateTransition("only a pending bill can be approved", httpx.Blocker("invoice_not_pending", "the bill is "+locked.Status.Status())) } + // A pending bill cannot carry a paid amount: a payment applies only + // to an approved or partial bill (ADR 0008 7.4), so this is a legacy + // or hand edited row, and amount_open sits where migration 106 + // clamped it, below the total. The approve posts the whole total + // while amount_open stands still, which would leave money in 2010 no + // bill owes (review r8 P3-1), so the act is refused and the row is + // left for the operator to mend by hand. + if locked.AmountPaidCents != 0 { + return httpx.InvalidStateTransition("a pending bill cannot carry a paid amount; mend amount_paid by hand", + httpx.Blocker("has_paid_amount", fmt.Sprintf("%d cents are paid against this bill", int64(locked.AmountPaidCents)))) + } + // The approve decides on the ledger like every AP act (review r4 + // P3-2): a legacy PENDING bill whose posting ran before the status + // moved (the base's sync could post first; migration 106 links only + // APPROVED or PARTIAL, so such a row carries its entry linked by + // hand or not at all) already holds its credit in 2010, and an + // approve would post a second one no bill owes. The refusal names + // the live entry, whatever depth its reversal chain has reached + // (P1-1's chain lens); nothing is written. + if number, other, found, err := s.repo.LiveInvoiceEntryOtherThan(ctx, id, uuid.Nil); err != nil { + return err + } else if found { + return httpx.InvalidStateTransition("the bill already holds a live journal entry, so an approve would post its own entry beside the net amount already standing in accounts payable", + httpx.Blocker("entry_not_linked", fmt.Sprintf("journal entry #%d (%s) still leaves a net amount standing in accounts payable for this bill, as a credit or as a debit, whichever way its chain nets; mend the bill's ledger by hand before approving", number, other))) + } lines, err := s.repo.LinesWithCodes(ctx, id) if err != nil { return err @@ -469,16 +494,28 @@ func (s *Service) postInvoiceEntry(ctx context.Context, inv *Summary, lines []Li } // void is the transition that reverses the entry: a pending bill has nothing -// posted and simply dies; an approved one's entry is reversed whole; a bill -// that prices to zero posted nothing and voids with nothing to reverse. A bill -// with money applied is refused: the payments have to be undone first. A bill -// that owes (approved or partial, a nonzero total) but has no linked entry is -// refused too: no route links an entry to a bill, and a legacy bill of that -// shape may hold the removed sync's one leg credit, which a plain void would -// strand. So is a linked entry that does not balance (that one leg shape, -// linked by hand): its reversal would not balance either. Both refusals are a -// stated limit: such a bill stays approved and payable until a later repair -// item. +// posted and simply dies; an approved one's linked entry is reversed whole +// when its credit stands live in the control account; a bill whose entry is +// not live (one that prices to zero, a legacy bill whose posting was +// swallowed, or an entry the GL routes have already undone, linked or not) +// holds no credit in the control account and voids with nothing to reverse. +// Liveness is decided on the whole reversal chain, never on one step of it +// (review r4 P1-1): a bill's credit is live exactly when the POSTED entries +// of its VENDOR_INVOICE entry and that entry's whole reversal chain leave a +// net amount on 2010, so a reversal that a later reversal put back leaves +// the credit live again at any depth. A bill with money applied is refused: +// the payments have to be undone first. So is a bill that holds a live +// VENDOR_INVOICE entry it is not linked to (a legacy bill: its posting was +// swallowed, it holds the removed sync's one leg credit, a hand edit may +// even have zeroed its total after the approve, or it was posted twice and +// only the newest entry is linked, the duplicate named by the refusal): a +// plain void would strand that credit. A linked entry that does not balance +// (that one leg shape, linked by hand) is refused too, because its reversal +// would not balance either, and so is a linked entry whose chain still nets +// a live credit while the one reversal row the entry may hold is already +// taken (the row is not posted, or the chain was reinstated behind it). +// Every refusal is a stated limit: such a bill keeps its status until a +// later repair item. func (s *Service) void(ctx context.Context, id uuid.UUID, pre Precondition, reason string, who Caller) (*Invoice, error) { if reason == "" { return nil, httpx.BadRequest("one or more fields failed validation", @@ -501,33 +538,83 @@ func (s *Service) void(ctx context.Context, id uuid.UUID, pre Precondition, reas if !from.allows(StatusVoided) { return httpx.InvalidStateTransition("a " + from.Status() + " bill cannot be voided") } - // An approved or partial bill that owes but has no linked entry (a - // legacy bill: its posting was swallowed, or it holds the removed - // sync's one leg credit) must not void silently: a credit may stay in - // the control account with nothing to reverse. A zero total bill posted - // nothing and owes nothing, so it voids. - if (from == StatusApproved || from == StatusPartial) && locked.GLEntryID == nil && locked.TotalCents != 0 { - return httpx.InvalidStateTransition("the bill has no linked journal entry, so a void could leave its credit in accounts payable", - httpx.Blocker("entry_not_linked", "no route links a journal entry to a bill yet; the bill stays approved and payable until a later repair item")) + // A bill that holds a posted, unreversed VENDOR_INVOICE entry it is + // not linked to (a legacy bill: its posting was swallowed, or it holds + // the removed sync's one leg credit, and a hand edit may have zeroed + // its total after the approve) must not void silently: the credit + // would stay in the control account with nothing to reverse, whatever + // the bill's total says now (review r8 P3-2). A bill with no live + // entry holds no credit in the ledger, so it voids with nothing to + // reverse. + live, err := s.repo.HasPostedInvoiceEntry(ctx, id) + if err != nil { + return err + } + if locked.GLEntryID == nil && live { + return httpx.InvalidStateTransition("the bill holds a posted journal entry it is not linked to, so a void could leave the entry's net amount standing in accounts payable", + httpx.Blocker("entry_not_linked", "no route links a journal entry to a bill yet; the bill keeps its status until a later repair item")) } var reversal *uuid.UUID if locked.GLEntryID != nil { - entry, err := s.gl.PostReversal(ctx, gl.ReversalInput{ - EntryID: *locked.GLEntryID, EntryDate: s.now(), Currency: locked.Currency, - Reason: reason, PostedBy: who.Actor, - }) + // A second live entry beside the linked one (the double posted + // shape: the base read the status outside its transaction, so a + // double click could post it twice) must not be stranded under a + // VOIDED bill: no route could reach it again (review r2 P3-1). + if number, other, found, err := s.repo.LiveInvoiceEntryOtherThan(ctx, id, *locked.GLEntryID); err != nil { + return err + } else if found { + return httpx.InvalidStateTransition("the bill holds a second live journal entry beside its linked one, so a void would strand that entry's credit in accounts payable", + httpx.Blocker("entry_not_linked", fmt.Sprintf("journal entry #%d (%s) is also live for this bill; reverse it before voiding", number, other))) + } + // The linked arm is keyed on the ledger too, never on the pointer + // alone (review r2 P2-1): an entry the GL routes have already + // reversed or voided holds no live credit, so the void has + // nothing to reverse rather than a failure. Liveness is the net + // of the whole reversal chain (review r4 P1-1), so a reversal of + // a reversal that put the credit back reads live again. A live + // entry whose one reversal row is already taken (the row is not + // posted, or the chain was reinstated behind it) cannot be + // reversed again: the void is refused rather than stranding the + // credit. + liveLinked, reversalTaken, err := s.repo.LinkedEntryLedger(ctx, *locked.GLEntryID) if err != nil { - if errors.Is(err, gl.ErrPeriodClosed) { - return conflictBlocker("period_closed", "the reversal would be dated into a closed fiscal period") + return err + } + if liveLinked && reversalTaken { + return httpx.InvalidStateTransition("the linked entry's reversal chain still nets a nonzero amount on accounts payable while the one reversal the entry may hold is taken, so no new reversal can post", + httpx.Blocker("reversal_not_posted", "a net amount stands in accounts payable, as a credit or as a debit, whichever way the chain nets; mend the linked entry's reversal chain before voiding the bill; the bill keeps its status until then")) + } + if liveLinked { + entry, err := s.gl.PostReversal(ctx, gl.ReversalInput{ + EntryID: *locked.GLEntryID, EntryDate: s.now(), Currency: locked.Currency, + Reason: reason, PostedBy: who.Actor, + }) + if err != nil { + if errors.Is(err, gl.ErrPeriodClosed) { + return conflictBlocker("period_closed", "the reversal would be dated into a closed fiscal period") + } + if errors.Is(err, gl.ErrUnbalanced) { + return httpx.InvalidStateTransition("the linked entry does not balance and a void would mirror it into another unbalanced entry", + httpx.Blocker("entry_unbalanced", "the bill's linked journal entry does not balance (the removed sync's one leg shape); the bill stays approved and payable until a later repair item")) + } + if errors.Is(err, gl.ErrAlreadyReversed) { + return httpx.InvalidStateTransition("the linked entry's reversal appeared while the void ran, so its credit is already undone", + httpx.Blocker("reversal_not_posted", "the linked journal entry is reversed; the bill keeps its status until then")) + } + // A GL void of the linked entry committed between the + // void's ledger read and the reversal's own read of the + // entry (review r6 P3-1): the same 409 as the other lost + // races, nothing written, and the retry voids with + // nothing to reverse. + if errors.Is(err, gl.ErrNotPosted) { + return httpx.InvalidStateTransition("the linked entry was voided while the void ran, so there is nothing left to reverse", + httpx.Blocker("reversal_not_posted", "the linked journal entry is no longer posted; the bill keeps its status until then")) + } + return fmt.Errorf("failed to reverse the vendor invoice entry: %w", err) } - if errors.Is(err, gl.ErrUnbalanced) { - return httpx.InvalidStateTransition("the linked entry does not balance and a void would mirror it into another unbalanced entry", - httpx.Blocker("entry_unbalanced", "the bill's linked journal entry does not balance (the removed sync's one leg shape); the bill stays approved and payable until a later repair item")) + if entry != nil { + reversal = &entry.ID } - return fmt.Errorf("failed to reverse the vendor invoice entry: %w", err) - } - if entry != nil { - reversal = &entry.ID } } if err := s.repo.MarkVoided(ctx, id); err != nil { @@ -591,7 +678,9 @@ func (s *Service) ListVendorInvoices(ctx context.Context, vendorID *uuid.UUID, s // PayVendor records a payment to a vendor and applies it to the bills it // names, in id order, under their locks: only an approved or partial bill // takes money (ADR 0008 7.4). The entry posts through gl.PostEntry inside the -// act's transaction; a posting failure fails the act. +// act's transaction in the bills' currency (a payment naming bills of more +// than one currency is refused, review r2 P2-2); a posting failure fails the +// act. func (s *Service) PayVendor(ctx context.Context, in *PayInput, who Caller) (*APPayment, error) { // The invoices are locked in id order (ADR 0008 section 9 step 5a), so // every contender takes them in one order; the money then applies in the @@ -634,6 +723,23 @@ func (s *Service) PayVendor(ctx context.Context, in *PayInput, who Caller) (*APP locked[id] = inv } + // The payment's entry posts in the bills' currency, so the control + // account ties per currency (the trial balance never sums across two, + // review r2 P2-2): every named bill must share one currency, and a + // payment that spans two is refused naming the bills that differ. + currency := "" + var setter uuid.UUID + for _, id := range lockedIDs { + c := locked[id].Currency + if currency == "" { + currency, setter = c, id + } else if c != currency { + return httpx.InvalidStateTransition("a payment cannot span bills of different currencies", + httpx.Blocker("currency_mismatch", fmt.Sprintf("%s is %s and %s is %s", + locked[setter].Number, currency, locked[id].Number, c))) + } + } + var events []outbox.Event remaining := in.AmountCents for _, id := range in.InvoiceIDs { @@ -680,7 +786,7 @@ func (s *Service) PayVendor(ctx context.Context, in *PayInput, who Caller) (*APP ref := p.ID entry, err := s.gl.PostEntry(ctx, gl.PostingInput{ EntryDate: mustDate(in.PaymentDate), Memo: "Vendor payment " + p.ID.String(), - Source: gl.SourceVendorPmt, SourceRefID: &ref, PostedBy: who.Actor, + Source: gl.SourceVendorPmt, SourceRefID: &ref, Currency: currency, PostedBy: who.Actor, Legs: []gl.Leg{ {AccountCode: gl.AccountCodeAP, Description: "Accounts Payable", Debit: in.AmountCents - remaining}, {AccountCode: gl.AccountCodeCash, Description: "Cash", Credit: in.AmountCents - remaining}, diff --git a/core/internal/ap/wire_test.go b/core/internal/ap/wire_test.go index f195356e..7fd67254 100644 --- a/core/internal/ap/wire_test.go +++ b/core/internal/ap/wire_test.go @@ -657,6 +657,110 @@ func TestWire_ApproveAccountMissing(t *testing.T) { } } +// RULE (review r8 P3-1): a pending bill cannot carry a paid amount. A payment +// applies only to an approved or partial bill, so amount_paid on a pending +// bill is a legacy or hand edited row; migration 106 clamped its amount_open +// below the total (a bill paid past its total sits at zero), and the approve +// posts the whole total while amount_open stands still, so 2010 would hold +// money no bill owes. The approve refuses such a bill; nothing is written. +func TestWire_ApproveRefusesLegacyPaidAmount(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + + // The unknown status shape: a bill the migration made PENDING, paid past + // its total, its open clamped to zero (what 106's LEAST leaves). + id1, r1 := f.enter("V-PAID-1", "1", 40000) // 4.00 + mustExec(t, f.db, `UPDATE vendor_invoices SET amount_paid = 9.00, amount_open = 0.00 WHERE id = $1`, id1) + // The drift shape: paid part of its total, its open clamped to the rest. + id2, r2 := f.enter("V-PAID-2", "1", 100000) // 10.00 + mustExec(t, f.db, `UPDATE vendor_invoices SET amount_paid = 3.00, amount_open = 7.00 WHERE id = $1`, id2) + + for _, bill := range []struct { + id string + rev int64 + }{{id1, r1}, {id2, r2}} { + a := f.approve(bill.id, bill.rev) + if a.status != http.StatusConflict { + t.Fatalf("approve a pending bill with a paid amount = %d, want 409: %s", a.status, a.raw) + } + if code, blockers, _ := errorOf(t, a); code != "invalid_state_transition" || blockers[0] != "has_paid_amount" { + t.Errorf("approve a pending bill with a paid amount = %q %v, want has_paid_amount", code, blockers) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'PENDING' AND revision = 1`, bill.id); n != 1 { + t.Error("a refused approve changed the bill") + } + if n := f.count(`SELECT count(*) FROM gl_journal_entries WHERE source = 'VENDOR_INVOICE' AND source_ref_id = $1`, bill.id); n != 0 { + t.Errorf("%d entries were written for a refused approve", n) + } + if !f.balanced() { + t.Error("an entry does not balance") + } + if got, want := f.controlBalance(), f.openSum(); got != want { + t.Errorf("after the refused approve: control %d, open %d", got, want) + } + } +} + +// RULE (PR 89 r1 P2-1): an omitted currency comes from the currency.default +// setting, read by the create's resolve (USD where the setting is absent). +// Parse leaves the field empty when the request omits it, so the setting is +// read on the route; a currency the request does name is validated as an ISO +// 4217 code of three capital letters. +func TestWire_CreateCurrencyFromSetting(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + // Migration 091 seeds the row ('USD') and the whole base reads it, so the + // test borrows it briefly and puts it back; a parallel package's read + // (the order service's customer facts CROSS JOIN it) must never find it + // gone. + restore := `INSERT INTO system_settings (key, value) VALUES ('currency.default', 'USD') + ON CONFLICT (key) DO UPDATE SET value = 'USD'` + t.Cleanup(func() { mustExec(t, f.db, restore) }) + mustExec(t, f.db, `UPDATE system_settings SET value = 'EUR' WHERE key = 'currency.default'`) + + // An omitted currency takes the setting's value. + body := f.createBody("V-CUR-1", "1", 1000) + r := f.do("POST", "/api/v1/ap/invoices", body) + if r.status != http.StatusCreated { + t.Fatalf("create with the currency omitted = %d: %s", r.status, r.raw) + } + if got := str(t, r.body, "currency"); got != "EUR" { + t.Errorf("an omitted currency = %q, want the setting's EUR", got) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND currency = 'EUR'`, str(t, r.body, "id")); n != 1 { + t.Error("the bill was not stored in the setting's currency") + } + mustExec(t, f.db, restore) + + // A named currency still wins, and a bad one is still refused. + body2 := f.createBody("V-CUR-2", "1", 1000) + body2["currency"] = "CAD" + r2 := f.do("POST", "/api/v1/ap/invoices", body2) + if r2.status != http.StatusCreated { + t.Fatalf("create with a named currency = %d: %s", r2.status, r2.raw) + } + if got := str(t, r2.body, "currency"); got != "CAD" { + t.Errorf("a named currency = %q, want CAD", got) + } + body3 := f.createBody("V-CUR-3", "1", 1000) + body3["currency"] = "usd" + r3 := f.do("POST", "/api/v1/ap/invoices", body3) + if r3.status != http.StatusBadRequest { + t.Fatalf("create with a lowercase currency = %d, want 400: %s", r3.status, r3.raw) + } + if _, _, fields := errorOf(t, r3); len(fields) == 0 || fields[0] != "currency" { + t.Errorf("create with a lowercase currency = %v, want currency", fields) + } + + // A create posts nothing, so the tie stands where it was. + if !f.balanced() { + t.Error("an entry does not balance") + } + if got, want := f.controlBalance(), f.openSum(); got != want { + t.Errorf("after the creates: control %d, open %d", got, want) + } +} + // RULE (ADR 0001 section 4): a client mistake is a 400 naming the field, never // a 500. A payment naming one bill twice would apply it twice (the second pass // reuses the stale locked summary) and only the amount_open CHECK stops it, so @@ -768,14 +872,6 @@ func TestWire_PayAmountIsCents(t *testing.T) { } } -// RULE (ADR 0008 7.4): a bill approved before migration 106 carries its entry -// but no pointer to it; the migration links the newest posted, unreversed, -// balanced VENDOR_INVOICE entry, and the void reverses exactly that entry, so -// the tie (every entry balanced, 2010 equal to what the open bills owe) holds -// for a legacy bill as for a new one. A legacy approved bill whose entry is -// missing is refused rather than voided silently with the credit stranded in -// 2010, and so is one whose linked entry is the base's one leg shape: a void -// would mirror it into another unbalanced entry. // A bill that prices to zero (a zero price, or a positive price that rounds // to 0 cents at the line) approves with no entry, because there is nothing to // post. It owes nothing, so its void has nothing to reverse and goes through: @@ -822,6 +918,19 @@ func TestWire_VoidZeroBill(t *testing.T) { } } +// RULE (ADR 0008 7.4): a bill approved before migration 106 carries its entry +// but no pointer to it; the migration links the newest posted, unreversed, +// balanced VENDOR_INVOICE entry, and the void reverses exactly that entry, so +// the tie (every entry balanced, 2010 equal to what the open bills owe) holds +// for a legacy bill as for a new one. A legacy approved bill that holds a +// posted, unreversed entry it is not linked to is refused rather than voided +// silently with the credit stranded in 2010, and so is one whose linked entry +// is the base's one leg shape: a void would mirror it into another unbalanced +// entry. A legacy bill whose entry never posted holds no credit in 2010 and +// voids with nothing to reverse. So does a pending bill that holds a live +// entry it is not linked to (the removed sync could post before the status +// moved): its void is refused the same way, whatever its total says (review +// r1 P3-2). func TestWire_VoidLegacyApprovedBill(t *testing.T) { testutil.LockOutboxTables(t) f := newFixture(t, testutil.RequireDB(t)) @@ -868,9 +977,11 @@ func TestWire_VoidLegacyApprovedBill(t *testing.T) { t.Error("the void lost the legacy entry link") } - // The unlinked legacy bill: its entry never posted (the base swallowed the - // journal error). The void is refused with a blocker, and nothing is - // written: no reversal, no status change, the tie untouched. + // The unlinked legacy bill whose entry never posted (the base swallowed + // the journal error): nothing of its own stands in 2010, so the void has + // nothing to reverse and goes through (review r8 P3-2 keys the no + // reversal arm on the live entry, not the total). The bill dies owing + // nothing more, and the tie the hand deletion broke is mended. id2, r1b := f.enter("V-LEG-2", "1", 2000) // 20.00 if a := f.approve(id2, r1b); a.status != http.StatusOK { t.Fatal(a.raw) @@ -881,17 +992,23 @@ func TestWire_VoidLegacyApprovedBill(t *testing.T) { mustExec(t, f.db, `DELETE FROM gl_journal_entries WHERE source_ref_id = $1`, id2) before := f.count(`SELECT count(*) FROM gl_journal_entries`) rv := f.void(id2, 2, "legacy bill voided") - if rv.status != http.StatusConflict { - t.Fatalf("void the unlinked legacy bill = %d, want 409: %s", rv.status, rv.raw) + if rv.status != http.StatusOK { + t.Fatalf("void the unlinked legacy bill = %d, want 200: %s", rv.status, rv.raw) } - if code, blockers, _ := errorOf(t, rv); code != "invalid_state_transition" || blockers[0] != "entry_not_linked" { - t.Errorf("void the unlinked legacy bill = %q %v, want entry_not_linked", code, blockers) - } - if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'APPROVED' AND revision = 2`, id2); n != 1 { - t.Error("a refused legacy void changed the bill") + if str(t, rv.body, "status") != "voided" { + t.Errorf("the unlinked legacy bill = %s, want voided", rv.raw) } if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before { - t.Errorf("%d entries were written for a refused void", after-before) + t.Errorf("%d entries were written for a void with nothing to reverse", after-before) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'VOIDED' AND amount_open = 0.00`, id2); n != 1 { + t.Error("the unlinked legacy bill did not die owing nothing") + } + if !f.balanced() { + t.Error("an entry does not balance") + } + if got, want := f.controlBalance(), f.openSum(); got != want || want != 0 { + t.Errorf("after the unlinked legacy void: control %d, open %d, want both 0", got, want) } // The base's real legacy shape: the entry the base's approve posted has @@ -930,8 +1047,7 @@ func TestWire_VoidLegacyApprovedBill(t *testing.T) { // The tie after the refused act: the only unbalanced entry in the fixture // is the seeded legacy one (no act of the fixture wrote an unbalanced // entry), and the refusal moved no money: the control account and the - // open sum stand where the seed left them (the earlier unlinked bill's - // deleted entry is the legacy gap, its credit absent from 2010). + // open sum stand where the seed left them. if n := f.count(`SELECT count(*) FROM gl_journal_entries e JOIN (SELECT journal_entry_id, SUM(debit) d, SUM(credit) c FROM gl_journal_lines GROUP BY journal_entry_id) s ON s.journal_entry_id = e.id @@ -948,6 +1064,1332 @@ func TestWire_VoidLegacyApprovedBill(t *testing.T) { if got, want := f.openSum(), openBefore; got != want { t.Errorf("after the refused one leg void: open %d, want the untouched %d", got, want) } + + // The pending shape: a bill whose entry posted while the status never + // moved off pending (the removed sync could post before the status did), + // holding a live, balanced entry it is not linked to. A plain void would + // strand the credit under a VOIDED bill, so it is refused the same way as + // an approved one (review r1 P3-2). + id4, r1d := f.enter("V-LEG-4", "1", 120000) // 12.00 + if a := f.approve(id4, r1d); a.status != http.StatusOK { + t.Fatal(a.raw) + } + mustExec(t, f.db, `UPDATE vendor_invoices SET status = 'PENDING', gl_entry_id = NULL, + approved_by = NULL, approved_at = NULL WHERE id = $1`, id4) + control4, open4 := f.controlBalance(), f.openSum() + before4 := f.count(`SELECT count(*) FROM gl_journal_entries`) + pv := f.void(id4, 2, "pending bill voided") + if pv.status != http.StatusConflict { + t.Fatalf("void a pending bill holding a live entry = %d, want 409: %s", pv.status, pv.raw) + } + if code, blockers, _ := errorOf(t, pv); code != "invalid_state_transition" || blockers[0] != "entry_not_linked" { + t.Errorf("void a pending bill holding a live entry = %q %v, want entry_not_linked", code, blockers) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'PENDING' AND revision = 2`, id4); n != 1 { + t.Error("a refused void changed the bill") + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before4 { + t.Errorf("%d entries were written for a refused void", after-before4) + } + // The only unbalanced entry in the fixture stays the seeded legacy one + // (the refusal moved no money). + if n := f.count(`SELECT count(*) FROM gl_journal_entries e + JOIN (SELECT journal_entry_id, SUM(debit) d, SUM(credit) c FROM gl_journal_lines GROUP BY journal_entry_id) s + ON s.journal_entry_id = e.id + WHERE e.id IN (SELECT id FROM gl_journal_entries + WHERE source_ref_id IN (SELECT id FROM vendor_invoices WHERE vendor_id = $1) + OR source_ref_id IN (SELECT id FROM ap_payments WHERE vendor_id = $1) + OR reverses_entry_id IN (SELECT gl_entry_id FROM vendor_invoices WHERE vendor_id = $1)) + AND s.d <> s.c`, f.vendor); n != 1 { + t.Errorf("%d unbalanced entries stand, want only the seeded legacy one", n) + } + if got, want := f.controlBalance(), control4; got != want { + t.Errorf("after the refused pending void: control %d, want the untouched %d", got, want) + } + if got, want := f.openSum(), open4; got != want { + t.Errorf("after the refused pending void: open %d, want the untouched %d", got, want) + } +} + +// RULE (review r8 P3-2): the void's no reversal arm is keyed on the bill +// holding no posted, unreversed VENDOR_INVOICE entry, not on its total. A +// legacy bill hand edited to a zero total after its approve still holds the +// credit its entry posted, and a plain void would strand that credit in 2010 +// under a voided bill: such a bill is refused when the entry is not linked, +// and voids with the entry reversed when it is. A bill with no live entry at +// all (a zero total bill that posted nothing) holds no credit and voids with +// nothing to reverse (TestWire_VoidZeroBill and the unlinked case above). +func TestWire_VoidHandEditedZeroTotalBill(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + + // The hand edited shape: approved over the base's one leg credit (which + // migration 106 leaves unlinked), its total then edited to zero and its + // open with it. + id, r1 := f.enter("V-EDIT-1", "1", 100000) // 10.00 + if a := f.approve(id, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + mustExec(t, f.db, `DELETE FROM gl_journal_lines WHERE journal_entry_id IN + (SELECT id FROM gl_journal_entries WHERE source_ref_id = $1) + AND account_id <> (SELECT id FROM gl_accounts WHERE code = '2010')`, id) + mustExec(t, f.db, `UPDATE vendor_invoices SET gl_entry_id = NULL, total = 0.00, amount_open = 0.00 WHERE id = $1`, id) + if f.balanced() { + t.Fatal("the seeded one leg entry balances; the seed did not take") + } + control, open := f.controlBalance(), f.openSum() + before := f.count(`SELECT count(*) FROM gl_journal_entries`) + v := f.void(id, 2, "hand edited bill voided") + if v.status != http.StatusConflict { + t.Fatalf("void a hand edited zero total bill over a live entry = %d, want 409: %s", v.status, v.raw) + } + if code, blockers, _ := errorOf(t, v); code != "invalid_state_transition" || blockers[0] != "entry_not_linked" { + t.Errorf("void a hand edited zero total bill = %q %v, want entry_not_linked", code, blockers) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'APPROVED' AND revision = 2`, id); n != 1 { + t.Error("a refused hand edited void changed the bill") + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before { + t.Errorf("%d entries were written for a refused void", after-before) + } + // The tie after the refused act: the refusal moved no money, and the only + // unbalanced entry is the seeded legacy one. + if got, want := f.controlBalance(), control; got != want { + t.Errorf("after the refused hand edited void: control %d, want the untouched %d", got, want) + } + if got, want := f.openSum(), open; got != want { + t.Errorf("after the refused hand edited void: open %d, want the untouched %d", got, want) + } + + // The linked shape: the same hand edit over a linked, balanced entry + // voids with the entry reversed, so 2010 gives the credit back and the + // bill dies owing nothing. + id2, r2 := f.enter("V-EDIT-2", "1", 100000) // 10.00 + if a := f.approve(id2, r2); a.status != http.StatusOK { + t.Fatal(a.raw) + } + mustExec(t, f.db, `UPDATE vendor_invoices SET total = 0.00, amount_open = 0.00 WHERE id = $1`, id2) + control2, open2 := f.controlBalance(), f.openSum() + v2 := f.void(id2, 2, "hand edited bill voided") + if v2.status != http.StatusOK { + t.Fatalf("void a linked zero total bill = %d, want 200: %s", v2.status, v2.raw) + } + if str(t, v2.body, "status") != "voided" { + t.Errorf("the linked zero total bill = %s, want voided", v2.raw) + } + if n := f.count(`SELECT count(*) FROM gl_journal_entries r JOIN gl_journal_entries o ON o.id = r.reverses_entry_id + WHERE o.source_ref_id = $1`, id2); n != 1 { + t.Errorf("%d reversals of the linked entry, want exactly 1", n) + } + if got, want := f.controlBalance(), control2-1000; got != want { + t.Errorf("after the linked void: control %d, want the reversed %d", got, want) + } + if got, want := f.openSum(), open2; got != want { + t.Errorf("after the linked void: open %d, want the untouched %d", got, want) + } + if n := f.count(`SELECT count(*) FROM gl_journal_entries e + JOIN (SELECT journal_entry_id, SUM(debit) d, SUM(credit) c FROM gl_journal_lines GROUP BY journal_entry_id) s + ON s.journal_entry_id = e.id + WHERE e.id IN (SELECT id FROM gl_journal_entries + WHERE source_ref_id IN (SELECT id FROM vendor_invoices WHERE vendor_id = $1) + OR source_ref_id IN (SELECT id FROM ap_payments WHERE vendor_id = $1) + OR reverses_entry_id IN (SELECT gl_entry_id FROM vendor_invoices WHERE vendor_id = $1)) + AND s.d <> s.c`, f.vendor); n != 1 { + t.Errorf("%d unbalanced entries stand, want only the seeded legacy one", n) + } +} + +// RULE (review r2 P1-1): a reversal counts against a bill's credit only +// while it is POSTED. The trial balance sums POSTED entries alone, so an +// entry whose reversal was voided keeps its credit live in 2010, and a bill +// that holds such an entry must not void with nothing reversed: the void is +// refused entry_not_linked and nothing is written, whatever the reversal row +// says. The shape is reachable with no hand SQL on the bill: the GL void +// route voids any posted entry, a reversal included, and migration 106 +// leaves such a bill unlinked. +func TestWire_VoidRefusesUnlinkedBillWhoseReversalIsVoid(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + id, r1 := f.enter("V-RVOID-1", "1", 500000) // 50.00 + if a := f.approve(id, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry := f.glEntryOf(id) + if rv := f.glReverse(entry); rv.status != http.StatusOK { + t.Fatalf("reverse the entry = %d: %s", rv.status, rv.raw) + } + if vv := f.glVoidEntry(f.glReversalOf(entry)); vv.status != http.StatusOK { + t.Fatalf("void the reversal = %d: %s", vv.status, vv.raw) + } + // The bill is the shape migration 106 leaves unlinked (its link predicate + // reads a reversal row of any status as an undo): the entry's credit + // stands live in 2010 while the bill still owes it. + mustExec(t, f.db, `UPDATE vendor_invoices SET gl_entry_id = NULL WHERE id = $1`, id) + control, open := f.controlBalance(), f.openSum() + if control != 5000 || open != 5000 { + t.Fatalf("control %d, open %d, want both 5000", control, open) + } + before := f.count(`SELECT count(*) FROM gl_journal_entries`) + v := f.void(id, 2, "wrong bill") + if v.status != http.StatusConflict { + t.Fatalf("void a bill whose entry's reversal is void = %d, want 409: %s", v.status, v.raw) + } + if code, blockers, _ := errorOf(t, v); code != "invalid_state_transition" || blockers[0] != "entry_not_linked" { + t.Errorf("void a bill whose entry's reversal is void = %q %v, want entry_not_linked", code, blockers) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'APPROVED' AND revision = 2`, id); n != 1 { + t.Error("a refused void changed the bill") + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before { + t.Errorf("%d entries were written for a refused void", after-before) + } + if n := f.count(`SELECT count(*) FROM gl_journal_entries e + WHERE e.source = 'VENDOR_INVOICE' AND e.source_ref_id = $1 + AND e.status = 'POSTED' AND e.reverses_entry_id IS NULL + AND NOT EXISTS (SELECT 1 FROM gl_journal_entries r WHERE r.reverses_entry_id = e.id AND r.status = 'POSTED')`, id); n != 1 { + t.Errorf("%d live credits stand for the bill, want 1", n) + } + if !f.balanced() { + t.Error("an entry does not balance") + } + if got, want := f.controlBalance(), control; got != want { + t.Errorf("after the refused void: control %d, want the untouched %d", got, want) + } + if got, want := f.openSum(), open; got != want { + t.Errorf("after the refused void: open %d, want the untouched %d", got, want) + } +} + +// RULE (review r2 P2-1): the void's linked arm is keyed on the ledger too, +// never on the pointer alone. A linked entry the GL routes have already +// reversed or voided holds no live credit in 2010, so the bill voids with +// nothing to reverse (never a 500). A linked entry whose reversal was itself +// voided keeps its credit live while the one reversal row the entry may hold +// is already taken, so no new reversal can post: that void is refused with +// the reversal_not_posted blocker and nothing is written. +func TestWire_VoidLinkedEntryAlreadyUndoneByGL(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + + // Reversed by the GL reverse route: the credit is already undone, so the + // void owes the ledger nothing and the open the bill carried closes. + id1, r1 := f.enter("V-GLU-1", "1", 300000) // 30.00 + if a := f.approve(id1, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + if rv := f.glReverse(f.glEntryOf(id1)); rv.status != http.StatusOK { + t.Fatalf("reverse the entry = %d: %s", rv.status, rv.raw) + } + control1, open1 := f.controlBalance(), f.openSum() + if control1 != 0 || open1 != 3000 { + t.Fatalf("control %d, open %d, want 0 and 3000", control1, open1) + } + before1 := f.count(`SELECT count(*) FROM gl_journal_entries`) + v1 := f.void(id1, 2, "wrong bill") + if v1.status != http.StatusOK { + t.Fatalf("void a bill whose linked entry the GL route reversed = %d, want 200: %s", v1.status, v1.raw) + } + if str(t, v1.body, "status") != "voided" { + t.Errorf("the bill = %s, want voided", v1.raw) + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before1 { + t.Errorf("%d entries were written for a void with nothing to reverse", after-before1) + } + if !f.balanced() { + t.Error("an entry does not balance") + } + if got, want := f.controlBalance(), control1; got != want { + t.Errorf("after the void: control %d, want the untouched %d", got, want) + } + if got, want := f.openSum(), open1-3000; got != want { + t.Errorf("after the void: open %d, want %d", got, want) + } + + // Voided by the GL void route: the entry is not POSTED, so it counts + // nowhere in 2010; the bill voids with nothing to reverse. + id2, r2 := f.enter("V-GLU-2", "1", 400000) // 40.00 + if a := f.approve(id2, r2); a.status != http.StatusOK { + t.Fatal(a.raw) + } + if vv := f.glVoidEntry(f.glEntryOf(id2)); vv.status != http.StatusOK { + t.Fatalf("void the entry = %d: %s", vv.status, vv.raw) + } + control2, open2 := f.controlBalance(), f.openSum() + if control2 != 0 || open2 != 4000 { + t.Fatalf("control %d, open %d, want 0 and 4000", control2, open2) + } + before2 := f.count(`SELECT count(*) FROM gl_journal_entries`) + v2 := f.void(id2, 2, "wrong bill") + if v2.status != http.StatusOK { + t.Fatalf("void a bill whose linked entry the GL route voided = %d, want 200: %s", v2.status, v2.raw) + } + if str(t, v2.body, "status") != "voided" { + t.Errorf("the bill = %s, want voided", v2.raw) + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before2 { + t.Errorf("%d entries were written for a void with nothing to reverse", after-before2) + } + if !f.balanced() { + t.Error("an entry does not balance") + } + if got, want := f.controlBalance(), control2; got != want { + t.Errorf("after the void: control %d, want the untouched %d", got, want) + } + if got, want := f.openSum(), open2-4000; got != want { + t.Errorf("after the void: open %d, want %d", got, want) + } + + // The reversal voided by the GL void route: the original credit stands + // live again (a voided reversal undoes nothing), but the entry's one + // reversal row is taken, so the void cannot reverse the credit either: + // refused, nothing written, the tie keeps holding. + id3, r3 := f.enter("V-GLU-3", "1", 500000) // 50.00 + if a := f.approve(id3, r3); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry3 := f.glEntryOf(id3) + if rv := f.glReverse(entry3); rv.status != http.StatusOK { + t.Fatalf("reverse the entry = %d: %s", rv.status, rv.raw) + } + if vv := f.glVoidEntry(f.glReversalOf(entry3)); vv.status != http.StatusOK { + t.Fatalf("void the reversal = %d: %s", vv.status, vv.raw) + } + control3, open3 := f.controlBalance(), f.openSum() + if control3 != 5000 || open3 != 5000 { + t.Fatalf("control %d, open %d, want both 5000", control3, open3) + } + before3 := f.count(`SELECT count(*) FROM gl_journal_entries`) + v3 := f.void(id3, 2, "wrong bill") + if v3.status != http.StatusConflict { + t.Fatalf("void a bill whose linked entry's reversal is void = %d, want 409: %s", v3.status, v3.raw) + } + if code, blockers, _ := errorOf(t, v3); code != "invalid_state_transition" || blockers[0] != "reversal_not_posted" { + t.Errorf("void a bill whose linked entry's reversal is void = %q %v, want reversal_not_posted", code, blockers) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'APPROVED' AND revision = 2`, id3); n != 1 { + t.Error("a refused void changed the bill") + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before3 { + t.Errorf("%d entries were written for a refused void", after-before3) + } + if !f.balanced() { + t.Error("an entry does not balance") + } + if got, want := f.controlBalance(), control3; got != want { + t.Errorf("after the refused void: control %d, want the untouched %d", got, want) + } + if got, want := f.openSum(), open3; got != want { + t.Errorf("after the refused void: open %d, want the untouched %d", got, want) + } + + // The entry voided by the GL void route and its reversal voided with it + // (review r3 P3-3's untested first branch): neither POSTED entry counts + // in 2010, so the chain holds no credit and the void has nothing to + // reverse, whatever the reversal row standing on the entry says. + id4, r4 := f.enter("V-GLU-4", "1", 600000) // 60.00 + if a := f.approve(id4, r4); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry4 := f.glEntryOf(id4) + if rv := f.glReverse(entry4); rv.status != http.StatusOK { + t.Fatalf("reverse the entry = %d: %s", rv.status, rv.raw) + } + if vv := f.glVoidEntry(entry4); vv.status != http.StatusOK { + t.Fatalf("void the entry = %d: %s", vv.status, vv.raw) + } + if vv := f.glVoidEntry(f.glReversalOf(entry4)); vv.status != http.StatusOK { + t.Fatalf("void the reversal = %d: %s", vv.status, vv.raw) + } + control4, open4 := f.controlBalance(), f.openSum() + if control4 != control3 || open4 != open3+6000 { + t.Fatalf("control %d, open %d, want the shape's own %d more owed and no more credit (control %d, open %d)", control4, open4, 6000, control3, open3) + } + before4 := f.count(`SELECT count(*) FROM gl_journal_entries`) + v4 := f.void(id4, 2, "wrong bill") + if v4.status != http.StatusOK { + t.Fatalf("void a bill whose entry and reversal are both void = %d, want 200: %s", v4.status, v4.raw) + } + if str(t, v4.body, "status") != "voided" { + t.Errorf("the bill = %s, want voided", v4.raw) + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before4 { + t.Errorf("%d entries were written for a void with nothing to reverse", after-before4) + } + if !f.balanced() { + t.Error("an entry does not balance") + } + if got, want := f.controlBalance(), control4; got != want { + t.Errorf("after the void: control %d, want the untouched %d", got, want) + } + if got, want := f.openSum(), open4-6000; got != want { + t.Errorf("after the void: open %d, want %d", got, want) + } +} + +// RULE (review r2 P2-2): a payment's entry posts in the bills' currency, not +// the setting's, so 2010 ties per currency; a payment naming bills of more +// than one currency is refused with the currency_mismatch blocker naming the +// bills, and nothing is written. +func TestWire_PaymentCurrencyFollowsTheBills(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + // Migration 091 seeds the row and the whole base reads it, so the test + // borrows it briefly and puts it back; the shared lock every wire test + // holds keeps the parallel packages off it for the window. + restore := `INSERT INTO system_settings (key, value) VALUES ('currency.default', 'USD') + ON CONFLICT (key) DO UPDATE SET value = 'USD'` + t.Cleanup(func() { mustExec(t, f.db, restore) }) + mustExec(t, f.db, `UPDATE system_settings SET value = 'EUR' WHERE key = 'currency.default'`) + + // A bill entered while the setting read EUR, paid after the setting went + // back to USD: the payment entry must post in the bill's EUR. + eur, r1 := f.enter("V-CURPAY-1", "1", 1000000) // 100.00 + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND currency = 'EUR'`, eur); n != 1 { + t.Fatal("the bill was not entered in the setting's EUR") + } + mustExec(t, f.db, restore) + if a := f.approve(eur, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + if got, want := f.controlBalanceIn("EUR"), f.openSumIn("EUR"); got != want || want != 10000 { + t.Fatalf("after the approve: EUR control %d, open %d, want both 10000", got, want) + } + if p := f.pay(10000, eur); p.status != http.StatusCreated { + t.Fatalf("pay the EUR bill = %d: %s", p.status, p.raw) + } + if n := f.count(`SELECT count(*) FROM gl_journal_entries e + JOIN ap_payments p ON p.id = e.source_ref_id + WHERE e.source = 'VENDOR_PAYMENT' AND p.vendor_id = $1 AND e.currency = 'EUR'`, f.vendor); n != 1 { + t.Errorf("%d payment entries post in the bill's EUR, want 1", n) + } + if !f.balanced() { + t.Error("the payment entry does not balance") + } + for _, c := range []string{"EUR", "USD", "CAD"} { + if got, want := f.controlBalanceIn(c), f.openSumIn(c); got != want { + t.Errorf("after the payment: %s control %d, open %d, want equal", c, got, want) + } + } + + // A payment naming bills of two currencies is refused, naming them, and + // nothing is written. + usd, r2 := f.enter("V-CURPAY-2", "1", 500000) // 50.00 + cadBody := f.createBody("V-CURPAY-3", "1", 700000) + cadBody["currency"] = "CAD" + cr := f.do("POST", "/api/v1/ap/invoices", cadBody) + if cr.status != http.StatusCreated { + t.Fatalf("enter the CAD bill = %d: %s", cr.status, cr.raw) + } + cad, r3 := str(t, cr.body, "id"), rev(t, cr) + if a := f.approve(usd, r2); a.status != http.StatusOK { + t.Fatal(a.raw) + } + if got, want := f.controlBalanceIn("USD"), f.openSumIn("USD"); got != want || want != 5000 { + t.Fatalf("after the USD approve: USD control %d, open %d, want both 5000", got, want) + } + if a := f.approve(cad, r3); a.status != http.StatusOK { + t.Fatal(a.raw) + } + if got, want := f.controlBalanceIn("CAD"), f.openSumIn("CAD"); got != want || want != 7000 { + t.Fatalf("after the CAD approve: CAD control %d, open %d, want both 7000", got, want) + } + payments := f.count(`SELECT count(*) FROM ap_payments WHERE vendor_id = $1`, f.vendor) + applications := f.count(`SELECT count(*) FROM ap_payment_applications a JOIN vendor_invoices i ON i.id = a.invoice_id WHERE i.vendor_id = $1`, f.vendor) + entries := f.count(`SELECT count(*) FROM gl_journal_entries`) + mixed := f.pay(12000, usd, cad) + if mixed.status != http.StatusConflict { + t.Fatalf("pay bills of two currencies = %d, want 409: %s", mixed.status, mixed.raw) + } + if code, blockers, _ := errorOf(t, mixed); code != "invalid_state_transition" || blockers[0] != "currency_mismatch" { + t.Errorf("pay bills of two currencies = %q %v, want currency_mismatch", code, blockers) + } + for _, number := range []string{f.number(usd), f.number(cad)} { + if !strings.Contains(string(mixed.raw), number) { + t.Errorf("the refusal does not name %s: %s", number, mixed.raw) + } + } + if after := f.count(`SELECT count(*) FROM ap_payments WHERE vendor_id = $1`, f.vendor); after != payments { + t.Errorf("%d payments were written for a refused payment", after-payments) + } + if after := f.count(`SELECT count(*) FROM ap_payment_applications a JOIN vendor_invoices i ON i.id = a.invoice_id WHERE i.vendor_id = $1`, f.vendor); after != applications { + t.Errorf("%d applications were written for a refused payment", after-applications) + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != entries { + t.Errorf("%d entries were written for a refused payment", after-entries) + } + if !f.balanced() { + t.Error("an entry does not balance") + } + for _, c := range []string{"EUR", "USD", "CAD"} { + if got, want := f.controlBalanceIn(c), f.openSumIn(c); got != want { + t.Errorf("after the refused payment: %s control %d, open %d, want equal", c, got, want) + } + } +} + +// RULE (review r2 P3-1): a double posted bill does not void with its +// duplicate credit live. A live VENDOR_INVOICE entry beside the linked one +// holds a second credit in 2010 that no route could reach under a VOIDED +// bill, so the void is refused entry_not_linked naming the other entry, and +// nothing is written. +func TestWire_VoidRefusesDoublePostedBill(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + id, r1 := f.enter("V-DUP-ENT", "1", 200000) // 20.00 + if a := f.approve(id, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + // The double posted shape: a second POSTED, balanced VENDOR_INVOICE entry + // for the bill, beside the linked one. + entry := f.glEntryOf(id) + dup := uuid.New() + mustExec(t, f.db, `INSERT INTO gl_journal_entries (id, entry_date, memo, source, source_ref_id, status, posted_by, currency) + SELECT $2, entry_date, memo || ' (duplicate)', source, source_ref_id, status, posted_by, currency + FROM gl_journal_entries WHERE id = $1`, entry, dup) + mustExec(t, f.db, `INSERT INTO gl_journal_lines (id, journal_entry_id, account_id, description, debit, credit) + SELECT gen_random_uuid(), $2, account_id, description, debit, credit + FROM gl_journal_lines WHERE journal_entry_id = $1`, entry, dup) + control, open := f.controlBalance(), f.openSum() + if control != 4000 || open != 2000 { + t.Fatalf("control %d, open %d, want 4000 and 2000", control, open) + } + before := f.count(`SELECT count(*) FROM gl_journal_entries`) + v := f.void(id, 2, "wrong bill") + if v.status != http.StatusConflict { + t.Fatalf("void a double posted bill = %d, want 409: %s", v.status, v.raw) + } + if code, blockers, _ := errorOf(t, v); code != "invalid_state_transition" || blockers[0] != "entry_not_linked" { + t.Errorf("void a double posted bill = %q %v, want entry_not_linked", code, blockers) + } + if !strings.Contains(string(v.raw), dup.String()) { + t.Errorf("the refusal does not name the duplicate entry: %s", v.raw) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'APPROVED' AND revision = 2`, id); n != 1 { + t.Error("a refused void changed the bill") + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before { + t.Errorf("%d entries were written for a refused void", after-before) + } + if n := f.count(`SELECT count(*) FROM gl_journal_entries r JOIN gl_journal_entries o ON o.id = r.reverses_entry_id WHERE o.source_ref_id = $1`, id); n != 0 { + t.Errorf("%d reversals were written for a refused void", n) + } + if !f.balanced() { + t.Error("an entry does not balance") + } + if got, want := f.controlBalance(), control; got != want { + t.Errorf("after the refused void: control %d, want the untouched %d", got, want) + } + if got, want := f.openSum(), open; got != want { + t.Errorf("after the refused void: open %d, want the untouched %d", got, want) + } +} + +// RULE (review r4 P1-1): a bill's credit is live exactly when the POSTED +// entries of its VENDOR_INVOICE entry and that entry's whole reversal chain +// leave a net amount on 2010, and every AP act decides on that net, never on +// one step of the chain. A reversal that a later reversal put back (a +// reinstatement) leaves the credit live at any depth, and a chain whose +// POSTED entries cancel holds none, whatever a single reversal row says. +func TestWire_VoidDecidesOnTheWholeReversalChain(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + + shapes := []struct { + name string + depth int + voided []int + }{ + {"CHAIN-D0", 0, nil}, + {"CHAIN-D1", 1, nil}, + {"CHAIN-D2", 2, nil}, // the reinstatement, review r4 P1-1 + {"CHAIN-D3", 3, nil}, + {"CHAIN-D1-R1V", 1, []int{1}}, // review r2 P1-1 through the routes + {"CHAIN-D2-R2V", 2, []int{2}}, + {"CHAIN-D3-R3V", 3, []int{3}}, + {"CHAIN-D2-R1V", 2, []int{1}}, // the reversal voided after its own reversal: twice the credit stands + {"CHAIN-D3-R1V", 3, []int{1}}, + } + + for i, shape := range shapes { + cents := int64(1100 + i*100) + wantNet := netMultiple(shape.depth, shape.voided...) * cents + + // The void of the linked bill. The chain built, the bill keeps its + // link and its APPROVED status, revision 2. + id, entry := buildChain(t, f, shape.name+"-LV", cents, shape.depth, shape.voided...) + if got := chainNet(f, entry); got != wantNet { + t.Fatalf("%s: the chain nets %d, want %d (the shape itself is wrong)", shape.name, got, wantNet) + } + control, open := f.controlBalance(), f.openSum() + before := f.count(`SELECT count(*) FROM gl_journal_entries`) + v := f.void(id, 2, "chain test") + switch { + case wantNet == 0: + // Nothing stands in 2010: the void owes the ledger nothing. + if v.status != http.StatusOK { + t.Errorf("%s: void a bill whose chain nets zero = %d, want 200: %s", shape.name, v.status, v.raw) + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before { + t.Errorf("%s: %d entries were written for a void with nothing to reverse", shape.name, after-before) + } + if got, want := f.controlBalance(), control; got != want { + t.Errorf("%s: after the void: control %d, want the untouched %d", shape.name, got, want) + } + if got, want := f.openSum(), open-cents; got != want { + t.Errorf("%s: after the void: open %d, want %d", shape.name, got, want) + } + if got := chainNet(f, entry); got != 0 { + t.Errorf("%s: after the void: the chain still nets %d on 2010, want 0", shape.name, got) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'VOIDED'`, id); n != 1 { + t.Errorf("%s: the bill is not VOIDED after its void", shape.name) + } + case wantNet != 0 && shape.depth == 0: + // The plain bill: its live entry is reversed whole. + if v.status != http.StatusOK { + t.Errorf("%s: void a bill whose entry stands live = %d, want 200: %s", shape.name, v.status, v.raw) + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before+1 { + t.Errorf("%s: %d entries were written for a reversing void, want 1", shape.name, after-before) + } + if got, want := f.controlBalance(), control-cents; got != want { + t.Errorf("%s: after the void: control %d, want %d", shape.name, got, want) + } + if got, want := f.openSum(), open-cents; got != want { + t.Errorf("%s: after the void: open %d, want %d", shape.name, got, want) + } + default: + // A live chain whose entry already holds a reversal row: no new + // reversal can post, so the void is refused naming the shape. + if v.status != http.StatusConflict { + t.Errorf("%s: void a bill whose chain nets %d with a reversal row taken = %d, want 409: %s", shape.name, wantNet, v.status, v.raw) + } + if v.status == http.StatusConflict { + if code, blockers, _ := errorOf(t, v); code != "invalid_state_transition" || blockers[0] != "reversal_not_posted" { + t.Errorf("%s: the void refusal = %q %v, want reversal_not_posted", shape.name, code, blockers) + } + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before { + t.Errorf("%s: %d entries were written for a refused void", shape.name, after-before) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'APPROVED' AND revision = 2`, id); n != 1 { + t.Errorf("%s: a refused void changed the bill", shape.name) + } + if got, want := f.controlBalance(), control; got != want { + t.Errorf("%s: after the refused void: control %d, want the untouched %d", shape.name, got, want) + } + if got, want := f.openSum(), open; got != want { + t.Errorf("%s: after the refused void: open %d, want the untouched %d", shape.name, got, want) + } + if got := chainNet(f, entry); got != wantNet { + t.Errorf("%s: after the refused void: the chain nets %d, want the live %d under the bill that still owes it", shape.name, got, wantNet) + } + } + if !f.balanced() { + t.Errorf("%s: an entry does not balance after the void", shape.name) + } + + // The void of the same shape with the link nulled (the legacy + // unlinked bill migration 106 leaves behind). + id, entry = buildChain(t, f, shape.name+"-UV", cents, shape.depth, shape.voided...) + if got := chainNet(f, entry); got != wantNet { + t.Fatalf("%s unlinked: the chain nets %d, want %d", shape.name, got, wantNet) + } + mustExec(t, f.db, `UPDATE vendor_invoices SET gl_entry_id = NULL WHERE id = $1`, id) + control, open = f.controlBalance(), f.openSum() + before = f.count(`SELECT count(*) FROM gl_journal_entries`) + v = f.void(id, 2, "chain test") + if wantNet == 0 { + if v.status != http.StatusOK { + t.Errorf("%s unlinked: void a bill whose chain nets zero = %d, want 200: %s", shape.name, v.status, v.raw) + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before { + t.Errorf("%s unlinked: %d entries were written for a void with nothing to reverse", shape.name, after-before) + } + if got, want := f.controlBalance(), control; got != want { + t.Errorf("%s unlinked: after the void: control %d, want the untouched %d", shape.name, got, want) + } + if got, want := f.openSum(), open-cents; got != want { + t.Errorf("%s unlinked: after the void: open %d, want %d", shape.name, got, want) + } + if got := chainNet(f, entry); got != 0 { + t.Errorf("%s unlinked: after the void: the chain still nets %d on 2010, want 0", shape.name, got) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'VOIDED'`, id); n != 1 { + t.Errorf("%s unlinked: the bill is not VOIDED after its void", shape.name) + } + } else { + if v.status != http.StatusConflict { + t.Errorf("%s unlinked: void a bill whose chain nets %d = %d, want 409: %s", shape.name, wantNet, v.status, v.raw) + } + if v.status == http.StatusConflict { + if code, blockers, _ := errorOf(t, v); code != "invalid_state_transition" || blockers[0] != "entry_not_linked" { + t.Errorf("%s unlinked: the void refusal = %q %v, want entry_not_linked", shape.name, code, blockers) + } + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before { + t.Errorf("%s unlinked: %d entries were written for a refused void", shape.name, after-before) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'APPROVED' AND revision = 2`, id); n != 1 { + t.Errorf("%s unlinked: a refused void changed the bill", shape.name) + } + if got, want := f.controlBalance(), control; got != want { + t.Errorf("%s unlinked: after the refused void: control %d, want the untouched %d", shape.name, got, want) + } + if got, want := f.openSum(), open; got != want { + t.Errorf("%s unlinked: after the refused void: open %d, want the untouched %d", shape.name, got, want) + } + if got := chainNet(f, entry); got != wantNet { + t.Errorf("%s unlinked: after the refused void: the chain nets %d, want the live %d under the bill that still owes it", shape.name, got, wantNet) + } + } + if !f.balanced() { + t.Errorf("%s: an entry does not balance after the unlinked void", shape.name) + } + + } +} + +// chainNet is the net 2010 amount of the POSTED entries of the entry's whole +// reversal chain, in cents (the trial balance's own lens). +func chainNet(f *fixture, entry string) int64 { + f.t.Helper() + return f.count(`WITH RECURSIVE c(id) AS (SELECT id FROM gl_journal_entries WHERE id = $1 + UNION SELECT e.id FROM gl_journal_entries e JOIN c ON e.reverses_entry_id = c.id) + SELECT ROUND(COALESCE(SUM(l.credit - l.debit), 0) * 100)::bigint FROM gl_journal_lines l + JOIN gl_journal_entries e ON e.id = l.journal_entry_id AND e.status = 'POSTED' + JOIN gl_accounts a ON a.id = l.account_id AND a.code = '2010' WHERE e.id IN (SELECT id FROM c)`, entry) +} + +// buildChain approves a bill of cents and grows its linked entry's reversal +// chain to depth (1 is the entry's own reversal, 2 that reversal's, and so +// on), then voids the named chain positions through the GL routes. It +// answers the bill id and the linked entry. +func buildChain(t *testing.T, f *fixture, name string, cents int64, depth int, voided ...int) (string, string) { + t.Helper() + id, r1 := f.enter(name, "1", cents*100) + if a := f.approve(id, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry := f.glEntryOf(id) + cur := entry + for i := 1; i <= depth; i++ { + if rv := f.glReverse(cur); rv.status != http.StatusOK { + t.Fatalf("%s: reverse chain depth %d = %d: %s", name, i, rv.status, rv.raw) + } + cur = f.glReversalOf(cur) + } + for _, p := range voided { + e := entry + for i := 0; i < p; i++ { + e = f.glReversalOf(e) + } + if vv := f.glVoidEntry(e); vv.status != http.StatusOK { + t.Fatalf("%s: void chain position %d = %d: %s", name, p, vv.status, vv.raw) + } + } + return id, entry +} + +// netMultiple is the net a chain of the depth leaves on 2010 in whole +// credits: the entry counts +1, its reversal -1, the reversal's reversal +1, +// and a voided position counts 0. +func netMultiple(depth int, voided ...int) int64 { + gone := map[int]bool{} + for _, p := range voided { + gone[p] = true + } + var net int64 + for k := 0; k <= depth; k++ { + if gone[k] { + continue + } + if k%2 == 0 { + net++ + } else { + net-- + } + } + return net +} + +// RULE (review r4 P3-2 on P1-1's chain lens): the approve decides on the +// same liveness as the void, never on the bill's status alone. A bill that +// already holds a live journal entry (a legacy PENDING row whose posting ran +// before the status moved, the link kept or lost by hand, whatever depth its +// reversal chain has reached) must not approve into a second credit: the +// approve is refused with the entry_not_linked blocker naming the live +// entry, and nothing is written. A bill whose chain holds no credit approves +// into the one credit it owes. +func TestWire_ApproveDecidesOnTheWholeReversalChain(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + shapes := []struct { + name string + depth int + voided []int + }{ + {"CHAIN-A-D0", 0, nil}, + {"CHAIN-A-D1", 1, nil}, + {"CHAIN-A-D2", 2, nil}, + {"CHAIN-A-D3", 3, nil}, + {"CHAIN-A-D1-R1V", 1, []int{1}}, + {"CHAIN-A-D2-R2V", 2, []int{2}}, + {"CHAIN-A-D3-R3V", 3, []int{3}}, + {"CHAIN-A-D2-R1V", 2, []int{1}}, + {"CHAIN-A-D3-R1V", 3, []int{1}}, + } + for i, shape := range shapes { + cents := int64(2100 + i*100) + wantNet := netMultiple(shape.depth, shape.voided...) * cents + for _, linked := range []bool{false, true} { + arm := "-AL" + if !linked { + arm = "-AU" + } + id, entry := buildChain(t, f, shape.name+arm, cents, shape.depth, shape.voided...) + if got, want := chainNet(f, entry), wantNet; got != want { + t.Fatalf("%s: the chain nets %d, want %d (the shape itself is wrong)", shape.name, got, want) + } + // The legacy shape: the posting ran before the status moved, so + // the bill sits PENDING over the live chain, the link kept or + // nulled by hand. + mustExec(t, f.db, `UPDATE vendor_invoices SET status = 'PENDING' WHERE id = $1`, id) + if !linked { + mustExec(t, f.db, `UPDATE vendor_invoices SET gl_entry_id = NULL WHERE id = $1`, id) + } + control, open := f.controlBalance(), f.openSum() + before := f.count(`SELECT count(*) FROM gl_journal_entries`) + a := f.approve(id, 2) + if wantNet == 0 { + if a.status != http.StatusOK { + t.Errorf("%s approve linked=%v: approve a bill whose chain nets zero = %d, want 200: %s", shape.name, linked, a.status, a.raw) + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before+1 { + t.Errorf("%s approve linked=%v: %d entries were written for the approve, want 1", shape.name, linked, after-before) + } + if got, want := f.controlBalance(), control+cents; got != want { + t.Errorf("%s approve linked=%v: after the approve: control %d, want %d", shape.name, linked, got, want) + } + if got, want := f.openSum(), open+cents; got != want { + t.Errorf("%s approve linked=%v: after the approve: open %d, want %d", shape.name, linked, got, want) + } + if got := chainNet(f, f.glEntryOf(id)); got != cents { + t.Errorf("%s approve linked=%v: the new entry's chain nets %d, want the %d the bill owes", shape.name, linked, got, cents) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'APPROVED' AND amount_open * 100 = $2`, id, cents); n != 1 { + t.Errorf("%s approve linked=%v: the approved bill does not owe its %d cents", shape.name, linked, cents) + } + } else { + if a.status != http.StatusConflict { + t.Errorf("%s approve linked=%v: approve a bill whose chain nets %d = %d, want 409: %s", shape.name, linked, wantNet, a.status, a.raw) + } + if a.status == http.StatusConflict { + if code, blockers, _ := errorOf(t, a); code != "invalid_state_transition" || blockers[0] != "entry_not_linked" { + t.Errorf("%s approve linked=%v: the refusal = %q %v, want entry_not_linked", shape.name, linked, code, blockers) + } + if !strings.Contains(string(a.raw), "journal entry #") { + t.Errorf("%s approve linked=%v: the refusal does not name the live entry: %s", shape.name, linked, a.raw) + } + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before { + t.Errorf("%s approve linked=%v: %d entries were written for a refused approve", shape.name, linked, after-before) + } + if n := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'PENDING' AND revision = 2`, id); n != 1 { + t.Errorf("%s approve linked=%v: a refused approve changed the bill", shape.name, linked) + } + if got, want := f.controlBalance(), control; got != want { + t.Errorf("%s approve linked=%v: after the refused approve: control %d, want the untouched %d", shape.name, linked, got, want) + } + if got, want := f.openSum(), open; got != want { + t.Errorf("%s approve linked=%v: after the refused approve: open %d, want the untouched %d", shape.name, linked, got, want) + } + if got := chainNet(f, entry); got != wantNet { + t.Errorf("%s approve linked=%v: after the refused approve: the chain nets %d, want the live %d it held", shape.name, linked, got, wantNet) + } + } + if !f.balanced() { + t.Errorf("%s: an entry does not balance after the approve linked=%v", shape.name, linked) + } + } + } +} + +// The reproducer of review r4 P1-1, as the review wrote it: reverse the bill's +// entry, then reverse that reversal (a reinstatement), then void the bill. +// The credit is back in 2010, so the void must refuse, linked or not, and the +// bill must never sit VOIDED with its credit live. +func TestWire_VoidRefusesReinstatedEntry(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + for _, unlink := range []bool{false, true} { + id, r1 := f.enter(fmt.Sprintf("V-REINST-%v", unlink), "1", 160000) // 16.00 + if a := f.approve(id, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry := f.glEntryOf(id) + if rv := f.glReverse(entry); rv.status != http.StatusOK { + t.Fatalf("reverse the entry = %d: %s", rv.status, rv.raw) + } + // Reverse the reversal: the original credit is reinstated in 2010. + if rr := f.glReverse(f.glReversalOf(entry)); rr.status != http.StatusOK { + t.Fatalf("reverse the reversal = %d: %s", rr.status, rr.raw) + } + if unlink { + mustExec(t, f.db, `UPDATE vendor_invoices SET gl_entry_id = NULL WHERE id = $1`, id) + } + live := func() int64 { + return f.count(`WITH RECURSIVE c(id) AS (SELECT id FROM gl_journal_entries WHERE id = $1 + UNION SELECT e.id FROM gl_journal_entries e JOIN c ON e.reverses_entry_id = c.id) + SELECT ROUND(COALESCE(SUM(l.credit - l.debit), 0) * 100)::bigint FROM gl_journal_lines l + JOIN gl_journal_entries e ON e.id = l.journal_entry_id AND e.status = 'POSTED' + JOIN gl_accounts a ON a.id = l.account_id AND a.code = '2010' WHERE e.id IN (SELECT id FROM c)`, entry) + } + if got := live(); got != 1600 { + t.Fatalf("the reinstated credit = %d, want 1600", got) + } + // The ties after the acts that built the shape: every entry balanced, + // and the control account holding exactly what the open bills owe + // (both arms share the fixture, so the tie is control against open). + if !f.balanced() { + t.Fatalf("unlinked=%v: an entry does not balance before the void", unlink) + } + control, open := f.controlBalance(), f.openSum() + if control != open || open == 0 { + t.Fatalf("unlinked=%v: the control %d against the open %d before the void, want the tie over a nonzero amount", unlink, control, open) + } + v := f.void(id, 2, "wrong bill") + if v.status != http.StatusConflict { + t.Errorf("unlinked=%v: void a bill whose credit was reinstated = %d, want 409: %s", unlink, v.status, v.raw) + } + if got := live(); got != 0 && f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'VOIDED'`, id) == 1 { + t.Errorf("unlinked=%v: the bill is VOIDED with %d cents of its credit live in 2010", unlink, got) + } + // The ties after the refused void: nothing moved (the review's r5 + // P3-2: the test asserted the status and the chain net, never the + // ledger sums). + if !f.balanced() { + t.Errorf("unlinked=%v: an entry does not balance after the refused void", unlink) + } + if got := f.controlBalance(); got != control { + t.Errorf("unlinked=%v: the control = %d after the refused void, want the untouched %d", unlink, got, control) + } + if got := f.openSum(); got != open { + t.Errorf("unlinked=%v: the open = %d after the refused void, want the untouched %d", unlink, got, open) + } + if got := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'APPROVED'`, id); got != 1 { + t.Errorf("unlinked=%v: %d APPROVED rows after the refused void, want the bill kept", unlink, got) + } + } +} + +// RULE (review r4 P3-3): a void's reversal posts in the original entry's own +// currency, never in the bill's current one. The entry priced the credit +// into its currency when it posted, so undoing it in another currency leaves +// 2010 holding both halves in different currencies (the trial balance never +// sums across two). The shape is legacy only (no route changes a bill's +// currency), built here by hand like the review's probe. +func TestWire_VoidReversalKeepsTheEntryCurrency(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + id, r1 := f.enter("V-CUR-ENTRY", "1", 250000) // 25.00 + if a := f.approve(id, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry := f.glEntryOf(id) + if got := f.count(`SELECT count(*) FROM gl_journal_entries WHERE id = $1 AND currency = 'USD'`, entry); got != 1 { + t.Fatalf("the entry's currency = %d USD rows, want the setting's USD", got) + } + // The hand edit no route allows: the bill now says CAD over a USD entry. + mustExec(t, f.db, `UPDATE vendor_invoices SET currency = 'CAD' WHERE id = $1`, id) + if got, want := f.controlBalanceIn("USD"), int64(2500); got != want { + t.Fatalf("the USD control = %d, want %d", got, want) + } + if got, want := f.openSumIn("CAD"), int64(2500); got != want { + t.Fatalf("the CAD open = %d, want %d", got, want) + } + v := f.void(id, 2, "wrong currency") + if v.status != http.StatusOK { + t.Fatalf("void the hand edited bill = %d: %s", v.status, v.raw) + } + if n := f.count(`SELECT count(*) FROM gl_journal_entries WHERE reverses_entry_id = $1 AND currency = 'USD'`, entry); n != 1 { + t.Errorf("the void's reversal is not in the entry's USD: %d USD reversals, want 1", n) + } + if !f.balanced() { + t.Error("an entry does not balance") + } + for _, currency := range []string{"USD", "CAD"} { + if got, want := f.controlBalanceIn(currency), int64(0); got != want { + t.Errorf("the %s control = %d, want %d after the void", currency, got, want) + } + if got, want := f.openSumIn(currency), int64(0); got != want { + t.Errorf("the %s open = %d, want %d after the void", currency, got, want) + } + } +} + +// RULE (review r4 P3-1, the race r3 P3-1 read): an AP void racing a GL +// reverse of the linked entry must never answer 500. Whichever side loses +// the insert meets the one reversal index (ux_gl_entry_single_reversal), and +// that loss maps to the same 409 reversal_not_posted as the belt, with +// nothing written: every round leaves exactly one reversal of the entry, the +// bill either voided or still voidable, and the tie holding. +func TestWire_VoidReverseRaceNever500(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + for round := 1; round <= 20; round++ { + id, r1 := f.enter(fmt.Sprintf("V-RACE-%02d", round), "1", 1400000) // 140.00 + if a := f.approve(id, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry := f.glEntryOf(id) + apRes, glRes := make(chan resp, 1), make(chan resp, 1) + start := make(chan struct{}) + go func() { <-start; apRes <- f.void(id, 2, "race round") }() + go func() { <-start; glRes <- f.glReverse(entry) }() + close(start) + ap, gl := <-apRes, <-glRes + if ap.status >= 500 { + t.Errorf("round %d: the AP void answered %d in the race: %s", round, ap.status, ap.raw) + } + if gl.status >= 500 { + t.Errorf("round %d: the GL reverse answered %d in the race: %s", round, gl.status, gl.raw) + } + if n := f.count(`SELECT count(*) FROM gl_journal_entries WHERE reverses_entry_id = $1`, entry); n != 1 { + t.Fatalf("round %d: %d reversals of the entry stand, want exactly one winner", round, n) + } + if voided := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'VOIDED'`, id); voided == 0 { + // The GL side won the reversal: the void refused (or landed too + // late to reverse anything), and its retry must now void with + // nothing to reverse, closing the gap the GL act made. + if ap.status != http.StatusConflict && ap.status != http.StatusOK { + t.Errorf("round %d: the AP void answered %d with the bill not voided: %s", round, ap.status, ap.raw) + } + before := f.count(`SELECT count(*) FROM gl_journal_entries`) + retry := f.void(id, 2, "race round retry") + if retry.status != http.StatusOK { + t.Errorf("round %d: the retry void after the GL reversal = %d, want 200: %s", round, retry.status, retry.raw) + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries`); after != before { + t.Errorf("round %d: %d entries were written by the retry void with nothing to reverse", round, after-before) + } + } + if !f.balanced() { + t.Errorf("round %d: an entry does not balance after the race", round) + } + if got, want := f.controlBalance(), f.openSum(); got != want || got != 0 { + t.Errorf("round %d: control %d against open %d, want both 0 after the round", round, got, want) + } + } +} + +// The reproducer of review r6 P2-1, as the review wrote it: a CAD bill whose +// entry the GL reverse route reversed (the reversal posted in the setting's +// USD before the fix, so the chain held CAD 20 and USD -20) must not void +// 200 over a stranded CAD credit: the liveness net never sums across two +// currencies, so the chain reads live in its CAD half and the void refuses. +func TestWire_VoidRefusesCrossCurrencyReversedEntry(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + body := f.createBody("V-XCUR", "1", 200000) // 20.00 + body["currency"] = "CAD" // the setting stays USD + c := f.do("POST", "/api/v1/ap/invoices", body) + if c.status != http.StatusCreated { + t.Fatal(c.raw) + } + id := str(t, c.body, "id") + if a := f.approve(id, rev(t, c)); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry := f.glEntryOf(id) + if rv := f.glReverse(entry); rv.status != http.StatusOK { + t.Fatalf("reverse the entry = %d: %s", rv.status, rv.raw) + } + v := f.void(id, 2, "wrong bill") + if v.status == http.StatusOK && f.controlBalanceIn("CAD") != f.openSumIn("CAD") { + t.Errorf("void = 200 and the bill is VOIDED with CAD control %d against CAD open %d: the CAD credit is stranded", + f.controlBalanceIn("CAD"), f.openSumIn("CAD")) + } +} + +// RULE (review r6 P2-1, the belt): the chain liveness nets per currency, +// never across two (the trial balance never sums across two currencies), so +// a chain holding two currencies whose nets cancel still reads live in the +// currency that holds the amount, however the chain arose (legacy rows +// included). The shape is built directly in the ledger: the GL route's own +// reversal carries the entry's currency since the route's fix, so no route +// can build it any more. The void, its unlinked arm and the legacy approve +// all decide on the same per currency net. +func TestWire_VoidRefusesCrossCurrencyChainBuiltInTheLedger(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + // handReversal inserts, directly in the ledger, a POSTED reversal of the + // entry in the named currency: the wrong currency reversal shape. + handReversal := func(entry uuid.UUID, currency string) { + t.Helper() + id := uuid.New() + mustExec(t, f.db, `INSERT INTO gl_journal_entries (id, entry_date, memo, source, source_ref_id, status, posted_by, reverses_entry_id, created_at, updated_at, currency) + VALUES ($1, now(), 'hand built wrong currency reversal', 'REVERSAL', $2, 'POSTED', 'test', $2, now(), now(), $3)`, id, entry, currency) + mustExec(t, f.db, `INSERT INTO gl_journal_lines (id, journal_entry_id, account_id, description, debit, credit) + SELECT gen_random_uuid(), $1, l.account_id, 'hand built reversal: ' || l.description, l.credit, l.debit + FROM gl_journal_lines l WHERE l.journal_entry_id = $2`, id, entry) + } + chainNetIn := func(entry, currency string) int64 { + return f.count(`WITH RECURSIVE c(id) AS (SELECT id FROM gl_journal_entries WHERE id = $1 + UNION SELECT e.id FROM gl_journal_entries e JOIN c ON e.reverses_entry_id = c.id) + SELECT ROUND(COALESCE(SUM(l.credit - l.debit), 0) * 100)::bigint FROM gl_journal_lines l + JOIN gl_journal_entries e ON e.id = l.journal_entry_id AND e.status = 'POSTED' + JOIN gl_accounts a ON a.id = l.account_id AND a.code = '2010' AND e.currency = $2 + WHERE e.id IN (SELECT id FROM c)`, entry, currency) + } + + // The linked arm: a CAD bill whose entry's reversal was built in USD. + body := f.createBody("V-XCUR-LEDGER", "1", 200000) // 20.00 + body["currency"] = "CAD" // the setting stays USD + c := f.do("POST", "/api/v1/ap/invoices", body) + if c.status != http.StatusCreated { + t.Fatal(c.raw) + } + id := str(t, c.body, "id") + if a := f.approve(id, rev(t, c)); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry := f.glEntryOf(id) + if got, want := f.controlBalanceIn("CAD"), int64(2000); got != want { + t.Fatalf("the CAD control after the approve = %d, want %d", got, want) + } + if got, want := f.openSumIn("CAD"), int64(2000); got != want { + t.Fatalf("the CAD open after the approve = %d, want %d", got, want) + } + if !f.balanced() { + t.Fatal("an entry does not balance after the approve") + } + handReversal(uuid.MustParse(entry), "USD") + if got, want := chainNetIn(entry, "CAD"), int64(2000); got != want { + t.Fatalf("the chain nets %d in CAD, want %d (the hand edit)", got, want) + } + if got, want := chainNetIn(entry, "USD"), int64(-2000); got != want { + t.Fatalf("the chain nets %d in USD, want %d (the hand edit)", got, want) + } + entries := f.count(`SELECT count(*) FROM gl_journal_entries WHERE id IN (`+fixtureEntries+`)`, f.vendor) + v := f.void(id, 2, "wrong currency chain") + if v.status != http.StatusConflict { + t.Fatalf("void a bill whose chain holds two currencies = %d, want 409: %s", v.status, v.raw) + } + if _, blockers, _ := errorOf(t, v); len(blockers) != 1 || blockers[0] != "reversal_not_posted" { + t.Errorf("the void's blockers = %v, want [reversal_not_posted]", blockers) + } + if got := f.count(`SELECT count(*) FROM gl_journal_entries WHERE id IN (`+fixtureEntries+`)`, f.vendor); got != entries { + t.Errorf("the refused void wrote %d entries, want none", got-entries) + } + if got := f.count(`SELECT count(*) FROM vendor_invoices WHERE id = $1 AND status = 'APPROVED'`, id); got != 1 { + t.Errorf("the bill left APPROVED: %d rows say so", got) + } + if got, want := f.controlBalanceIn("CAD"), int64(2000); got != want { + t.Errorf("the CAD control after the refusal = %d, want %d (nothing written)", got, want) + } + if got, want := f.openSumIn("CAD"), int64(2000); got != want { + t.Errorf("the CAD open after the refusal = %d, want %d (nothing written)", got, want) + } + if got, want := f.controlBalanceIn("USD"), int64(-2000); got != want { + t.Errorf("the USD control after the refusal = %d, want %d (the hand edit's gap, untouched)", got, want) + } + if !f.balanced() { + t.Error("an entry does not balance after the refusal") + } + + // The unlinked arm of the same shape: the liveness read of a bill with no + // linked entry nets per currency too, so the unlink cannot make the CAD + // half of the chain disappear from the decision. + mustExec(t, f.db, `UPDATE vendor_invoices SET gl_entry_id = NULL WHERE id = $1`, id) + v2 := f.void(id, 2, "wrong currency chain, unlinked") + if v2.status != http.StatusConflict { + t.Fatalf("void the unlinked bill = %d, want 409: %s", v2.status, v2.raw) + } + if _, blockers, _ := errorOf(t, v2); len(blockers) != 1 || blockers[0] != "entry_not_linked" { + t.Errorf("the unlinked void's blockers = %v, want [entry_not_linked]", blockers) + } + if got := f.count(`SELECT count(*) FROM gl_journal_entries WHERE id IN (`+fixtureEntries+`)`, f.vendor); got != entries { + t.Errorf("the refused unlinked void wrote %d entries, want none", got-entries) + } + if !f.balanced() { + t.Error("an entry does not balance after the unlinked refusal") + } + + // The legacy approve arm: a PENDING bill whose hand built chain holds two + // currencies is refused, never approved into a second credit. + body2 := f.createBody("V-XCUR-PENDING", "1", 200000) // 20.00 + body2["currency"] = "CAD" + c2 := f.do("POST", "/api/v1/ap/invoices", body2) + if c2.status != http.StatusCreated { + t.Fatal(c2.raw) + } + id2 := str(t, c2.body, "id") + e2 := uuid.New() + mustExec(t, f.db, `INSERT INTO gl_journal_entries (id, entry_date, memo, source, source_ref_id, status, posted_by, created_at, updated_at, currency) + VALUES ($1, now(), 'hand built bill entry', 'VENDOR_INVOICE', $2, 'POSTED', 'test', now(), now(), 'CAD')`, e2, id2) + mustExec(t, f.db, `INSERT INTO gl_journal_lines (id, journal_entry_id, account_id, description, debit, credit) + VALUES (gen_random_uuid(), $1, $2, 'hand built bill line', 20.00, 0), + (gen_random_uuid(), $1, (SELECT id FROM gl_accounts WHERE code = '2010'), 'hand built bill line', 0, 20.00)`, e2, f.expense) + handReversal(e2, "USD") + a2 := f.approve(id2, 1) + if a2.status != http.StatusConflict { + t.Fatalf("approve a pending bill whose chain holds two currencies = %d, want 409: %s", a2.status, a2.raw) + } + if _, blockers, _ := errorOf(t, a2); len(blockers) != 1 || blockers[0] != "entry_not_linked" { + t.Errorf("the approve's blockers = %v, want [entry_not_linked]", blockers) + } + if got, want := f.controlBalanceIn("CAD"), int64(4000); got != want { + t.Errorf("the CAD control after the refused approve = %d, want %d (the two hand shapes, nothing written)", got, want) + } + if got, want := f.openSumIn("CAD"), int64(2000); got != want { + t.Errorf("the CAD open after the refused approve = %d, want %d (the pending bill owes nothing yet)", got, want) + } + if !f.balanced() { + t.Error("an entry does not balance after the refused approve") + } +} + +// RULE (review r6 P3-1): an AP void racing a GL void of the linked entry must +// never answer 500. The void's ledger read can see the entry POSTED while the +// GL void commits beneath it, and the reversal it then books meets an entry +// that is no longer POSTED: that loss maps to the same 409 reversal_not_posted +// as the other lost races, nothing written, and the retry voids with nothing +// to reverse. Fifty rounds, never a 500 on either side. +func TestWire_VoidVoidRaceNever500(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + for round := 1; round <= 50; round++ { + id, r1 := f.enter(fmt.Sprintf("V-VRACE-%02d", round), "1", 1400000) // 140.00 + if a := f.approve(id, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry := f.glEntryOf(id) + controlBefore, openBefore := f.controlBalanceIn("USD"), f.openSumIn("USD") + apRes, glRes := make(chan resp, 1), make(chan resp, 1) + start := make(chan struct{}) + go func() { <-start; apRes <- f.void(id, 2, "race round") }() + go func() { <-start; glRes <- f.glVoidEntry(entry) }() + close(start) + ap, gl := <-apRes, <-glRes + if ap.status >= 500 { + t.Errorf("round %d: the AP void answered %d in the race: %s", round, ap.status, ap.raw) + } + if gl.status >= 500 { + t.Errorf("round %d: the GL void answered %d in the race: %s", round, gl.status, gl.raw) + } + if ap.status != http.StatusOK && ap.status != http.StatusConflict { + t.Errorf("round %d: the AP void answered %d, want 200 or 409: %s", round, ap.status, ap.raw) + } + if ap.status == http.StatusConflict { + _, blockers, _ := errorOf(t, ap) + if len(blockers) != 1 || blockers[0] != "reversal_not_posted" { + t.Errorf("round %d: the lost race's blockers = %v, want [reversal_not_posted]", round, blockers) + } + // Nothing was written by the refused void: the bill is still there + // to void, and the retry voids with nothing to reverse. + before := f.count(`SELECT count(*) FROM gl_journal_entries WHERE id IN (`+fixtureEntries+`)`, f.vendor) + retry := f.void(id, 2, "race round retry") + if retry.status != http.StatusOK { + t.Errorf("round %d: the retry void after the GL void = %d, want 200: %s", round, retry.status, retry.raw) + } + if after := f.count(`SELECT count(*) FROM gl_journal_entries WHERE id IN (`+fixtureEntries+`)`, f.vendor); after != before { + t.Errorf("round %d: %d entries were written by the retry void with nothing to reverse", round, after-before) + } + } + if !f.balanced() { + t.Errorf("round %d: an entry does not balance after the race", round) + } + // The tie after the round is asserted on the refused arm alone, as a + // delta against the round's own snapshot: the GL void that won + // beneath the refusal is the one ledger move (its entry no longer + // sums in the control), the retry then voids the bill with nothing to + // reverse, and the AP act wrote nothing. Where the AP void won, the + // GL void of the entry beside the void's own reversal is the known + // out of scope shape (the GL item), so no whole tie holds there. + if ap.status == http.StatusConflict { + if got, want := f.controlBalanceIn("USD"), controlBefore-14000; got != want { + t.Errorf("round %d: USD control %d after the refused void, want %d (the GL void's gap alone)", round, got, want) + } + if got, want := f.openSumIn("USD"), openBefore-14000; got != want { + t.Errorf("round %d: USD open %d after the refused void and its retry, want %d (the retry closed the round's bill)", round, got, want) + } + } + } +} + +// RULE (review r5 P3-1): a net negative chain (the original entry was +// reversed and then voided, so the reversal's debit stands alone on 2010) is +// refused by words that are true: the liveness net may stand as a credit or +// as a debit, so every refusal names a net amount on accounts payable and +// none says a credit is live where a debit stands. +func TestWire_VoidRefusalWordsMatchANetNegativeChain(t *testing.T) { + testutil.LockOutboxTables(t) + f := newFixture(t, testutil.RequireDB(t)) + id, r1 := f.enter("V-NEGNET", "1", 160000) // 16.00 + if a := f.approve(id, r1); a.status != http.StatusOK { + t.Fatal(a.raw) + } + entry := f.glEntryOf(id) + if rv := f.glReverse(entry); rv.status != http.StatusOK { + t.Fatalf("reverse the entry = %d: %s", rv.status, rv.raw) + } + if vv := f.glVoidEntry(entry); vv.status != http.StatusOK { + t.Fatalf("void the original entry = %d: %s", vv.status, vv.raw) + } + net := func() int64 { + return f.count(`WITH RECURSIVE c(id) AS (SELECT id FROM gl_journal_entries WHERE id = $1 + UNION SELECT e.id FROM gl_journal_entries e JOIN c ON e.reverses_entry_id = c.id) + SELECT ROUND(COALESCE(SUM(l.credit - l.debit), 0) * 100)::bigint FROM gl_journal_lines l + JOIN gl_journal_entries e ON e.id = l.journal_entry_id AND e.status = 'POSTED' + JOIN gl_accounts a ON a.id = l.account_id AND a.code = '2010' WHERE e.id IN (SELECT id FROM c)`, entry) + } + if got := net(); got != -1600 { + t.Fatalf("the chain nets %d, want -1600 (the reversal's debit alone stands)", got) + } + entries := f.count(`SELECT count(*) FROM gl_journal_entries WHERE id IN (`+fixtureEntries+`)`, f.vendor) + words := func(act string, r resp) { + t.Helper() + if r.status != http.StatusConflict { + t.Fatalf("%s = %d, want 409: %s", act, r.status, r.raw) + } + e, ok := r.body["error"].(map[string]any) + if !ok { + t.Fatalf("%s: no error envelope in %s", act, r.raw) + } + msg, _ := e["message"].(string) + if !strings.Contains(msg, "net amount") && !strings.Contains(msg, "nonzero") { + t.Errorf("%s: the refusal's message does not name the net amount that stands: %s", act, msg) + } + if strings.Contains(msg, "credit") { + t.Errorf("%s: the refusal's message says a credit is live while a debit stands: %s", act, msg) + } + } + // The linked arm: the reversal's debit stands, the entry's one reversal + // row is taken, so the void is refused. + words("the linked void", f.void(id, 2, "wrong bill")) + // The unlinked arm: the same chain read through the unlinked predicate. + mustExec(t, f.db, `UPDATE vendor_invoices SET gl_entry_id = NULL WHERE id = $1`, id) + words("the unlinked void", f.void(id, 2, "wrong bill, unlinked")) + // The approve arm: a hand set PENDING bill over the same chain. + mustExec(t, f.db, `UPDATE vendor_invoices SET status = 'PENDING' WHERE id = $1`, id) + words("the approve", f.approve(id, 2)) + // Every refusal wrote nothing and the ties did not move. + if got := f.count(`SELECT count(*) FROM gl_journal_entries WHERE id IN (`+fixtureEntries+`)`, f.vendor); got != entries { + t.Errorf("the three refusals wrote %d entries, want none", got-entries) + } + if got := net(); got != -1600 { + t.Errorf("the chain nets %d after the refusals, want the untouched -1600", got) + } + if !f.balanced() { + t.Error("an entry does not balance after the refusals") + } + if got, want := f.controlBalance(), int64(-1600); got != want { + t.Errorf("the control = %d after the refusals, want the GL acts' %d", got, want) + } + if got, want := f.openSum(), int64(0); got != want { + t.Errorf("the open = %d after the refusals, want %d (the bill is PENDING)", got, want) + } } // RULE (ADR 0008 section 11): the branch wall on every read and write. A diff --git a/core/internal/gl/postentry.go b/core/internal/gl/postentry.go index 46a5c852..03b43c8c 100644 --- a/core/internal/gl/postentry.go +++ b/core/internal/gl/postentry.go @@ -12,6 +12,7 @@ import ( "github.com/gablelbm/gable/pkg/database" "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgconn" ) // Account codes the sales postings name (ADR 0005 section 8.1), beside the @@ -120,14 +121,33 @@ func (s *Service) PostEntry(ctx context.Context, in PostingInput) (*JournalEntry if strings.Contains(err.Error(), closedPeriodMessagePart) { return nil, fmt.Errorf("%w: %v", ErrPeriodClosed, err) } + // A reversal whose entry was reversed by a concurrent act while this + // one ran: the insert meets the one reversal index (a pre-check + // cannot see an uncommitted contender), and the loss maps to + // ErrAlreadyReversed so the caller answers the same refusal as the + // belt, never a 500 (review r4 P3-1). The map is keyed on the index's + // own name, not on the SQLSTATE alone (review r5 P3-3): today + // ux_gl_entry_single_reversal is the only unique index an entry + // insert can meet, and a later one must pass through unmapped rather + // than read as a lost reversal race. + if in.ReversesEntryID != nil { + var pg *pgconn.PgError + if errors.As(err, &pg) && pg.Code == "23505" && pg.ConstraintName == "ux_gl_entry_single_reversal" { + return nil, fmt.Errorf("%w: another reversal of the entry committed first", ErrAlreadyReversed) + } + } return nil, err } return entry, nil } -// ReversalInput names the entry to reverse and the business date and currency -// of the reversal (ADR 0005 section 8.2: a void reverses the document's whole -// entry, dated the void date, in the document's currency). +// ReversalInput names the entry to reverse and the business date of the +// reversal (ADR 0005 section 8.2: a void reverses the document's whole entry, +// dated the void date). The reversal posts in the entry's own currency (the +// entry priced its credit into that currency when it posted, so undoing it +// in another would leave the control account holding both halves in +// different currencies, review r4 P3-3); the input's currency is only the +// fallback for an entry that carries none. type ReversalInput struct { EntryID uuid.UUID EntryDate time.Time @@ -140,6 +160,13 @@ type ReversalInput struct { // points at (the one-reversal-per-entry index would refuse it too). var ErrAlreadyReversed = errors.New("gl: the entry is already reversed") +// ErrNotPosted is the refusal to reverse an entry that is no longer POSTED +// (a concurrent act voided it while this one ran between its own two reads +// of the entry): the callers that race a GL act map it beside +// ErrAlreadyReversed, so the loss answers the same 409, never a 500 (review +// r6 P3-1). +var ErrNotPosted = errors.New("gl: only a POSTED entry can be reversed") + // PostReversal books the net zero reversal of a POSTED entry through the // caller's transaction: the same accounts, debits and credits swapped, source // REVERSAL, reverses_entry_id set, dated the given business date (period @@ -154,7 +181,7 @@ func (s *Service) PostReversal(ctx context.Context, in ReversalInput) (*JournalE return nil, err } if orig.Status != StatusPosted { - return nil, fmt.Errorf("gl: only a POSTED entry can be reversed (current: %s)", orig.Status) + return nil, fmt.Errorf("%w (current: %s)", ErrNotPosted, orig.Status) } if reversed, err := s.repo.IsReversed(ctx, in.EntryID); err != nil { return nil, err @@ -171,8 +198,12 @@ func (s *Service) PostReversal(ctx context.Context, in ReversalInput) (*JournalE Debit: l.Credit, Credit: l.Debit}) } origID := orig.ID + currency := orig.Currency + if currency == "" { + currency = in.Currency + } return s.PostEntry(ctx, PostingInput{ EntryDate: in.EntryDate, Memo: memo, Source: SourceReversal, SourceRefID: &origID, - ReversesEntryID: &origID, Currency: in.Currency, PostedBy: in.PostedBy, Legs: legs, + ReversesEntryID: &origID, Currency: currency, PostedBy: in.PostedBy, Legs: legs, }) } diff --git a/core/internal/gl/postentry_test.go b/core/internal/gl/postentry_test.go index 9c15811a..e2db8691 100644 --- a/core/internal/gl/postentry_test.go +++ b/core/internal/gl/postentry_test.go @@ -18,6 +18,7 @@ import ( "github.com/gablelbm/gable/internal/gl" "github.com/gablelbm/gable/internal/testutil" "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgconn" ) func TestPostEntry(t *testing.T) { @@ -172,3 +173,52 @@ func TestPostReversal(t *testing.T) { t.Errorf("the reversal credits AR %d, %v, want 1100 (the original's debit)", arCredit, err) } } + +// RULE (review r5 P3-3): a unique violation on a reversal insert maps to +// ErrAlreadyReversed only when the violated index is the one reversal index +// ux_gl_entry_single_reversal. Any other unique violation on the insert (a +// later unique index on the table) passes through unmapped, so it is never +// misreported as a lost reversal race. The probe index here stands in for +// such a later index. +func TestPostEntryMapsOnlyTheReversalIndexViolation(t *testing.T) { + db := testutil.RequireDB(t) + ctx := context.Background() + svc := gl.NewService(gl.NewRepository(db), nil, slog.Default()) + ref := uuid.New() + t.Cleanup(func() { + _, _ = db.Pool.Exec(ctx, `DELETE FROM gl_journal_lines WHERE journal_entry_id IN (SELECT id FROM gl_journal_entries WHERE source_ref_id = $1)`, ref) + _, _ = db.Pool.Exec(ctx, `DELETE FROM gl_journal_entries WHERE source_ref_id = $1`, ref) + }) + sentinel := "r5p3 probe " + uuid.NewString() + index := "gv1_probe_ix_" + uuid.NewString()[:8] + if _, err := db.Pool.Exec(ctx, `CREATE UNIQUE INDEX `+index+` ON gl_journal_entries (memo) WHERE memo = '`+sentinel+`'`); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _, _ = db.Pool.Exec(ctx, `DROP INDEX IF EXISTS `+index) }) + entry := func(memo string, reverses *uuid.UUID) gl.PostingInput { + return gl.PostingInput{EntryDate: time.Now(), Memo: memo, Source: gl.SourceInvoice, SourceRefID: &ref, + Currency: "USD", ReversesEntryID: reverses, + Legs: []gl.Leg{{AccountCode: gl.AccountCodeAR, Debit: 1100}, {AccountCode: gl.AccountCodeRevenue, Credit: 1100}}} + } + err := db.RunInTx(ctx, func(ctx context.Context) error { + first, err := svc.PostEntry(ctx, entry(sentinel, nil)) + if err != nil || first == nil { + t.Fatalf("the first entry: %v, %v", first, err) + } + // The second entry collides on the probe index (its memo repeats); + // the entry it reverses holds no reversal row, so the one reversal + // index is not the violated one. + _, err = svc.PostEntry(ctx, entry(sentinel, &first.ID)) + return err + }) + if err == nil { + t.Fatal("the colliding insert posted, want the probe index to refuse it") + } + var pg *pgconn.PgError + if !errors.As(err, &pg) || pg.Code != "23505" || pg.ConstraintName != index { + t.Fatalf("the collision = %v, want a 23505 on the probe index %s", err, index) + } + if errors.Is(err, gl.ErrAlreadyReversed) { + t.Errorf("a violation of %s mapped to ErrAlreadyReversed: only ux_gl_entry_single_reversal may map there, and this loss is not a reversal race", index) + } +} diff --git a/core/internal/gl/repository.go b/core/internal/gl/repository.go index cc43e436..6373e1d2 100644 --- a/core/internal/gl/repository.go +++ b/core/internal/gl/repository.go @@ -225,14 +225,14 @@ func (r *PostgresRepository) CreateJournalEntry(ctx context.Context, entry *Jour func (r *PostgresRepository) GetJournalEntry(ctx context.Context, id uuid.UUID) (*JournalEntry, error) { queryHeader := ` - SELECT id, entry_number, entry_date, memo, source, source_ref_id, status, COALESCE(posted_by, ''), created_at, updated_at + SELECT id, entry_number, entry_date, memo, source, source_ref_id, status, COALESCE(posted_by, ''), created_at, updated_at, currency FROM gl_journal_entries WHERE id = $1 ` var e JournalEntry err := r.db.GetExecutor(ctx).QueryRow(ctx, queryHeader, id).Scan( &e.ID, &e.EntryNumber, &e.EntryDate, &e.Memo, &e.Source, - &e.SourceRefID, &e.Status, &e.PostedBy, &e.CreatedAt, &e.UpdatedAt, + &e.SourceRefID, &e.Status, &e.PostedBy, &e.CreatedAt, &e.UpdatedAt, &e.Currency, ) if err != nil { if err == pgx.ErrNoRows { diff --git a/core/internal/gl/service.go b/core/internal/gl/service.go index 554e0341..3e81fab4 100644 --- a/core/internal/gl/service.go +++ b/core/internal/gl/service.go @@ -188,7 +188,11 @@ func (s *Service) VoidJournalEntry(ctx context.Context, id uuid.UUID) error { // same accounts, debits and credits swapped, linked via reverses_entry_id. // The original stays POSTED (it happened); the reversal cancels its effect. // Enforced once per entry (DB unique index + a pre-check). The reversal is -// dated today, so it is period-checked at post time like any other entry. +// dated today, so it is period-checked at post time like any other entry, and +// it posts in the original's own currency: the entry priced its legs into +// that currency, so a reversal in the setting's code would leave the control +// account holding both halves of one undo in different currencies (review r6 +// P2-1). func (s *Service) ReverseJournalEntry(ctx context.Context, id uuid.UUID, reason string) (uuid.UUID, error) { orig, err := s.repo.GetJournalEntry(ctx, id) if err != nil { @@ -218,6 +222,7 @@ func (s *Service) ReverseJournalEntry(ctx context.Context, id uuid.UUID, reason Status: StatusPosted, PostedBy: "system", ReversesEntryID: &orig.ID, + Currency: orig.Currency, } for _, l := range orig.Lines { rev.Lines = append(rev.Lines, JournalLine{ diff --git a/docs/modules/ap.md b/docs/modules/ap.md index 0e755df5..cf46acf0 100644 --- a/docs/modules/ap.md +++ b/docs/modules/ap.md @@ -12,10 +12,15 @@ the due date, the tax) and the priced lines (the goods or services, the quantity, the unit price, the GL account the line's money lands on). A payment names the bills it applies to (in the order the desk pays them) and locks that order on the database so concurrent payments cannot -deadlock or both spend the same open amount (repeats are refused as -`invoice_ids[i] (appears twice)` by `PayRequest.Parse`, and over-payment -as `exceeds_open` by `PayVendor`). Both acts post one journal entry -inside one transaction: a payment that posts `DR 2010 / CR 1010`, and an +deadlock or both spend the same open amount (a repeated bill id in one +payment's `invoice_ids` is refused as `invoice_ids[i] (appears twice)` +by `PayRequest.Parse`, and over-payment as `exceeds_open` by +`PayVendor`; a repeated payment, the same request sent twice, is a +replay caught by the `Idempotency-Key`, not by these checks; a payment +naming bills of two currencies is refused as `currency_mismatch`). Both +acts post one journal entry +inside one transaction: a payment that posts `DR 2010 / CR 1010` in the +bills' currency, and an approve that builds the entry from the lines and credits the whole total to `2010`. @@ -48,17 +53,41 @@ blocker; a chart of accounts code that is missing is the `entry_unbalanced` blocker. When the bill is wrong, the desk voids it through the same route with -`to: "voided"` and a reason. A pending bill voids with nothing posted; -an approved bill's entry is reversed whole through `gl.PostReversal`; -the bill's `amount_open` drops to zero; a bill that prices to zero -posted nothing and voids with nothing to reverse. A bill with money -applied is refused with `has_payments` (the payments have to be undone -first); a partial bill is refused there before the void reaches the -linked-entry checks. An approved bill with a nonzero total and no -linked entry is refused with `entry_not_linked`, and one whose linked -entry does not balance with `entry_unbalanced`: both stay approved and -payable until a later repair item (no route today links an entry to a -bill). Both refusals write nothing. +`to: "voided"` and a reason. A pending bill voids with nothing posted. +A bill's credit is live exactly when the POSTED entries of its +`VENDOR_INVOICE` entry and that entry's whole reversal chain leave a net +amount on `2010`, and the void decides on that net, never on one step of +the chain: a reversal that a later reversal put back leaves the credit +live at any depth, and a chain whose POSTED entries cancel holds none. +The net is summed per currency, never across two (the trial balance never +sums across two), so a chain holding two currencies whose whole sums +cancel still reads live in the currency that holds the amount, however +the chain arose: legacy rows included. The net may stand as a credit or +as a debit (a chain whose original entry was reversed and then voided +holds the reversal's debit alone), and the refusals name a net amount +standing in accounts payable, never a credit alone. A +bill whose entry holds no live credit (one that prices to zero, or one +the GL routes have already undone, linked or not) voids with nothing to +reverse; a linked live entry with no reversal row is reversed whole +through `gl.PostReversal`, in the entry's own currency. A bill with +money applied is refused with `has_payments` (the payments have to be +undone first); a partial bill is refused there before the void reaches +the linked-entry checks. A bill holding a live entry it is not linked to +is refused with `entry_not_linked` (the duplicate is named when the bill +was posted twice), one whose linked entry does not balance with +`entry_unbalanced`, and one whose linked entry is live while its one +reversal row is already taken (the row is not posted, or the chain was +reinstated behind it, so no new reversal can post) with +`reversal_not_posted`: all stay approved and payable until a later +repair item (no route today links an entry to a bill). Every refusal +writes nothing, and a reversal insert that loses a race on the one +reversal index answers the same `reversal_not_posted` 409, never a 500; +so does a GL void of the linked entry that commits while the void runs +(the entry is no longer POSTED, there is nothing left to reverse, +nothing is written and the retry voids with nothing to reverse). +The approve decides on the same liveness: a pending bill that already +holds a live journal entry is refused `entry_not_linked` naming it, +rather than posting a second credit no bill owes. Payments land as the desk cuts the checks. `POST /ap/payments` takes the vendor, the amount in cents, the method (`CHECK`, `ACH` or @@ -68,8 +97,11 @@ on the same bill) and applies only to bills in `approved` or `partial` (a `pending`, `paid` or `voided` bill is the `invoice_not_approved` blocker). A bill of another vendor is a 400 naming `invoice_ids[i]`, a repeated id a 400 naming its index. More than the named bills still -owe is the `exceeds_open` blocker and writes nothing. The applied sum -posts `DR 2010 / CR 1010` inside the act's transaction. Each settled +owe is the `exceeds_open` blocker and writes nothing. The named bills +must share one currency: a payment spanning two is the +`currency_mismatch` blocker naming the bills that differ, and writes +nothing. The applied sum posts `DR 2010 / CR 1010` in the bills' +currency inside the act's transaction. Each settled bill writes `vendor_invoice.partial` (when more is still owed) or `vendor_invoice.paid` (when this payment closes it). @@ -121,7 +153,7 @@ scope. | `vendor_name` | text | Snapshot of the vendor name. | | `branch_id` | UUID | The branch the bill belongs to. | | `vendor_invoice_number` | text | The vendor's own number, 1 to 64 characters, unique per vendor. | -| `currency` | ISO 4217 | The bill's currency; an omitted currency is `USD`. | +| `currency` | ISO 4217 | The bill's currency; an omitted currency comes from the `currency.default` setting (`USD` where the setting is absent). | | `invoice_date` | date | YYYY-MM-DD. | | `due_date` | date | YYYY-MM-DD. | | `po_id` | UUID, nullable | The purchase order this bill was matched to (FK since migration 106). | @@ -213,7 +245,9 @@ target is a 403 naming `branch_id`), `vendor_invoice_number` (1 to 64 characters, unique per vendor; a duplicate is a 409 naming the field), `invoice_date`, `due_date` (both YYYY-MM-DD), optional `po_id` (an unknown PO is a 400 naming `po_id`), optional `currency` -(an omitted currency is `USD`; a request's currency +(an omitted currency comes from the `currency.default` setting, which +the create's `resolve` reads (`USD` where the setting is absent); a +request's currency must be an ISO 4217 code of three capital letters), `tax_cents`, `notes` (at most 2000 characters) and `lines` (at least one line, the same set of fields as the line schema minus `id`, `position` and @@ -258,25 +292,41 @@ transaction: 1. Lock the bill (`repo.Lock`); check the revision under the lock. 2. Refuse `409 invoice_not_pending` if the bill is not `pending`. -3. Load the lines with their account codes (`repo.LinesWithCodes`). -4. Refuse `409 line_needs_account` if any line's code is empty, naming +3. Refuse `409 has_paid_amount` if `amount_paid_cents` is not zero: a + payment applies only to an `approved` or `partial` bill, so a paid + amount on a `pending` bill is a legacy or hand edited row whose + `amount_open` sits where migration 106 clamped it, below the total; + the approve would post the whole total while `amount_open` stands + still, leaving money in `2010` no bill owes. The row is left for the + operator to mend by hand (a stated limit). +4. Refuse `409 entry_not_linked` if the bill already holds a live + journal entry, naming it by its number and id: a legacy `pending` + row whose posting ran before the status moved (the base's sync could + post first; migration 106 links only `approved` or `partial`, so the + row carries its entry linked by hand or not at all) already leaves a + net amount standing in `2010` (as a credit or as a debit, whichever + way its chain nets), and the approve would post its own entry beside + it. Live is the chain net of the void (below), never one reversal + level; nothing is written. +5. Load the lines with their account codes (`repo.LinesWithCodes`). +6. Refuse `409 line_needs_account` if any line's code is empty, naming the line's position. -5. Build the entry (`postInvoiceEntry`): a debit leg per line on the +7. Build the entry (`postInvoiceEntry`): a debit leg per line on the line's account, the tax spread across the line accounts pro rata to their extensions with the last line taking the remainder - (`tax * line_total_cents / subtotal_cents`, the remainder accumulated - into the last line), the credit to `gl.AccountCodeAP = "2010"` for - the whole total. -6. Post the entry through `gl.PostEntry` in the same transaction. The + (`tax_cents * line_total_cents / subtotal_cents`, the remainder accumulated + into the last line), the credit to `gl.AccountCodeAP = "2010"` for the + whole total. +8. Post the entry through `gl.PostEntry` in the same transaction. The posting can raise `period_closed` (closed fiscal period), `account_missing` (chart of accounts code absent) or `entry_unbalanced` (debits and credits do not balance); each is mapped to a 409 by the module and fails the act (ADR 0008 7.4). -7. Mark the bill approved with the linked `gl_entry_id` and the +9. Mark the bill approved with the linked `gl_entry_id` and the approver (the caller's UUID when the actor is a parseable UUID, else nil for an in-process caller; `s.ApproveInvoice` passes the caller-given approver id). -8. Write the audit row, then the event +10. Write the audit row, then the event (`vendor_invoice.approved`) in the same transaction (ADR 0003 section 2). @@ -296,22 +346,40 @@ transaction: cents applied; the payments have to be undone first. 4. Refuse `409 invalid_state_transition` if the bill's `from` does not allow `voided` (the `partial` and `paid` rows in the map). -5. Refuse `409 entry_not_linked` if the bill is `approved` or `partial`, - has a nonzero `total_cents` and no `gl_entry_id`: a credit may stay - in `2010` with nothing to reverse, so the bill stays approved and - payable (a stated limit). -6. If the bill has a `gl_entry_id`, post the reversal through - `gl.PostReversal` with the void time as `EntryDate` and the reason. - `PostReversal` can raise `period_closed` (mapped to 409) or - `entry_unbalanced` (the removed sync's one leg shape, mapped to 409 - `entry_unbalanced`); the latter also stays approved and payable (a +5. Refuse `409 entry_not_linked` if the bill holds a live + `VENDOR_INVOICE` journal entry it is not linked to (`repo + .HasPostedInvoiceEntry`, the predicate migration 106 links by, made + chain wide): a credit may stay in `2010` with nothing to reverse, + whatever the bill's `total_cents` says now (a hand edit may even have + zeroed the total after the approve), so the bill keeps its status (a stated limit). +6. If the bill has a `gl_entry_id`, read the ledger through `repo + .LinkedEntryLedger` and post the reversal through `gl.PostReversal` + with the void time as `EntryDate` and the reason. `PostReversal` can + raise `period_closed` (mapped to 409) or `entry_unbalanced` (the + removed sync's one leg shape, mapped to 409 `entry_unbalanced`); the + latter also keeps its status (a stated limit). 7. Mark the bill voided; write the audit row, then the event (`vendor_invoice.voided`) in the same transaction. -A bill that prices to zero posted nothing and owes nothing: there is -no `gl_entry_id`, the reversal block is skipped, and the void goes -through with nothing to reverse. +Liveness, for the linked and the unlinked arm alike, is the net the +POSTED entries of the entry's whole reversal chain leave on `2010` (the +trial balance's own lens): an entry whose chain nets nothing holds no +credit and voids with nothing to reverse (a bill that prices to zero +posted nothing, and a legacy bill whose posting was swallowed or whose +entry the GL routes have already undone holds nothing to undo), a live +entry with no reversal row is reversed whole in the entry's own +currency, and a live entry whose one reversal row is already taken (the +row is not posted, or a reversal of a reversal put the credit back +behind it, so no new reversal can post under the one reversal index) is +refused `409 reversal_not_posted` naming the shape, nothing written. The +net may stand as a credit or as a debit, and the refusals name a net +amount standing in accounts payable, never a credit alone. A +reversal insert that loses a race with a concurrent GL reverse meets +the one reversal index and answers the same 409, never a 500; the map +is keyed on that index's own name (`ux_gl_entry_single_reversal`), so a +later unique index on the entry tables is never misreported as a lost +reversal race. ### Pay @@ -374,7 +442,10 @@ posts only when `len(locked) > 0`. The void posts a reversal of the bill's `gl_entry_id` through `gl.PostReversal`, with `EntryDate: time.Now()` (the void time) and the -call's `reason`. The bill's `gl_entry_id` stays set (the link is kept +call's `reason`. The reversal carries the original entry's own currency +(the entry priced its credit into that currency when it posted; the +bill's current currency, which a hand edit may have changed, never +moves it). The bill's `gl_entry_id` stays set (the link is kept for the audit trail). `AccountCodeAP = "2010"` (Accounts Payable) is declared at @@ -434,20 +505,36 @@ only AP route with a payload branch. `POST /ap/payments` carries no outside the caller's wall is a 400 naming `invoice_ids[i]`, `LockForPayment` at `repository.go:307-319`). The branch resolves in three steps (`service.go:263-278`): the payload `branch_id`, the -caller's context branch, or the `default_branch_id` setting. The -`currency` is `Parse`'s `USD` on the route (input.go:117-122); the -`currency.default` setting is not read on the create path. +caller's context branch, or the `default_branch_id` setting. An omitted +`currency` is left empty by `Parse` and the create's `resolve` reads the +`currency.default` setting for it (`USD` where the setting is absent), +on the route and in process alike. ## Stated limits These are limits the module lives with until a later repair item. -- An `approved` or `partial` bill with a nonzero `total_cents` and no - linked `gl_entry_id` is refused a `to: "voided"` with `409 - entry_not_linked`: the credit may stay in `2010` with nothing to - reverse, so the bill stays approved and payable. This is the legacy - unlinked bill (its posting was swallowed, or it holds the removed - sync's one leg credit). +- A bill that holds a live `VENDOR_INVOICE` journal entry it is not + linked to is refused a `to: "voided"` with `409 entry_not_linked`: a + net amount may stay in `2010` with nothing to reverse, so the bill keeps + its status. This is the legacy unlinked bill (its posting was + swallowed, it holds the removed sync's one leg credit, or its total + was hand edited after the approve); live means the entry's whole + reversal chain nets a nonzero amount on `2010` over its POSTED + entries, per currency, as a credit or as a debit. A legacy bill with + no live entry at all voids with nothing to + reverse, owing nothing through the ledger. +- A bill whose linked entry is live while its one reversal row is + already taken (the row is not posted, or a reversal of a reversal put + the credit back behind it) is refused the same void with `409 + reversal_not_posted`: no new reversal can post under the one reversal + index, so the chain must be mended by hand first. The refusal names + the net amount standing in `2010`, as a credit or as a debit, + whichever way the chain nets. +- A pending bill that already holds a live journal entry is refused the + approve with `409 entry_not_linked` naming it: the row already leaves + a net amount standing in `2010`, and mending the ledger by hand comes + before any approve. - A bill linked to an entry that does not balance (the removed sync's one leg shape, debit absent) is refused the same void with `409 entry_unbalanced`. Its reversal would mirror the imbalance into @@ -533,14 +620,15 @@ curl -X POST http://localhost:8080/api/v1/ap/invoices \ -H 'X-Branch-Id: ' \ -H 'Idempotency-Key: 1' \ -H 'Content-Type: application/json' \ - -d @ap-invoice-create.json + -d '{"vendor_id":"","vendor_invoice_number":"INV-1001","invoice_date":"","due_date":"","tax_cents":0,"lines":[{"description":"2x4x8 SPF","quantity":"100","unit_price_ten_thousandths":41250,"gl_account_id":""}]}' ``` -with a body that names the seeded vendor, the seeded branch, a -1 to 64 character `vendor_invoice_number`, the two dates as -YYYY-MM-DD, an ISO 4217 currency, `tax_cents`, optional `notes` and at -least one line (`description`, `quantity` as a decimal string, -`unit_price_ten_thousandths` int64 and the line's `gl_account_id`): +The body names the seeded vendor, a 1 to 64 character +`vendor_invoice_number`, the two dates as YYYY-MM-DD, optional `notes` +and at least one line (`description`, `quantity` as a decimal string, +`unit_price_ten_thousandths` int64 and the line's `gl_account_id`; the +currency may be named and otherwise comes from the `currency.default` +setting). The approve then follows: ``` curl -X POST http://localhost:8080/api/v1/ap/invoices//transitions \ diff --git a/docs/refactor/CONTRACT-CHANGES.md b/docs/refactor/CONTRACT-CHANGES.md index b6471441..69ec7588 100644 --- a/docs/refactor/CONTRACT-CHANGES.md +++ b/docs/refactor/CONTRACT-CHANGES.md @@ -291,3 +291,19 @@ get an entry. | C4-1b | migration 106 `ap_wire_contract` (operational, no route) | `vendor_invoices` had no revision, number, branch, currency or `amount_open`; `po_id` was a bare UUID column with no foreign key; `vendor_invoice_lines` had no `position`, purchase line, product or freight charge links and a scale 2 unit price; nothing was unique | The C4-1b migration of ADR 0008 section 12: `created_at` NOT NULL, `revision`, `number` (`AP-`, a sequence, backfilled in `(created_at, id)` order, the DEFAULT minting through the writer's transaction), `branch_id` (backfilled from the purchase order's branch when that column exists, else the default branch; a defaults trigger serves raw inserts), `currency` (from the `currency.default` setting), a status CHECK on today's values (lowercase spellings uppercased and unknown values become PENDING, each with a notice; cancel-like values and negative totals become VOIDED, see the review fix row above), `po_id` a real foreign key with orphans set null (notice), `UNIQUE (vendor_id, invoice_number)` after duplicates are suffixed `#2`, `#3` in `(created_at, id)` order (notice), `amount_open` (the total less paid, zero on a voided bill; the review fix row above gives the rule now in force), `gl_entry_id`; the lines gain `position` (backfilled in `(created_at, id)` order), `purchase_order_line_id`, `product_id`, a `unit_price` widened to scale 4 and `po_freight_charge_id` (written from package C's freight rule on; today's lines are not linked, the positional pairing being the defect); the keyset indexes. The down reverses each step, keeps the suffixed duplicates, normalized statuses and nulled orphans (each valid old shape data), and refuses while any unit price is finer than cents | | C4-1b | ap module scope and wiring (ADR 0002) | The AP routes sat behind `RequireRole` only, with no branch middleware, no outbox and no audit log; `SyncVendorInvoice` and `SyncVendorPayment` lived in `internal/gl` | The routes sit behind `scoped("admin", "owner", "finance")` with the branch middleware and the branch guard on the create's payload branch; the service carries the outbox writer, the audit logger and the database as transaction runner, wired in serve as every converted module is; the two broken sync functions are deleted from `internal/gl` (their only caller was AP), and `gl` gains the `2010` account code constant beside the sales postings' | | C4-1b | goldens that follow | `ap` created a bill in float dollars and read it back; `ap_payments` scripted the 401 approve, the float create and payments that paid a pending bill | `ap` and `ap_payments` re-recorded on the converted routes: the create in cents, ten thousandths and a decimal string quantity, the 428 transition without a precondition, the approve through `/transitions` and its 409 on a second attempt, the events feed step, the partial and settling payments, the paid bill's further payment refused `invoice_not_approved`, the void with payments refused `has_payments`, the validation 400, the status filters and the pending void | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions` to approved, on a pending bill carrying a paid amount (review fix) | A legacy pending bill with `amount_paid <> 0` (an unknown status migration 106 made PENDING, or a hand edited row) approved whole: the entry credited `2010` with the entire total while `amount_open` stood at the migration's clamp (zero on a bill paid past its total, the rest on a partly paid one), so `2010` held money no bill owed and no route cleared it | The approve refuses it with a 409 `invalid_state_transition` and the `has_paid_amount` blocker naming the paid cents; nothing is written and the row is left for the operator to mend by hand (new data cannot reach the shape: the pay applies only to an approved or partial bill) | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions` to voided, the no reversal arm (review fix) | The arm was keyed on the bill's total: an approved bill with a nonzero total and no `gl_entry_id` was refused `entry_not_linked` (even a legacy bill whose entry never posted or was already reversed, holding no credit in `2010`), while a legacy bill hand edited to a zero total after its approve voided with nothing reversed, stranding its posted credit in `2010` under a VOIDED bill | The arm is keyed on the ledger: a bill that holds a posted, unreversed `VENDOR_INVOICE` entry it is not linked to (a legacy bill whose posting was swallowed, one holding the removed sync's one leg credit, or one whose total was hand edited after its approve, whatever its total says now) is refused `entry_not_linked` and keeps its status; a bill with no live entry (one that prices to zero, or a legacy bill whose entry never posted or was already reversed) holds no credit and voids with nothing to reverse; a linked live entry still voids reversed whole | +| C4-1b | ap: `POST /api/v1/ap/invoices` currency default (review fix, PR 89 r1 P2-1) | `Parse` set an omitted currency to `USD` before the service saw it, so the `currency.default` setting was never read on the route (only an in-process caller with an empty currency reached it) | `Parse` leaves an omitted currency empty and the create's `resolve` reads the `currency.default` setting for it (`USD` where the setting is absent), on the route as in process; a currency the request names is still validated as an ISO 4217 code of three capital letters | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions` to voided, a reversal of any status (review fix, PR 99 r2 P1-1) | The void's live entry predicate counted a reversal row of any status as an undo, but the trial balance sums POSTED entries alone, so a bill whose entry's reversal was voided (the GL void route voids any posted entry, a reversal included) still held its credit live in `2010` while the predicate read the bill as dead: the unlinked bill voided 200 with nothing reversed and the credit stood stranded under a VOIDED bill | A reversal counts only while it is POSTED: the predicate matches the trial balance, and such a bill is refused `entry_not_linked` with nothing written, its credit staying reachable under a bill that still owes it (migration 106's link predicate reads a reversal of any status as an undo too, which leaves such a bill unlinked: the safer side, since the entry's one reversal row is taken and the void could not reverse it anyway) | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions` to voided, the linked arm (review fix, PR 99 r2 P2-1) | With `gl_entry_id` set the void called `gl.PostReversal` with no live check, so a bill whose linked entry the GL routes had already reversed or voided answered 500 `internal_error` (the already reversed and the not POSTED refusals are mapped nowhere), contrary to the arm's own rule that a bill holding no credit voids with nothing to reverse | The linked arm is keyed on the ledger: a linked entry that is not live (not POSTED, or a POSTED reversal points at it, or it was deleted by hand) holds no credit and the bill voids with nothing to reverse; a linked entry that is live while a not POSTED reversal row points at it is refused with the 409 `reversal_not_posted` blocker and nothing is written (its credit stands in `2010`, and the one reversal an entry may hold is already taken, so no new reversal can post); never a 500 | +| C4-1b | ap: `POST /api/v1/ap/payments` entry currency (review fix, PR 99 r2 P2-2) | `PayVendor` posted its entry with no currency, so the entry took the `currency.default` setting's code whatever the bills carried: paying a bill entered under a since changed setting, or a named currency bill, posted the payment in the setting's currency and `2010` drifted per currency (the trial balance never sums across two) | The payment entry posts in the bills' currency, and a payment naming bills of more than one currency is refused with the 409 `currency_mismatch` blocker naming the bills that differ, nothing written; `2010` ties per currency after every act | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions` to voided, a double posted bill (review fix, PR 99 r2 P3-1) | A bill holding a live `VENDOR_INVOICE` entry beside its linked one (the base read the status outside its transaction, so a double click could post it twice; only the newest is linked) voided 200 reversing the linked entry alone, stranding the duplicate's credit in `2010` under a VOIDED bill no route could reach | The void is refused with the 409 `entry_not_linked` blocker naming the other live entry by its number and id; nothing is written and the bill keeps its status until the duplicate is reversed | +| C4-1b | ap: a legacy pending bill carrying a paid amount (review note, PR 99 r1 P3-3) | Such a bill can be neither approved (the `has_paid_amount` refusal) nor voided (the `has_payments` refusal runs first), and no route edits `amount_paid` | Stated limit: the row is parked for the operator to mend by hand, as the blocker text says; a later repair item should give the operator a route (new data cannot reach the shape: the pay applies only to an approved or partial bill) | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions` to voided, liveness over the whole reversal chain (review fix, PR 99 r4 P1-1) | Both liveness predicates (`HasPostedInvoiceEntry` for the unlinked arm, `LinkedEntryLedger` for the linked one) and the double posted check looked one level down the chain: a POSTED reversal pointing at the bill's entry read as dead even when that reversal's own reversal (posted through the public GL reverse route) had put the credit back, so the void answered 200 with nothing reversed and left the bill VOIDED with its full credit live in `2010`, linked or not (a regression this PR introduced: the base answered 500 or `entry_not_linked`, each writing nothing) | A bill's credit is live exactly when the POSTED entries of its VENDOR_INVOICE entry and that entry's whole reversal chain (walked down `reverses_entry_id` through entries of any status, summed on `2010` as the trial balance sums it) leave a net amount; the void, the double posted check and the approve all decide on that net. A live linked entry whose one reversal row is already taken (of any status, so no new reversal can post under the one reversal index) is refused with the 409 `reversal_not_posted` blocker naming the shape, nothing written | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions` to approved, a bill already holding a live entry (review fix, PR 99 r4 P3-2) | A legacy PENDING bill whose posting ran before the status moved (the base's sync could post first; migration 106 links only APPROVED or PARTIAL, so the row carries its entry unlinked) approved whole into a second credit: `2010` held the total twice while the bill owed it once, and a whole payment later left the surplus under a PAID bill (predates the PR; the PR's void made the shape better, not worse) | The approve refuses it with a 409 `invalid_state_transition` and the `entry_not_linked` blocker naming the live entry by its number and id, nothing written, whatever depth the entry's reversal chain has reached (the chain lens above); a bill whose chain holds no credit approves into the one credit it owes | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions` to voided racing a GL reverse (review fix, PR 99 r4 P3-1) | When an AP void and a GL reverse of the same linked entry raced, the loser's reversal insert met the one reversal index `ux_gl_entry_single_reversal` (a pre-check cannot see the uncommitted contender) and the duplicate key error passed up unmapped, so the AP void answered 500 `internal_error` on a money route the PR body says the `ErrAlreadyReversed` belt covers; nothing was written and the ledger held, so a retry voided cleanly | `gl.PostEntry` maps a unique violation on a reversal insert to `ErrAlreadyReversed`, which the void answers with the same 409 `reversal_not_posted` as the belt, nothing written: never a 500, exactly one reversal of the entry stands whichever side wins, and the bill voids or stays voidable | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions` to voided, the reversal's currency (review fix, PR 99 r4 P3-3) | The void handed `gl.PostReversal` the bill's current currency, and the reversal took the currency it was handed, so a bill whose currency had been hand edited after its approve (no route changes it; legacy only) voided with the reversal posted in the bill's new currency: the entry's credit stood in one currency and its reversal in another, so `2010` tied in neither (the trial balance never sums across two) | `gl.PostReversal` posts in the original entry's own currency (`GetJournalEntry` reads it), the input's currency kept only as the fallback for an entry that carries none; both currencies tie at zero after the void | +| C4-1b | gl: `POST /api/v1/gl/journal-entries/{id}/reverse`, the reversal's currency (review fix, PR 99 r6 P2-1) | The route built its reversal with no currency, so the insert took the `currency.default` setting's code instead of the entry's: reversing a CAD entry under a USD setting posted the reversal in USD, and with the AP acts' chain liveness summing the two currencies together the AP void read the chain as dead (CAD 20 and USD -20 net to zero), answered 200, reversed nothing and left the bill VOIDED with its CAD credit live in the CAD ledger (a regression this PR introduced: on the base the same shape answered 500 with nothing written) | The reversal posts in the original entry's own currency (`GetJournalEntry` reads it), so both halves of one undo sit in one currency and `2010` ties per currency after the act | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions`, the chain liveness net per currency (review fix, PR 99 r6 P2-1) | Both liveness reads (`HasPostedInvoiceEntry` and `LiveInvoiceEntryOtherThan` through `chainLive`, and `LinkedEntryLedger` with its own copy of the walk) summed `credit - debit` on `2010` across every currency in the chain at once, so a chain holding two currencies whose whole sums cancel read dead (the trial balance never sums across two, yet liveness did): the AP void and the legacy approve then answered 200 over an amount still standing in one of the two currencies, whatever way the chain had arisen (a legacy reversal posted in the wrong currency included) | The net is computed per currency and the chain is live exactly when any one currency nets nonzero: such a chain is refused (the void with the `reversal_not_posted` blocker on the linked arm, `entry_not_linked` unlinked and on the legacy approve), nothing written, and no mixed currency chain, however it arose, can read as dead | +| C4-1b | ap: `POST /api/v1/ap/invoices/{id}/transitions` to voided racing a GL void of the linked entry (review fix, PR 99 r6 P3-1) | When a GL void of the linked entry committed between the void's ledger read and the reversal's own read of the entry, `gl.PostReversal`'s plain refusal to reverse an entry that is no longer POSTED passed up unmapped, so the AP void answered 500 `internal_error` on a money route (nothing was written by the loser; sequentially the same shape voids 200 with nothing to reverse) | The refusal is the typed `gl.ErrNotPosted`, which the void maps beside `ErrAlreadyReversed` to the same 409 `reversal_not_posted`, nothing written: a fifty round race never answers 500 on either side, the loser's retry voids 200 with nothing to reverse, and every entry stays balanced | +| C4-1b | ap: the liveness refusals' words on a net negative chain (review fix, PR 99 r5 P3-1) | A chain whose original entry was reversed and then voided holds the reversal's debit alone on `2010`, yet every liveness refusal said a credit was live ("still holds its credit live in accounts payable", "could leave its credit in accounts payable", "would post a second credit"): true for the credit shapes, untrue for the debit one (the refusal itself was right and wrote nothing; only the words lied) | The refusals name the net amount standing in accounts payable, as a credit or as a debit, whichever way the chain nets, on the void's linked and unlinked arms and the legacy approve alike, in the code, ap.yaml and the module docs; a test drives the net negative shape through all three arms and asserts the message never says a credit is live | diff --git a/web/packages/api-client/src/schema.d.ts b/web/packages/api-client/src/schema.d.ts index 1b6ee295..1e513733 100644 --- a/web/packages/api-client/src/schema.d.ts +++ b/web/packages/api-client/src/schema.d.ts @@ -519,7 +519,7 @@ export interface paths { put?: never; /** * Approve or void a vendor invoice - * @description Role guard admin, owner or finance, held to the caller's branch wall. Replaces the old /approve route. The body names the target status and carries the revision (or If-Match); a write with neither is 428, a mismatch 409 stale_revision. To approved: only from pending; the entry posts through gl.PostEntry inside the transition's transaction (a debit leg per line on the line's account, the tax spread across the line accounts pro rata, the credit to Accounts Payable), and a posting failure fails the act (a closed period is a 409 blocker period_closed; a line with no account is a 409 blocker line_needs_account; a required chart of accounts code that is missing is a 409 blocker account_missing naming it). To voided: from pending (nothing posted) or approved (the entry is reversed whole; a bill that prices to zero posted nothing and voids with nothing to reverse); a reason is required, a bill with money applied is refused with the has_payments blocker, an approved or partial bill that owes but has no linked journal entry (a legacy bill whose posting was swallowed, or one holding the removed sync's one leg credit) with the entry_not_linked blocker, and a bill whose linked entry does not balance with the entry_unbalanced blocker; nothing is written on either refusal. No route links an entry to a bill yet: such a bill stays approved and payable until a later repair item (a stated limit). + * @description Role guard admin, owner or finance, held to the caller's branch wall. Replaces the old /approve route. The body names the target status and carries the revision (or If-Match); a write with neither is 428, a mismatch 409 stale_revision. To approved: only from pending; the entry posts through gl.PostEntry inside the transition's transaction (a debit leg per line on the line's account, the tax spread across the line accounts pro rata, the credit to Accounts Payable), and a posting failure fails the act (a closed period is a 409 blocker period_closed; a line with no account is a 409 blocker line_needs_account; a required chart of accounts code that is missing is a 409 blocker account_missing naming it; a pending bill that already carries a paid amount, a legacy or hand edited row no payment of its own could write, is a 409 blocker has_paid_amount naming the cents, and the row is left for the operator to mend by hand; a pending bill that already holds a live journal entry, a legacy row whose posting ran before the status moved with its entry linked by hand or not at all, is a 409 blocker entry_not_linked naming the live entry by its number and id, nothing written). To voided: from pending or approved; a bill's credit is live exactly when the POSTED entries of its VENDOR_INVOICE entry and that entry's whole reversal chain leave a net amount on Accounts Payable (a reversal of a reversal puts the credit back, whatever depth the chain has reached; the net is summed per currency, never across two, as the trial balance sums it, so a chain holding two currencies whose whole sums cancel still reads live in the currency that holds the amount, however the chain arose; the net may stand as a credit or as a debit, and the refusals name a net amount standing on Accounts Payable, never a credit alone), and the void decides on that net, never on one step of the chain: a bill whose entry holds no live credit (one that prices to zero, a legacy bill whose posting was swallowed, or an entry the GL routes have already undone, linked or not) voids with nothing to reverse, a linked live entry with no reversal row is reversed whole in the entry's own currency, and a linked live entry whose one reversal row is already taken (the row is not posted, or the chain was reinstated behind it, so no new reversal can post) is refused with the reversal_not_posted blocker naming the shape; a reason is required, a bill with money applied is refused with the has_payments blocker, a bill that holds a live journal entry it is not linked to (a legacy bill whose posting was swallowed, one holding the removed sync's one leg credit, one whose total was hand edited after its approve, or one posted twice, the duplicate named) with the entry_not_linked blocker, and a bill whose linked entry does not balance with the entry_unbalanced blocker; nothing is written on any refusal, and a reversal insert that loses a race with a concurrent GL reverse on the same entry answers the same reversal_not_posted 409, never a 500, and so does a GL void of the linked entry that commits while the void runs (the entry is no longer POSTED, there is nothing left to reverse, nothing is written and the retry voids with nothing to reverse). No route links an entry to a bill yet: such a bill keeps its status until a later repair item (a stated limit). */ post: operations["apInvoiceTransition"]; delete?: never; @@ -543,7 +543,7 @@ export interface paths { put?: never; /** * Pay a vendor - * @description Role guard admin, owner or finance. The request's money is int64 amount_cents (a decimal amount is a 400 naming it; the old float dollars amount is gone); the rest of the shape keeps its today form (uppercase methods) until the payment routes' own conversion. The payment locks the named bills in id order and applies only to bills in approved or partial (a pending, paid or voided bill is a 409 blocker invoice_not_approved; a bill of another vendor is a 400 naming invoice_ids[i]); a repeated id in invoice_ids is a 400 naming its index (appears twice); more than the named bills still owe is a 409 blocker exceeds_open. The entry posts through gl.PostEntry inside the act's transaction (DR Accounts Payable, CR Cash), and a posting failure fails the act (a closed period is a 409 blocker period_closed; a missing chart of accounts code is a 409 blocker account_missing naming it). + * @description Role guard admin, owner or finance. The request's money is int64 amount_cents (a decimal amount is a 400 naming it; the old float dollars amount is gone); the rest of the shape keeps its today form (uppercase methods) until the payment routes' own conversion. The payment locks the named bills in id order and applies only to bills in approved or partial (a pending, paid or voided bill is a 409 blocker invoice_not_approved; a bill of another vendor is a 400 naming invoice_ids[i]); a repeated id in invoice_ids is a 400 naming its index (appears twice); more than the named bills still owe is a 409 blocker exceeds_open. The named bills must share one currency: a payment spanning two is a 409 blocker currency_mismatch naming the bills that differ. The entry posts through gl.PostEntry inside the act's transaction in the bills' currency (DR Accounts Payable, CR Cash), and a posting failure fails the act (a closed period is a 409 blocker period_closed; a missing chart of accounts code is a 409 blocker account_missing naming it). */ post: operations["apPaymentCreate"]; delete?: never; @@ -2142,7 +2142,7 @@ export interface paths { put?: never; /** * Reverse a posted journal entry - * @description Role guard admin or owner. Books a posted, net-zero reversing entry dated today and linked by reverses_entry_id. The optional body reason is appended to the memo; an undecodable or absent body is ignored. Only a POSTED entry that has not already been reversed and has at least 2 lines can be reversed. Every refusal, including an unknown id, is a 400. + * @description Role guard admin or owner. Books a posted, net-zero reversing entry dated today and linked by reverses_entry_id. The optional body reason is appended to the memo; an undecodable or absent body is ignored. Only a POSTED entry that has not already been reversed and has at least 2 lines can be reversed. The reversal posts in the original entry's own currency (the entry priced its legs into that currency, so a reversal in the currency.default setting's code would leave the control account holding both halves of one undo in different currencies). Every refusal, including an unknown id, is a 400. */ post: operations["glJournalEntryReverse"]; delete?: never; @@ -2852,7 +2852,7 @@ export interface paths { put?: never; /** * Write off part or all of an invoice's open amount - * @description Roles admin, owner, finance; a reason is required. Writes an amount of an unpaid or partial invoice off as bad debt: DR 5040 / CR 1020, a WRITE_OFF application and a WRITE_OFF subledger row. A write off that takes the open amount to zero closes the invoice as written_off, else it stays partial. The precondition is the invoice's revision. Refused with 409 exceeds_open_amount, invoice_not_open (invalid_state_transition) or period_closed. Writes invoice.written_off when it closes the invoice, else invoice.partial, and customer.updated. Reversing the application (POST /ar/applications/{id}/reverse) reopens the invoice. + * @description Roles admin, owner, finance; a reason is required. Writes an amount of an unpaid or partial invoice off as bad debt: DR 5040 / CR 1020, a WRITE_OFF application and a WRITE_OFF subledger row. A write off that takes the open amount to zero closes the invoice as written_off, else it stays partial. The precondition is the invoice's revision. Refused with 409 exceeds_open_amount, invalid_state_transition (the invoice is neither unpaid nor partial) or period_closed. Writes invoice.written_off when it closes the invoice, else invoice.partial, and customer.updated. Reversing the application (POST /ar/applications/{id}/reverse) reopens the invoice. */ post: operations["invoiceWriteOff"]; delete?: never;