New endpoints for fetching alerts & subscriptions (#17531)

src/metabase/api/alert.clj

@@ -20,12 +20,20 @@
 
 (api/defendpoint GET "/"
   "Fetch all alerts"
-  [archived]
-  {archived (s/maybe su/BooleanString)}
-  (as-> (pulse/retrieve-alerts {:archived? (Boolean/parseBoolean archived)}) <>
+  [archived user_id]
+  {archived (s/maybe su/BooleanString)
+   user_id  (s/maybe su/IntGreaterThanZero)}
+  (as-> (pulse/retrieve-alerts {:archived? (Boolean/parseBoolean archived)
+                                :user-id   user_id}) <>
     (filter mi/can-read? <>)
     (hydrate <> :can_write)))
 
+(api/defendpoint GET "/:id"
+  "Fetch an alert by ID"
+  [id]
+  (-> (api/read-check (pulse/retrieve-alert id))
+      (hydrate :can_write)))
+
 (api/defendpoint GET "/question/:id"
   "Fetch all questions for the given question (`Card`) id"
   [id]

src/metabase/api/pulse.clj

@@ -31,12 +31,16 @@
 (u/ignore-exceptions (classloader/require 'metabase-enterprise.sandbox.api.util))
 
 (api/defendpoint GET "/"
-  "Fetch all Pulses"
-  [archived dashboard_id]
+  "Fetch all Pulses. If `dashboard_id` is specified, restricts results to dashboard subscriptions
+  associated with that dashboard. If `user_id` is specified, restricts results to pulses or subscriptions
+  created by the user, or for which the user is a known recipient."
+  [archived dashboard_id user_id]
   {archived     (s/maybe su/BooleanString)
-   dashboard_id (s/maybe su/IntGreaterThanZero)}
+   dashboard_id (s/maybe su/IntGreaterThanZero)
+   user_id      (s/maybe su/IntGreaterThanZero)}
   (as-> (pulse/retrieve-pulses {:archived?    (Boolean/parseBoolean archived)
-                                :dashboard-id dashboard_id}) <>
+                                :dashboard-id dashboard_id
+                                :user-id      user_id}) <>
     (filter mi/can-read? <>)
     (hydrate <> :can_write)))
 

src/metabase/models/pulse.clj

@@ -218,38 +218,57 @@
           hydrate-notification
           notification->alert))
 
