From f340a3aecb9b393b7e5490382512b90bc9b6f08f Mon Sep 17 00:00:00 2001
From: Ngoc Khuat <qn.khuat@gmail.com>
Date: Fri, 8 Nov 2024 09:31:43 +0700
Subject: [PATCH] [Notification] Send notification options (#49576)
---
src/metabase/api/pulse.clj | 2 +-
src/metabase/events/notification.clj | 4 ++--
src/metabase/notification/core.clj | 21 ++++++++++++++++-----
src/metabase/pulse/send.clj | 2 +-
src/metabase/task/notification.clj | 2 +-
src/metabase/task/send_pulses.clj | 2 +-
test/metabase/events/notification_test.clj | 4 ++--
test/metabase/notification/test_util.clj | 5 ++---
8 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/src/metabase/api/pulse.clj b/src/metabase/api/pulse.clj
index 0247d8b665b..43241a2cbb8 100644
--- a/src/metabase/api/pulse.clj
+++ b/src/metabase/api/pulse.clj
@@ -356,7 +356,7 @@
;; make sure any email addresses that are specified are allowed before sending the test Pulse.
(doseq [channel channels]
(pulse-channel/validate-email-domains channel))
- (binding [notification/*send-notification!* notification/send-notification-sync!]
+ (binding [notification/*default-options* {:notification/sync? true}]
(pulse/send-pulse! (assoc body :creator_id api/*current-user-id*)))
{:ok true})
diff --git a/src/metabase/events/notification.clj b/src/metabase/events/notification.clj
index 7bd30143932..f7fd1496ba1 100644
--- a/src/metabase/events/notification.clj
+++ b/src/metabase/events/notification.clj
@@ -76,8 +76,8 @@
:notification_ids (map :id notifications)}}
(log/infof "Found %d notifications for event: %s" (count notifications) topic)
(doseq [notification notifications]
- (notification/*send-notification!* (assoc notification :payload {:event_info (maybe-hydrate-event-info topic event-info)
- :event_topic topic})))))))
+ (notification/send-notification! (assoc notification :payload {:event_info (maybe-hydrate-event-info topic event-info)
+ :event_topic topic})))))))
(methodical/defmethod events/publish-event! ::notification
[topic event-info]
diff --git a/src/metabase/notification/core.clj b/src/metabase/notification/core.clj
index 36343ba99fd..80c272d1d5d 100644
--- a/src/metabase/notification/core.clj
+++ b/src/metabase/notification/core.clj
@@ -3,6 +3,7 @@
(:require
[metabase.notification.payload.core :as notification.payload]
[metabase.notification.send :as notification.send]
+ [metabase.util.malli :as mu]
[potemkin :as p]))
;; ------------------------------------------------------------------------------------------------;;
@@ -13,10 +14,20 @@
[notification.payload
notification-payload
Notification
- NotificationPayload]
- [notification.send
- send-notification-sync!])
+ NotificationPayload])
-(def ^:dynamic *send-notification!*
+(def ^:private Options
+ [:map
+ [:notification/sync? :boolean]])
+
+(def ^:dynamic *default-options*
+ "The default options for sending a notification."
+ {:notification/sync? false})
+
+(mu/defn send-notification!
"The function to send a notification. Defaults to `notification.send/send-notification-async!`."
- notification.send/send-notification-async!)
+ [notification & {:keys [] :as options} :- [:maybe Options]]
+ (let [options (merge *default-options* options)]
+ (if (:notification/sync? options)
+ (notification.send/send-notification-sync! notification)
+ (notification.send/send-notification-async! notification))))
diff --git a/src/metabase/pulse/send.clj b/src/metabase/pulse/send.clj
index 1a3bc94eae4..7212e145b68 100644
--- a/src/metabase/pulse/send.clj
+++ b/src/metabase/pulse/send.clj
@@ -93,7 +93,7 @@
(select-keys (-> pulse :cards first) [:include_xls :include_csv :pivot_results :format_rows]))
:handlers [(get-notification-handler pulse-channel :notification/alert)]}))
-(def ^:private send-notification! (requiring-resolve 'metabase.notification.core/*send-notification!*))
+(def ^:private send-notification! (requiring-resolve 'metabase.notification.core/send-notification!))
(defn- send-pulse!*
[{:keys [channels channel-ids] :as pulse} dashboard]
diff --git a/src/metabase/task/notification.clj b/src/metabase/task/notification.clj
index 5b6f7395ae5..9ceedebfdaa 100644
--- a/src/metabase/task/notification.clj
+++ b/src/metabase/task/notification.clj
@@ -98,7 +98,7 @@
:notification_subscription_id subscription-id
:cron_schedule (:cron_schedule subscription)
:notification_ids [notification-id]}}
- (notification/*send-notification!* (t2/select-one :model/Notification notification-id)))
+ (notification/send-notification! (t2/select-one :model/Notification notification-id)))
(log/infof "Sent notification %d for subscription %d" notification-id subscription-id)
(catch Exception e
(log/errorf e "Failed to send notification %d for subscription %d" notification-id subscription-id)
diff --git a/src/metabase/task/send_pulses.clj b/src/metabase/task/send_pulses.clj
index 24a6799661e..8e9a89c4867 100644
--- a/src/metabase/task/send_pulses.clj
+++ b/src/metabase/task/send_pulses.clj
@@ -56,7 +56,7 @@
:channel-ids (seq channel-ids)}}
(when-let [pulse (models.pulse/retrieve-notification pulse-id :archived false)]
(log/debugf "Starting Pulse Execution: %d" pulse-id)
- (binding [notification/*send-notification!* notification/send-notification-sync!]
+ (binding [notification/*default-options* {:notification/sync? true}]
(pulse/send-pulse! pulse :channel-ids channel-ids))
(log/debugf "Finished Pulse Execution: %d" pulse-id)
:done))
diff --git a/test/metabase/events/notification_test.clj b/test/metabase/events/notification_test.clj
index 162c812fffc..2290bf9a7b8 100644
--- a/test/metabase/events/notification_test.clj
+++ b/test/metabase/events/notification_test.clj
@@ -37,7 +37,7 @@
sent-notis (atom [])]
(testing "publishing event will send all the actively subscribed notifciations"
(mt/with-dynamic-redefs
- [notification/*send-notification!* (fn [notification] (swap! sent-notis conj notification))
+ [notification/send-notification! (fn [notification] (swap! sent-notis conj notification))
events.notification/supported-topics #{:event/test-notification}]
(events/publish-event! topic {::hi true})
(is (=? [[(:id n-1) {:event_info {::hi true}}]
@@ -59,7 +59,7 @@
nil)
(testing "publish an event that is not supported for notifications will not send any notifications"
(mt/with-dynamic-redefs
- [notification/*send-notification!* (fn [notification] (swap! sent-notis conj notification))
+ [notification/send-notification! (fn [notification] (swap! sent-notis conj notification))
events.notification/supported-topics #{}]
(events/publish-event! :event/unsupported-topic {::hi true})
(is (empty? @sent-notis))))))))
diff --git a/test/metabase/notification/test_util.clj b/test/metabase/notification/test_util.clj
index bf4628f30ef..1eebf520d97 100644
--- a/test/metabase/notification/test_util.clj
+++ b/test/metabase/notification/test_util.clj
@@ -6,7 +6,6 @@
[metabase.events.notification :as events.notification]
[metabase.notification.core :as notification]
[metabase.notification.payload.core :as notification.payload]
- [metabase.notification.send :as notification.send]
[metabase.test :as mt]
[metabase.util :as u]))
@@ -39,7 +38,7 @@
(defmacro with-send-notification-sync
"Notifications are sent async by default, wrap the body in this macro to send them synchronously."
[& body]
- `(binding [notification/*send-notification!* #'notification.send/send-notification-sync!]
+ `(binding [notification/*default-options* {:notification/sync? true}]
~@body))
(defn do-with-captured-channel-send!
@@ -85,7 +84,7 @@
"Macro that sets up the notification testing environment."
[& body]
`(mt/with-model-cleanup [:model/Notification]
- (notification.tu/with-send-notification-sync
+ (with-send-notification-sync
~@body)))
;; ------------------------------------------------------------------------------------------------;;
--
GitLab