Merge pull request #2413 from metabase/pulse-tidy

Remove stray error message in pulses

src/metabase/api/pulse.clj

@@ -13,7 +13,7 @@
                              [pulse-channel :refer [channel-types]])
             [metabase.query-processor :as qp]
             [metabase.pulse :as p]
-            [metabase.task.send-pulses :refer [send-pulse!]]
+            [metabase.pulse.render :as render]
             [metabase.util :as u]))
 
 
@@ -87,9 +87,9 @@
   (let [card (Card id)]
     (read-check Database (:database (:dataset_query card)))
     (let [data (:data (qp/dataset-query (:dataset_query card) {:executed_by *current-user-id*}))]
-      {:status 200, :body (html [:html [:body {:style "margin: 0;"} (binding [p/*include-title* true
-                                                                              p/*include-buttons* true]
-                                                                      (p/render-pulse-card card data))]])})))
+      {:status 200, :body (html [:html [:body {:style "margin: 0;"} (binding [render/*include-title* true
+                                                                              render/*include-buttons* true]
+                                                                      (render/render-pulse-card card data))]])})))
 
 (defendpoint GET "/preview_card_info/:id"
   "Get JSON object containing HTML rendering of a `Card` with ID and other information."
@@ -98,9 +98,9 @@
     (read-check Database (:database (:dataset_query card)))
     (let [result    (qp/dataset-query (:dataset_query card) {:executed_by *current-user-id*})
           data      (:data result)
-          card-type (p/detect-pulse-card-type card data)
-          card-html (html (binding [p/*include-title* true]
-                            (p/render-pulse-card card data)))]
+          card-type (render/detect-pulse-card-type card data)
+          card-html (html (binding [render/*include-title* true]
+                            (render/render-pulse-card card data)))]
       {:id              id
        :pulse_card_type card-type
        :pulse_card_html card-html
@@ -112,8 +112,8 @@
   (let [card (Card id)]
     (read-check Database (:database (:dataset_query card)))
     (let [data (:data (qp/dataset-query (:dataset_query card) {:executed_by *current-user-id*}))
-          ba   (binding [p/*include-title* true]
-                 (p/render-pulse-card-to-png card data))]
+          ba   (binding [render/*include-title* true]
+                 (render/render-pulse-card-to-png card data))]
       {:status 200, :headers {"Content-Type" "image/png"}, :body (new java.io.ByteArrayInputStream ba) })))
 
 (defendpoint POST "/test"
@@ -122,7 +122,7 @@
   {name     [Required NonEmptyString]
    cards    [Required ArrayOfMaps]
    channels [Required ArrayOfMaps]}
-  (send-pulse! body)
+  (p/send-pulse! body)
   {:ok true})
 
 (define-routes)

src/metabase/email/messages.clj

@@ -5,7 +5,7 @@
             [stencil.core :as stencil]
             [metabase.email :as email]
             [metabase.models.setting :as setting]
-            [metabase.pulse :as p]
+            [metabase.pulse.render :as render]
             [metabase.util :as u]
             (metabase.util [quotation :as quotation]
                            [urls :as url])))
@@ -108,17 +108,17 @@
   "Take a pulse object and list of results, returns an array of attachment objects for an email"
   [pulse results]
   (let [images       (atom [])
-        body         (binding [p/*include-title* true
-                               p/*render-img-fn* (partial render-image images)]
+        body         (binding [render/*include-title* true
+                               render/*render-img-fn* (partial render-image images)]
                        (vec (cons :div (for [result results]
-                                         (p/render-pulse-section result)))))
+                                         (render/render-pulse-section result)))))
         data-quote   (quotation/random-quote)
         message-body (stencil/render-file "metabase/email/pulse"
                                           {:emailType       "pulse"
                                            :pulse           (html body)
                                            :pulseName       (:name pulse)
-                                           :sectionStyle    p/section-style
-                                           :colorGrey4      p/color-gray-4
+                                           :sectionStyle    render/section-style
+                                           :colorGrey4      render/color-gray-4
                                            :quotation       (:quote data-quote)
                                            :quotationAuthor (:author data-quote)
                                            :logoFooter      true})]

src/metabase/metabot.clj

@@ -10,11 +10,10 @@
             (manifold [bus :as bus]
                       [deferred :as d]
                       [stream :as s])
-            [metabase.api.common :refer [let-404]]
             [metabase.db :refer [sel]]
             [metabase.integrations.slack :as slack]
             [metabase.models.setting :as setting]
-            [metabase.task.send-pulses :as pulses]
+            [metabase.pulse :as pulse]
             [metabase.util :as u]
             [metabase.util.urls :as urls]))
 
@@ -58,11 +57,10 @@
   (str "Uh oh! :cry:\n>" (.getMessage e)))
 
 (defmacro ^:private do-async {:style/indent 0} [& body]
-  `(do (future (try ~@body
-                    (catch Throwable e#
-                      (log/error (u/format-color '~'red (u/filtered-stacktrace e#)))
-                      (slack/post-chat-message! *channel-id* (format-exception e#)))))
-       nil))
+  `(future (try ~@body
+                (catch Throwable e#
+                  (log/error (u/format-color '~'red (u/filtered-stacktrace e#)))
+                  (slack/post-chat-message! *channel-id* (format-exception e#))))))
 
 (defn- format-cards
   "Format a sequence of Cards as a nice multiline list for use in responses."
@@ -95,17 +93,14 @@
   ([]
    "Show which card? Give me a part of a card name or its ID and I can show it to you. If you don't know which card you want, try `metabot list`.")
   ([card-id-or-name & _]
-   (let-404 [{card-id :id} (id-or-name->card card-id-or-name)]
-     (do-async (pulses/send-pulse! {:cards    [{:id card-id}]
-                                    :channels [{:enabled        true
-                                                :channel_type   "slack"
-                                                :recipients     []
-                                                :details        {:channel *channel-id*}
-                                                :schedule_type  "hourly"
-                                                :schedule_day   "mon"
-                                                :schedule_hour  8
-                                                :schedule_frame "first"}]})))
-   "Ok, just a second..."))
+   (if-let [{card-id :id} (id-or-name->card card-id-or-name)]
+     (do
+       (do-async (let [attachments (pulse/create-and-upload-slack-attachments! [(pulse/execute-card card-id)])]
+                   (slack/post-chat-message! *channel-id*
+                                             nil
+                                             attachments)))
+       "Ok, just a second...")
+     (throw (Exception. "Not Found")))))
 
 
 (defn meme:up-and-to-the-right
@@ -187,7 +182,11 @@
     (when (and (human-message? event)
                (> (event-timestamp-ms event) start-time))
       (binding [*channel-id* (:channel event)]
-        (do-async (handle-slack-message event))))))
+        (do (future (try
+                      (handle-slack-message event)
+                      (catch Throwable t
+                        (slack/post-chat-message! *channel-id* (format-exception t)))))
+            nil)))))
 
 
 ;;; # ------------------------------------------------------------ Websocket Connection Stuff ------------------------------------------------------------

src/metabase/task/send_pulses.clj

@@ -6,19 +6,11 @@
             [clojurewerkz.quartzite.schedule.cron :as cron]
             (clj-time [core :as time]
                       [predicates :as timepr])
-            [metabase.api.common :refer [let-404]]
-            [metabase.email :as email]
-            [metabase.email.messages :as messages]
-            [metabase.integrations.slack :as slack]
-            (metabase.models [card :refer [Card]]
-                             [pulse :refer [Pulse], :as pulse]
+            (metabase.models [pulse :refer [Pulse], :as pulse]
                              [pulse-channel :as pulse-channel]
                              [setting :as setting])
-            (metabase [pulse :as p]
-                      [task :as task]
-                      [util :as u])
-            [metabase.query-processor :as qp]
-            [metabase.util.urls :as urls]))
+            [metabase.pulse :as p]
+            [metabase.task :as task]))
 
 
 (declare send-pulses!)
@@ -82,72 +74,6 @@
 ;;; ## ---------------------------------------- PULSE SENDING ----------------------------------------
 
 
-;; TODO: this is probably something that could live somewhere else and just be reused by us
-(defn- execute-card
-  "Execute the query for a single card."
-  [card-id]
-  {:pre [(integer? card-id)]}
-  (let-404 [card (Card card-id)]
-    (let [{:keys [creator_id dataset_query]} card]
-      (try
-        {:card card :result (qp/dataset-query dataset_query {:executed_by creator_id})}
-        (catch Throwable t
-          (log/warn (format "Error running card query (%n)" card-id) t))))))
-
-(defn- send-email-pulse!
-  "Send a `Pulse` email given a list of card results to render and a list of recipients to send to."
-  [{:keys [id name] :as pulse} results recipients]
-  (log/debug (format "Sending Pulse (%d: %s) via Channel :email" id name))
-  (let [email-subject    (str "Pulse: " name)
-        email-recipients (filterv u/is-email? (map :email recipients))]
-    (email/send-message
-      :subject      email-subject
-      :recipients   email-recipients
-      :message-type :attachments
-      :message      (messages/render-pulse-email pulse results))))
-
-(defn- create-and-upload-slack-attachment!
-  "Create an attachment in Slack for a given Card by rendering its result into an image and uploading it."
-  [channel-id {{card-id :id, card-name :name, :as card} :card, {:keys [data]} :result}]
-  (let [image-byte-array (p/render-pulse-card-to-png card data)
-        slack-file-url   (slack/upload-file! image-byte-array "image.png" channel-id)]
-    {:title      card-name
-     :title_link (urls/card-url card-id)
-     :image_url  slack-file-url
-     :fallback   card-name}))
-
-(defn send-slack-pulse!
-  "Post a `Pulse` to a slack channel given a list of card results to render and details about the slack destination."
-  [{pulse-name :name, :as pulse} results channel-id]
-  {:pre [(string? channel-id)]}
-  (log/debug (u/format-color 'cyan "Sending Pulse (%d: %s) via Slack" (:id pulse) (:name pulse)))
-  (when-let [metabase-files-channel (slack/get-or-create-files-channel!)]
-    (let [attachments (doall (for [result results]
-                               (create-and-upload-slack-attachment! (:id metabase-files-channel) result)))]
-      (slack/post-chat-message! channel-id
-                                (when (seq pulse-name)
-                                  (str "Pulse: " pulse-name)) ; if `:name` is specified pass it as the `:text` part of the Slack message
-                                attachments))))
-
-(defn send-pulse!
-  "Execute and Send a `Pulse`, optionally specifying the specific `PulseChannels`.  This includes running each
-   `PulseCard`, formatting the results, and sending the results to any specified destination.
-
-   Example:
-       (send-pulse! pulse)                       Send to all Channels
-       (send-pulse! pulse :channel-ids [312])    Send only to Channel with :id = 312"
-  [{:keys [cards] :as pulse} & {:keys [channel-ids]}]
-  {:pre [(map? pulse) (every? map? cards) (every? :id cards)]}
-  (let [results     (for [card cards]
-                      (execute-card (:id card)))
-        channel-ids (or channel-ids (mapv :id (:channels pulse)))]
-    (doseq [channel-id channel-ids]
-      (let [{:keys [channel_type details recipients]} (some #(when (= channel-id (:id %)) %)
-                                                            (:channels pulse))]
-        (condp = (keyword channel_type)
-          :email (send-email-pulse! pulse results recipients)
-          :slack (send-slack-pulse! pulse results (:channel details)))))))
-
 (defn- send-pulses!
   "Send any `Pulses` which are scheduled to run in the current day/hour.  We use the current time and determine the
    hour of the day and day of the week according to the defined reporting timezone, or UTC.  We then find all `Pulses`
@@ -163,7 +89,7 @@
       (try
         (log/debug (format "Starting Pulse Execution: %d" pulse-id))
         (when-let [pulse (pulse/retrieve-pulse pulse-id)]
-          (send-pulse! pulse :channel-ids (mapv :id (get channels-by-pulse pulse-id))))
+          (p/send-pulse! pulse :channel-ids (mapv :id (get channels-by-pulse pulse-id))))
         (log/debug (format "Finished Pulse Execution: %d" pulse-id))
         (catch Throwable e
           (log/error "Error sending pulse:" pulse-id e))))))