Accounting audit fixes: revenue default IsActive + deposit account guard
Audit of this session's accounting changes (sub-type→type dropdowns, deposit account picker, default GL accounts) found no ledger-drift bugs. Two fixes applied: - Default revenue account now requires IsActive (mirrors the 4000 fallback), so a deactivated default isn't silently posted to. - DepositsController.Record blocks recording when the 2300 Customer Deposits liability exists but no deposit/bank account resolves — that would post a one-sided entry. When 2300 doesn't exist (no accounting), nothing posts, so the deposit is still allowed. ACCOUNTING_AUDIT.md updated: O9 footgun surface widened by the default- accounts feature (now mitigated/documented), plus the 2026-06-20 review notes and the resolved deposit-imbalance item. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
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.
|
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
|
## Status
|
||||||
**All findings O1–O9 + the read-path sweep are resolved** on `dev` (O9 by policy decision — expense at
|
**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
|
purchase — needing no code change). The optional structural follow-up is the JournalEntry single-source
|
||||||
|
|||||||
@@ -97,6 +97,20 @@ public class DepositsController : Controller
|
|||||||
}
|
}
|
||||||
depositAcctId ??= await GetCheckingAccountIdAsync(currentUser.CompanyId);
|
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
|
var deposit = new Deposit
|
||||||
{
|
{
|
||||||
ReceiptNumber = receiptNumber,
|
ReceiptNumber = receiptNumber,
|
||||||
@@ -119,7 +133,6 @@ public class DepositsController : Controller
|
|||||||
await _unitOfWork.CompleteAsync();
|
await _unitOfWork.CompleteAsync();
|
||||||
|
|
||||||
// GL: DR Checking (cash received) / CR Customer Deposits 2300 (liability until applied to invoice).
|
// 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.DebitAsync(depositAcctId, deposit.Amount);
|
||||||
await _accountBalanceService.CreditAsync(custDepositsAcctId, deposit.Amount);
|
await _accountBalanceService.CreditAsync(custDepositsAcctId, deposit.Amount);
|
||||||
|
|
||||||
|
|||||||
@@ -413,10 +413,13 @@ public class InvoicesController : Controller
|
|||||||
: new Dictionary<int, CatalogItem>();
|
: new Dictionary<int, CatalogItem>();
|
||||||
|
|
||||||
// Fall back to the company's configured default revenue account when a catalog item
|
// 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;
|
Account? defaultRevenueAccount = null;
|
||||||
if (prefs?.DefaultRevenueAccountId != 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
|
defaultRevenueAccount ??= await _unitOfWork.Accounts
|
||||||
.FirstOrDefaultAsync(a => a.AccountNumber == "4000" && a.IsActive);
|
.FirstOrDefaultAsync(a => a.AccountNumber == "4000" && a.IsActive);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user