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 <noreply@anthropic.com>
This commit is contained in:
2026-06-20 09:26:52 -04:00
parent 8b9a3dff41
commit 687aedf7a4
15 changed files with 186 additions and 48 deletions
@@ -366,11 +366,13 @@ public class BankReconciliationsController : Controller
private async Task PopulateAccountDropdownAsync() private async Task PopulateAccountDropdownAsync()
{ {
var companyId = _tenantContext.GetCurrentCompanyId() ?? 0; 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( var accounts = await _unitOfWork.Accounts.FindAsync(
a => a.CompanyId == companyId && a.IsActive a => a.CompanyId == companyId && a.IsActive
&& (a.AccountSubType == AccountSubType.Checking && (a.AccountType == AccountType.Asset
|| a.AccountSubType == AccountSubType.Savings || a.AccountType == AccountType.Liability));
|| a.AccountSubType == AccountSubType.Cash));
ViewBag.AccountSelectList = accounts ViewBag.AccountSelectList = accounts
.OrderBy(a => a.AccountNumber) .OrderBy(a => a.AccountNumber)
@@ -452,12 +452,12 @@ public class BillsController : Controller
var dto = _mapper.Map<BillDto>(bill); var dto = _mapper.Map<BillDto>(bill);
// Payment form defaults // 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( var bankAccounts = (await _unitOfWork.Accounts.FindAsync(
a => a.CompanyId == bill.CompanyId && a => a.CompanyId == bill.CompanyId &&
(a.AccountSubType == AccountSubType.Cash || (a.AccountType == AccountType.Asset ||
a.AccountSubType == AccountSubType.Checking || a.AccountType == AccountType.Liability)))
a.AccountSubType == AccountSubType.Savings ||
a.AccountSubType == AccountSubType.CreditCard)))
.OrderBy(a => a.AccountNumber) .OrderBy(a => a.AccountNumber)
.ToList(); .ToList();
@@ -63,7 +63,8 @@ public class DepositsController : Controller
string paymentMethod, string paymentMethod,
DateTime receivedDate, DateTime receivedDate,
string? reference, string? reference,
string? notes) string? notes,
int? depositAccountId = null)
{ {
try try
{ {
@@ -80,7 +81,21 @@ public class DepositsController : Controller
if (currentUser == null) return Unauthorized(); if (currentUser == null) return Unauthorized();
var receiptNumber = await GenerateReceiptNumberAsync(currentUser.CompanyId); 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 var deposit = new Deposit
{ {
@@ -93,7 +108,7 @@ public class DepositsController : Controller
ReceivedDate = receivedDate, ReceivedDate = receivedDate,
Reference = reference, Reference = reference,
Notes = notes, Notes = notes,
DepositAccountId = checkingAcctId, DepositAccountId = depositAcctId,
RecordedById = currentUser.Id, RecordedById = currentUser.Id,
CompanyId = currentUser.CompanyId, CompanyId = currentUser.CompanyId,
CreatedAt = DateTime.UtcNow, 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). // GL: DR Checking (cash received) / CR Customer Deposits 2300 (liability until applied to invoice).
var custDepositsAcctId = await GetCustomerDepositsAccountIdAsync(currentUser.CompanyId); var custDepositsAcctId = await GetCustomerDepositsAccountIdAsync(currentUser.CompanyId);
await _accountBalanceService.DebitAsync(checkingAcctId, deposit.Amount); await _accountBalanceService.DebitAsync(depositAcctId, deposit.Amount);
await _accountBalanceService.CreditAsync(custDepositsAcctId, deposit.Amount); await _accountBalanceService.CreditAsync(custDepositsAcctId, deposit.Amount);
return Json(new return Json(new
@@ -1642,10 +1642,15 @@ public class InventoryController : Controller
var accounts = await _unitOfWork.Accounts.FindAsync(a => a.CompanyId == companyId && a.IsActive); 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 ViewBag.InventoryAccounts = accounts
.Where(a => a.AccountType == AccountType.Asset .Where(a => a.AccountType == AccountType.Asset)
&& a.AccountSubType == AccountSubType.Inventory) .OrderByDescending(a => a.AccountSubType == AccountSubType.Inventory)
.OrderBy(a => a.AccountNumber) .ThenBy(a => a.AccountNumber)
.Select(a => new SelectListItem($"{a.AccountNumber} {a.Name}", a.Id.ToString())) .Select(a => new SelectListItem($"{a.AccountNumber} {a.Name}", a.Id.ToString()))
.ToList(); .ToList();
@@ -446,6 +446,9 @@ public class JobsController : Controller
ViewBag.JobInvoiceStatus = jobInvoice?.Status; ViewBag.JobInvoiceStatus = jobInvoice?.Status;
ViewBag.JobVoidedInvoices = voidedInvoices; 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 // Workers dropdown for inline assignment
await PopulateWorkersDropdown(); await PopulateWorkersDropdown();
@@ -302,6 +302,9 @@ public class QuotesController : Controller
var quoteDto = _mapper.Map<QuoteDto>(quote); var quoteDto = _mapper.Map<QuoteDto>(quote);
// Bank/asset accounts the deposit can land in (deposit modal dropdown)
ViewBag.DepositAccounts = await AccountingDropdownHelper.LoadDepositAccountsAsync(_unitOfWork, companyId);
// Get customer info if exists // Get customer info if exists
if (quote.CustomerId.HasValue) if (quote.CustomerId.HasValue)
{ {
@@ -17,6 +17,27 @@ internal static class AccountingDropdownHelper
/// Returns pre-projected SelectListItem collections so controllers avoid duplicating the /// Returns pre-projected SelectListItem collections so controllers avoid duplicating the
/// LINQ-to-SelectListItem transform. /// LINQ-to-SelectListItem transform.
/// </summary> /// </summary>
/// <summary>
/// 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).
/// </summary>
internal static async Task<List<SelectListItem>> 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<AccountingDropdowns> LoadAsync(IUnitOfWork unitOfWork) internal static async Task<AccountingDropdowns> LoadAsync(IUnitOfWork unitOfWork)
{ {
var vendors = await unitOfWork.Vendors.FindAsync(v => v.IsActive); 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())) .Select(a => new SelectListItem(accountLabel(a), a.Id.ToString()))
.ToList(), .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 ApAccounts = allAccounts
.Where(a => a.AccountSubType == AccountSubType.AccountsPayable) .Where(a => a.AccountType == AccountType.Liability)
.OrderBy(a => a.AccountNumber) .OrderBy(a => a.AccountNumber)
.Select(a => new SelectListItem(accountLabel(a), a.Id.ToString())) .Select(a => new SelectListItem(accountLabel(a), a.Id.ToString()))
.ToList(), .ToList(),
// Payment sources span both Assets (cash/checking/savings) and Liabilities
// (credit cards, lines of credit), so include both top-level types.
BankAccounts = allAccounts BankAccounts = allAccounts
.Where(a => a.AccountSubType == AccountSubType.Cash || .Where(a => a.AccountType == AccountType.Asset ||
a.AccountSubType == AccountSubType.Checking || a.AccountType == AccountType.Liability)
a.AccountSubType == AccountSubType.Savings ||
a.AccountSubType == AccountSubType.CreditCard)
.OrderBy(a => a.AccountNumber) .OrderBy(a => a.AccountNumber)
.Select(a => new SelectListItem(accountLabel(a), a.Id.ToString())) .Select(a => new SelectListItem(accountLabel(a), a.Id.ToString()))
.ToList(), .ToList(),
@@ -435,6 +435,7 @@
@section Scripts { @section Scripts {
<partial name="_ValidationScriptsPartial" /> <partial name="_ValidationScriptsPartial" />
<script>const inventoryFormIsCreate = true;</script> <script>const inventoryFormIsCreate = true;</script>
<script src="~/js/inventory-vendor-match.js"></script>
<partial name="_InventoryColorFamilyScripts" /> <partial name="_InventoryColorFamilyScripts" />
<script src="~/js/inventory-catalog-lookup.js"></script> <script src="~/js/inventory-catalog-lookup.js"></script>
<script src="~/js/inventory-duplicate-check.js"></script> <script src="~/js/inventory-duplicate-check.js"></script>
@@ -452,6 +452,7 @@
@section Scripts { @section Scripts {
<partial name="_ValidationScriptsPartial" /> <partial name="_ValidationScriptsPartial" />
<script src="~/js/inventory-vendor-match.js"></script>
<partial name="_InventoryColorFamilyScripts" /> <partial name="_InventoryColorFamilyScripts" />
<script src="~/js/inventory-catalog-lookup.js"></script> <script src="~/js/inventory-catalog-lookup.js"></script>
<script src="~/js/inventory-duplicate-check.js"></script> <script src="~/js/inventory-duplicate-check.js"></script>
@@ -195,17 +195,15 @@
function autoMatchVendor() { function autoMatchVendor() {
if (!isCoatingCategory(categorySelect?.value)) return; if (!isCoatingCategory(categorySelect?.value)) return;
if (!vendorSel || vendorSel.value) return; // don't overwrite an existing selection if (typeof window.matchInventoryVendor === 'function') {
const mfr = (manufacturerEl?.value?.trim() ?? '').toLowerCase(); window.matchInventoryVendor(vendorSel, manufacturerEl?.value, null);
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 (manufacturerEl) { 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) { if (colorNameEl) {
colorNameEl.addEventListener('input', autoComposeName); colorNameEl.addEventListener('input', autoComposeName);
@@ -421,15 +419,15 @@
aiFilledColorFamilies = true; aiFilledColorFamilies = true;
} }
// Vendor: match by name (case-insensitive) against dropdown options // Vendor: match on the Manufacturer field first (almost always populated and equal to
if (data.vendorName) { // the vendor for the shop's distributors); fall back to the AI's price-derived vendorName.
{
const vendorSel = document.getElementById('field-vendor'); const vendorSel = document.getElementById('field-vendor');
if (vendorSel && (forceRefill || !vendorSel.value)) { const mfrName = document.getElementById('field-manufacturer')?.value || data.manufacturer;
const needle = data.vendorName.toLowerCase(); if (typeof window.matchInventoryVendor === 'function' &&
const match = Array.from(vendorSel.options).find(o => window.matchInventoryVendor(vendorSel, mfrName, data.vendorName, { force: forceRefill })) {
o.text.toLowerCase().includes(needle) || needle.includes(o.text.toLowerCase().trim()) filled.push('Vendor');
); aiFilledVendor = true;
if (match) { vendorSel.value = match.value; filled.push('Vendor'); aiFilledVendor = true; }
} }
} }
@@ -1306,6 +1306,17 @@
<label class="form-label fw-semibold">Date Received <span class="text-danger">*</span></label> <label class="form-label fw-semibold">Date Received <span class="text-danger">*</span></label>
<input type="date" class="form-control" id="depositDate" name="receivedDate" required value="@(DateTime.Today.ToString("yyyy-MM-dd"))" /> <input type="date" class="form-control" id="depositDate" name="receivedDate" required value="@(DateTime.Today.ToString("yyyy-MM-dd"))" />
</div> </div>
@{ var depositAccounts = ViewBag.DepositAccounts as List<Microsoft.AspNetCore.Mvc.Rendering.SelectListItem>; }
@if (depositAccounts != null && depositAccounts.Count > 0)
{
<div class="mb-3">
<label class="form-label fw-semibold">Deposit To</label>
<select class="form-select" id="depositAccount" name="depositAccountId" asp-items="depositAccounts">
<option value="">Default (first checking/cash account)</option>
</select>
<small class="form-text text-muted">Bank or asset account this deposit is recorded against.</small>
</div>
}
<div class="mb-3"> <div class="mb-3">
<label class="form-label">Reference (check #, card last 4, etc.)</label> <label class="form-label">Reference (check #, card last 4, etc.)</label>
<input type="text" class="form-control" id="depositReference" name="reference" maxlength="200" /> <input type="text" class="form-control" id="depositReference" name="reference" maxlength="200" />
@@ -1898,6 +1898,17 @@
<label class="form-label fw-semibold">Date Received <span class="text-danger">*</span></label> <label class="form-label fw-semibold">Date Received <span class="text-danger">*</span></label>
<input type="date" class="form-control" id="depositDate" name="receivedDate" required value="@(DateTime.Today.ToString("yyyy-MM-dd"))" /> <input type="date" class="form-control" id="depositDate" name="receivedDate" required value="@(DateTime.Today.ToString("yyyy-MM-dd"))" />
</div> </div>
@{ var depositAccounts = ViewBag.DepositAccounts as List<Microsoft.AspNetCore.Mvc.Rendering.SelectListItem>; }
@if (depositAccounts != null && depositAccounts.Count > 0)
{
<div class="mb-3">
<label class="form-label fw-semibold">Deposit To</label>
<select class="form-select" id="depositAccount" name="depositAccountId" asp-items="depositAccounts">
<option value="">Default (first checking/cash account)</option>
</select>
<small class="form-text text-muted">Bank or asset account this deposit is recorded against.</small>
</div>
}
<div class="mb-3"> <div class="mb-3">
<label class="form-label">Reference (check #, card last 4, etc.)</label> <label class="form-label">Reference (check #, card last 4, etc.)</label>
<input type="text" class="form-control" id="depositReference" name="reference" maxlength="200" /> <input type="text" class="form-control" id="depositReference" name="reference" maxlength="200" />
@@ -198,14 +198,12 @@
filled.push('Image'); 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'); const vendorSel = document.getElementById('field-vendor');
if (vendorSel && !vendorSel.value && item.vendorName) { const mfrName = document.getElementById('field-manufacturer')?.value;
const needle = item.vendorName.toLowerCase(); if (typeof window.matchInventoryVendor === 'function' &&
const match = Array.from(vendorSel.options).find(o => window.matchInventoryVendor(vendorSel, mfrName, item.vendorName)) {
o.text.toLowerCase().includes(needle) || needle.includes(o.text.toLowerCase().trim()) filled.push('Vendor');
);
if (match) { vendorSel.value = match.value; filled.push('Vendor'); }
} }
document.dispatchEvent(new CustomEvent('inventory:identity-changed')); document.dispatchEvent(new CustomEvent('inventory:identity-changed'));
@@ -401,12 +401,10 @@
} }
const vendorSel = document.getElementById('field-vendor'); const vendorSel = document.getElementById('field-vendor');
if (vendorSel && !vendorSel.value && data.vendorName) { const mfrName = document.getElementById('field-manufacturer')?.value || data.manufacturer;
const needle = data.vendorName.toLowerCase(); if (typeof window.matchInventoryVendor === 'function' &&
const match = Array.from(vendorSel.options).find(o => window.matchInventoryVendor(vendorSel, mfrName, data.vendorName)) {
o.text.toLowerCase().includes(needle) || needle.includes(o.text.toLowerCase().trim()) filled.push('Vendor');
);
if (match) { vendorSel.value = match.value; filled.push('Vendor'); }
} }
const catalogNote = data.wasInCatalog const catalogNote = data.wasInCatalog
@@ -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;
};
})();