From ede9600751df757bdcbe2da2bace1a1ea793b616 Mon Sep 17 00:00:00 2001
From: Noah Moss <32746338+noahmoss@users.noreply.github.com>
Date: Mon, 6 Dec 2021 15:30:33 -0500
Subject: [PATCH] Omit thousands separator in XLSX format string if specified
in viz setting (#19228)
---
src/metabase/query_processor/streaming/xlsx.clj | 11 +++++++++--
test/metabase/query_processor/streaming/xlsx_test.clj | 9 ++++++++-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/metabase/query_processor/streaming/xlsx.clj b/src/metabase/query_processor/streaming/xlsx.clj
index be282221a80..751b2b04d4a 100644
--- a/src/metabase/query_processor/streaming/xlsx.clj
+++ b/src/metabase/query_processor/streaming/xlsx.clj
@@ -24,6 +24,7 @@
(def ^:private number-setting-keys
"If any of these settings are present, we should format the column as a number."
#{::mb.viz/number-style
+ ::mb.viz/number-separators
::mb.viz/currency
::mb.viz/currency-style
::mb.viz/currency-in-header
@@ -95,6 +96,7 @@
;; Custom number formatting options are not set
(not (seq (dissoc format-settings
::mb.viz/number-style
+ ::mb.viz/number-separators
::mb.viz/scale
::mb.viz/prefix
::mb.viz/suffix)))))
@@ -111,10 +113,15 @@
merged-settings (if is-currency?
(merge-global-settings format-settings :type/Currency)
format-settings)
+ base-string (if (= (::mb.viz/number-separators format-settings) ".")
+ ;; Omit thousands separator if ommitted in the format settings. Otherwise ignore
+ ;; number separator settings, since custom separators are not supported in XLSX.
+ "###0"
+ "#,##0")
base-strings (if (default-number-format? merged-settings)
;; [int-format, float-format]
- ["#,##0", "#,##0.##"]
- (repeat 2 (apply str "#,##0" (when (> decimals 0) (apply str "." (repeat decimals "0"))))))]
+ [base-string (str base-string ".##")]
+ (repeat 2 (apply str base-string (when (> decimals 0) (apply str "." (repeat decimals "0"))))))]
(condp = (::mb.viz/number-style merged-settings)
"percent"
(map #(str % "%") base-strings)
diff --git a/test/metabase/query_processor/streaming/xlsx_test.clj b/test/metabase/query_processor/streaming/xlsx_test.clj
index ad78c9fa36e..f8a92e57ba3 100644
--- a/test/metabase/query_processor/streaming/xlsx_test.clj
+++ b/test/metabase/query_processor/streaming/xlsx_test.clj
@@ -61,7 +61,14 @@
(is (= "#,##0E+0" (format-string {::mb.viz/decimals -1, ::mb.viz/number-style "scientific"})))
(is (= "[$$]#,##0" (format-string {::mb.viz/decimals -1,
::mb.viz/currency-in-header false,
- ::mb.viz/number-style "currency"}))))
+ ::mb.viz/number-style "currency"})))
+
+ ;; Thousands separator can be omitted
+ (is (= ["###0" "###0.##"] (format-string {::mb.viz/number-separators "."})))
+ ;; Custom separators are not supported
+ (is (= ["#,##0" "#,##0.##"] (format-string {::mb.viz/number-separators ", "})))
+ (is (= ["#,##0" "#,##0.##"] (format-string {::mb.viz/number-separators ".,"})))
+ (is (= ["#,##0" "#,##0.##"] (format-string {::mb.viz/number-separators ".’"}))))
(testing "Scale"
;; Scale should not affect format string since it is applied to the actual data prior to export
--
GitLab