Fix custom formula wizard bugs and add field name validation

- Fix Add Field blanking inputs: cfFields was IIFE-scoped so inline oninput handlers couldn't reach it; expose cfUpdateField on window - Fix ManualUnitPrice dropped in buildItemFromData: condition excluded isCustomFormulaItem, causing FixedRate items to reprice from scratch - Fix formula card missing on job pages: load CustomFormulaTemplates in PopulateJobItemDropDownsAsync so Details, EditItems, and Edit all get it; add customFormulaTemplates + formulaEvalUrl to Details and EditItems pageMeta - Add NCalc field name validation: client-side inline feedback (is-invalid + message on oninput) and pre-save sweep; server-side ValidateTemplateFields on Create and Update; rules: letter-start, letters/digits/underscores only, no duplicates, "rate" reserved Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 10:28:41 -04:00
parent 1eba50cf0f
commit 4650ba3d4d
6 changed files with 105 additions and 8 deletions
@@ -3001,6 +3001,9 @@ public class CompanySettingsController : Controller
        if (!ModelState.IsValid)
            return Json(new { success = false, message = "Invalid data." });

+        var fieldError = ValidateTemplateFields(dto.FieldsJson);
+        if (fieldError != null) return Json(new { success = false, message = fieldError });
+
        var companyId = _tenantContext.GetCurrentCompanyId()!.Value;
        var entity = _mapper.Map<CustomItemTemplate>(dto);
        entity.CompanyId = companyId;
@@ -3019,6 +3022,9 @@ public class CompanySettingsController : Controller
        if (!ModelState.IsValid)
            return Json(new { success = false, message = "Invalid data." });

+        var fieldError = ValidateTemplateFields(dto.FieldsJson);
+        if (fieldError != null) return Json(new { success = false, message = fieldError });
+
        var companyId = _tenantContext.GetCurrentCompanyId()!.Value;
        var entity = await _unitOfWork.CustomItemTemplates.GetByIdAsync(dto.Id);
        if (entity == null || entity.CompanyId != companyId)
@@ -3136,6 +3142,44 @@ public class CompanySettingsController : Controller

        return Json(result);
    }
+
+    /// <summary>
+    /// Validates field variable names in a fieldsJson array against NCalc identifier rules:
+    /// must start with a letter, contain only letters/digits/underscores, and not use the
+    /// reserved name "rate" (which is auto-populated from the template's Default Rate).
+    /// Returns an error message string on failure, or null if all names are valid.
+    /// </summary>
+    private static string? ValidateTemplateFields(string? fieldsJson)
+    {
+        if (string.IsNullOrWhiteSpace(fieldsJson)) return null;
+
+        List<System.Text.Json.JsonElement>? fields;
+        try
+        {
+            fields = System.Text.Json.JsonSerializer.Deserialize<List<System.Text.Json.JsonElement>>(fieldsJson);
+        }
+        catch { return "Invalid fields JSON."; }
+
+        if (fields == null) return null;
+
+        var nameRegex = new System.Text.RegularExpressions.Regex(@"^[a-zA-Z][a-zA-Z0-9_]*$");
+        var seen = new HashSet<string>(StringComparer.Ordinal);
+
+        foreach (var field in fields)
+        {
+            var name = field.TryGetProperty("name", out var nameProp) ? nameProp.GetString() ?? "" : "";
+            if (string.IsNullOrEmpty(name))
+                return "All fields must have a variable name.";
+            if (name == "rate")
+                return $"\"rate\" is a reserved variable name — it is pre-populated from the template's Default Rate.";
+            if (!nameRegex.IsMatch(name))
+                return $"Invalid field name \"{name}\": must start with a letter and contain only letters, digits, or underscores.";
+            if (!seen.Add(name))
+                return $"Duplicate field name \"{name}\".";
+        }
+
+        return null;
+    }
 }

 public record SaveTemplateJsonRequest(int Id, string? Subject, string? Body);
@@ -3340,6 +3340,13 @@ public class JobsController : Controller
        var useMetric = await _tenantContext.UseMetricSystemAsync();
        ViewBag.UseMetric = useMetric;
        ViewBag.AreaUnit = _measurementService.GetAreaUnitLabel(useMetric);
+
+        var formulaTemplates = await _unitOfWork.CustomItemTemplates.FindAsync(t => t.CompanyId == companyId && t.IsActive);
+        ViewBag.CustomFormulaTemplates = formulaTemplates.OrderBy(t => t.DisplayOrder).ThenBy(t => t.Name)
+            .Select(t => new { id = t.Id, name = t.Name, description = t.Description, outputMode = t.OutputMode,
+                fieldsJson = t.FieldsJson, formula = t.Formula, defaultRate = t.DefaultRate, rateLabel = t.RateLabel,
+                diagramImagePath = string.IsNullOrEmpty(t.DiagramImagePath) ? null
+                    : Url.Action("TemplateDiagram", "CompanySettings", new { templateId = t.Id }) }).ToList();
    }

    /// <summary>
@@ -2489,7 +2489,9 @@
    "useMetric": @Json.Serialize((bool)(ViewBag.UseMetric ?? false)),
    "pricingUrl": "@Url.Action("CalculatePricing", "Jobs")",
    "itemsFieldPrefix": "JobItems",
-    "aiRecalcUrl": "@Url.Action("AiRecalcPrice", "Quotes")"
+    "aiRecalcUrl": "@Url.Action("AiRecalcPrice", "Quotes")",
+    "customFormulaTemplates": @Json.Serialize(ViewBag.CustomFormulaTemplates ?? new List<object>()),
+    "formulaEvalUrl": "@Url.Action("EvaluateFormula", "CompanySettings")"
 }
 </script>

