From 7dc9d02eb8a582ef8248397857d7a00ae3d039cb Mon Sep 17 00:00:00 2001
From: Howon Lee <hlee.howon@gmail.com>
Date: Fri, 21 Jan 2022 10:34:03 -0800
Subject: [PATCH] Static viz card vs. normal errors 18676 (#19771)
Static viz errors are of basically two kinds:
1. Someone futzed up their query, which we can't do anything about
2. We futzed up the rendering, which is our fault.
Display those two errors differently.
---
src/metabase/pulse/render.clj | 11 ++++++++---
src/metabase/pulse/render/body.clj | 23 +++++++++++++++++++++--
test/metabase/pulse/render/body_test.clj | 8 ++++++++
test/metabase/pulse/render_test.clj | 7 +++++++
4 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/src/metabase/pulse/render.clj b/src/metabase/pulse/render.clj
index 9af34674c1b..2e5c195b383 100644
--- a/src/metabase/pulse/render.clj
+++ b/src/metabase/pulse/render.clj
@@ -133,7 +133,7 @@
[render-type timezone-id :- (s/maybe s/Str) card dashcard {:keys [data error], :as results}]
(try
(when error
- (throw (ex-info (tru "Card has errors: {0}" error) results)))
+ (throw (ex-info (tru "Card has errors: {0}" error) (assoc results :card-error true))))
(let [chart-type (or (detect-pulse-chart-type card dashcard data)
(when (is-attached? card)
:attached)
@@ -141,8 +141,13 @@
(log/debug (trs "Rendering pulse card with chart-type {0} and render-type {1}" chart-type render-type))
(body/render chart-type render-type timezone-id card dashcard data))
(catch Throwable e
- (log/error e (trs "Pulse card render error"))
- (body/render :error nil nil nil nil nil))))
+ (if (:card-error (ex-data e))
+ (do
+ (log/error e (trs "Pulse card query error"))
+ (body/render :card-error nil nil nil nil nil))
+ (do
+ (log/error e (trs "Pulse card render error"))
+ (body/render :render-error nil nil nil nil nil))))))
(defn- card-href
[card]
diff --git a/src/metabase/pulse/render/body.clj b/src/metabase/pulse/render/body.clj
index 4478dffe142..7125a65a505 100644
--- a/src/metabase/pulse/render/body.clj
+++ b/src/metabase/pulse/render/body.clj
@@ -21,8 +21,23 @@
[schema.core :as s])
(:import [java.text DecimalFormat DecimalFormatSymbols]))
+(def ^:private card-error-rendered-info
+ "Default rendered-info map when there is an error running a card on the card run.
+ Is a delay due to the call to `trs`."
+ (delay {:attachments
+ nil
+
+ :content
+ [:div {:style (style/style
+ (style/font-style)
+ {:color style/color-error
+ :font-weight 700
+ :padding :16px})}
+ (trs "There was a problem with this question.")]}))
+
(def ^:private error-rendered-info
- "Default rendered-info map when there is an error displaying a card. Is a delay due to the call to `trs`."
+ "Default rendered-info map when there is an error displaying a card on the static viz side.
+ Is a delay due to the call to `trs`."
(delay {:attachments
nil
@@ -839,6 +854,10 @@
[:br]
(trs "Please view this card in Metabase.")]})
-(s/defmethod render :error :- common/RenderedPulseCard
+(s/defmethod render :card-error :- common/RenderedPulseCard
+ [_ _ _ _ _ _]
+ @card-error-rendered-info)
+
+(s/defmethod render :render-error :- common/RenderedPulseCard
[_ _ _ _ _ _]
@error-rendered-info)
diff --git a/test/metabase/pulse/render/body_test.clj b/test/metabase/pulse/render/body_test.clj
index 8a307e011aa..133af07f79f 100644
--- a/test/metabase/pulse/render/body_test.clj
+++ b/test/metabase/pulse/render/body_test.clj
@@ -250,6 +250,14 @@
{:cols test-columns-with-date-semantic-type :rows test-data}
(count test-columns))))))
+(deftest error-test
+ (testing "renders error"
+ (= "An error occurred while displaying this card."
+ (-> (body/render :render-error nil nil nil nil nil) :content last)))
+ (testing "renders card error"
+ (= "There was a problem with this question."
+ (-> (body/render :card-error nil nil nil nil nil) :content last))))
+
(defn- render-scalar-value [results]
(-> (body/render :scalar nil pacific-tz nil nil results)
:content
diff --git a/test/metabase/pulse/render_test.clj b/test/metabase/pulse/render_test.clj
index dce4112b765..77398d58275 100644
--- a/test/metabase/pulse/render_test.clj
+++ b/test/metabase/pulse/render_test.clj
@@ -30,6 +30,13 @@
:breakout [!month.date]}))
[:td _ "November 2015"])))))
+(deftest render-error-test
+ (testing "gives us a proper error if we have erroring card"
+ (is (= (get-in (render/render-pulse-card-for-display
+ nil nil
+ {:error "some error"}) [1 2 4 2 2])
+ "There was a problem with this question."))))
+
(deftest detect-pulse-chart-type-test
(is (= :scalar
(render/detect-pulse-chart-type {:display :anything}
--
GitLab