From 716b87b817179372dc0b60b102851a1d2128ff25 Mon Sep 17 00:00:00 2001
From: adam-james <21064735+adam-james-v@users.noreply.github.com>
Date: Thu, 5 Jan 2023 12:24:14 -0800
Subject: [PATCH] Disallow axis ranges with the same min/max, which causes
divide by zero error (#27518)
* Disallow axis ranges with the same min/max, which causes divide by zero error
The group-axes private fn is used to try group series axes together if the ranges of the series are 'close enough',
which is calculated by determining the percentage overlap the ranges share.
As the comparison is made, the first series in the list will always be compared with itself, and should have
1.0 (100%) overlap, as the ranges are going to be identical. The divide by zero issue arises when this first series,
and potentially any other series, has a 'range' where the min and max are the same. This happens if the series has the
same value in every row.
With this change, these ranges are considered invalid, and we avoid the divide by zero.
* Unskip e2e repro
---
.../27427-static-viz-divide-by-zero.cy.spec.js | 2 +-
src/metabase/pulse/render/body.clj | 6 ++++--
test/metabase/pulse/render/body_test.clj | 18 +++++++++++++++++-
3 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/frontend/test/metabase/scenarios/visualizations/reproductions/27427-static-viz-divide-by-zero.cy.spec.js b/frontend/test/metabase/scenarios/visualizations/reproductions/27427-static-viz-divide-by-zero.cy.spec.js
index 848587ef32c..75b74764d55 100644
--- a/frontend/test/metabase/scenarios/visualizations/reproductions/27427-static-viz-divide-by-zero.cy.spec.js
+++ b/frontend/test/metabase/scenarios/visualizations/reproductions/27427-static-viz-divide-by-zero.cy.spec.js
@@ -16,7 +16,7 @@ const questionDetails = {
},
};
-describe.skip("issue 27427", () => {
+describe("issue 27427", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
diff --git a/src/metabase/pulse/render/body.clj b/src/metabase/pulse/render/body.clj
index 13799d31f24..7a391f3e4b5 100644
--- a/src/metabase/pulse/render/body.clj
+++ b/src/metabase/pulse/render/body.clj
@@ -548,7 +548,10 @@
[col-a col-b]
(let [[min-a min-b] (map #(get-in % [:fingerprint :type :type/Number :min]) [col-a col-b])
[max-a max-b] (map #(get-in % [:fingerprint :type :type/Number :max]) [col-a col-b])
- valid-ranges? (and min-a min-b max-a max-b)
+ valid-ranges? (and min-a min-b max-a max-b
+ ;; ranges with same min and max won't be considered ranges.
+ (not= min-a max-a)
+ (not= min-b max-b))
overlapping-and-valid? (and valid-ranges?
(or (<= min-a min-b max-a)
(<= min-a max-b max-a)))]
@@ -739,7 +742,6 @@
series-seqs [(if (= (count x-cols) 1)
(single-x-axis-combo-series enforced-type joined-rows x-cols y-cols data card-name)
(double-x-axis-combo-series enforced-type joined-rows x-cols y-cols data card-name))]
-
labels (combo-label-info x-cols y-cols viz-settings)
settings (->ts-viz (first x-cols) (first y-cols) labels viz-settings)]
(image-bundle/make-image-bundle
diff --git a/test/metabase/pulse/render/body_test.clj b/test/metabase/pulse/render/body_test.clj
index f914bdeb764..e7f993de2f6 100644
--- a/test/metabase/pulse/render/body_test.clj
+++ b/test/metabase/pulse/render/body_test.clj
@@ -646,7 +646,23 @@
(testing "Multiple series with close values does not split y-axis."
(is (not (axes-split? rows))))
(testing "Multiple series with far values does split y-axis."
- (is (axes-split? (conj rows ["D" 3 70]))))))
+ (is (axes-split? (conj rows ["D" 3 70]))))
+ (testing "Multiple axes split does not fail when a series has the same value for all of its rows #27427"
+ (let [rows [["Category" "Series A" "Series B"]
+ ["A" 1 1.3]
+ ["B" 1 1.9]
+ ["C" 1 4]]
+ axes-split? (fn [rows]
+ (let [text (-> rows first last)]
+ ;; there is always 1 node with the series name in the legend
+ ;; so we see if the series name shows up a second time, which will
+ ;; be the axis label, indicating that there is indeed a split
+ (< 1 (-> rows
+ (render.tu/make-viz-data :bar {})
+ :viz-tree
+ (render.tu/nodes-with-text text)
+ count))))]
+ (is (axes-split? rows))))))
(deftest ^:parallel x-and-y-axis-label-info-test
(let [x-col {:display_name "X col"}
--
GitLab