Format Rows toggle also applies to Column Names (#42543)

* Format Rows toggle also applies to Column Names Fixes: #42500 When format-rows is false, we want to also pass that true/false value to the title column formatting. That is, if a column is renamed and formatting is applied, we want to use that column's name. But, if a column is renamed and the user exports unformatted csv or json, the custom column name should NOT be used, as was the behaviour in prior versions. * write a better test explanation
metabase · May 11, 2024 · 8e72f29 · 8e72f29
1 parent 7b10580
commit 8e72f29
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 13 deletions.
diff --git a/src/metabase/query_processor/streaming/common.clj b/src/metabase/query_processor/streaming/common.clj
@@ -94,7 +94,7 @@
 
 (defn column-titles
   "Generates the column titles that should be used in the export, taking into account viz settings."
-  [ordered-cols col-settings]
+  [ordered-cols col-settings format-rows?]
   (for [col ordered-cols]
     (let [id-or-name      (or (and (:remapped_from col) (:fk_field_id col))
                               (:id col)
@@ -107,7 +107,7 @@
           merged-settings (if is-currency?
                             (merge-global-settings format-settings :type/Currency)
                             format-settings)
-          column-title    (or (::mb.viz/column-title merged-settings)
+          column-title    (or (when format-rows? (::mb.viz/column-title merged-settings))
                               (:display_name col)
                               (:name col))]
       (if (and is-currency? (::mb.viz/currency-in-header merged-settings true))

diff --git a/src/metabase/query_processor/streaming/csv.clj b/src/metabase/query_processor/streaming/csv.clj
@@ -31,7 +31,7 @@
     (reify qp.si/StreamingResultsWriter
       (begin! [_ {{:keys [ordered-cols results_timezone format-rows?]
                    :or   {format-rows? true}} :data} viz-settings]
-        (let [col-names (common/column-titles ordered-cols (::mb.viz/column-settings viz-settings))]
+        (let [col-names (common/column-titles ordered-cols (::mb.viz/column-settings viz-settings) format-rows?)]
           (vreset! ordered-formatters
                    (if format-rows?
                      (mapv #(formatter/create-formatter results_timezone % viz-settings) ordered-cols)

diff --git a/src/metabase/query_processor/streaming/json.clj b/src/metabase/query_processor/streaming/json.clj
@@ -35,7 +35,7 @@
                    :or   {format-rows? true}} :data} viz-settings]
         ;; TODO -- wouldn't it make more sense if the JSON downloads used `:name` preferentially? Seeing how JSON is
         ;; probably going to be parsed programmatically
-        (vreset! col-names (common/column-titles ordered-cols (::mb.viz/column-settings viz-settings)))
+        (vreset! col-names (common/column-titles ordered-cols (::mb.viz/column-settings viz-settings) format-rows?))
         (vreset! ordered-formatters
                    (if format-rows?
                      (mapv #(formatter/create-formatter results_timezone % viz-settings) ordered-cols)

diff --git a/src/metabase/query_processor/streaming/xlsx.clj b/src/metabase/query_processor/streaming/xlsx.clj
@@ -461,7 +461,7 @@
         (doseq [i (range (count ordered-cols))]
           (.trackColumnForAutoSizing ^SXSSFSheet sheet i))
         (setup-header-row! sheet (count ordered-cols))
-        (spreadsheet/add-row! sheet (common/column-titles ordered-cols col-settings)))
+        (spreadsheet/add-row! sheet (common/column-titles ordered-cols col-settings true)))
 
       (write-row! [_ row row-num ordered-cols {:keys [output-order] :as viz-settings}]
         (let [ordered-row  (if output-order

diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
@@ -3483,17 +3483,26 @@
                 (mt/user-http-request :crowberto :post 400 "card" card-data)))))))
 
 (deftest ^:parallel format-export-middleware-test
-  (testing "The `:format-export?` query processor middleware has the intended effect on file exports."
+  (testing "The `:format-rows` query processor middleware results in formatted/unformatted rows when set to true/false."
     (let [q             {:database (mt/id)
                          :type     :native
                          :native   {:query "SELECT 2000 AS number, '2024-03-26'::DATE AS date;"}}
-          output-helper {:csv  (fn [output] (->> output csv/read-csv last))
-                         :json (fn [output] (->> output (map (juxt :NUMBER :DATE)) last))}]
-      (t2.with-temp/with-temp [Card {card-id :id} {:display :table :dataset_query q}]
-        (doseq [[export-format apply-formatting? expected] [[:csv true ["2,000" "March 26, 2024"]]
-                                                            [:csv false ["2000" "2024-03-26"]]
-                                                            [:json true ["2,000" "March 26, 2024"]]
-                                                            [:json false [2000 "2024-03-26"]]]]
+          output-helper {:csv  (fn [output] (->> output csv/read-csv))
+                         :json (fn [[row]] [(map name (keys row)) (vals row)])}]
+      (t2.with-temp/with-temp [Card {card-id :id} {:dataset_query q
+                                                   :display       :table
+                                                   :visualization_settings
+                                                   {:column_settings
+                                                    {"[\"name\",\"NUMBER\"]" {:column_title "Custom Title"}
+                                                     "[\"name\",\"DATE\"]"   {:column_title "Custom Title 2"}}}}]
+        (doseq [[export-format apply-formatting? expected] [[:csv true [["Custom Title" "Custom Title 2"]
+                                                                        ["2,000" "March 26, 2024"]]]
+                                                            [:csv false [["NUMBER" "DATE"]
+                                                                         ["2000" "2024-03-26"]]]
+                                                            [:json true [["Custom Title" "Custom Title 2"]
+                                                                         ["2,000" "March 26, 2024"]]]
+                                                            [:json false [["NUMBER" "DATE"]
+                                                                          [2000 "2024-03-26"]]]]]
           (testing (format "export_format %s yields expected output for %s exports." apply-formatting? export-format)
               (is (= expected
                      (->> (mt/user-http-request