diff --git a/src/metabase/email/messages.clj b/src/metabase/email/messages.clj index 2f0d1cb5447665bbea6d0c751d15487e5e1e3f0d..e506ed95412495004c9ec2ded380064d303d378b 100644 --- a/src/metabase/email/messages.clj +++ b/src/metabase/email/messages.clj @@ -380,7 +380,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) diff --git a/test/metabase/dashboard_subscription_test.clj b/test/metabase/dashboard_subscription_test.clj index c8c5d7339b5b8393120a2e8cecfb92c95624ec8a..325b953909e95e2fb61d596705ae78975153d271 100644 --- a/test/metabase/dashboard_subscription_test.clj +++ b/test/metabase/dashboard_subscription_test.clj @@ -939,3 +939,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 #"_"))))))) diff --git a/test/metabase/pulse/pulse_integration_test.clj b/test/metabase/pulse/pulse_integration_test.clj index e2916894c832257791792c89fbb69074eeb50681..b17979d355df1a11d709223c2aeb7156845f57ca 100644 --- a/test/metabase/pulse/pulse_integration_test.clj +++ b/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." diff --git a/test/metabase/pulse/test_util.clj b/test/metabase/pulse/test_util.clj index c8e863f001d4f767d55013737769e9397cfe8dd1..cc470826bc4953142dffb1b5054179a2d8798b16 100644 --- a/test/metabase/pulse/test_util.clj +++ b/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'"