Update pulse code to allow user-specified x and y axes

The previous code had two issues. The first was a bug in using the
user-specified y-axis column for checking if a goal had been
met. Beyond that the pulse code didn't recognize those user specified
x and y axes either. This commit uses the user specified columns, when
they are not present defaults to the previous first/second column
behavior in the resultset.

src/metabase/pulse.clj

@@ -97,19 +97,19 @@
   (every? is-card-empty? results))
 
 (defn- goal-met? [{:keys [alert_above_goal] :as pulse} results]
-  (let [first-result    (first results)
-        goal-comparison (if alert_above_goal <= >=)
-        comparison-col-index (ui/goal-comparison-column first-result)
-        goal-val (ui/find-goal-value first-result)]
+  (let [first-result         (first results)
+        goal-comparison      (if alert_above_goal <= >=)
+        goal-val             (ui/find-goal-value first-result)
+        comparison-col-rowfn (ui/make-goal-comparison-rowfn (:card first-result)
+                                                            (get-in first-result [:result :data]))]
 
-    (when-not (and goal-val comparison-col-index)
+    (when-not (and goal-val comparison-col-rowfn)
       (throw (Exception. (str (tru "Unable to compare results to goal for alert.")
-                              (tru "Question ID is '{0}' with visualization settings '{1}'"
+                              (tru "Question ID is ''{0}'' with visualization settings ''{1}''"
                                    (get-in results [:card :id])
                                    (pr-str (get-in results [:card :visualization_settings])))))))
-
     (some (fn [row]
-            (goal-comparison goal-val (nth row comparison-col-index)))
+            (goal-comparison goal-val (comparison-col-rowfn row)))
           (get-in first-result [:result :data :rows]))))
 
 (defn- alert-or-pulse [pulse]

src/metabase/pulse/render.clj

@@ -10,7 +10,9 @@
             [clojure.tools.logging :as log]
             [hiccup.core :refer [h html]]
             [metabase.util :as u]
-            [metabase.util.urls :as urls]
+            [metabase.util
+             [ui-logic :as ui-logic]
+             [urls :as urls]]
             [puppetlabs.i18n.core :refer [tru trs]]
             [schema.core :as s])
   (:import cz.vutbr.web.css.MediaSpec
@@ -483,30 +485,35 @@
     :attachment {content-id image-url}
     :inline     nil))
 
+(defn- graphing-columns [card {:keys [cols] :as data}]
+  [(or (ui-logic/x-axis-rowfn card data)
+       first)
+   (or (ui-logic/y-axis-rowfn card data)
+       second)])
+
 (s/defn ^:private render:sparkline :- RenderedPulseCard
-  [render-type timezone card {:keys [rows cols]}]
-  (let [ft-row (if (datetime-field? (first cols))
+  [render-type timezone card {:keys [rows cols] :as data}]
+  (let [[x-axis-rowfn y-axis-rowfn] (graphing-columns card data)
+        ft-row (if (datetime-field? (x-axis-rowfn cols))
                  #(.getTime ^Date (u/->Timestamp %))
                  identity)
-        rows   (if (> (ft-row (ffirst rows))
-                      (ft-row (first (last rows))))
+        rows   (if (> (ft-row (x-axis-rowfn (first rows)))
+                      (ft-row (x-axis-rowfn (last rows))))
                  (reverse rows)
                  rows)
-        xs     (for [row  rows
-                     :let [x (first row)]]
-                 (ft-row x))
+        xs     (map (comp ft-row x-axis-rowfn) rows)
         xmin   (apply min xs)
         xmax   (apply max xs)
         xrange (- xmax xmin)
         xs'    (map #(/ (double (- % xmin)) xrange) xs)
-        ys     (map second rows)
+        ys     (map y-axis-rowfn rows)
         ymin   (apply min ys)
         ymax   (apply max ys)
         yrange (max 1 (- ymax ymin))                    ; `(max 1 ...)` so we don't divide by zero
         ys'    (map #(/ (double (- % ymin)) yrange) ys) ; cast to double to avoid "Non-terminating decimal expansion" errors
         rows'  (reverse (take-last 2 rows))
-        values (map (comp format-number second) rows')
-        labels (format-timestamp-pair timezone (map first rows') (first cols))
+        values (map (comp format-number y-axis-rowfn) rows')
+        labels (format-timestamp-pair timezone (map x-axis-rowfn rows') (x-axis-rowfn cols))
         image-bundle (make-image-bundle render-type (render-sparkline-to-png xs' ys' 524 130))]
 
     {:attachments (when image-bundle
@@ -550,11 +557,12 @@
 (defn detect-pulse-card-type
   "Determine the pulse (visualization) type of a CARD, e.g. `:scalar` or `:bar`."
   [card data]
-  (let [col-count (-> data :cols count)
-        row-count (-> data :rows count)
-        col-1 (-> data :cols first)
-        col-2 (-> data :cols second)
-        aggregation (-> card :dataset_query :query :aggregation first)]
+  (let [col-count                 (-> data :cols count)
+        row-count                 (-> data :rows count)
+        [col-1-rowfn col-2-rowfn] (graphing-columns card data)
+        col-1                     (col-1-rowfn (:cols data))
+        col-2                     (col-2-rowfn (:cols data))
+        aggregation               (-> card :dataset_query :query :aggregation first)]
     (cond
       (or (= aggregation :rows)
           (contains? #{:pin_map :state :country} (:display card))) nil

src/metabase/util/ui_logic.clj

@@ -27,11 +27,8 @@
   "For graphs with goals, this function returns the index of the default column that should be used to compare against
   the goal. This follows the frontend code getDefaultLineAreaBarColumns closely with a slight change (detailed in the
   code)"
-  [results]
-  (let [graph-type (get-in results [:card :display])
-        [col-1 col-2 col-3 :as all-cols] (get-in results [:result :data :cols])
-        cols-count (count all-cols)]
-
+  [{graph-type :display :as card} {[col-1 col-2 col-3 :as all-cols] :cols :as result}]
+  (let [cols-count (count all-cols)]
     (cond
       ;; Progress goals return a single row and column, compare that
       (= :progress graph-type)
@@ -63,19 +60,39 @@
 
 (defn- column-name->index
   "The results seq is seq of vectors, this function returns the index in that vector of the given `COLUMN-NAME`"
-  [results ^String column-name]
-  (when column-name
-    (first (map-indexed (fn [idx column]
-                          (when (.equalsIgnoreCase column-name (:name column))
-                            idx))
-                        (get-in results [:result :data :cols])))))
-
-(defn goal-comparison-column
+  [^String column-name {:keys [cols] :as result}]
+  (first (remove nil? (map-indexed (fn [idx column]
+                                     (when (.equalsIgnoreCase column-name (:name column))
+                                       idx))
+                                   cols))))
+
+(defn- graph-column-index [viz-kwd card results]
+  (when-let [metrics-col-index (some-> card
+                                       (get-in [:visualization_settings viz-kwd])
+                                       first
+                                       (column-name->index results))]
+    (fn [row]
+      (nth row metrics-col-index))))
+
+(defn y-axis-rowfn
+  "This is used as the Y-axis column in the UI"
+  [card results]
+  (graph-column-index :graph.metrics card results))
+
+(defn x-axis-rowfn
+  "This is used as the X-axis column in the UI"
+  [card results]
+  (graph-column-index :graph.dimensions card results))
+
+(defn make-goal-comparison-rowfn
   "For a given resultset, return the index of the column that should be used for the goal comparison. This can come
   from the visualization settings if the column is specified, or from our default column logic"
-  [result]
-  (or (column-name->index result (get-in result [:card :visualization_settings :graph.metrics]))
-      (default-goal-column-index result)))
+  [card result]
+  (if-let [user-specified-rowfn (y-axis-rowfn card result)]
+    user-specified-rowfn
+    (when-let [default-col-index (default-goal-column-index card result)]
+      (fn [row]
+        (nth row default-col-index)))))
 
 (defn find-goal-value
   "The goal value can come from a progress goal or a graph goal_value depending on it's type"

test/metabase/pulse_test.clj

@@ -228,6 +228,34 @@
      (send-pulse! (retrieve-pulse-or-alert pulse-id))
      (et/summarize-multipart-email #"Test card.*has reached its goal"))))
 
+;; Native query with user-specified x and y axis
+(expect
+  (rasta-alert-email "Metabase alert: Test card has reached its goal"
+                     [{"Test card.*has reached its goal" true}, png-attachment])
+  (tt/with-temp* [Card                  [{card-id :id}  {:name          "Test card"
+                                                         :dataset_query {:database (data/id)
+                                                                         :type     :native
+                                                                         :native   {:query (str "select count(*) as total_per_day, date as the_day "
+                                                                                                "from checkins "
+                                                                                                "group by date")}}
+                                                         :display :line
+                                                         :visualization_settings {:graph.show_goal true
+                                                                                  :graph.goal_value 5.9
+                                                                                  :graph.dimensions ["the_day"]
+                                                                                  :graph.metrics ["total_per_day"]}}]
+                  Pulse                 [{pulse-id :id} {:alert_condition  "goal"
+                                                         :alert_first_only false
+                                                         :alert_above_goal true}]
+                  PulseCard             [_             {:pulse_id pulse-id
+                                                        :card_id  card-id
+                                                        :position 0}]
+                  PulseChannel          [{pc-id :id}   {:pulse_id pulse-id}]
+                  PulseChannelRecipient [_             {:user_id          (rasta-id)
+                                                        :pulse_channel_id pc-id}]]
+    (email-test-setup
+     (send-pulse! (retrieve-pulse-or-alert pulse-id))
+     (et/summarize-multipart-email #"Test card.*has reached its goal"))))
+
 ;; Above goal alert, with no data above goal
 (expect
   {}