diff --git a/src/metabase/task/send_pulses.clj b/src/metabase/task/send_pulses.clj
index 48dba94606a22e4f9b9a8f9cba873c38dff7f07e..a73f7a714adf443e70476e20c52e320c6c9665b5 100644
--- a/src/metabase/task/send_pulses.clj
+++ b/src/metabase/task/send_pulses.clj
@@ -100,14 +100,19 @@
"Delete PulseChannels that have no recipients and no channel set for a pulse, returns the channel ids that were deleted."
[pulse-id]
(when-let [ids-to-delete (seq
- (for [channel (t2/select [:model/PulseChannel :id :details]
+ (for [channel (t2/select [:model/PulseChannel :id :details :channel_id :channel_type]
:pulse_id pulse-id
:id [:not-in {:select [[:pulse_channel_id :id]]
:from :pulse_channel_recipient
:group-by [:pulse_channel_id]
:having [:>= :%count.* [:raw 1]]}])
- :when (and (empty? (get-in channel [:details :emails]))
- (not (get-in channel [:details :channel])))]
+ :when (case (:channel_type channel)
+ :email
+ (empty? (get-in channel [:details :emails]))
+ :slack
+ (empty? (get-in channel [:details :channel]))
+ :http
+ (nil? (:channel_id channel)))]
(:id channel)))]
(log/infof "Deleting %d PulseChannels with id: %s due to having no recipients" (count ids-to-delete) (str/join ", " ids-to-delete))
(t2/delete! :model/PulseChannel :id [:in ids-to-delete])
diff --git a/test/metabase/channel/http_test.clj b/test/metabase/channel/http_test.clj
index 1f84e72d5be18017bf1add508f2af77bc42efa84..f5eba2376ca21ad8010bf42dda32cb07b55d6747 100644
--- a/test/metabase/channel/http_test.clj
+++ b/test/metabase/channel/http_test.clj
@@ -7,7 +7,7 @@
[compojure.core :as compojure]
[medley.core :as m]
[metabase.channel.core :as channel]
- [metabase.pulse :as pulse]
+ [metabase.task.send-pulses :as task.send-pulses]
[metabase.test :as mt]
[metabase.util.i18n :refer [deferred-tru]]
[ring.adapter.jetty :as jetty]
@@ -328,10 +328,10 @@
:model/Channel {chn-id :id} {:type :channel/http
:details {:url (str url (:path receive-route))
:auth-method "none"}}
- :model/PulseChannel _ {:pulse_id pulse-id
+ :model/PulseChannel {pc-id :id} {:pulse_id pulse-id
:channel_type "http"
:channel_id chn-id}]
- (pulse/send-pulse! pulse)
+ (#'task.send-pulses/send-pulse!* pulse-id [pc-id])
(is (=? {:body {:type "alert"
:alert_id pulse-id
:alert_creator_id (mt/malli=? int?)
diff --git a/test/metabase/task/send_pulses_test.clj b/test/metabase/task/send_pulses_test.clj
index f28a93be70b664491c2abf390edb23cfd8de0f78..b10d6e4c2eb6c44ed2db102dd9d0a7275665399b 100644
--- a/test/metabase/task/send_pulses_test.clj
+++ b/test/metabase/task/send_pulses_test.clj
@@ -30,33 +30,72 @@
(t2/count PulseChannel)))
(is (:archived (t2/select-one Pulse :id pulse-id)))))
- (testing "Has PulseChannelRecipient"
- (mt/with-temp [Pulse {pulse-id :id} {}
- PulseChannel {pc-id :id} {:pulse_id pulse-id
- :channel_type :email}
- PulseChannelRecipient _ {:user_id (mt/user->id :rasta)
- :pulse_channel_id pc-id}]
- (#'task.send-pulses/clear-pulse-channels-no-recipients! pulse-id)
- (is (= 1
- (t2/count PulseChannel)))))
+ (testing "emails"
+ (testing "keep if has PulseChannelRecipient"
+ (mt/with-temp [Pulse {pulse-id :id} {}
+ PulseChannel {pc-id :id} {:pulse_id pulse-id
+ :channel_type :email}
+ PulseChannelRecipient _ {:user_id (mt/user->id :rasta)
+ :pulse_channel_id pc-id}]
+ (#'task.send-pulses/clear-pulse-channels-no-recipients! pulse-id)
+ (is (= 1
+ (t2/count PulseChannel)))))
- (testing "Has email"
- (mt/with-temp [Pulse {pulse-id :id} {}
- PulseChannel _ {:pulse_id pulse-id
- :channel_type :email
- :details {:emails ["test@metabase.com"]}}]
- (#'task.send-pulses/clear-pulse-channels-no-recipients! pulse-id)
- (is (= 1
- (t2/count PulseChannel)))))
+ (testing "keep if has external email"
+ (mt/with-temp [Pulse {pulse-id :id} {}
+ PulseChannel _ {:pulse_id pulse-id
+ :channel_type :email
+ :details {:emails ["test@metabase.com"]}}]
+ (#'task.send-pulses/clear-pulse-channels-no-recipients! pulse-id)
+ (is (= 1
+ (t2/count PulseChannel)))))
- (testing "Has channel"
- (mt/with-temp [Pulse {pulse-id :id} {}
- PulseChannel _ {:pulse_id pulse-id
- :channel_type :slack
- :details {:channel ["#test"]}}]
- (#'task.send-pulses/clear-pulse-channels-no-recipients! pulse-id)
- (is (= 1
- (t2/count PulseChannel)))))))
+ (testing "clear if no recipients"
+ (mt/with-temp [Pulse {pulse-id :id} {}
+ PulseChannel _ {:pulse_id pulse-id
+ :channel_type :email}]
+ (#'task.send-pulses/clear-pulse-channels-no-recipients! pulse-id)
+ (is (= 0
+ (t2/count PulseChannel))))))
+
+ (testing "slack"
+ (testing "Has channel"
+ (mt/with-temp [Pulse {pulse-id :id} {}
+ PulseChannel _ {:pulse_id pulse-id
+ :channel_type :slack
+ :details {:channel "#test"}}]
+ (#'task.send-pulses/clear-pulse-channels-no-recipients! pulse-id)
+ (is (= 1
+ (t2/count PulseChannel)))))
+
+ (testing "No channel"
+ (mt/with-temp [Pulse {pulse-id :id} {}
+ PulseChannel _ {:pulse_id pulse-id
+ :channel_type :slack
+ :details {:channel nil}}]
+ (#'task.send-pulses/clear-pulse-channels-no-recipients! pulse-id)
+ (is (= 0
+ (t2/count PulseChannel))))))
+
+ (testing "http"
+ (testing "do not clear if has a channel_id"
+ (mt/with-temp [:model/Channel {channel-id :id} {:type :channel/metabase-test
+ :details {}}
+ :model/Pulse {pulse-id :id} {}
+ :model/PulseChannel _ {:pulse_id pulse-id
+ :channel_id channel-id
+ :channel_type "http"}]
+ (#'task.send-pulses/clear-pulse-channels-no-recipients! pulse-id)
+ (is (= 1
+ (t2/count :model/PulseChannel)))))
+
+ (testing "clear if there is no channel_id"
+ (mt/with-temp [:model/Pulse {pulse-id :id} {}
+ :model/PulseChannel _ {:pulse_id pulse-id
+ :channel_type :http}]
+ (#'task.send-pulses/clear-pulse-channels-no-recipients! pulse-id)
+ (is (= 0
+ (t2/count :model/PulseChannel))))))))
(def ^:private daily-at-1am
{:schedule_type "daily"