When an admin deletes their own alert, don't send an email

Previously whenever an alert was deleted it would notify the creator of the deletion, even if the creator was the one that did the deleting. This commit changes that behavior to only notify if the creator is different from the user deleting the alert.
andyZheng · Nov 16, 2017 · 563bbba · 563bbba
1 parent a89592d
commit 563bbba
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 13 deletions.
diff --git a/src/metabase/api/alert.clj b/src/metabase/api/alert.clj
@@ -161,26 +161,32 @@
 (defn- collect-alert-recipients [alert]
   (set (:recipients (email-channel alert))))
 
+(defn- notify-on-delete-if-needed!
+  "When an alert is deleted, we notify the creator that their alert is being deleted and any recipieint that they are
+  no longer going to be receiving that alert"
+  [alert]
+  (when (email/email-configured?)
+    (let [{creator-id :id :as creator} (:creator alert)
+          ;; The creator might also be a recipient, no need to notify them twice
+          recipients (remove #(= (:id creator) (:id %)) (collect-alert-recipients alert))]
+
+      (doseq [recipient recipients]
+        (messages/send-admin-unsubscribed-alert-email! alert recipient @api/*current-user*))
+
+      (when-not (= creator-id api/*current-user-id*)
+        (messages/send-admin-deleted-your-alert! alert creator @api/*current-user*)))))
+
 (api/defendpoint DELETE "/:id"
   "Remove an alert"
   [id]
   (api/let-404 [alert (pulse/retrieve-alert id)]
     (api/check-superuser)
 
-    ;; When an alert is deleted, we notify the creator that their alert is being deleted and any recipieint that they
-    ;; are no longer going to be receiving that alert
-    (let [creator (:creator alert)
-          ;; The creator might also be a recipient, no need to notify them twice
-          recipients (remove #(= (:id creator) (:id %)) (collect-alert-recipients alert))]
-
-      (db/delete! Pulse :id id)
+    (db/delete! Pulse :id id)
 
-      (events/publish-event! :alert-delete (assoc alert :actor_id api/*current-user-id*))
+    (events/publish-event! :alert-delete (assoc alert :actor_id api/*current-user-id*))
 
-      (when (email/email-configured?)
-        (doseq [recipient recipients]
-          (messages/send-admin-unsubscribed-alert-email! alert recipient @api/*current-user*))
-        (messages/send-admin-deleted-your-alert! alert creator @api/*current-user*)))
+    (notify-on-delete-if-needed! alert)
 
     api/generic-204-no-content))
 

diff --git a/test/metabase/api/alert_test.clj b/test/metabase/api/alert_test.clj
@@ -557,7 +557,7 @@
                       PulseChannelRecipient [_             {:user_id          (user->id :rasta)
                                                             :pulse_channel_id pc-id-1}]]
   [1
-   1                                    ;<-- Alert should not be deleted
+   1 ;;<-- Alert should not be deleted
    (rasta-unsubscribe-email {"Foo" true})]
   (with-alert-setup
    [(count ((user->client :rasta) :get 200 (format "alert/question/%d" card-id)))
@@ -675,3 +675,21 @@
     (count ((user->client :rasta) :get 200 (format "alert/question/%d" card-id)))
     (et/regex-email-bodies #"Crowberto Corv deleted an alert"
                            #"Crowberto Corv unsubscribed you from alerts")]))
+
+;; When an admin deletes their own alert, it should not notify them
+(tt/expect-with-temp [Card                 [{card-id :id}  (basic-alert-query)]
+                      Pulse                [{pulse-id :id} {:alert_condition   "rows"
+                                                            :alert_first_only  false
+                                                            :creator_id        (user->id :crowberto)}]
+                      PulseCard             [_             {:pulse_id pulse-id
+                                                            :card_id  card-id
+                                                            :position 0}]
+                      PulseChannel          [{pc-id :id}   {:pulse_id pulse-id}]
+                      PulseChannelRecipient [_             {:user_id          (user->id :crowberto)
+                                                            :pulse_channel_id pc-id}]]
+  [1 nil 0 {}]
+  (with-alert-setup
+   [(count ((user->client :crowberto) :get 200 (format "alert/question/%d" card-id)))
+    ((user->client :crowberto) :delete 204 (format "alert/%d" pulse-id))
+    (count ((user->client :crowberto) :get 200 (format "alert/question/%d" card-id)))
+    (et/regex-email-bodies #".*")]))