Merge pull request #1176 from we-promise/claude/fix-issue-1138-uRWb6

Fix decimal separator handling in money input fields

app/javascript/controllers/convert_to_trade_controller.js

@@ -1,4 +1,5 @@
 import { Controller } from "@hotwired/stimulus"
+import parseLocaleFloat from "utils/parse_locale_float"
 
 export default class extends Controller {
   static targets = ["customWrapper", "customField", "tickerSelect", "qtyField", "priceField", "priceWarning", "priceWarningMessage"]
@@ -42,8 +43,8 @@ export default class extends Controller {
 
     // Calculate the implied/entered price
     let enteredPriceCents = null
-    const qty = Number.parseFloat(this.qtyFieldTarget?.value)
-    const enteredPrice = Number.parseFloat(this.priceFieldTarget?.value)
+    const qty = parseLocaleFloat(this.qtyFieldTarget?.value)
+    const enteredPrice = parseLocaleFloat(this.priceFieldTarget?.value)
 
     if (enteredPrice && enteredPrice > 0) {
       // User entered a price directly

app/javascript/controllers/cost_basis_form_controller.js

@@ -1,4 +1,5 @@
 import { Controller } from "@hotwired/stimulus"
+import parseLocaleFloat from "utils/parse_locale_float"
 
 // Handles bidirectional conversion between total cost basis and per-share cost
 // in the manual cost basis entry form.
@@ -9,7 +10,7 @@ export default class extends Controller {
   // Called when user types in the total cost basis field
   // Updates the per-share display and input to show the calculated value
   updatePerShare() {
-    const total = Number.parseFloat(this.totalTarget.value) || 0
+    const total = parseLocaleFloat(this.totalTarget.value)
     const qty = this.qtyValue || 1
     const perShare = qty > 0 ? (total / qty).toFixed(2) : "0.00"
     this.perShareValueTarget.textContent = perShare
@@ -21,7 +22,7 @@ export default class extends Controller {
   // Called when user types in the per-share field
   // Updates the total cost basis field with the calculated value
   updateTotal() {
-    const perShare = Number.parseFloat(this.perShareTarget.value) || 0
+    const perShare = parseLocaleFloat(this.perShareTarget.value)
     const qty = this.qtyValue || 1
     const total = (perShare * qty).toFixed(2)
     this.totalTarget.value = total

app/javascript/controllers/drawer_cost_basis_controller.js

@@ -1,4 +1,5 @@
 import { Controller } from "@hotwired/stimulus"
+import parseLocaleFloat from "utils/parse_locale_float"
 
 // Handles the inline cost basis editor in the holding drawer.
 // Shows/hides the form and handles bidirectional total <-> per-share conversion.
@@ -13,7 +14,7 @@ export default class extends Controller {
 
   // Called when user types in total cost basis field
   updatePerShare() {
-    const total = Number.parseFloat(this.totalTarget.value) || 0
+    const total = parseLocaleFloat(this.totalTarget.value)
     const qty = this.qtyValue || 1
     const perShare = qty > 0 ? (total / qty).toFixed(2) : "0.00"
     this.perShareValueTarget.textContent = perShare
@@ -24,7 +25,7 @@ export default class extends Controller {
 
   // Called when user types in per-share field
   updateTotal() {
-    const perShare = Number.parseFloat(this.perShareTarget.value) || 0
+    const perShare = parseLocaleFloat(this.perShareTarget.value)
     const qty = this.qtyValue || 1
     const total = (perShare * qty).toFixed(2)
     this.totalTarget.value = total

app/javascript/controllers/money_field_controller.js

@@ -1,4 +1,5 @@
 import { Controller } from "@hotwired/stimulus";
+import parseLocaleFloat from "utils/parse_locale_float";
 import { CurrenciesService } from "services/currencies_service";
 
 // Connects to data-controller="money-field"
@@ -15,10 +16,12 @@ export default class extends Controller {
     new CurrenciesService().get(currency).then((currency) => {
       this.amountTarget.step = currency.step;
 
-      if (Number.isFinite(this.amountTarget.value)) {
-        this.amountTarget.value = Number.parseFloat(
-          this.amountTarget.value,
-        ).toFixed(currency.default_precision);
+      const rawValue = this.amountTarget.value.trim();
+      if (rawValue !== "") {
+        const parsedAmount = parseLocaleFloat(rawValue);
+        if (Number.isFinite(parsedAmount)) {
+          this.amountTarget.value = parsedAmount.toFixed(currency.default_precision);
+        }
       }
 
       this.symbolTarget.innerText = currency.symbol;

app/javascript/utils/parse_locale_float.js

@@ -0,0 +1,24 @@
+// Parses a float from a string that may use either commas or dots as decimal separators.
+// Handles formats like "1,234.56" (English) and "1.234,56" (French/European).
+export default function parseLocaleFloat(value) {
+  if (typeof value !== "string") return Number.parseFloat(value) || 0
+
+  const cleaned = value.replace(/\s/g, "")
+  const lastComma = cleaned.lastIndexOf(",")
+  const lastDot = cleaned.lastIndexOf(".")
+
+  if (lastComma > lastDot) {
+    // When there's no dot present and exactly 3 digits follow the last comma,
+    // treat comma as a thousands separator (e.g., "1,234" → 1234, "12,345" → 12345)
+    const digitsAfterComma = cleaned.length - lastComma - 1
+    if (lastDot === -1 && digitsAfterComma === 3) {
+      return Number.parseFloat(cleaned.replace(/,/g, "")) || 0
+    }
+
+    // Comma is the decimal separator (e.g., "1.234,56" or "256,54")
+    return Number.parseFloat(cleaned.replace(/\./g, "").replace(",", ".")) || 0
+  }
+
+  // Dot is the decimal separator (e.g., "1,234.56" or "256.54")
+  return Number.parseFloat(cleaned.replace(/,/g, "")) || 0
+}

test/javascript/parse_locale_float_test.mjs

@@ -0,0 +1,124 @@
+import { describe, it } from "node:test"
+import assert from "node:assert/strict"
+
+// Inline the function to avoid needing a bundler for ESM imports
+function parseLocaleFloat(value) {
+  if (typeof value !== "string") return Number.parseFloat(value) || 0
+
+  const cleaned = value.replace(/\s/g, "")
+  const lastComma = cleaned.lastIndexOf(",")
+  const lastDot = cleaned.lastIndexOf(".")
+
+  if (lastComma > lastDot) {
+    const digitsAfterComma = cleaned.length - lastComma - 1
+    if (lastDot === -1 && digitsAfterComma === 3) {
+      return Number.parseFloat(cleaned.replace(/,/g, "")) || 0
+    }
+
+    return Number.parseFloat(cleaned.replace(/\./g, "").replace(",", ".")) || 0
+  }
+
+  return Number.parseFloat(cleaned.replace(/,/g, "")) || 0
+}
+
+describe("parseLocaleFloat", () => {
+  describe("dot as decimal separator", () => {
+    it("parses simple decimal", () => {
+      assert.equal(parseLocaleFloat("256.54"), 256.54)
+    })
+
+    it("parses with thousands comma", () => {
+      assert.equal(parseLocaleFloat("1,234.56"), 1234.56)
+    })
+
+    it("parses multiple thousands separators", () => {
+      assert.equal(parseLocaleFloat("1,234,567.89"), 1234567.89)
+    })
+
+    it("parses integer with dot-zero", () => {
+      assert.equal(parseLocaleFloat("100.00"), 100)
+    })
+  })
+
+  describe("comma as decimal separator (European/French)", () => {
+    it("parses simple decimal", () => {
+      assert.equal(parseLocaleFloat("256,54"), 256.54)
+    })
+
+    it("parses with thousands dot", () => {
+      assert.equal(parseLocaleFloat("1.234,56"), 1234.56)
+    })
+
+    it("parses multiple thousands separators", () => {
+      assert.equal(parseLocaleFloat("1.234.567,89"), 1234567.89)
+    })
+
+    it("parses two-digit decimal", () => {
+      assert.equal(parseLocaleFloat("10,50"), 10.5)
+    })
+
+    it("parses single-digit decimal", () => {
+      assert.equal(parseLocaleFloat("10,5"), 10.5)
+    })
+  })
+
+  describe("ambiguous comma with 3 trailing digits treated as thousands separator", () => {
+    it("treats 1,234 as one thousand two hundred thirty-four", () => {
+      assert.equal(parseLocaleFloat("1,234"), 1234)
+    })
+
+    it("treats 12,345 as twelve thousand three hundred forty-five", () => {
+      assert.equal(parseLocaleFloat("12,345"), 12345)
+    })
+
+    it("treats 1,000 as one thousand", () => {
+      assert.equal(parseLocaleFloat("1,000"), 1000)
+    })
+  })
+
+  describe("integers", () => {
+    it("parses plain integer", () => {
+      assert.equal(parseLocaleFloat("100"), 100)
+    })
+
+    it("parses zero", () => {
+      assert.equal(parseLocaleFloat("0"), 0)
+    })
+  })
+
+  describe("whitespace handling", () => {
+    it("strips leading/trailing spaces", () => {
+      assert.equal(parseLocaleFloat("  256.54  "), 256.54)
+    })
+
+    it("strips thousands space separator", () => {
+      assert.equal(parseLocaleFloat("1 234,56"), 1234.56)
+    })
+  })
+
+  describe("edge cases", () => {
+    it("returns 0 for empty string", () => {
+      assert.equal(parseLocaleFloat(""), 0)
+    })
+
+    it("returns 0 for non-numeric string", () => {
+      assert.equal(parseLocaleFloat("abc"), 0)
+    })
+
+    it("returns 0 for undefined", () => {
+      assert.equal(parseLocaleFloat(undefined), 0)
+    })
+
+    it("returns 0 for null", () => {
+      assert.equal(parseLocaleFloat(null), 0)
+    })
+
+    it("passes through numeric values", () => {
+      assert.equal(parseLocaleFloat(42.5), 42.5)
+    })
+
+    it("returns 0 for NaN", () => {
+      assert.equal(parseLocaleFloat(NaN), 0)
+    })
+  })
+})