From 687aedf7a40fbdc0c23d797e063b233f43aa8bf4 Mon Sep 17 00:00:00 2001 From: Scott Pouliot Date: Sat, 20 Jun 2026 09:26:52 -0400 Subject: [PATCH] Fix account dropdowns: vendor auto-select + sub-type filtering Inventory vendor auto-select: match the dropdown off the Manufacturer field (almost always populated and equal to the vendor for the shop's distributors) instead of the AI's price-conditional vendorName, which was only returned when a price was scraped. Centralizes the logic in a shared inventory-vendor-match.js used by catalog lookup, AI lookup, label scan, and manual entry; skips brands sold by multiple distributors (PPG, KP Pigments) so those stay manual. Account dropdowns filtered by sub-type now filter by parent AccountType, so accounts a company classifies under a non-standard sub-type still appear: Inventory account (Asset), AP account (Liability), pay-from/bank and Bank Reconciliation pickers (Asset + Liability). Deposit account is now a user-selectable dropdown on the Job and Quote deposit modals (Asset + Liability accounts) instead of a silent auto-pick of the first Checking/Cash account; falls back to the old behavior when left blank, and validates the chosen account belongs to the company. Co-Authored-By: Claude Opus 4.8 --- .../BankReconciliationsController.cs | 8 ++- .../Controllers/BillsController.cs | 8 +-- .../Controllers/DepositsController.cs | 23 +++++-- .../Controllers/InventoryController.cs | 11 ++- .../Controllers/JobsController.cs | 3 + .../Controllers/QuotesController.cs | 3 + .../Helpers/AccountingDropdownHelper.cs | 35 ++++++++-- .../Views/Inventory/Create.cshtml | 1 + .../Views/Inventory/Edit.cshtml | 1 + .../_InventoryColorFamilyScripts.cshtml | 30 ++++----- .../Views/Jobs/Details.cshtml | 11 +++ .../Views/Quotes/Details.cshtml | 11 +++ .../wwwroot/js/inventory-catalog-lookup.js | 12 ++-- .../wwwroot/js/inventory-label-scan.js | 10 ++- .../wwwroot/js/inventory-vendor-match.js | 67 +++++++++++++++++++ 15 files changed, 186 insertions(+), 48 deletions(-) create mode 100644 src/PowderCoating.Web/wwwroot/js/inventory-vendor-match.js diff --git a/src/PowderCoating.Web/Controllers/BankReconciliationsController.cs b/src/PowderCoating.Web/Controllers/BankReconciliationsController.cs index 9a8fe73..49cd142 100644 --- a/src/PowderCoating.Web/Controllers/BankReconciliationsController.cs +++ b/src/PowderCoating.Web/Controllers/BankReconciliationsController.cs @@ -366,11 +366,13 @@ public class BankReconciliationsController : Controller private async Task PopulateAccountDropdownAsync() { var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; + // Reconcilable accounts: any Asset (bank/cash) or Liability (credit card, line of + // credit) account. Filter by parent AccountType, not sub-type, so an account the + // company classified differently still shows up for reconciliation. var accounts = await _unitOfWork.Accounts.FindAsync( a => a.CompanyId == companyId && a.IsActive - && (a.AccountSubType == AccountSubType.Checking - || a.AccountSubType == AccountSubType.Savings - || a.AccountSubType == AccountSubType.Cash)); + && (a.AccountType == AccountType.Asset + || a.AccountType == AccountType.Liability)); ViewBag.AccountSelectList = accounts .OrderBy(a => a.AccountNumber) diff --git a/src/PowderCoating.Web/Controllers/BillsController.cs b/src/PowderCoating.Web/Controllers/BillsController.cs index 4121d29..7536f41 100644 --- a/src/PowderCoating.Web/Controllers/BillsController.cs +++ b/src/PowderCoating.Web/Controllers/BillsController.cs @@ -452,12 +452,12 @@ public class BillsController : Controller var dto = _mapper.Map(bill); // Payment form defaults + // Payment sources: filter by parent AccountType (Asset or Liability), not sub-type, + // so accounts a company classified under a different sub-type still appear. var bankAccounts = (await _unitOfWork.Accounts.FindAsync( a => a.CompanyId == bill.CompanyId && - (a.AccountSubType == AccountSubType.Cash || - a.AccountSubType == AccountSubType.Checking || - a.AccountSubType == AccountSubType.Savings || - a.AccountSubType == AccountSubType.CreditCard))) + (a.AccountType == AccountType.Asset || + a.AccountType == AccountType.Liability))) .OrderBy(a => a.AccountNumber) .ToList(); diff --git a/src/PowderCoating.Web/Controllers/DepositsController.cs b/src/PowderCoating.Web/Controllers/DepositsController.cs index fc1f8e1..b299fc8 100644 --- a/src/PowderCoating.Web/Controllers/DepositsController.cs +++ b/src/PowderCoating.Web/Controllers/DepositsController.cs @@ -63,7 +63,8 @@ public class DepositsController : Controller string paymentMethod, DateTime receivedDate, string? reference, - string? notes) + string? notes, + int? depositAccountId = null) { try { @@ -80,7 +81,21 @@ public class DepositsController : Controller if (currentUser == null) return Unauthorized(); var receiptNumber = await GenerateReceiptNumberAsync(currentUser.CompanyId); - var checkingAcctId = await GetCheckingAccountIdAsync(currentUser.CompanyId); + + // Resolve the bank/asset account the deposit lands in. The user now picks this on + // the form; if they didn't (or the value is stale), fall back to the legacy + // auto-pick of the first Checking/Cash account. Validate any user-supplied id + // belongs to this company (defense in depth — the global filter alone isn't enough). + int? depositAcctId = null; + if (depositAccountId.HasValue) + { + var chosen = await _unitOfWork.Accounts.FirstOrDefaultAsync( + a => a.Id == depositAccountId.Value + && a.CompanyId == currentUser.CompanyId + && a.IsActive); + depositAcctId = chosen?.Id; + } + depositAcctId ??= await GetCheckingAccountIdAsync(currentUser.CompanyId); var deposit = new Deposit { @@ -93,7 +108,7 @@ public class DepositsController : Controller ReceivedDate = receivedDate, Reference = reference, Notes = notes, - DepositAccountId = checkingAcctId, + DepositAccountId = depositAcctId, RecordedById = currentUser.Id, CompanyId = currentUser.CompanyId, CreatedAt = DateTime.UtcNow, @@ -105,7 +120,7 @@ public class DepositsController : Controller // GL: DR Checking (cash received) / CR Customer Deposits 2300 (liability until applied to invoice). var custDepositsAcctId = await GetCustomerDepositsAccountIdAsync(currentUser.CompanyId); - await _accountBalanceService.DebitAsync(checkingAcctId, deposit.Amount); + await _accountBalanceService.DebitAsync(depositAcctId, deposit.Amount); await _accountBalanceService.CreditAsync(custDepositsAcctId, deposit.Amount); return Json(new diff --git a/src/PowderCoating.Web/Controllers/InventoryController.cs b/src/PowderCoating.Web/Controllers/InventoryController.cs index 357b15d..cdc3676 100644 --- a/src/PowderCoating.Web/Controllers/InventoryController.cs +++ b/src/PowderCoating.Web/Controllers/InventoryController.cs @@ -1642,10 +1642,15 @@ public class InventoryController : Controller var accounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && a.IsActive); + // Show ALL asset accounts, not just the Inventory sub-type. Companies that created + // their inventory account manually often land on a different asset sub-type (e.g. + // Other Current Asset), which previously left this dropdown empty. Listing every + // asset account lets them pick whatever they actually use; Inventory sub-type + // accounts are surfaced first as the recommended choice. ViewBag.InventoryAccounts = accounts - .Where(a => a.AccountType == AccountType.Asset - && a.AccountSubType == AccountSubType.Inventory) - .OrderBy(a => a.AccountNumber) + .Where(a => a.AccountType == AccountType.Asset) + .OrderByDescending(a => a.AccountSubType == AccountSubType.Inventory) + .ThenBy(a => a.AccountNumber) .Select(a => new SelectListItem($"{a.AccountNumber} – {a.Name}", a.Id.ToString())) .ToList(); diff --git a/src/PowderCoating.Web/Controllers/JobsController.cs b/src/PowderCoating.Web/Controllers/JobsController.cs index f502f85..b3c00d7 100644 --- a/src/PowderCoating.Web/Controllers/JobsController.cs +++ b/src/PowderCoating.Web/Controllers/JobsController.cs @@ -446,6 +446,9 @@ public class JobsController : Controller ViewBag.JobInvoiceStatus = jobInvoice?.Status; ViewBag.JobVoidedInvoices = voidedInvoices; + // Bank/asset accounts the deposit can land in (deposit modal dropdown) + ViewBag.DepositAccounts = await AccountingDropdownHelper.LoadDepositAccountsAsync(_unitOfWork, companyId); + // Workers dropdown for inline assignment await PopulateWorkersDropdown(); diff --git a/src/PowderCoating.Web/Controllers/QuotesController.cs b/src/PowderCoating.Web/Controllers/QuotesController.cs index 575db2d..21ad960 100644 --- a/src/PowderCoating.Web/Controllers/QuotesController.cs +++ b/src/PowderCoating.Web/Controllers/QuotesController.cs @@ -302,6 +302,9 @@ public class QuotesController : Controller var quoteDto = _mapper.Map(quote); + // Bank/asset accounts the deposit can land in (deposit modal dropdown) + ViewBag.DepositAccounts = await AccountingDropdownHelper.LoadDepositAccountsAsync(_unitOfWork, companyId); + // Get customer info if exists if (quote.CustomerId.HasValue) { diff --git a/src/PowderCoating.Web/Helpers/AccountingDropdownHelper.cs b/src/PowderCoating.Web/Helpers/AccountingDropdownHelper.cs index 34faa96..5b7ea58 100644 --- a/src/PowderCoating.Web/Helpers/AccountingDropdownHelper.cs +++ b/src/PowderCoating.Web/Helpers/AccountingDropdownHelper.cs @@ -17,6 +17,27 @@ internal static class AccountingDropdownHelper /// Returns pre-projected SelectListItem collections so controllers avoid duplicating the /// LINQ-to-SelectListItem transform. /// + /// + /// Loads the accounts a customer deposit can land in — any active Asset or Liability + /// account for the company (filtered by parent AccountType, not sub-type, so accounts a + /// company classified differently still appear). Checking/Cash accounts sort to the top + /// as the usual choice. Used to populate the deposit modal's account dropdown on the Job + /// and Quote details pages. CompanyId is filtered explicitly (defense in depth). + /// + internal static async Task> LoadDepositAccountsAsync(IUnitOfWork unitOfWork, int companyId) + { + var accounts = await unitOfWork.Accounts.FindAsync( + a => a.CompanyId == companyId && a.IsActive + && (a.AccountType == AccountType.Asset || a.AccountType == AccountType.Liability)); + + return accounts + .OrderByDescending(a => a.AccountSubType == AccountSubType.Checking || + a.AccountSubType == AccountSubType.Cash) + .ThenBy(a => a.AccountNumber) + .Select(a => new SelectListItem($"{a.AccountNumber} – {a.Name}", a.Id.ToString())) + .ToList(); + } + internal static async Task LoadAsync(IUnitOfWork unitOfWork) { var vendors = await unitOfWork.Vendors.FindAsync(v => v.IsActive); @@ -50,17 +71,21 @@ internal static class AccountingDropdownHelper .Select(a => new SelectListItem(accountLabel(a), a.Id.ToString())) .ToList(), + // Filter by parent AccountType only — not sub-type. Companies classify their + // own accounts differently (e.g. a "Line of Credit" they treat as a payable), + // so listing every account of the right top-level type lets them pick what they + // actually use instead of silently hiding accounts on a sub-type mismatch. ApAccounts = allAccounts - .Where(a => a.AccountSubType == AccountSubType.AccountsPayable) + .Where(a => a.AccountType == AccountType.Liability) .OrderBy(a => a.AccountNumber) .Select(a => new SelectListItem(accountLabel(a), a.Id.ToString())) .ToList(), + // Payment sources span both Assets (cash/checking/savings) and Liabilities + // (credit cards, lines of credit), so include both top-level types. BankAccounts = allAccounts - .Where(a => a.AccountSubType == AccountSubType.Cash || - a.AccountSubType == AccountSubType.Checking || - a.AccountSubType == AccountSubType.Savings || - a.AccountSubType == AccountSubType.CreditCard) + .Where(a => a.AccountType == AccountType.Asset || + a.AccountType == AccountType.Liability) .OrderBy(a => a.AccountNumber) .Select(a => new SelectListItem(accountLabel(a), a.Id.ToString())) .ToList(), diff --git a/src/PowderCoating.Web/Views/Inventory/Create.cshtml b/src/PowderCoating.Web/Views/Inventory/Create.cshtml index 880c7c5..64d9b7d 100644 --- a/src/PowderCoating.Web/Views/Inventory/Create.cshtml +++ b/src/PowderCoating.Web/Views/Inventory/Create.cshtml @@ -435,6 +435,7 @@ @section Scripts { + diff --git a/src/PowderCoating.Web/Views/Inventory/Edit.cshtml b/src/PowderCoating.Web/Views/Inventory/Edit.cshtml index bdbe6ed..c4d51a0 100644 --- a/src/PowderCoating.Web/Views/Inventory/Edit.cshtml +++ b/src/PowderCoating.Web/Views/Inventory/Edit.cshtml @@ -452,6 +452,7 @@ @section Scripts { + diff --git a/src/PowderCoating.Web/Views/Inventory/_InventoryColorFamilyScripts.cshtml b/src/PowderCoating.Web/Views/Inventory/_InventoryColorFamilyScripts.cshtml index 2df2682..f09e057 100644 --- a/src/PowderCoating.Web/Views/Inventory/_InventoryColorFamilyScripts.cshtml +++ b/src/PowderCoating.Web/Views/Inventory/_InventoryColorFamilyScripts.cshtml @@ -195,17 +195,15 @@ function autoMatchVendor() { if (!isCoatingCategory(categorySelect?.value)) return; - if (!vendorSel || vendorSel.value) return; // don't overwrite an existing selection - const mfr = (manufacturerEl?.value?.trim() ?? '').toLowerCase(); - if (!mfr) return; - const match = Array.from(vendorSel.options).find(o => - o.text.toLowerCase().includes(mfr) || mfr.includes(o.text.toLowerCase().trim()) - ); - if (match) vendorSel.value = match.value; + if (typeof window.matchInventoryVendor === 'function') { + window.matchInventoryVendor(vendorSel, manufacturerEl?.value, null); + } } if (manufacturerEl) { - manufacturerEl.addEventListener('input', autoMatchVendor); + // Use 'change' (fires on blur with the full value) rather than 'input' so partial + // mid-typing values like "P" don't trigger a wrong vendor pick. + manufacturerEl.addEventListener('change', autoMatchVendor); } if (colorNameEl) { colorNameEl.addEventListener('input', autoComposeName); @@ -421,15 +419,15 @@ aiFilledColorFamilies = true; } - // Vendor: match by name (case-insensitive) against dropdown options - if (data.vendorName) { + // Vendor: match on the Manufacturer field first (almost always populated and equal to + // the vendor for the shop's distributors); fall back to the AI's price-derived vendorName. + { const vendorSel = document.getElementById('field-vendor'); - if (vendorSel && (forceRefill || !vendorSel.value)) { - const needle = data.vendorName.toLowerCase(); - const match = Array.from(vendorSel.options).find(o => - o.text.toLowerCase().includes(needle) || needle.includes(o.text.toLowerCase().trim()) - ); - if (match) { vendorSel.value = match.value; filled.push('Vendor'); aiFilledVendor = true; } + const mfrName = document.getElementById('field-manufacturer')?.value || data.manufacturer; + if (typeof window.matchInventoryVendor === 'function' && + window.matchInventoryVendor(vendorSel, mfrName, data.vendorName, { force: forceRefill })) { + filled.push('Vendor'); + aiFilledVendor = true; } } diff --git a/src/PowderCoating.Web/Views/Jobs/Details.cshtml b/src/PowderCoating.Web/Views/Jobs/Details.cshtml index 8b3ba56..7f0e9e1 100644 --- a/src/PowderCoating.Web/Views/Jobs/Details.cshtml +++ b/src/PowderCoating.Web/Views/Jobs/Details.cshtml @@ -1306,6 +1306,17 @@ + @{ var depositAccounts = ViewBag.DepositAccounts as List; } + @if (depositAccounts != null && depositAccounts.Count > 0) + { +
+ + + Bank or asset account this deposit is recorded against. +
+ }
diff --git a/src/PowderCoating.Web/Views/Quotes/Details.cshtml b/src/PowderCoating.Web/Views/Quotes/Details.cshtml index 9a67187..71ee562 100644 --- a/src/PowderCoating.Web/Views/Quotes/Details.cshtml +++ b/src/PowderCoating.Web/Views/Quotes/Details.cshtml @@ -1898,6 +1898,17 @@
+ @{ var depositAccounts = ViewBag.DepositAccounts as List; } + @if (depositAccounts != null && depositAccounts.Count > 0) + { +
+ + + Bank or asset account this deposit is recorded against. +
+ }
diff --git a/src/PowderCoating.Web/wwwroot/js/inventory-catalog-lookup.js b/src/PowderCoating.Web/wwwroot/js/inventory-catalog-lookup.js index 60c82b9..ca2b32f 100644 --- a/src/PowderCoating.Web/wwwroot/js/inventory-catalog-lookup.js +++ b/src/PowderCoating.Web/wwwroot/js/inventory-catalog-lookup.js @@ -198,14 +198,12 @@ filled.push('Image'); } - // Vendor dropdown — match by name + // Vendor dropdown — match on the Manufacturer field first, catalog vendor name as fallback const vendorSel = document.getElementById('field-vendor'); - if (vendorSel && !vendorSel.value && item.vendorName) { - const needle = item.vendorName.toLowerCase(); - const match = Array.from(vendorSel.options).find(o => - o.text.toLowerCase().includes(needle) || needle.includes(o.text.toLowerCase().trim()) - ); - if (match) { vendorSel.value = match.value; filled.push('Vendor'); } + const mfrName = document.getElementById('field-manufacturer')?.value; + if (typeof window.matchInventoryVendor === 'function' && + window.matchInventoryVendor(vendorSel, mfrName, item.vendorName)) { + filled.push('Vendor'); } document.dispatchEvent(new CustomEvent('inventory:identity-changed')); diff --git a/src/PowderCoating.Web/wwwroot/js/inventory-label-scan.js b/src/PowderCoating.Web/wwwroot/js/inventory-label-scan.js index 0086409..419223b 100644 --- a/src/PowderCoating.Web/wwwroot/js/inventory-label-scan.js +++ b/src/PowderCoating.Web/wwwroot/js/inventory-label-scan.js @@ -401,12 +401,10 @@ } const vendorSel = document.getElementById('field-vendor'); - if (vendorSel && !vendorSel.value && data.vendorName) { - const needle = data.vendorName.toLowerCase(); - const match = Array.from(vendorSel.options).find(o => - o.text.toLowerCase().includes(needle) || needle.includes(o.text.toLowerCase().trim()) - ); - if (match) { vendorSel.value = match.value; filled.push('Vendor'); } + const mfrName = document.getElementById('field-manufacturer')?.value || data.manufacturer; + if (typeof window.matchInventoryVendor === 'function' && + window.matchInventoryVendor(vendorSel, mfrName, data.vendorName)) { + filled.push('Vendor'); } const catalogNote = data.wasInCatalog diff --git a/src/PowderCoating.Web/wwwroot/js/inventory-vendor-match.js b/src/PowderCoating.Web/wwwroot/js/inventory-vendor-match.js new file mode 100644 index 0000000..2e9bf96 --- /dev/null +++ b/src/PowderCoating.Web/wwwroot/js/inventory-vendor-match.js @@ -0,0 +1,67 @@ +/** + * Shared vendor-dropdown auto-select for the Inventory Create/Edit forms. + * + * Why this exists: catalog lookup, AI lookup, label scan, and manual manufacturer entry + * all need to pick the right Vendor option, and they used to each carry their own copy of + * the matching logic. They disagreed on WHAT to match on — the AI path keyed off the + * price-derived `vendorName` (which is null unless a price was scraped), so the vendor + * only got selected "sometimes". This centralizes the rule: + * + * For ~95% of powders the manufacturer IS the vendor (Prismatic, Columbia, + * All Powder Paints, Tiger, Powder Buy The Pound). So match on the Manufacturer + * field first — it's almost always populated — and only fall back to the + * AI/catalog-supplied vendor name when the manufacturer is blank. + * + * Brands sold by more than one distributor (e.g. PPG, KP Pigments) are intentionally + * skipped so the user picks the vendor manually rather than getting a wrong guess. + */ +(function () { + 'use strict'; + + // Brands carried by multiple distributors — never auto-pick a vendor for these. + // Lowercase; matched as a substring against the manufacturer name. Extend as needed. + const AMBIGUOUS_BRANDS = ['ppg', 'kp pigments', 'kp pigment']; + + function normalize(s) { + return (s || '').toLowerCase().trim(); + } + + function isAmbiguousBrand(name) { + const n = normalize(name); + return n.length > 0 && AMBIGUOUS_BRANDS.some(b => n.includes(b)); + } + + /** + * Selects the vendor dropdown option that best matches a manufacturer/vendor name. + * + * @param {HTMLSelectElement} vendorSelect the #field-vendor element + * @param {string} manufacturerName primary name to match on (the Manufacturer field) + * @param {string} fallbackVendorName AI/catalog vendor name, used only if manufacturer is blank + * @param {{force?: boolean}} [opts] force=true overrides an existing selection (bad-match retry) + * @returns {boolean} true if a vendor option was selected. + */ + window.matchInventoryVendor = function (vendorSelect, manufacturerName, fallbackVendorName, opts) { + opts = opts || {}; + if (!vendorSelect) return false; + // Don't clobber a choice the user (or a prior fill) already made, unless forcing a re-fill. + if (vendorSelect.value && !opts.force) return false; + + // Manufacturer drives the match; the price-derived vendor name is only a fallback. + const name = normalize(manufacturerName) || normalize(fallbackVendorName); + if (!name) return false; + + // Brands sold by multiple distributors stay manual — don't guess. + if (isAmbiguousBrand(name)) return false; + + const match = Array.from(vendorSelect.options).find(function (o) { + const t = normalize(o.text); + // Skip the placeholder and the "Add new vendor" sentinel; require a real name to + // avoid spurious substring hits (e.g. empty option text matches everything). + if (!o.value || o.value === '__new__' || t.length < 3) return false; + return t.includes(name) || name.includes(t); + }); + + if (match) { vendorSelect.value = match.value; return true; } + return false; + }; +})();