From 91ed19c2b18248c33fade9c8387bcf838674d268 Mon Sep 17 00:00:00 2001 From: Scott Pouliot Date: Fri, 19 Jun 2026 21:08:16 -0400 Subject: [PATCH] Credit AR for gift-certificate redemptions in balance recompute (audit O7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ApplyGiftCertificate posts DR 2500 Gift Certificate Liability / CR AR, but the AR recompute only subtracted payments and credit-memo applications — so the redemption's 2500 debit was recomputed while its AR credit was not, leaving the Trial Balance out of balance by the total gift-certificate amount redeemed and overstating AR on the Balance Sheet. Subtract GC redemptions from AR in both recompute engines: - FinancialReportService: Balance Sheet (gcRedeemedBs) and Trial Balance (gcRedeemedTb) - LedgerService: AR section (dated rows) and ComputePriorBalanceAsync (prior balance) AR Aging was already correct (uses BalanceDue, which includes GiftCertificateRedeemed). Adds a LedgerService regression test. Build clean; 292 unit tests pass. Co-Authored-By: Claude Opus 4.8 --- docs/ACCOUNTING_AUDIT.md | 62 ++++++++++++++++++- .../Services/FinancialReportService.cs | 14 +++++ .../Services/LedgerService.cs | 26 ++++++++ .../LedgerServiceTests.cs | 36 +++++++++++ 4 files changed, 136 insertions(+), 2 deletions(-) diff --git a/docs/ACCOUNTING_AUDIT.md b/docs/ACCOUNTING_AUDIT.md index 80d9f67..caf0356 100644 --- a/docs/ACCOUNTING_AUDIT.md +++ b/docs/ACCOUNTING_AUDIT.md @@ -130,6 +130,64 @@ explicit `CompanyId` predicate, matching the standing rule in CLAUDE.md. ~19 loo `companyId` source per controller: `_tenantContext.GetCurrentCompanyId()` where available, else the in-scope entity's `CompanyId`, else `_userManager` current user. Build clean; 291 unit tests pass. +--- + +## Whole-system re-audit (2026-06-19) — NEW findings, OPEN + +Root cause shared by all three: the GL is kept two ways — **direct postings** via +`AccountBalanceService.Debit/CreditAsync` (authoritative `Account.CurrentBalance`) and **recomputes** +via `LedgerService` (drives `RecalculateAllAsync`) and `FinancialReportService` (drives the reports). +Every posting type must be mirrored in *both* recompute engines or balances diverge. Several postings +are **not** mirrored, so: (a) "Recalculate Balances" corrupts those accounts, (b) the Balance +Reconciliation report shows false drift, and in one case (O7) the **Trial Balance does not balance**. +O2 (Sales Discounts 4950) was the first instance found and fixed; these are the rest. + +### O6 — Inventory consumption COGS not in either recompute — **MEDIUM** +- `JobsController:3019` and `InventoryController:1855` post **DR COGS / CR Inventory** when an inventory + item with both `CogsAccountId` and `InventoryAccountId` is consumed on a job. Neither `LedgerService` + nor `FinancialReportService` reads this (no JobUsage/InventoryTransaction COGS section). +- **Impact:** COGS understated (profit overstated) and Inventory asset overstated on P&L/Balance Sheet + relative to the posted `CurrentBalance`; a recalc wipes the effect; reconciliation flags drift. TB still + *balances* (both sides omitted). Only active for items with both account mappings set. +- **Fix direction:** add a COGS/Inventory section to both recompute engines driven by `InventoryTransaction` + consumption rows (needs the posted cost captured on the transaction, e.g. `TotalCost`), or — better — + post these via real `JournalEntry` lines so the JE recompute path already covers them. + +### O7 — Gift-certificate redemptions credit AR but AR recompute omitted it — **RESOLVED** +- `InvoicesController.ApplyGiftCertificate:3137` posts **DR 2500 GC Liability / CR AR** (no Payment row, + no `AmountPaid` change — only `invoice.GiftCertificateRedeemed`). The **2500 debit IS** recomputed + (GC redemptions), but the **AR credit is NOT**: AR recompute = `sum(Total) − Payments − CreditMemoApplications` + in both `FinancialReportService` (BS line 358 / TB line 1129) and `LedgerService` (AR section). +- **Impact:** because only one side of the entry is recomputed, the **Trial Balance is out of balance by the + total GC redeemed**; Balance Sheet AR is overstated; recalc corrupts AR. (AR **Aging** is correct — it uses + `BalanceDue`, which includes `GiftCertificateRedeemed`.) Active for any company that redeems GCs on invoices. +- **Fix applied:** GC redemptions now subtracted from AR in both recompute engines — `FinancialReportService` + Balance Sheet (`gcRedeemedBs`) and Trial Balance (`gcRedeemedTb`), and `LedgerService` AR section + prior + balance. Mirrors the `cmApplied` treatment. Regression test + `GetAccountLedgerAsync_AR_GiftCertificateRedemption_CreditsAccountsReceivable`. Build clean; 292 tests pass. + +### O8 — Written-off invoices misstated in AR recompute — **MEDIUM** +- `InvoicesController.WriteOff:1689` posts **DR Bad Debt / CR AR** for the balance due and marks the invoice + `WrittenOff`. In the recompute, written-off invoices' `Total` is still counted as an AR **debit** + (`arDebits` only excludes Draft/Voided), while `FinancialReportService` **excludes** their payments from AR + credits (Status != WrittenOff filter) and neither engine models the write-off CR — so a written-off invoice + shows its full `Total` as open AR on recompute. `LedgerService` and `FinancialReportService` also disagree + (Ledger does not apply the WrittenOff payment filter). +- **Impact:** AR overstated by written-off balances on recompute/reports; recalc corrupts AR; the two engines + disagree. Active when invoices are written off. +- **Fix direction:** treat `WrittenOff` consistently — exclude written-off invoices' `Total` from `arDebits` + (or model the write-off CR) and align the two engines. + +**Architectural note:** these keep recurring because reports re-derive balances from source documents in +parallel with the live postings. The durable fix is to make **every** financial event post `JournalEntry` +lines and drive all reports/recompute from those lines alone (single source of truth), retiring the +per-document re-derivation. Worth considering before the ledger grows further. + ## Status -All audit findings **O1–O4 are resolved** on `dev`. Original audit numbering #1–3/#5/#6/#8 remains -unrecoverable (see top). The accounting batch has **not** been merged to `master`/production yet. +Findings **O1–O5, O7, + the read-path sweep are resolved** on `dev`. **O6 and O8 remain OPEN** — both need the +JournalEntry approach (post a balanced JE at consumption/write-off time) plus a one-time historical backfill, +because neither event leaves a reliably recompute-able marker (O6: no flag distinguishing COGS-posting +reductions, and posted cost uses AverageCost while the transaction stores UnitCost; O8: the write-off's +bad-debt account and amount aren't stored as a ledger record, so mirroring only the AR side would re-break the +trial balance). +Original audit numbering #1–3/#5/#6/#8 remains unrecoverable (see top). Nothing merged to `master` yet. diff --git a/src/PowderCoating.Infrastructure/Services/FinancialReportService.cs b/src/PowderCoating.Infrastructure/Services/FinancialReportService.cs index 0ce8cce..81dce42 100644 --- a/src/PowderCoating.Infrastructure/Services/FinancialReportService.cs +++ b/src/PowderCoating.Infrastructure/Services/FinancialReportService.cs @@ -365,6 +365,13 @@ public class FinancialReportService : IFinancialReportService .Where(a => a.CompanyId == companyId && a.AppliedDate <= asOfEnd && a.Invoice.Status != InvoiceStatus.Voided) .SumAsync(a => (decimal?)a.AmountApplied) ?? 0; arCredits += cmAppliedBs; + // Gift-certificate redemptions also credit AR (ApplyGiftCertificate posts DR 2500 / CR AR). + // Mirror the posting here so AR is not overstated and the entry's two sides stay balanced. + var gcRedeemedBs = await _context.GiftCertificateRedemptions + .Where(r => r.CompanyId == companyId && !r.IsDeleted && r.RedeemedDate <= asOfEnd + && r.Invoice.Status != InvoiceStatus.Voided) + .SumAsync(r => (decimal?)r.AmountRedeemed) ?? 0; + arCredits += gcRedeemedBs; // Customer Credits (2350): a credit memo books DR Sales Discounts / CR Customer Credits on issue, // then DR Customer Credits / CR AR on apply. Contra-revenue (retained earnings) = issued amount @@ -1132,6 +1139,13 @@ public class FinancialReportService : IFinancialReportService && p.Invoice.Status != InvoiceStatus.WrittenOff) .SumAsync(p => (decimal?)p.Amount) ?? 0m; arTotalCredits += cmApplied; // credit memo applications reduce AR balance + // Gift-certificate redemptions credit AR too (DR 2500 / CR AR). Without this the redemption's + // 2500 debit is recomputed but its AR credit is not, leaving the trial balance out of balance. + var gcRedeemedTb = await _context.GiftCertificateRedemptions + .Where(r => r.CompanyId == companyId && !r.IsDeleted && r.RedeemedDate <= asOfEnd + && r.Invoice.Status != InvoiceStatus.Voided) + .SumAsync(r => (decimal?)r.AmountRedeemed) ?? 0m; + arTotalCredits += gcRedeemedTb; // Cash refunds reverse the sale: revenue portion → DR Sales Returns (4960), tax portion → // DR Sales Tax Payable (relieves the liability), cash → CR bank (refundsByAcct below). They no diff --git a/src/PowderCoating.Infrastructure/Services/LedgerService.cs b/src/PowderCoating.Infrastructure/Services/LedgerService.cs index 097edca..cc8d7ef 100644 --- a/src/PowderCoating.Infrastructure/Services/LedgerService.cs +++ b/src/PowderCoating.Infrastructure/Services/LedgerService.cs @@ -350,6 +350,27 @@ public class LedgerService : ILedgerService LinkId = cm.InvoiceId }); + // Gift-certificate redemptions reduce open AR (CREDIT) — ApplyGiftCertificate posts DR 2500 / CR AR. + var arGcRedemptions = await _context.GiftCertificateRedemptions + .Include(r => r.Invoice) + .Include(r => r.GiftCertificate) + .Where(r => !r.IsDeleted && r.RedeemedDate >= fromDate && r.RedeemedDate <= toDate + && r.Invoice.Status != InvoiceStatus.Voided) + .ToListAsync(); + + foreach (var r in arGcRedemptions) + entries.Add(new LedgerEntryDto + { + Date = r.RedeemedDate, + Reference = r.GiftCertificate?.CertificateCode ?? $"GC-{r.GiftCertificateId}", + Source = "Gift Certificate", + Description = $"GC redeemed on {r.Invoice?.InvoiceNumber}", + Debit = 0, + Credit = r.AmountRedeemed, + LinkController = "Invoices", + LinkId = r.InvoiceId + }); + // NOTE: cash refunds no longer touch AR. Under the "reverse the sale" model they debit // Sales Returns + Sales Tax Payable and credit the bank (see section 5b above). } @@ -751,6 +772,11 @@ public class LedgerService : ILedgerService .Where(a => a.AppliedDate < beforeDate && a.Invoice.Status != InvoiceStatus.Voided) .SumAsync(a => (decimal?)a.AmountApplied) ?? 0; + // Gift-certificate redemptions credit AR (DR 2500 / CR AR), same as in GetAccountLedgerAsync. + credits += await _context.GiftCertificateRedemptions + .Where(r => !r.IsDeleted && r.RedeemedDate < beforeDate && r.Invoice.Status != InvoiceStatus.Voided) + .SumAsync(r => (decimal?)r.AmountRedeemed) ?? 0; + // NOTE: cash refunds no longer debit AR — they reverse the sale (Sales Returns + Sales Tax), // handled in section 5b above. } diff --git a/tests/PowderCoating.UnitTests/LedgerServiceTests.cs b/tests/PowderCoating.UnitTests/LedgerServiceTests.cs index 96aac77..c3c3b28 100644 --- a/tests/PowderCoating.UnitTests/LedgerServiceTests.cs +++ b/tests/PowderCoating.UnitTests/LedgerServiceTests.cs @@ -619,6 +619,42 @@ public class LedgerServiceTests Assert.Equal(-150m, ledger.ClosingBalance); } + // ── Gift-certificate redemptions credit AR (DR 2500 / CR AR) — must appear in the AR ── + // ── recompute or the trial balance is left out of balance by the redeemed amount (O7). ── + + [Fact] + public async Task GetAccountLedgerAsync_AR_GiftCertificateRedemption_CreditsAccountsReceivable() + { + await using var context = CreateContext(); + SeedAccount(context, id: 1, AccountSubType.AccountsReceivable); + SeedCustomer(context, id: 1); + context.Invoices.Add(new Invoice + { + Id = 10, CompanyId = 1, InvoiceNumber = "INV-GC", CustomerId = 1, + Status = InvoiceStatus.Sent, InvoiceDate = InPeriod, Total = 200m + }); + context.GiftCertificates.Add(new GiftCertificate + { + Id = 5, CompanyId = 1, CertificateCode = "GC-5", OriginalAmount = 50m, RedeemedAmount = 50m, + IssueDate = InPeriod, Status = GiftCertificateStatus.FullyRedeemed + }); + context.GiftCertificateRedemptions.Add(new GiftCertificateRedemption + { + Id = 1, CompanyId = 1, GiftCertificateId = 5, InvoiceId = 10, + AmountRedeemed = 50m, RedeemedDate = InPeriod + }); + await context.SaveChangesAsync(); + + var ledger = await CreateService(context).GetAccountLedgerAsync(1, PeriodStart, PeriodEnd); + + var invoiceEntry = ledger!.Entries.Single(e => e.Source == "Invoice"); + var gcEntry = ledger.Entries.Single(e => e.Source == "Gift Certificate"); + Assert.Equal(200m, invoiceEntry.Debit); + Assert.Equal(50m, gcEntry.Credit); + Assert.Equal(0m, gcEntry.Debit); + Assert.Equal(150m, ledger.ClosingBalance); // debit-normal AR: 200 invoiced − 50 redeemed + } + private static LedgerService CreateService(ApplicationDbContext context) => new LedgerService(context, Mock.Of>());