+(defn- query-as [model query]
+  (db/do-post-select model (db/query query)))
+
 (s/defn retrieve-alerts :- [PulseInstance]
   "Fetch all Alerts."
   ([]
    (retrieve-alerts nil))
 
-  ([{:keys [archived?]
+  ([{:keys [archived? user-id]
      :or   {archived? false}}]
-   (for [alert (hydrate-notifications (db/select Pulse
-                                        :alert_condition [:not= nil]
-                                        :archived        archived?
-                                        {:order-by [[:%lower.name :asc]]}))
-         :let [alert (notification->alert alert)]
-         ;; if for whatever reason the Alert doesn't have a Card associated with it (e.g. the Card was deleted) don't
-         ;; return the Alert -- it's basically orphaned/invalid at this point. See #13575 -- we *should* be deleting
-         ;; Alerts if their associated PulseCard is deleted, but that's not currently the case.
-         :when (:card alert)]
-     alert)))
-
-(defn- query-as [model query]
-  (db/do-post-select model (db/query query)))
+   (let [query {:select    [:p.* [:%lower.p.name :lower-name]]
+                :modifiers [:distinct]
+                :from      [[Pulse :p]]
+                :left-join (when user-id
+                             [[PulseChannel :pchan] [:= :p.id :pchan.pulse_id]
+                              [PulseChannelRecipient :pcr] [:= :pchan.id :pcr.pulse_channel_id]])
+                :where     [:and
+                            [:not= :p.alert_condition nil]
+                            [:= :p.archived archived?]
+                            (when user-id
+                              [:or
+                               [:= :p.creator_id user-id]
+                               [:= :pcr.user_id user-id]])]
+                :order-by  [[:lower-name :asc]]}]
+     (for [alert (hydrate-notifications (query-as Pulse query))
+           :let [alert (notification->alert alert)]
+          ;; if for whatever reason the Alert doesn't have a Card associated with it (e.g. the Card was deleted) don't
+          ;; return the Alert -- it's basically orphaned/invalid at this point. See #13575 -- we *should* be deleting
+          ;; Alerts if their associated PulseCard is deleted, but that's not currently the case.
+           :when (:card alert)]
+       alert))))
 
 (s/defn retrieve-pulses :- [PulseInstance]
   "Fetch all `Pulses`."
-  [{:keys [archived? dashboard-id]
+  [{:keys [archived? dashboard-id user-id]
     :or   {archived? false}}]
   (let [query {:select    [:p.* [:%lower.p.name :lower-name]]
                :modifiers [:distinct]
                :from      [[Pulse :p]]
+               :left-join (when user-id
+                            [[PulseChannel :pchan] [:= :p.id :pchan.pulse_id]
+                             [PulseChannelRecipient :pcr] [:= :pchan.id :pcr.pulse_channel_id]])
                :where     [:and
                            [:= :p.alert_condition nil]
                            [:= :p.archived archived?]
-                           [:= :p.dashboard_id dashboard-id]]
+                           (when dashboard-id
+                             [:= :p.dashboard_id dashboard-id])
+                           (when user-id
+                             [:or
+                              [:= :p.creator_id user-id]
+                              [:= :pcr.user_id user-id]])]
                :order-by  [[:lower-name :asc]]}]
     (for [pulse (query-as Pulse query)]
       (-> pulse

test/metabase/api/alert_test.clj

@@ -145,7 +145,7 @@
 
 ;; by default, archived Alerts should be excluded
 
-(deftest get-alert-tests
+(deftest get-alerts-test
   (testing "archived alerts should be excluded"
     (is (= #{"Not Archived"}
            (with-alert-in-collection [_ _ not-archived-alert]
@@ -153,7 +153,7 @@
                (db/update! Pulse (u/the-id not-archived-alert) :name "Not Archived")
                (db/update! Pulse (u/the-id archived-alert)     :name "Archived", :archived true)
                (with-alerts-in-readable-collection [not-archived-alert archived-alert]
-                 (set (map :name ((user->client :rasta) :get 200 "alert")))))))))
+                 (set (map :name (mt/user-http-request :rasta :get 200 "alert")))))))))
 
   (testing "fetch archived alerts"
     (is (= #{"Archived"}
@@ -162,7 +162,39 @@
                (db/update! Pulse (u/the-id not-archived-alert) :name "Not Archived")
                (db/update! Pulse (u/the-id archived-alert)     :name "Archived", :archived true)
                (with-alerts-in-readable-collection [not-archived-alert archived-alert]
-                 (set (map :name ((user->client :rasta) :get 200 "alert?archived=true"))))))))))
+                 (set (map :name (mt/user-http-request :rasta :get 200 "alert?archived=true")))))))))
+
+  (testing "fetch alerts by user ID -- should return alerts created by the user,
+           or alerts for which the user is a known recipient"
+    (with-alert-in-collection [_ _ creator-alert]
+      (with-alert-in-collection [_ _ recipient-alert]
+        (with-alert-in-collection [_ _ other-alert]
+          (with-alerts-in-readable-collection [creator-alert recipient-alert other-alert]
+            (db/update! Pulse (u/the-id creator-alert) :name "LuckyCreator" :creator_id (mt/user->id :lucky))
+            (db/update! Pulse (u/the-id recipient-alert) :name "LuckyRecipient")
+            (db/update! Pulse (u/the-id other-alert) :name "Other")
+            (mt/with-temp* [PulseChannel [pulse-channel {:pulse_id (u/the-id recipient-alert)}]
+                            PulseChannelRecipient [_ {:pulse_channel_id (u/the-id pulse-channel), :user_id (mt/user->id :lucky)}]]
+              (is (= #{"LuckyCreator" "LuckyRecipient"}
+                      (set (map :name (mt/user-http-request :rasta :get 200 (str "alert?user_id=" (mt/user->id :lucky)))))))
+              (is (= #{"LuckyRecipient" "Other"}
+                      (set (map :name (mt/user-http-request :rasta :get 200 (str "alert?user_id=" (mt/user->id :rasta)))))))
+              (is (= #{}
+                      (set (map :name (mt/user-http-request :rasta :get 200 (str "alert?user_id=" (mt/user->id :trashbird))))))))))))))
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; |                                               GET /api/alert/:id                                               |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(deftest get-alert-test
+  (testing "an alert can be fetched by ID"
+    (with-alert-in-collection [_ _ alert]
+      (with-alerts-in-readable-collection [alert]
+        (is (= (u/the-id alert)
+               (:id (mt/user-http-request :rasta :get 200 (str "alert/" (u/the-id alert)))))))))
+
+  (testing "fetching a non-existing alert returns an error"
+    (mt/user-http-request :rasta :get 404 (str "alert/" 123))))
 
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                                POST /api/alert                                                 |

test/metabase/api/pulse_test.clj

@@ -801,7 +801,22 @@
                       Pulse [archived-pulse     {:name "Archived", :archived true}]]
         (with-pulses-in-readable-collection [not-archived-pulse archived-pulse]
           (is (= #{"Archived"}
-                 (set (map :name (mt/user-http-request :rasta :get 200 "pulse?archived=true"))))))))))
+                 (set (map :name (mt/user-http-request :rasta :get 200 "pulse?archived=true"))))))))
+
+    (testing "can fetch pulses by user ID -- should return pulses created by the user,
+           or pulses for which the user is a known recipient"
+      (mt/with-temp* [Pulse                 [creator-pulse   {:name "LuckyCreator" :creator_id (mt/user->id :lucky)}]
+                      Pulse                 [recipient-pulse {:name "LuckyRecipient"}]
+                      Pulse                 [other-pulse     {:name "Other"}]
+                      PulseChannel          [pulse-channel   {:pulse_id (u/the-id recipient-pulse)}]
+                      PulseChannelRecipient [_               {:pulse_channel_id (u/the-id pulse-channel),
+                                                              :user_id (mt/user->id :lucky)}]]
+        (is (= #{"LuckyCreator" "LuckyRecipient"}
+               (set (map :name (mt/user-http-request :rasta :get 200 (str "pulse?user_id=" (mt/user->id :lucky)))))))
+        (is (= #{"LuckyRecipient" "Other"}
+               (set (map :name (mt/user-http-request :rasta :get 200 (str "pulse?user_id=" (mt/user->id :rasta)))))))
+        (is (= #{}
+               (set (map :name (mt/user-http-request :rasta :get 200 (str "pulse?user_id=" (mt/user->id :trashbird)))))))))))
 
 
 ;;; +----------------------------------------------------------------------------------------------------------------+