@@ -179,7 +179,9 @@
    "useMetric": @Json.Serialize((bool)(ViewBag.UseMetric ?? false)),
    "pricingUrl": "@Url.Action("CalculatePricing", "Jobs")",
    "itemsFieldPrefix": "JobItems",
-    "aiRecalcUrl": "@Url.Action("AiRecalcPrice", "Quotes")"
+    "aiRecalcUrl": "@Url.Action("AiRecalcPrice", "Quotes")",
+    "customFormulaTemplates": @Json.Serialize(ViewBag.CustomFormulaTemplates ?? new List<object>()),
+    "formulaEvalUrl": "@Url.Action("EvaluateFormula", "CompanySettings")"
 }
 </script>

@@ -137,6 +137,37 @@
        cfRenderFields();
    };

+    window.cfUpdateField = function (i, key, val, isNumber) {
+        cfFields[i][key] = isNumber ? (parseFloat(val) || 0) : val;
+        if (key === 'name') cfValidateFieldNameInput(i, val);
+    };
+
+    function cfValidateFieldName(name) {
+        if (!name) return 'Field variable name is required.';
+        if (name === 'rate') return '"rate" is reserved — it is pre-populated from the template\'s Default Rate.';
+        if (!/^[a-zA-Z][a-zA-Z0-9_]*$/.test(name)) return 'Must start with a letter and contain only letters, digits, or underscores (no spaces).';
+        return null;
+    }
+
+    function cfValidateFieldNameInput(i, val) {
+        const inputs = document.querySelectorAll('#cfFieldsList .field-name-input');
+        const input = inputs[i];
+        if (!input) return;
+        const err = cfValidateFieldName(val);
+        input.classList.toggle('is-invalid', !!err);
+        let fb = input.nextElementSibling;
+        if (err) {
+            if (!fb || !fb.classList.contains('invalid-feedback')) {
+                fb = document.createElement('div');
+                fb.className = 'invalid-feedback';
+                input.after(fb);
+            }
+            fb.textContent = err;
+        } else if (fb && fb.classList.contains('invalid-feedback')) {
+            fb.remove();
+        }
+    }
+
    function cfRenderFields() {
        const el = document.getElementById('cfFieldsList');
        if (!cfFields.length) {
@@ -147,24 +178,24 @@
            <div class="border rounded p-2 mb-2 bg-light">
                <div class="row g-2 align-items-center">
                    <div class="col-3">
-                        <input type="text" class="form-control form-control-sm font-monospace"
+                        <input type="text" class="form-control form-control-sm font-monospace field-name-input${cfValidateFieldName(f.name) ? ' is-invalid' : ''}"
                               placeholder="var_name" value="${escHtml(f.name)}"
-                               oninput="cfFields[${i}].name=this.value" />
+                               oninput="cfUpdateField(${i},'name',this.value)" />${cfValidateFieldName(f.name) ? `<div class="invalid-feedback">${escHtml(cfValidateFieldName(f.name))}</div>` : ''}
                    </div>
                    <div class="col-3">
                        <input type="text" class="form-control form-control-sm"
                               placeholder="Label" value="${escHtml(f.label)}"
-                               oninput="cfFields[${i}].label=this.value" />
+                               oninput="cfUpdateField(${i},'label',this.value)" />
                    </div>
                    <div class="col-2">
                        <input type="text" class="form-control form-control-sm"
                               placeholder="Unit" value="${escHtml(f.unit ?? '')}"
-                               oninput="cfFields[${i}].unit=this.value" />
+                               oninput="cfUpdateField(${i},'unit',this.value)" />
                    </div>
                    <div class="col-3">
                        <input type="number" class="form-control form-control-sm"
                               placeholder="Default" value="${f.defaultValue ?? ''}"
-                               oninput="cfFields[${i}].defaultValue=parseFloat(this.value)||0" />
+                               oninput="cfUpdateField(${i},'defaultValue',this.value,true)" />
                    </div>
                    <div class="col-1 text-end">
                        <button type="button" class="btn btn-link text-danger p-0" onclick="cfRemoveField(${i})">
@@ -292,6 +323,17 @@
        const formula = document.getElementById('cfFormula').value.trim();
        if (!formula) { showCfError('Formula is required.'); return; }

+        const fieldErrors = cfFields.map((f, i) => ({ i, err: cfValidateFieldName(f.name) })).filter(x => x.err);
+        if (fieldErrors.length) {
+            fieldErrors.forEach(x => cfValidateFieldNameInput(x.i, cfFields[x.i].name));
+            showCfError(`Fix field name errors before saving: ${fieldErrors[0].err}`);
+            return;
+        }
+
+        const names = cfFields.map(f => f.name);
+        const dupes = names.filter((n, i) => names.indexOf(n) !== i);
+        if (dupes.length) { showCfError(`Duplicate field name: "${dupes[0]}"`); return; }
+
        const dto = {
            id,
            name,
@@ -2865,7 +2865,7 @@ function buildItemFromWizard() {
        surfaceAreaSqFt:   d.surfaceAreaSqFt || 0,
        estimatedMinutes:  d.estimatedMinutes || 0,
        catalogItemId:     d.catalogItemId || null,
-        manualUnitPrice:   isAi ? (d.manualUnitPrice ?? null) : (d.isGenericItem || d.isSalesItem ? (d.manualUnitPrice ?? null) : null),
+        manualUnitPrice:   isAi ? (d.manualUnitPrice ?? null) : (d.isGenericItem || d.isSalesItem || d.isCustomFormulaItem ? (d.manualUnitPrice ?? null) : null),
        powderCostOverride: d.powderCostOverride ?? null,
        isGenericItem:     !!d.isGenericItem,
        isLaborItem:       !!d.isLaborItem,