Don't use the slugify util for filenames (#42475)

* Don't use the slugify util for filenames

* Fix tests that looked for incorrect filenames

* add a test

* fix filename in pulse test util

src/metabase/email/messages.clj

@@ -382,7 +382,7 @@
     {:type         :attachment
      :content-type content-type
      :file-name    (format "%s_%s.%s"
-                           (or (u/slugify card-name) "query_result")
+                           (or card-name "query_result")
                            (u.date/format (t/zoned-date-time))
                            (name export-type))
      :content      (-> attachment-file .toURI .toURL)

test/metabase/dashboard_subscription_test.clj

@@ -943,3 +943,9 @@
                          (->> (mapv #(str/split % #",")))
                          first
                          count))))))))))
+
+(deftest attachment-filenames-stay-readable-test
+  (testing "Filenames remain human-readable (#41669)"
+    (let [tmp (#'messages/create-temp-file ".tmp")
+          {:keys [file-name]} (#'messages/create-result-attachment-map :csv "テストSQL質問" tmp)]
+      (is (= "テストSQL質問" (first (str/split file-name #"_")))))))

test/metabase/pulse/pulse_integration_test.clj

@@ -197,16 +197,6 @@
              (zipmap h (apply mapv vector r)))])))
      (into {}))))
 
-(defn- slugify-fname
-  "Slugify a filename while preserving the extension.
-  Useful for writing tests that require a stable filename as a key.
-
-  Eg. Some File Name.csv -> some_file_name.csv"
-  [s]
-  (let [parts (str/split s #"\.")
-        ext (last parts)]
-    (str (u/slugify (str/join "." (butlast parts))) "." ext)))
-
 (deftest apply-formatting-in-csv-dashboard-test
   (testing "An exported dashboard should preserve the formatting specified in the column metadata (#36320)"
     (with-metadata-data-cards [base-card-id model-card-id question-card-id]
@@ -236,11 +226,11 @@
                                               :user_id          (mt/user->id :rasta)}]
         (let [parsed-data (run-pulse-and-return-attached-csv-data pulse)]
           (testing "The base model has no special formatting"
-            (is (all-float? (get-in parsed-data [(slugify-fname "Base question - no special metadata.csv") "Tax Rate"]))))
+            (is (all-float? (get-in parsed-data ["Base question - no special metadata.csv" "Tax Rate"]))))
           (testing "The model with metadata formats the Tax Rate column with the user-defined semantic type"
-            (is (all-pct-2d? (get-in parsed-data [(slugify-fname "Model with percent semantic type.csv") "Tax Rate"]))))
+            (is (all-pct-2d? (get-in parsed-data ["Model with percent semantic type.csv" "Tax Rate"]))))
           (testing "The query based on the model uses the model's semantic typ information for formatting"
-            (is (all-pct-2d? (get-in parsed-data [(slugify-fname "Query based on model.csv") "Tax Rate"])))))))))
+            (is (all-pct-2d? (get-in parsed-data ["Query based on model.csv" "Tax Rate"])))))))))
 
 (deftest apply-formatting-in-csv-no-dashboard-test
   (testing "Exported cards should preserve the formatting specified in their column metadata (#36320)"
@@ -256,7 +246,7 @@
                        PulseChannelRecipient _ {:pulse_channel_id pulse-channel-id
                                                 :user_id          (mt/user->id :rasta)}]
           (let [parsed-data (run-pulse-and-return-attached-csv-data pulse)]
-            (is (all-float? (get-in parsed-data [(slugify-fname "Base question - no special metadata.csv") "Tax Rate"]))))))
+            (is (all-float? (get-in parsed-data ["Base question - no special metadata.csv" "Tax Rate"]))))))
       (testing "The attached data from the second question (a model) is percent formatted"
         (mt/with-temp [Pulse {pulse-id :id
                               :as      pulse} {:name "Test Pulse"}
@@ -268,7 +258,7 @@
                        PulseChannelRecipient _ {:pulse_channel_id pulse-channel-id
                                                 :user_id          (mt/user->id :rasta)}]
           (let [parsed-data (run-pulse-and-return-attached-csv-data pulse)]
-            (is (all-pct-2d? (get-in parsed-data [(slugify-fname "Model with percent semantic type.csv") "Tax Rate"]))))))
+            (is (all-pct-2d? (get-in parsed-data ["Model with percent semantic type.csv" "Tax Rate"]))))))
       (testing "The attached data from the last question (based on a a model) is percent formatted"
         (mt/with-temp [Pulse {pulse-id :id
                               :as      pulse} {:name "Test Pulse"}
@@ -280,7 +270,7 @@
                        PulseChannelRecipient _ {:pulse_channel_id pulse-channel-id
                                                 :user_id          (mt/user->id :rasta)}]
           (let [parsed-data (run-pulse-and-return-attached-csv-data pulse)]
-            (is (all-pct-2d? (get-in parsed-data [(slugify-fname "Query based on model.csv") "Tax Rate"])))))))))
+            (is (all-pct-2d? (get-in parsed-data ["Query based on model.csv" "Tax Rate"])))))))))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; Consistent Date Formatting ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
@@ -396,7 +386,7 @@
                      PulseChannelRecipient _ {:pulse_channel_id pulse-channel-id
                                               :user_id          (mt/user->id :rasta)}]
         (let [attached-data     (run-pulse-and-return-attached-csv-data pulse)
-              get-res           #(-> attached-data (get (slugify-fname %))
+              get-res           #(-> (get attached-data %)
                                      (update-vals first)
                                      (dissoc "X"))
               native-results    (get-res "NATIVE.csv")
@@ -584,16 +574,16 @@
                                           (into {})))]
             (testing "Renaming columns via viz settings is correctly applied to the CSV export"
               (is (= ["THE_ID" "ORDER TAX" "Total Amount" "Discount Applied ($)" "Amount Ordered" "Effective Tax Rate"]
-                     (attachment-name->cols (slugify-fname (format "%s.csv" base-card-name))))))
+                     (attachment-name->cols (format "%s.csv" base-card-name)))))
             (testing "A question derived from another question does not bring forward any renames"
               (is (= ["ID" "Tax" "Total" "Discount ($)" "Quantity" "Tax Rate"]
-                     (attachment-name->cols (slugify-fname (format "%s.csv" model-card-name))))))
+                     (attachment-name->cols (format "%s.csv" model-card-name)))))
             (testing "A model with custom metadata shows the renamed metadata columns"
               (is (= ["ID" "Tax" "Grand Total" "Amount of Discount ($)" "N" "Tax Rate"]
-                     (attachment-name->cols (slugify-fname (format "%s.csv" meta-model-card-name))))))
+                     (attachment-name->cols (format "%s.csv" meta-model-card-name)))))
             (testing "A question based on a model retains the curated metadata column names but overrides these with any existing visualization_settings"
               (is (= ["IDENTIFIER" "Tax" "Grand Total" "Amount of Discount ($)" "Count" "Tax Rate"]
-                     (attachment-name->cols (slugify-fname (format "%s.csv" question-card-name))))))))))))
+                     (attachment-name->cols (format "%s.csv" question-card-name)))))))))))
 
 (defn- run-pulse-and-return-scalars
   "Simulate sending the pulse email, get the html body of the response and return the scalar value of the card."

test/metabase/pulse/test_util.clj

@@ -84,14 +84,14 @@
 (def csv-attachment
   {:type         :attachment
    :content-type "text/csv"
-   :file-name    "test_card.csv",
+   :file-name    "Test card.csv",
    :content      java.net.URL
    :description  "More results for 'Test card'"
    :content-id   false})
 
 (def xls-attachment
   {:type         :attachment
-   :file-name    "test_card.xlsx"
+   :file-name    "Test card.xlsx"
    :content-type "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"
    :content      java.net.URL
    :description  "More results for 'Test card'"