diff --git a/src/metabase/api/pulse.clj b/src/metabase/api/pulse.clj index e7479c95bfbbdb5446cffe635e9b4e7a85859613..81d5e852158ed531d6d0f2b6ff6e57979597c490 100644 --- a/src/metabase/api/pulse.clj +++ b/src/metabase/api/pulse.clj @@ -114,7 +114,7 @@ :card-id id})] {:status 200, :body (html [:html [:body {:style "margin: 0;"} (binding [render/*include-title* true render/*include-buttons* true] - (render/render-pulse-card (p/defaulted-timezone card) card result))]])})) + (render/render-pulse-card-for-display (p/defaulted-timezone card) card result))]])})) (api/defendpoint GET "/preview_card_info/:id" "Get JSON object containing HTML rendering of a `Card` with ID and other information." @@ -126,7 +126,7 @@ data (:data result) card-type (render/detect-pulse-card-type card data) card-html (html (binding [render/*include-title* true] - (render/render-pulse-card (p/defaulted-timezone card) card result)))] + (render/render-pulse-card-for-display (p/defaulted-timezone card) card result)))] {:id id :pulse_card_type card-type :pulse_card_html card-html diff --git a/src/metabase/email.clj b/src/metabase/email.clj index 1ae54db739f85b24a5b0c3d88cdf7831695c9feb..258b543b1bbcd46c8252ea584484470e61ff12c2 100644 --- a/src/metabase/email.clj +++ b/src/metabase/email.clj @@ -6,20 +6,21 @@ [postal [core :as postal] [support :refer [make-props]]] + [puppetlabs.i18n.core :refer [tru trs]] [schema.core :as s]) (:import javax.mail.Session)) ;;; CONFIG ;; TODO - smtp-port should be switched to type :integer -(defsetting email-from-address "Email address you want to use as the sender of Metabase." :default "notifications@metabase.com") -(defsetting email-smtp-host "The address of the SMTP server that handles your emails.") -(defsetting email-smtp-username "SMTP username.") -(defsetting email-smtp-password "SMTP password.") -(defsetting email-smtp-port "The port your SMTP server uses for outgoing emails.") +(defsetting email-from-address (tru "Email address you want to use as the sender of Metabase.") :default "notifications@metabase.com") +(defsetting email-smtp-host (tru "The address of the SMTP server that handles your emails.")) +(defsetting email-smtp-username (tru "SMTP username.")) +(defsetting email-smtp-password (tru "SMTP password.")) +(defsetting email-smtp-port (tru "The port your SMTP server uses for outgoing emails.")) (defsetting email-smtp-security - "SMTP secure connection protocol. (tls, ssl, starttls, or none)" - :default "none" + (tru "SMTP secure connection protocol. (tls, ssl, starttls, or none)") + :default (tru "none") :setter (fn [new-value] (when-not (nil? new-value) (assert (contains? #{"tls" "ssl" "none" "starttls"} new-value))) @@ -71,7 +72,8 @@ {:style/indent 0} [{:keys [subject recipients message-type message]} :- EmailMessage] (when-not (email-smtp-host) - (throw (Exception. "SMTP host is not set."))) + (let [^String msg (tru "SMTP host is not set.")] + (throw (Exception. msg)))) ;; Now send the email (send-email! (smtp-settings) {:from (email-from-address) @@ -100,8 +102,7 @@ (try (send-message-or-throw! msg-args) (catch Throwable e - (println "Failed to send email:" e) - (log/warn e "Failed to send email") + (log/warn e (trs "Failed to send email")) {:error :ERROR :message (.getMessage e)}))) @@ -126,7 +127,7 @@ {:error :SUCCESS :message nil} (catch Throwable e - (log/error "Error testing SMTP connection:" (.getMessage e)) + (log/error e (trs "Error testing SMTP connection")) {:error :ERROR :message (.getMessage e)}))) diff --git a/src/metabase/email/messages.clj b/src/metabase/email/messages.clj index 3851986d5a13a435d90a68948969716f3a02e458..3dcaa7e71afe91aa703c84bd88c60015d9042687 100644 --- a/src/metabase/email/messages.clj +++ b/src/metabase/email/messages.clj @@ -178,55 +178,33 @@ :message-type :html :message message-body))) - -;; HACK: temporary workaround to postal requiring a file as the attachment -(defn- write-byte-array-to-temp-file - [^bytes img-bytes] - (u/prog1 (doto (File/createTempFile "metabase_pulse_image_" ".png") - .deleteOnExit) - (with-open [fos (FileOutputStream. <>)] - (.write fos img-bytes)))) - -(defn- hash-bytes - "Generate a hash to be used in a Content-ID" - [^bytes img-bytes] - (Math/abs ^Integer (Arrays/hashCode img-bytes))) - -(defn- render-image [images-atom, ^bytes image-bytes] - (let [content-id (str (hash-bytes image-bytes) "@metabase")] - (if-not (contains? @images-atom content-id) - (swap! images-atom assoc content-id image-bytes)) - (str "cid:" content-id))) - -(defn- write-image-content [[content-id bytes]] +(defn- make-message-attachment [[content-id url]] {:type :inline :content-id content-id :content-type "image/png" - :content (write-byte-array-to-temp-file bytes)}) + :content url}) -(defn- pulse-context [body pulse] +(defn- pulse-context [pulse] (merge {:emailType "pulse" - :pulse (html body) :pulseName (:name pulse) :sectionStyle render/section-style :colorGrey4 render/color-gray-4 :logoFooter true} (random-quote-context))) -(defn- render-message-body - [message-template message-context images] - (vec (cons {:type "text/html; charset=utf-8" :content (stencil/render-file message-template message-context)} - (map write-image-content images)))) +(defn- render-message-body [message-template message-context timezone results] + (let [rendered-cards (binding [render/*include-title* true] + ;; doall to ensure we haven't exited the binding before the valures are created + (doall (map #(render/render-pulse-section timezone %) results))) + message-body (assoc message-context :pulse (html (vec (cons :div (map :content rendered-cards))))) + attachments (apply merge (map :attachments rendered-cards))] + (vec (cons {:type "text/html; charset=utf-8" :content (stencil/render-file message-template message-body)} + (map make-message-attachment attachments))))) (defn render-pulse-email "Take a pulse object and list of results, returns an array of attachment objects for an email" [timezone pulse results] - (let [images (atom {}) - body (binding [render/*include-title* true - render/*render-img-fn* (partial render-image images)] - (vec (cons :div (for [result results] - (render/render-pulse-section timezone result)))))] - (render-message-body "metabase/email/pulse" (pulse-context body pulse) (seq @images)))) + (render-message-body "metabase/email/pulse" (pulse-context pulse) timezone results)) (defn pulse->alert-condition-kwd "Given an `ALERT` return a keyword representing what kind of goal needs to be met." @@ -267,15 +245,10 @@ (defn render-alert-email "Take a pulse object and list of results, returns an array of attachment objects for an email" [timezone {:keys [alert_first_only] :as alert} results goal-value] - (let [images (atom {}) - body (binding [render/*include-title* true - render/*render-img-fn* (partial render-image images)] - (html (vec (cons :div (for [result results] - (render/render-pulse-section timezone result)))))) - message-ctx (default-alert-context alert (alert-results-condition-text goal-value))] + (let [message-ctx (default-alert-context alert (alert-results-condition-text goal-value))] (render-message-body "metabase/email/alert" - (assoc message-ctx :pulse body :firstRunOnly? alert_first_only) - (seq @images)))) + (assoc message-ctx :firstRunOnly? alert_first_only) + timezone results))) (def ^:private alert-condition-text {:meets "when this question meets its goal" diff --git a/src/metabase/pulse/render.clj b/src/metabase/pulse/render.clj index 56468aeec02e8844b0751d3ca217edf2164f36bf..59b8c2398633f88be709bca92c911c3df5a54af0 100644 --- a/src/metabase/pulse/render.clj +++ b/src/metabase/pulse/render.clj @@ -5,18 +5,21 @@ [format :as f]] [clojure [pprint :refer [cl-format]] - [string :as s]] + [string :as str]] [clojure.java.io :as io] [clojure.tools.logging :as log] [hiccup.core :refer [h html]] [metabase.util :as u] - [metabase.util.urls :as urls]) + [metabase.util.urls :as urls] + [puppetlabs.i18n.core :refer [tru trs]] + [schema.core :as s]) (:import cz.vutbr.web.css.MediaSpec [java.awt BasicStroke Color Dimension RenderingHints] java.awt.image.BufferedImage [java.io ByteArrayInputStream ByteArrayOutputStream] + java.net.URL java.nio.charset.StandardCharsets - java.util.Date + [java.util Arrays Date] javax.imageio.ImageIO org.apache.commons.io.IOUtils [org.fit.cssbox.css CSSNorm DOMAnalyzer DOMAnalyzer$Origin] @@ -75,6 +78,10 @@ :padding-right :1em :padding-top :8px})) +(def ^:private RenderedPulseCard + "Schema used for functions that operate on pulse card contents and their attachments" + {:attachments (s/maybe {s/Str URL}) + :content [s/Any]}) ;;; # ------------------------------------------------------------ HELPER FNS ------------------------------------------------------------ @@ -83,9 +90,9 @@ (style {:font-weight 400, :color \"white\"}) -> \"font-weight: 400; color: white;\"" [& style-maps] - (s/join " " (for [[k v] (into {} style-maps) - :let [v (if (keyword? v) (name v) v)]] - (str (name k) ": " v ";")))) + (str/join " " (for [[k v] (into {} style-maps) + :let [v (if (keyword? v) (name v) v)]] + (str (name k) ": " v ";")))) (defn- datetime-field? @@ -229,12 +236,13 @@ (.createLayout content-canvas window-size) (ImageIO/write (.getImage content-canvas) "png" os))) -(defn- render-html-to-png - ^bytes [html-body width] +(s/defn ^:private render-html-to-png :- bytes + [{:keys [content]} :- RenderedPulseCard + width] (let [html (html [:html [:body {:style (style {:margin 0 :padding 0 :background-color :white})} - html-body]]) + content]]) os (ByteArrayOutputStream.)] (render-to-png html os width) (.toByteArray os))) @@ -288,7 +296,7 @@ ;; If this column is remapped from another, it's already ;; in the output and should be skipped :when (not (:remapped_from maybe-remapped-col))] - (s/upper-case (name (or (:display_name col) (:name col))))) + (str/upper-case (name (or (:display_name col) (:name col))))) :bar-width (when include-bar? 99)}) (defn- query-results->row-seq @@ -336,23 +344,26 @@ " of " [:strong {:style (style {:color color-gray-3})} (format-number col-count)] " columns."])])) -(defn- render:table +(s/defn ^:private render:table :- RenderedPulseCard [timezone card {:keys [cols rows] :as data}] - [:div - (render-table (prep-for-html-rendering timezone cols rows nil nil cols-limit)) - (render-truncation-warning cols-limit (count cols) rows-limit (count rows))]) + {:attachments nil + :content [:div + (render-table (prep-for-html-rendering timezone cols rows nil nil cols-limit)) + (render-truncation-warning cols-limit (count cols) rows-limit (count rows))]}) -(defn- render:bar +(s/defn ^:private render:bar :- RenderedPulseCard [timezone card {:keys [cols rows] :as data}] (let [max-value (apply max (map second rows))] - [:div - (render-table (prep-for-html-rendering timezone cols rows second max-value 2)) - (render-truncation-warning 2 (count cols) rows-limit (count rows))])) + {:attachments nil + :content [:div + (render-table (prep-for-html-rendering timezone cols rows second max-value 2)) + (render-truncation-warning 2 (count cols) rows-limit (count rows))]})) -(defn- render:scalar +(s/defn ^:private render:scalar :- RenderedPulseCard [timezone card {:keys [cols rows]}] - [:div {:style (style scalar-style)} - (h (format-cell timezone (ffirst rows) (first cols)))]) + {:attachments nil + :content [:div {:style (style scalar-style)} + (h (format-cell timezone (ffirst rows) (first cols)))]}) (defn- render-sparkline-to-png "Takes two arrays of numbers between 0 and 1 and plots them as a sparkline" @@ -380,10 +391,54 @@ (* 2 sparkline-dot-radius) (* 2 sparkline-dot-radius))) (when-not (ImageIO/write image "png" os) ; returns `true` if successful -- see JavaDoc - (throw (Exception. "No approprate image writer found!"))) + (let [^String msg (tru "No approprate image writer found!")] + (throw (Exception. msg)))) (.toByteArray os))) -(defn- render:sparkline +(defn- hash-bytes + "Generate a hash to be used in a Content-ID" + [^bytes img-bytes] + (Math/abs ^Integer (Arrays/hashCode img-bytes))) + +(defn- hash-image-url + "Generate a hash to be used in a Content-ID" + [^java.net.URL url] + (-> url io/input-stream IOUtils/toByteArray hash-bytes)) + +(defn- content-id-reference [content-id] + (str "cid:" content-id)) + +(defn- mb-hash-str [image-hash] + (str image-hash "@metabase")) + +(defn- resource-path->content-id-pair [path] + (let [external-link-url (io/resource path)] + [(mb-hash-str (hash-image-url external-link-url)) + external-link-url])) + +(defn- write-byte-array-to-temp-file + [^bytes img-bytes] + (let [f (doto (java.io.File/createTempFile "metabase_pulse_image_" ".png") + .deleteOnExit)] + (with-open [fos (java.io.FileOutputStream. f)] + (.write fos img-bytes)) + f)) + +(defn- byte-array->content-id-pair + [^bytes img-bytes] + (let [f (write-byte-array-to-temp-file img-bytes)] + [(mb-hash-str (hash-bytes img-bytes)) + (io/as-url f)])) + +(def ^:private external-link-image + (delay + (resource-path->content-id-pair "frontend_client/app/assets/img/external_link.png"))) + +(def ^:private no-results-image + (delay + (resource-path->content-id-pair "frontend_client/app/assets/img/pulse_no_results@2x.png"))) + +(s/defn ^:private render:sparkline :- RenderedPulseCard [timezone card {:keys [rows cols]}] (let [ft-row (if (datetime-field? (first cols)) #(.getTime ^Date (u/->Timestamp %)) @@ -406,42 +461,47 @@ 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))] - [:div - [:img {:style (style {:display :block - :width :100%}) - :src (*render-img-fn* (render-sparkline-to-png xs' ys' 524 130))}] - [:table - [:tr - [:td {:style (style {:color color-brand - :font-size :24px - :font-weight 700 - :padding-right :16px})} - (first values)] - [:td {:style (style {:color color-gray-3 - :font-size :24px - :font-weight 700})} - (second values)]] - [:tr - [:td {:style (style {:color color-brand - :font-size :16px - :font-weight 700 - :padding-right :16px})} - (first labels)] - [:td {:style (style {:color color-gray-3 - :font-size :16px})} - (second labels)]]]])) - -(defn- render-image-with-filename [^String filename] - (*render-img-fn* (IOUtils/toByteArray (io/input-stream (io/resource filename))))) - -(defn- render:empty [_ _] - [:div {:style (style {:text-align :center})} - [:img {:style (style {:width :104px}) - :src (render-image-with-filename "frontend_client/app/assets/img/pulse_no_results@2x.png")}] - [:div {:style (style {:margin-top :8px - :color color-gray-4})} - "No results"]]) + labels (format-timestamp-pair timezone (map first rows') (first cols)) + [content-id png-url] (byte-array->content-id-pair (render-sparkline-to-png xs' ys' 524 130))] + + {:attachments (when (and content-id png-url) + {content-id png-url}) + :content [:div + [:img {:style (style {:display :block + :width :100%}) + :src (content-id-reference content-id)}] + [:table + [:tr + [:td {:style (style {:color color-brand + :font-size :24px + :font-weight 700 + :padding-right :16px})} + (first values)] + [:td {:style (style {:color color-gray-3 + :font-size :24px + :font-weight 700})} + (second values)]] + [:tr + [:td {:style (style {:color color-brand + :font-size :16px + :font-weight 700 + :padding-right :16px})} + (first labels)] + [:td {:style (style {:color color-gray-3 + :font-size :16px})} + (second labels)]]]]})) + +(s/defn ^:private render:empty :- RenderedPulseCard + [_ _] + (let [[content-id no-results-url] @no-results-image] + {:attachments (when (and content-id no-results-url) + {content-id no-results-url}) + :content [:div {:style (style {:text-align :center})} + [:img {:style (style {:width :104px}) + :src (content-id-reference content-id)}] + [:div {:style (style {:margin-top :8px + :color color-gray-4})} + "No results"]]})) (defn detect-pulse-card-type "Determine the pulse (visualization) type of a CARD, e.g. `:scalar` or `:bar`." @@ -467,61 +527,86 @@ (number-field? col-2)) :bar :else :table))) -(defn render-pulse-card - "Render a single CARD for a `Pulse` to Hiccup HTML. RESULT is the QP results." +(s/defn ^:private make-title-if-needed :- (s/maybe RenderedPulseCard) + [card] + (when *include-title* + (let [[content-id link-url] (when *include-buttons* + @external-link-image)] + {:attachments (when (and content-id link-url) + {content-id link-url}) + :content [:table {:style (style {:margin-bottom :8px + :width :100%})} + [:tbody + [:tr + [:td [:span {:style header-style} + (-> card :name h)]] + [:td {:style (style {:text-align :right})} + (when *include-buttons* + [:img {:style (style {:width :16px}) + :width 16 + :src (content-id-reference content-id)}])]]]]}))) + +(s/defn ^:private render-pulse-card-body :- RenderedPulseCard [timezone card {:keys [data error]}] - [:a {:href (card-href card) - :target "_blank" - :style (style section-style - {:margin :16px - :margin-bottom :16px - :display :block - :text-decoration :none})} - (when *include-title* - [:table {:style (style {:margin-bottom :8px - :width :100%})} - [:tbody - [:tr - [:td [:span {:style header-style} - (-> card :name h)]] - [:td {:style (style {:text-align :right})} - (when *include-buttons* - [:img {:style (style {:width :16px}) - :width 16 - :src (render-image-with-filename "frontend_client/app/assets/img/external_link.png")}])]]]]) - (try - (when error - (throw (Exception. (str "Card has errors: " error)))) - (case (detect-pulse-card-type card data) - :empty (render:empty card data) - :scalar (render:scalar timezone card data) - :sparkline (render:sparkline timezone card data) - :bar (render:bar timezone card data) - :table (render:table timezone card data) - [:div {:style (style font-style - {:color "#F9D45C" - :font-weight 700})} - "We were unable to display this card." [:br] "Please view this card in Metabase."]) - (catch Throwable e - (log/warn "Pulse card render error:" e) - [:div {:style (style font-style - {:color "#EF8C8C" - :font-weight 700 - :padding :16px})} - "An error occurred while displaying this card."]))]) - - -(defn render-pulse-section + (try + (when error + (let [^String msg (tru "Card has errors: {0}" error)] + (throw (Exception. msg)))) + (case (detect-pulse-card-type card data) + :empty (render:empty card data) + :scalar (render:scalar timezone card data) + :sparkline (render:sparkline timezone card data) + :bar (render:bar timezone card data) + :table (render:table timezone card data) + {:attachments nil + :content [:div {:style (style font-style + {:color "#F9D45C" + :font-weight 700})} + "We were unable to display this card." [:br] "Please view this card in Metabase."]}) + (catch Throwable e + (log/error e (trs "Pulse card render error")) + {:attachments nil + :content [:div {:style (style font-style + {:color "#EF8C8C" + :font-weight 700 + :padding :16px})} + "An error occurred while displaying this card."]}))) + +(s/defn render-pulse-card :- RenderedPulseCard + "Render a single CARD for a `Pulse` to Hiccup HTML. RESULT is the QP results." + [timezone card results] + (let [{title :content title-attachments :attachments} (make-title-if-needed card) + {pulse-body :content body-attachments :attachments} (render-pulse-card-body timezone card results)] + {:attachments (merge title-attachments body-attachments) + :content [:a {:href (card-href card) + :target "_blank" + :style (style section-style + {:margin :16px + :margin-bottom :16px + :display :block + :text-decoration :none})} + title + pulse-body]})) + +(defn render-pulse-card-for-display + "Same as `render-pulse-card` but isn't intended for an email, rather for previewing so there is no need for + attachments" + [timezone card results] + (:content (render-pulse-card timezone card results))) + +(s/defn render-pulse-section :- RenderedPulseCard "Render a specific section of a Pulse, i.e. a single Card, to Hiccup HTML." [timezone {:keys [card result]}] - [:div {:style (style {:margin-top :10px - :margin-bottom :20px - :border "1px solid #dddddd" - :border-radius :2px - :background-color :white - :box-shadow "0 1px 2px rgba(0, 0, 0, .08)"})} - (binding [*include-title* true] - (render-pulse-card timezone card result))]) + (let [{:keys [attachments content]} (binding [*include-title* true] + (render-pulse-card timezone card result))] + {:attachments attachments + :content [:div {:style (style {:margin-top :10px + :margin-bottom :20px + :border "1px solid #dddddd" + :border-radius :2px + :background-color :white + :box-shadow "0 1px 2px rgba(0, 0, 0, .08)"})} + content]})) (defn render-pulse-card-to-png "Render a PULSE-CARD as a PNG. DATA is the `:data` from a QP result (I think...)" diff --git a/test/metabase/pulse/render_test.clj b/test/metabase/pulse/render_test.clj index ba25f3160ad6e094c72e20df8ff7ebe43d04d288..68fba0d60f7ead181f8d2f463ca90504c316663e 100644 --- a/test/metabase/pulse/render_test.clj +++ b/test/metabase/pulse/render_test.clj @@ -134,33 +134,38 @@ {:bar-width nil, :row ["3" "34.05" "Aug 1, 2014" "The Gorbals"]}] (rest (prep-for-html-rendering pacific-tz test-columns-with-date-special-type test-data nil nil (count test-columns)))) +(defn- render-scalar-value [results] + (-> (render:scalar pacific-tz nil results) + :content + last)) + (expect "10" - (last (render:scalar pacific-tz nil {:cols [{:name "ID", - :display_name "ID", - :base_type :type/BigInteger - :special_type nil}] - :rows [[10]]}))) + (render-scalar-value {:cols [{:name "ID", + :display_name "ID", + :base_type :type/BigInteger + :special_type nil}] + :rows [[10]]})) (expect "10.12" - (last (render:scalar pacific-tz nil {:cols [{:name "floatnum", - :display_name "FLOATNUM", - :base_type :type/Float - :special_type nil}] - :rows [[10.12345]]}))) + (render-scalar-value {:cols [{:name "floatnum", + :display_name "FLOATNUM", + :base_type :type/Float + :special_type nil}] + :rows [[10.12345]]})) (expect "foo" - (last (render:scalar pacific-tz nil {:cols [{:name "stringvalue", - :display_name "STRINGVALUE", - :base_type :type/Text - :special_type nil}] - :rows [["foo"]]}))) + (render-scalar-value {:cols [{:name "stringvalue", + :display_name "STRINGVALUE", + :base_type :type/Text + :special_type nil}] + :rows [["foo"]]})) (expect "Apr 1, 2014" - (last (render:scalar pacific-tz nil {:cols [{:name "date", - :display_name "DATE", - :base_type :type/DateTime - :special_type nil}] - :rows [["2014-04-01T08:30:00.0000"]]}))) + (render-scalar-value {:cols [{:name "date", + :display_name "DATE", + :base_type :type/DateTime + :special_type nil}] + :rows [["2014-04-01T08:30:00.0000"]]})) diff --git a/test/metabase/pulse_test.clj b/test/metabase/pulse_test.clj index 4c71edea5d331f8c9702b41e7667895629cea46b..670eba881f5e5f31a9a9487b18fd433a655de434 100644 --- a/test/metabase/pulse_test.clj +++ b/test/metabase/pulse_test.clj @@ -65,7 +65,7 @@ {:type :inline, :content-id true, :content-type "image/png", - :content java.io.File}) + :content java.net.URL}) (defn- rasta-pulse-email [& [email]] (et/email-to :rasta (merge {:subject "Pulse: Pulse Name", @@ -366,7 +366,7 @@ (defn- attachment? [{message-type :type content-type :content-type content :content}] (and (= :inline message-type) (= "image/png" content-type) - (instance? java.io.File content))) + (instance? java.net.URL content))) ;; Test with a slack channel and an email (tt/expect-with-temp [Card [{card-id :id} (checkins-query {:breakout [["datetime-field" (data/id :checkins :date) "hour"]]})]