Skip to content
Snippets Groups Projects
Commit 81293e1c authored by Ryan Senior's avatar Ryan Senior
Browse files

Only include a CSV attachment for pulses when XLS is not selected

Previously we would always include a CSV attachment when there were
more rows/columns than would be included in the table. This ended up
being redundant for users that would prefer an XLS attachment and have
indicated that the pulse should include an XLS attachment. This commit
will only not attach a CSV if the user has select an XLS attachment
for that pulse.

Fixes #7302
parent a657ce32
No related branches found
No related tags found
No related merge requests found
......@@ -160,9 +160,10 @@
(defn include-csv-attachment?
"Returns true if this card and resultset should include a CSV attachment"
[{:keys [include_csv] :as card} {:keys [cols rows] :as result-data}]
[card {:keys [cols rows] :as result-data}]
(or (:include_csv card)
(and (= :table (detect-pulse-card-type card result-data))
(and (not (:include_xls card))
(= :table (detect-pulse-card-type card result-data))
(or
;; If some columns are not shown, include an attachment
(some (complement show-in-table?) cols)
......
......@@ -764,6 +764,53 @@
(send-pulse! (retrieve-notification pulse-id))
(et/summarize-multipart-email test-card-regex))))
;; With a "rows" type of pulse (table visualization) we should include the CSV by default
(expect
(-> (rasta-pulse-email)
;; There's no PNG with a table visualization, remove it from the expected results
(update-in ["rasta@metabase.com" 0 :body] (comp vector first))
(add-rasta-attachment csv-attachment))
(tt/with-temp* [Card [{card-id :id} {:name card-name
:dataset_query {:database (data/id)
:type :query
:query {:source-table (data/id :checkins)}}}]
Pulse [{pulse-id :id} {:name "Pulse Name"
:skip_if_empty false}]
PulseCard [_ {:pulse_id pulse-id
:card_id card-id
:position 0}]
PulseChannel [{pc-id :id} {:pulse_id pulse-id}]
PulseChannelRecipient [_ {:user_id (rasta-id)
:pulse_channel_id pc-id}]]
(email-test-setup
(send-pulse! (retrieve-pulse pulse-id))
(et/summarize-multipart-email #"Pulse Name"))))
;; If the pulse is already configured to send an XLS, no need to include a CSV
(expect
(-> (rasta-pulse-email)
;; There's no PNG with a table visualization, remove it from the expected results
(update-in ["rasta@metabase.com" 0 :body] (comp vector first))
(add-rasta-attachment xls-attachment))
(tt/with-temp* [Card [{card-id :id} {:name card-name
:dataset_query {:database (data/id)
:type :query
:query {:source-table (data/id :checkins)}}}]
Pulse [{pulse-id :id} {:name "Pulse Name"
:skip_if_empty false}]
PulseCard [_ {:pulse_id pulse-id
:card_id card-id
:position 0
:include_xls true}]
PulseChannel [{pc-id :id} {:pulse_id pulse-id}]
PulseChannelRecipient [_ {:user_id (rasta-id)
:pulse_channel_id pc-id}]]
(email-test-setup
(send-pulse! (retrieve-pulse pulse-id))
(et/summarize-multipart-email #"Pulse Name"))))
;; Basic test of card with CSV and XLS attachments, but no data. Should not include an attachment
(expect
(rasta-pulse-email)
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment