diff --git a/frontend/src/metabase/static-viz/components/FunnelChart/FunnelChart.tsx b/frontend/src/metabase/static-viz/components/FunnelChart/FunnelChart.tsx index 0baaf35d51b906b0243c3a66599c6a1194c8084c..a1f65e359d92b6d60bac9cb2bccdcf86263b16d9 100644 --- a/frontend/src/metabase/static-viz/components/FunnelChart/FunnelChart.tsx +++ b/frontend/src/metabase/static-viz/components/FunnelChart/FunnelChart.tsx @@ -118,10 +118,10 @@ const Funnel = ({ data, settings }: FunnelProps) => { <> <Text textAnchor="end" - fontWeight={700} y={firstMeasureTop} fontSize={layout.initialMeasureFontSize} fill="black" + style={{ fontWeight: 700 }} > {measure} </Text> diff --git a/src/metabase/pulse/render/body.clj b/src/metabase/pulse/render/body.clj index 38ddc011bbb14cdd5ad7097bb92efeab3931ac73..5c02fb07550935af39131589b5c0f347b1fb8557 100644 --- a/src/metabase/pulse/render/body.clj +++ b/src/metabase/pulse/render/body.clj @@ -686,7 +686,10 @@ (if (and last-value previous-value unit last-change) (let [value (format-cell timezone-id last-value metric-col viz-settings) previous (format-cell timezone-id previous-value metric-col viz-settings) - adj (if (pos? last-change) (tru "Up") (tru "Down"))] + adj (if (pos? last-change) (tru "Up") (tru "Down")) + delta-statement (if (= last-value previous-value) + "No change." + (str adj " " (percentage last-change) "."))] {:attachments nil :content [:div [:div {:style (style/style (style/scalar-style))} @@ -695,10 +698,10 @@ :font-size :16px :font-weight 700 :padding-right :16px})} - adj " " (percentage last-change) "." + delta-statement " Was " previous " last " (format-unit unit)]] :render/text (str value "\n" - adj " " (percentage last-change) "." + delta-statement " Was " previous " last " (format-unit unit))}) ;; In other words, defaults to plain scalar if we don't have actual changes {:attachments nil @@ -796,16 +799,17 @@ (s/defmethod render :funnel :- common/RenderedPulseCard [_ render-type timezone-id card _ {:keys [rows cols viz-settings] :as data}] - ;; x-axis-rowfn is always first, y-axis-rowfn is always second - (let [rows (common/row-preprocess first second rows) - [x-col y-col] cols - settings (->js-viz x-col y-col viz-settings) - settings (assoc settings - :step {:name (:display_name x-col) - :format (:x settings)} - :measure {:format (:y settings)}) - svg (js-svg/funnel rows settings) - image-bundle (image-bundle/make-image-bundle render-type svg)] + (let [[x-axis-rowfn + y-axis-rowfn] (common/graphing-column-row-fns card data) + rows (map (juxt x-axis-rowfn y-axis-rowfn) + (common/row-preprocess x-axis-rowfn y-axis-rowfn rows)) + [x-col y-col] cols + settings (as-> (->js-viz x-col y-col viz-settings) jsviz-settings + (assoc jsviz-settings :step {:name (:display_name x-col) + :format (:x jsviz-settings)} + :measure {:format (:y jsviz-settings)})) + svg (js-svg/funnel rows settings) + image-bundle (image-bundle/make-image-bundle render-type svg)] {:attachments (image-bundle/image-bundle->attachment image-bundle) diff --git a/test/metabase/pulse/render/body_test.clj b/test/metabase/pulse/render/body_test.clj index 133af07f79f251fe172daad52759ee2018c59197..0f61bb8cc898c3f0dd02c7c7885d11ac635558e1 100644 --- a/test/metabase/pulse/render/body_test.clj +++ b/test/metabase/pulse/render/body_test.clj @@ -308,13 +308,14 @@ :render/text (s/eq "foo")} (body/render :scalar nil pacific-tz nil nil results))))) (testing "for smartscalars" - (let [results {:cols [{:name "value", - :display_name "VALUE", - :base_type :type/Decimal} - {:name "time", - :display_name "TIME", - :base_type :type/DateTime - :effective_type :type/DateTime}] + (let [cols [{:name "value", + :display_name "VALUE", + :base_type :type/Decimal} + {:name "time", + :display_name "TIME", + :base_type :type/DateTime + :effective_type :type/DateTime}] + results {:cols cols :rows [[40.0 :this-month] [30.0 :last-month] [20.0 :month-before]] @@ -323,14 +324,17 @@ :last-change 1.333333 :col "value" :last-value 40.0}]} + sameres {:cols cols + :rows [[40.0 :this-month] + [40.0 :last-month] + [40.0 :month-before]] + :insights [{:previous-value 40.0 + :unit :month + :last-change 1.0 + :col "value" + :last-value 40.0}]} ;; by "dumb" it is meant "without nonnil insights" - dumbres {:cols [{:name "value", - :display_name "VALUE", - :base_type :type/Decimal} - {:name "time", - :display_name "TIME", - :base_type :type/DateTime - :effective_type :type/DateTime}] + dumbres {:cols cols :rows [[20.0 :month-before]] :insights [{:previous-value nil :unit nil @@ -339,6 +343,8 @@ :last-value 20.0}]}] (is (= "40.00\nUp 133.33%. Was 30.00 last month" (:render/text (body/render :smartscalar nil pacific-tz nil nil results)))) + (is (= "40.00\nNo change. Was 40.00 last month" + (:render/text (body/render :smartscalar nil pacific-tz nil nil sameres)))) (is (= "20.0\nNothing to compare to." (:render/text (body/render :smartscalar nil pacific-tz nil nil dumbres)))) (is (schema= {:attachments (s/eq nil) @@ -418,7 +424,7 @@ :base_type :type/BigInteger :semantic_type nil}]) -(def ^:private default-combo-columns +(def ^:private default-multi-columns [{:name "Price", :display_name "Price", :base_type :type/BigInteger @@ -465,7 +471,7 @@ (testing "Check multiseries in one card but without explicit combo" (is (has-inline-image? (render-multiseries-bar-graph - {:cols default-combo-columns + {:cols default-multi-columns :rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]]}))))) (defn- render-area-graph [results] @@ -494,7 +500,7 @@ (testing "Check multiseries in one card but without explicit combo" (is (has-inline-image? (render-multiseries-area-graph - {:cols default-combo-columns + {:cols default-multi-columns :rows [[10.0 1 1231 1] [5.0 10 nil 111] [2.50 20 11 1] [1.25 nil 1231 11]]}))))) (defn- render-waterfall [results] @@ -531,15 +537,15 @@ (deftest render-combo-test (testing "Render a combo graph with non-nil values for the x and y axis" (is (has-inline-image? - (render-combo {:cols default-combo-columns + (render-combo {:cols default-multi-columns :rows [[10.0 1 123 111] [5.0 10 12 111] [2.50 20 1337 12312] [1.25 30 -22 123124]]})))) (testing "Render a combo graph with multiple x axes" (is (has-inline-image? - (render-combo-multi-x {:cols default-combo-columns + (render-combo-multi-x {:cols default-multi-columns :rows [[10.0 "Bob" 123 123124] [5.0 "Dobbs" 12 23423] [2.50 "Robbs" 1337 234234] [1.25 "Mobbs" -22 1234123]]})))) (testing "Check to make sure we allow nil values for any axis" (is (has-inline-image? - (render-combo {:cols default-combo-columns + (render-combo {:cols default-multi-columns :rows [[nil 1 1 23453] [10.0 1 nil nil] [5.0 10 22 1337] [2.50 nil 22 1231] [1.25 nil nil 1231232]]}))))) ;; Test rendering a sparkline @@ -587,6 +593,11 @@ (render-funnel {:cols default-columns :rows [[10.0 1] [5.0 10] [2.50 20] [1.25 30]]})))) + (testing "Test that we can render a funnel with extraneous columns and also weird strings stuck in places" + (is (has-inline-image? + (render-funnel + {:cols default-multi-columns + :rows [[10.0 1 2 2] [5.0 10 "11.1" 1] ["2.50" 20 1337 0] [1.25 30 -2 "-2"]]})))) (testing "Test that we can have some nil values stuck everywhere" (is (has-inline-image? (render-funnel