Fix datetime formatting for pulse/dashboard subscription attachments (#18225)

src/metabase/email/messages.clj

@@ -23,6 +23,7 @@
             [metabase.query-processor.store :as qp.store]
             [metabase.query-processor.streaming :as qp.streaming]
             [metabase.query-processor.streaming.interface :as qp.streaming.i]
+            [metabase.query-processor.streaming.xlsx :as xlsx]
             [metabase.util :as u]
             [metabase.util.date-2 :as u.date]
             [metabase.util.i18n :as i18n :refer [deferred-trs trs tru]]
@@ -368,24 +369,25 @@
   Results are streamed synchronosuly. Caller is responsible for closing `os` when this call is complete."
   [export-format ^OutputStream os {{:keys [rows]} :data, database-id :database_id, :as results}]
   ;; make sure Database/driver info is available for the streaming results writers -- they might need this in order to
-    ;; get timezone information when writing results
+  ;; get timezone information when writing results
   (driver/with-driver (driver.u/database->driver database-id)
     (qp.store/with-store
       (qp.store/fetch-and-store-database! database-id)
-      (let [w                           (qp.streaming.i/streaming-results-writer export-format os)
-            cols                        (-> results :data :cols)
-            viz-settings                (-> results :data :viz-settings)
-            [ordered-cols output-order] (qp.streaming/order-cols cols viz-settings)
-            viz-settings'               (assoc viz-settings :output-order output-order)]
-        (qp.streaming.i/begin! w
-                               (assoc-in results [:data :ordered-cols] ordered-cols)
-                               viz-settings')
-        (dorun
-         (map-indexed
-          (fn [i row]
-            (qp.streaming.i/write-row! w row i ordered-cols viz-settings'))
-          rows))
-        (qp.streaming.i/finish! w results)))))
+      (binding [xlsx/*parse-temporal-string-values* true]
+        (let [w                           (qp.streaming.i/streaming-results-writer export-format os)
+              cols                        (-> results :data :cols)
+              viz-settings                (-> results :data :viz-settings)
+              [ordered-cols output-order] (qp.streaming/order-cols cols viz-settings)
+              viz-settings'               (assoc viz-settings :output-order output-order)]
+          (qp.streaming.i/begin! w
+                                 (assoc-in results [:data :ordered-cols] ordered-cols)
+                                 viz-settings')
+          (dorun
+           (map-indexed
+            (fn [i row]
+              (qp.streaming.i/write-row! w row i ordered-cols viz-settings'))
+            rows))
+          (qp.streaming.i/finish! w results))))))
 
 (defn- result-attachment
   [{{card-name :name, :as card} :card, {{:keys [rows], :as result-data} :data, :as result} :result}]

src/metabase/pulse.clj

@@ -45,7 +45,8 @@
               process-query (fn []
                               (binding [qp.perms/*card-id* card-id]
                                 (qp/process-query-and-save-with-max-results-constraints!
-                                 (assoc-in query [:middleware :process-viz-settings?] true)
+                                 (assoc query :middleware {:process-viz-settings? true
+                                                           :js-int-to-string?     false})
                                  (merge {:executed-by pulse-creator-id
                                          :context     :pulse
                                          :card-id     card-id}
@@ -76,7 +77,6 @@
                     :dashboard-id  (:id dashboard)
                     :context       :pulse ; TODO - we should support for `:dashboard-subscription` and use that to differentiate the two
                     :export-format :api
-                    :middleware    {:process-viz-settings? true}
                     :parameters    params
                     :middleware    {:process-viz-settings? true
                                     :js-int-to-string?     false}

src/metabase/query_processor/streaming/xlsx.clj

@@ -368,6 +368,11 @@
 (defmethod set-cell! nil [^Cell cell _ _]
   (.setBlank cell))
 
+(def ^:dynamic *parse-temporal-string-values*
+  "When true, XLSX exports will attempt to parse string values into corresponding java.time classes so that
+  formatting can be applied. This should be enabled for generation of pulse/dashboard subscription attachments."
+  false)
+
 (defn- add-row!
   "Adds a row of values to the spreadsheet. Values with the `scaled` viz setting are scaled prior to being added.
 
@@ -378,13 +383,20 @@
                   (inc (.getLastRowNum sheet)))
         row (.createRow sheet row-num)]
     (doseq [[value col index] (map vector values cols (range (count values)))]
-      (let [id-or-name (or (:id col) (:name col))
-            settings   (or (get col-settings {::mb.viz/field-id id-or-name})
-                           (get col-settings {::mb.viz/column-name id-or-name}))
-            scaled-val (if (and value (::mb.viz/scale settings))
-                         (* value (::mb.viz/scale settings))
-                         value)]
-        (set-cell! (.createCell ^SXSSFRow row ^Integer index) scaled-val id-or-name)))
+      (let [id-or-name   (or (:id col) (:name col))
+            settings     (or (get col-settings {::mb.viz/field-id id-or-name})
+                             (get col-settings {::mb.viz/column-name id-or-name}))
+            scaled-val   (if (and value (::mb.viz/scale settings))
+                           (* value (::mb.viz/scale settings))
+                           value)
+            ;; Temporal values are converted into strings in the format-rows QP middleware, which is enabled during
+            ;; dashboard subscription/pulse generation. If so, we should parse them here so that formatting is applied.
+            parsed-value (if (and *parse-temporal-string-values* (string? value))
+                           (try (u.date/parse value)
+                                ;; Fallback to plain string value if it couldn't be parsed
+                                (catch java.time.format.DateTimeParseException _ value))
+                           scaled-val)]
+        (set-cell! (.createCell ^SXSSFRow row ^Integer index) parsed-value id-or-name)))
     row))
 
 (defn- column-titles

test/metabase/query_processor/streaming/xlsx_test.clj

@@ -390,6 +390,12 @@
   (testing "LocalTime"
     (is (= [#inst "1899-12-31T10:12:06.000-00:00"]
            (second (xlsx-export [{:id 0, :name "Col"}] {} [[#t "10:12:06.681"]])))))
+  (testing "LocalDateTime formatted as a string; should be parsed when *parse-temporal-string-values* is true"
+    (is (= ["2020-03-28T10:12:06.681"]
+           (second (xlsx-export [{:id 0, :name "Col"}] {} [["2020-03-28T10:12:06.681"]]))))
+    (binding [xlsx/*parse-temporal-string-values* true]
+      (is (= [#inst "2020-03-28T10:12:06.681"]
+             (second (xlsx-export [{:id 0, :name "Col"}] {} [["2020-03-28T10:12:06.681"]]))))))
   (mt/with-everything-store
     (binding [metabase.driver/*driver* :h2]
       (testing "OffsetDateTime"