Do not clear http channels when sending pulse (#48330)

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])

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?)

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"