Skip to content
Snippets Groups Projects
Unverified Commit 1ea99689 authored by Mark Bastian's avatar Mark Bastian Committed by GitHub
Browse files

Text cards not skipped in pulses (#39216)

This PR updates the logic of skipping what cards are displayed in a pulse email by ensuring the type of the card being processed is a `:card` in addition to having no results.
parent 7f0d6718
No related branches found
No related tags found
No related merge requests found
......@@ -206,8 +206,12 @@
(concat [(tab->part tab)] (dashcards->part cards pulse dashboard))))))
(dashcards->part (t2/select :model/DashboardCard :dashboard_id dashboard-id) pulse dashboard))]
(if skip_if_empty
;; Remove any component of the parts that have no results when empty results aren't wanted
(remove (fn [part] (zero? (get-in part [:result :row_count] 0))) parts)
;; Remove cards that have no results when empty results aren't wanted
(remove (fn [{part-type :type :as part}]
(and
(= part-type :card)
(zero? (get-in part [:result :row_count] 0))))
parts)
parts)))))
(defn- database-id [card]
......
......@@ -730,6 +730,64 @@
(testing "Don't send a pulse if no results at all"
(is (nil? result)))))))))))
(deftest text-cards-are-not-skipped-when-empty-data-is-skipped-test
(testing "Do not skip text cards when filtering out pulse cards with empty results (#39190)"
(let [card-text "THIS IS TEXT THAT SHOULD NOT GO AWAY"]
(mt/dataset test-data
;; If we don't skip empty cards, we expect 2 cards.
;; If we do skip empty cards, we expect 1 card.
(doseq [[skip? expected-count] [[false 2] [true 1]]]
(mt/with-temp
[Card {base-card-id :id} {:name "Card1"
:dataset_query {:database (mt/id)
:type :query
:query {:source-table (mt/id :orders)
:fields [[:field (mt/id :orders :id) {:base-type :type/BigInteger}]
[:field (mt/id :orders :tax) {:base-type :type/Float}]]
:limit 2}}}
Card {empty-card-id :id} {:name "Card1"
:dataset_query {:database (mt/id)
:type :query
:query {:source-table (format "card__%s" base-card-id)
:filter [:= [:field "TAX" {:base-type :type/Float}] -1]}}}
Dashboard {dash-id :id} {:name "The Dashboard"}
DashboardCard _ {:dashboard_id dash-id
:visualization_settings
{:virtual_card {:display :text}
:text card-text}}
DashboardCard {base-dash-card-id :id} {:dashboard_id dash-id
:card_id base-card-id}
DashboardCard {empty-dash-card-id :id} {:dashboard_id dash-id
:card_id empty-card-id}
Pulse {pulse-id :id :as pulse} {:name "Only populated pulse"
:dashboard_id dash-id
:skip_if_empty skip?}
PulseCard _ {:pulse_id pulse-id
:card_id base-card-id
:dashboard_card_id base-dash-card-id
:include_csv true}
PulseCard _ {:pulse_id pulse-id
:card_id empty-card-id
:dashboard_card_id empty-dash-card-id
:include_csv true}
PulseChannel {pulse-channel-id :id} {:channel_type :email
:pulse_id pulse-id
:enabled true}
PulseChannelRecipient _ {:pulse_channel_id pulse-channel-id
:user_id (mt/user->id :rasta)}]
(mt/with-fake-inbox
(with-redefs [email/bcc-enabled? (constantly false)]
(mt/with-test-user nil
(metabase.pulse/send-pulse! pulse)))
(let [html-body (get-in @mt/inbox ["rasta@metabase.com" 0 :body 0 :content])]
(let [data-tables (hik.s/select
(hik.s/class "pulse-body")
(-> html-body hik/parse hik/as-hickory))]
(testing "The expected count will change if empty tables are skipped."
(is (= expected-count (count data-tables))))
(testing "The text card should always be present"
(is (true? (str/includes? html-body card-text)))))))))))))
(deftest xray-dashboards-work-test
(testing "Dashboards produced by generated by X-Rays should not produce bad results (#38350)"
(mt/dataset test-data
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment