[Fixes #12653] Remove nil rows when rendering a sparkline pulse (#13489)

src/metabase/pulse/render/body.clj

@@ -217,7 +217,7 @@
   [_ render-type timezone-id card {:keys [rows cols] :as data}]
   (let [[x-axis-rowfn
          y-axis-rowfn] (common/graphing-column-row-fns card data)
-        rows           (sparkline/sparkline-rows timezone-id card data)
+        rows           (sparkline/cleaned-rows timezone-id card data)
         last-rows      (reverse (take-last 2 rows))
         values         (for [row last-rows]
                          (some-> row y-axis-rowfn common/format-number))

src/metabase/pulse/render/sparkline.clj

@@ -99,16 +99,14 @@
     (image-bundle/make-image-bundle render-type (render-sparkline-to-png x-axis-values y-axis-values))))
 
 
-(s/defn sparkline-rows
+(s/defn cleaned-rows
   "Get sorted rows from query results, with nils removed, appropriate for rendering as a sparkline."
   [timezone-id :- (s/maybe s/Str) card {:keys [rows cols], :as data}]
-  (let [[x-axis-rowfn
-         y-axis-rowfn] (common/graphing-column-row-fns card data)
-        format-val     (format-val-fn timezone-id cols x-axis-rowfn)]
-    (common/non-nil-rows
-     x-axis-rowfn
-     y-axis-rowfn
-     (if (> (format-val (x-axis-rowfn (first rows)))
-            (format-val (x-axis-rowfn (last rows))))
-       (reverse rows)
-       rows))))
+  (let [[x-axis-rowfn y-axis-rowfn] (common/graphing-column-row-fns card data)
+        format-val                  (format-val-fn timezone-id cols x-axis-rowfn)
+        present-rows                (common/non-nil-rows x-axis-rowfn y-axis-rowfn rows)
+        formatted-value             (comp format-val x-axis-rowfn)]
+    (if (> (formatted-value (first present-rows))
+           (formatted-value (last present-rows)))
+      (reverse present-rows)
+      present-rows)))

test/metabase/pulse/render/sparkline_test.clj

 (ns metabase.pulse.render.sparkline-test
   (:require [clojure.test :refer :all]
             [java-time :as t]
-            [metabase.pulse.render.sparkline :as sparkline]))
+            [metabase.models.card :refer [Card]]
+            [metabase.pulse.render.sparkline :as sparkline]
+            [metabase.test :as mt]))
 
 (deftest format-val-fn-test
   "Make sure format-val-fn works correctly for all of the various temporal types"
@@ -20,3 +22,12 @@
       (testing (format "^%s %s" (.getName (class x)) x)
         (is (= true
                (boolean (f x))))))))
+
+(deftest cleaned-rows-test
+  (mt/with-temp Card [card]
+    (testing "it removes nils"
+      (is (=
+           [[1 10]
+            [2 20]]
+           (sparkline/cleaned-rows nil card {:rows [[1 10] [2 20] [nil 30] [4 nil]]
+                                               :cols []}))))))