diff --git a/docs/ACCOUNTING_AUDIT.md b/docs/ACCOUNTING_AUDIT.md index cfdc900..d433aea 100644 --- a/docs/ACCOUNTING_AUDIT.md +++ b/docs/ACCOUNTING_AUDIT.md @@ -209,6 +209,38 @@ does exactly this, which is why it was the cleanest. Worth considering before th item's `CogsAccountId` + `InventoryAccountId` — that would record the cost twice (once at purchase, once at consumption). Keep item COGS/Inventory account mappings empty under the expense-at-purchase policy. +#### O9 update (2026-06-20) — default GL accounts feature widened this surface +- The original O9 note assumed item `CogsAccountId`/`InventoryAccountId` are "set only via CSV import, never + by normal item creation." **That assumption no longer holds.** A new **Default Accounts** feature + (Chart of Accounts → "Set Defaults", stored on `CompanyPreferences.Default{Revenue,Cogs,Inventory}AccountId`) + pre-fills these fields on **normal** Inventory/Catalog item creation. A company that sets *both* a default + COGS and a default Inventory account will have new items post `DR COGS / CR Inventory` on consumption — + i.e. it opts the shop into the perpetual path. +- **Why it's still safe:** the defaults are **null by default**, so nothing changes until a company + deliberately sets them. The footgun (double-counting under expense-at-purchase) is surfaced with an inline + warning on the Default Accounts card and in the Settings help article. Posting and balance-recompute both + read the item's *stored* accounts, so there is **no recompute drift** — verified during the 2026-06-20 audit. +- **Revenue default fallback:** invoice lines now fall back to `DefaultRevenueAccountId` (active only), then + to account 4000 (`InvoicesController.Create`). Resolved at invoice-create time and stored on the + `InvoiceItem`, so recompute stays consistent. + +### 2026-06-20 audit — dropdown sub-type→type broadening + deposit account picker +Reviewed after broadening account dropdowns from sub-type to parent `AccountType` and adding a user-selectable +deposit account. **No ledger-drift bugs found.** Notes: +- **Deposit account picker ↔ recompute:** consistent. Live posting debits the chosen `DepositAccountId`, and + `LedgerService` reproduces the debit by `DepositAccountId == accountId` (lines ~78/724). Picking a non-default + deposit account recomputes correctly. +- **Bank / "pay-from" / bank-rec pickers** now list all Asset + Liability accounts with **no server-side type + guard** — a user could pick a nonsensical source (e.g. A/R). Postings stay sign-correct + (`AccountBalanceService` keys sign off `AccountSubType`), and this is the intended "trust the operator" + tradeoff, but there is no longer a guardrail. Accepted; noted here for visibility. +- **Latent deposit imbalance — RESOLVED (2026-06-20):** a deposit saved with a null `DepositAccountId` posted + `CR 2300` with no offsetting debit → unbalanced. `DepositsController.Record` now blocks recording when the + `2300` Customer Deposits account exists but no deposit/bank account resolves (user must pick one). When `2300` + doesn't exist (company not using accounting), no GL posts at all, so the deposit is still allowed through. +- **Pre-existing type/sub-type mismatch risk:** account create does not enforce a valid type↔sub-type pairing, + and sign convention keys off sub-type — a mis-paired account would post with the wrong sign. Backlog item. + ## Status **All findings O1–O9 + the read-path sweep are resolved** on `dev` (O9 by policy decision — expense at purchase — needing no code change). The optional structural follow-up is the JournalEntry single-source diff --git a/src/PowderCoating.Web/Controllers/DepositsController.cs b/src/PowderCoating.Web/Controllers/DepositsController.cs index b299fc8..10ced5d 100644 --- a/src/PowderCoating.Web/Controllers/DepositsController.cs +++ b/src/PowderCoating.Web/Controllers/DepositsController.cs @@ -97,6 +97,20 @@ public class DepositsController : Controller } depositAcctId ??= await GetCheckingAccountIdAsync(currentUser.CompanyId); + // Guard against an unbalanced GL posting: this deposit credits the Customer Deposits + // liability (2300). If that account exists but we have no bank/asset account to debit, + // the entry would be one-sided. Block it so the user picks a deposit account first. + // (When 2300 doesn't exist — e.g. a company not using accounting — no GL posts at all, + // so a missing bank account is harmless and the deposit is allowed through.) + var custDepositsAcctId = await GetCustomerDepositsAccountIdAsync(currentUser.CompanyId); + if (custDepositsAcctId != null && depositAcctId == null) + return Json(new + { + success = false, + message = "Select a deposit account (the bank/asset account this payment lands in) " + + "before recording. None is configured for your company yet." + }); + var deposit = new Deposit { ReceiptNumber = receiptNumber, @@ -119,7 +133,6 @@ public class DepositsController : Controller await _unitOfWork.CompleteAsync(); // GL: DR Checking (cash received) / CR Customer Deposits 2300 (liability until applied to invoice). - var custDepositsAcctId = await GetCustomerDepositsAccountIdAsync(currentUser.CompanyId); await _accountBalanceService.DebitAsync(depositAcctId, deposit.Amount); await _accountBalanceService.CreditAsync(custDepositsAcctId, deposit.Amount); diff --git a/src/PowderCoating.Web/Controllers/InvoicesController.cs b/src/PowderCoating.Web/Controllers/InvoicesController.cs index 055f32e..5d20d56 100644 --- a/src/PowderCoating.Web/Controllers/InvoicesController.cs +++ b/src/PowderCoating.Web/Controllers/InvoicesController.cs @@ -413,10 +413,13 @@ public class InvoicesController : Controller : new Dictionary(); // Fall back to the company's configured default revenue account when a catalog item - // has no specific account; if none is configured, fall back to the seeded 4000 account. + // has no specific account; if none is configured (or it has since been deactivated), + // fall back to the seeded 4000 account. The IsActive check mirrors the 4000 lookup so a + // deactivated default doesn't keep being posted to. Account? defaultRevenueAccount = null; if (prefs?.DefaultRevenueAccountId != null) - defaultRevenueAccount = await _unitOfWork.Accounts.GetByIdAsync(prefs.DefaultRevenueAccountId.Value); + defaultRevenueAccount = await _unitOfWork.Accounts.FirstOrDefaultAsync( + a => a.Id == prefs.DefaultRevenueAccountId.Value && a.IsActive); defaultRevenueAccount ??= await _unitOfWork.Accounts .FirstOrDefaultAsync(a => a.AccountNumber == "4000" && a.IsActive);