From ba02a664a918915460c0969cfa44a49af3487d02 Mon Sep 17 00:00:00 2001
From: Ibe Dwi <hi@ibedwi.dev>
Date: Wed, 15 Nov 2023 03:01:33 +0700
Subject: [PATCH] Fix misplaced negative sign on a currency visualization in
 Dashboard (#35632)

* fix: misplaced negative sign

* update formatting unit test

* chore: update test name
---
 .../src/metabase/lib/formatting/numbers.tsx   |  9 ++-
 .../lib/formatting/numbers.unit.spec.js       | 73 +++++++++++++++++++
 .../test/metabase/lib/formatting.unit.spec.js |  2 +-
 3 files changed, 82 insertions(+), 2 deletions(-)
 create mode 100644 frontend/src/metabase/lib/formatting/numbers.unit.spec.js

diff --git a/frontend/src/metabase/lib/formatting/numbers.tsx b/frontend/src/metabase/lib/formatting/numbers.tsx
index abbd9804553..11643af2e2d 100644
--- a/frontend/src/metabase/lib/formatting/numbers.tsx
+++ b/frontend/src/metabase/lib/formatting/numbers.tsx
@@ -182,7 +182,14 @@ function formatNumberCompact(value: number, options: FormatNumberOptionsType) {
       const { value: currency } = nf
         .formatToParts(value)
         .find((p: any) => p.type === "currency");
-      return currency + formatNumberCompactWithoutOptions(value);
+
+      const valueSign = value < 0 ? "-" : "";
+
+      return (
+        valueSign +
+        currency +
+        formatNumberCompactWithoutOptions(Math.abs(value))
+      );
     } catch (e) {
       // Intl.NumberFormat failed, so we fall back to a non-currency number
       return formatNumberCompactWithoutOptions(value);
diff --git a/frontend/src/metabase/lib/formatting/numbers.unit.spec.js b/frontend/src/metabase/lib/formatting/numbers.unit.spec.js
new file mode 100644
index 00000000000..ad48aff0061
--- /dev/null
+++ b/frontend/src/metabase/lib/formatting/numbers.unit.spec.js
@@ -0,0 +1,73 @@
+import { formatNumber, numberFormatterForOptions } from "./numbers";
+
+describe("formatNumber", () => {
+  it("should show the correct currency format (metabase#34242)", () => {
+    const numberFormatter = numberFormatterForOptions({
+      number_style: "currency",
+      currency: "USD",
+      currency_style: "symbol",
+      maximumFractionDigits: 2,
+    });
+
+    const compactResult = formatNumber(-500000, {
+      jsx: true,
+      remap: true,
+      field: "-500000",
+      currency: "USD",
+      number_style: "currency",
+      currency_style: "symbol",
+      currency_in_header: true,
+      number_separators: ".,",
+      _numberFormatter: numberFormatter,
+      _header_unit: "$",
+      column: {
+        display_name: "-500000",
+        source: "native",
+        field_ref: [
+          "field",
+          "-500000",
+          {
+            "base-type": "type/Integer",
+          },
+        ],
+        name: "-500000",
+        base_type: "type/Integer",
+        effective_type: "type/Integer",
+      },
+      _column_title_full: "-500000 ($)",
+      compact: true,
+    });
+    expect(compactResult).toEqual("-$500.0k");
+
+    const fullResult = formatNumber(-500000, {
+      compact: false,
+      maximumFractionDigits: 2,
+      jsx: true,
+      remap: true,
+      field: "-500000",
+      currency: "USD",
+      number_style: "currency",
+      currency_style: "symbol",
+      currency_in_header: true,
+      number_separators: ".,",
+      _numberFormatter: numberFormatter,
+      _header_unit: "$",
+      column: {
+        display_name: "-500000",
+        source: "native",
+        field_ref: [
+          "field",
+          "-500000",
+          {
+            "base-type": "type/Integer",
+          },
+        ],
+        name: "-500000",
+        base_type: "type/Integer",
+        effective_type: "type/Integer",
+      },
+      _column_title_full: "-500000 ($)",
+    });
+    expect(fullResult).toEqual("-$500,000.00");
+  });
+});
diff --git a/frontend/test/metabase/lib/formatting.unit.spec.js b/frontend/test/metabase/lib/formatting.unit.spec.js
index 39115b7f0bc..bf9a482dcca 100644
--- a/frontend/test/metabase/lib/formatting.unit.spec.js
+++ b/frontend/test/metabase/lib/formatting.unit.spec.js
@@ -156,7 +156,7 @@ describe("formatting", () => {
         expect(formatNumber(724.9, options)).toEqual("$724.90");
         expect(formatNumber(1234.56, options)).toEqual("$1.2k");
         expect(formatNumber(1234567.89, options)).toEqual("$1.2M");
-        expect(formatNumber(-1234567.89, options)).toEqual("$-1.2M");
+        expect(formatNumber(-1234567.89, options)).toEqual("-$1.2M");
         expect(
           formatNumber(1234567.89, { ...options, currency: "CNY" }),
         ).toEqual("CNÂ¥1.2M");
-- 
GitLab