From f573f9dddcf8bbf141f4d4fc3279b3db346378a7 Mon Sep 17 00:00:00 2001
From: Ngoc Khuat <qn.khuat@gmail.com>
Date: Wed, 23 Oct 2024 11:08:11 +0700
Subject: [PATCH] [Notification] Rework APIs for better modularization (#48876)
---
src/metabase/channel/core.clj | 24 +--
src/metabase/channel/{ => impl}/email.clj | 24 ++-
src/metabase/channel/{ => impl}/http.clj | 2 +-
src/metabase/channel/{ => impl}/slack.clj | 2 +-
src/metabase/core.clj | 3 -
src/metabase/db/custom_migrations.clj | 8 +-
.../email/alert_new_confirmation.mustache | 4 +-
src/metabase/email/new_user_invite.mustache | 28 +--
src/metabase/email/slack_token_error.mustache | 4 +-
src/metabase/events.clj | 7 +-
src/metabase/events/notification.clj | 67 ++-----
src/metabase/models/notification.clj | 1 +
src/metabase/notification/core.clj | 166 ++----------------
src/metabase/notification/payload/core.clj | 76 ++++++++
.../payload/impl/system_event.clj | 44 +++++
src/metabase/notification/send.clj | 134 ++++++++++++++
src/metabase/pulse/send.clj | 13 +-
src/metabase/util.cljc | 4 +-
src/metabase/util/namespaces.clj | 15 ++
test/metabase/api/channel_test.clj | 4 +-
.../channel/{ => impl}/email_test.clj | 2 +-
test/metabase/channel/slack_test.clj | 2 +-
test/metabase/dashboard_subscription_test.clj | 2 +-
test/metabase/events/notification_test.clj | 26 +--
test/metabase/models/notification_test.clj | 25 +--
test/metabase/models/user_test.clj | 5 +
.../notification/payload/core_test.clj | 29 +++
.../{ => payload/impl}/system_event_test.clj | 140 ++++++++-------
.../{core_test.clj => send_test.clj} | 28 +--
test/metabase/notification/test_util.clj | 32 ++--
test/metabase/test/initialize/plugins.clj | 4 +-
.../channel_template/hello_world.mustache | 2 +-
32 files changed, 509 insertions(+), 418 deletions(-)
rename src/metabase/channel/{ => impl}/email.clj (90%)
rename src/metabase/channel/{ => impl}/http.clj (99%)
rename src/metabase/channel/{ => impl}/slack.clj (99%)
create mode 100644 src/metabase/notification/payload/core.clj
create mode 100644 src/metabase/notification/payload/impl/system_event.clj
create mode 100644 src/metabase/notification/send.clj
rename test/metabase/channel/{ => impl}/email_test.clj (96%)
create mode 100644 test/metabase/notification/payload/core_test.clj
rename test/metabase/notification/{ => payload/impl}/system_event_test.clj (63%)
rename test/metabase/notification/{core_test.clj => send_test.clj} (86%)
diff --git a/src/metabase/channel/core.clj b/src/metabase/channel/core.clj
index c2641c50a82..f819e300cdf 100644
--- a/src/metabase/channel/core.clj
+++ b/src/metabase/channel/core.clj
@@ -3,9 +3,7 @@
The API is still in development and subject to change."
(:require
- [metabase.plugins.classloader :as classloader]
- [metabase.util :as u]
- [metabase.util.log :as log]))
+ [metabase.util :as u]))
(set! *warn-on-reflection* true)
@@ -29,14 +27,14 @@
channel-type))
(defmulti render-notification
- "Given a notification info, return a sequence of channel-specific messages.
+ "Given a notification payload, return a sequence of channel-specific messages.
The message format is channel-specific, one requirement is that it should be the same format that
the [[send!]] multimethod expects."
{:added "0.51.0"
- :arglists '([channel-type notification-info template recipients])}
- (fn [channel-type notification-info _template _recipients]
- [channel-type (:payload_type notification-info)]))
+ :arglists '([channel-type notification-payload template recipients])}
+ (fn [channel-type notification-payload _template _recipients]
+ [channel-type (:payload_type notification-payload)]))
(defmulti send!
"Send a message to a channel."
@@ -46,13 +44,7 @@
(:type channel)))
;; ------------------------------------------------------------------------------------------------;;
-;; Utils ;;
+;; Load the implementations ;;
;; ------------------------------------------------------------------------------------------------;;
-
-(defn find-and-load-metabase-channels!
- "Load namespaces that start with `metabase.channel."
- []
- (doseq [ns-symb u/metabase-namespace-symbols
- :when (.startsWith (name ns-symb) "metabase.channel.")]
- (log/info "Loading channel namespace:" (u/format-color :blue ns-symb))
- (classloader/require ns-symb)))
+(when-not *compile-files*
+ (u/find-and-load-namespaces! "metabase.channel.impl"))
diff --git a/src/metabase/channel/email.clj b/src/metabase/channel/impl/email.clj
similarity index 90%
rename from src/metabase/channel/email.clj
rename to src/metabase/channel/impl/email.clj
index 4a3b5f5d6b4..5d4b504d971 100644
--- a/src/metabase/channel/email.clj
+++ b/src/metabase/channel/impl/email.clj
@@ -1,4 +1,4 @@
-(ns metabase.channel.email
+(ns metabase.channel.impl.email
(:require
[metabase.channel.core :as channel]
[metabase.channel.params :as channel.params]
@@ -7,7 +7,6 @@
[metabase.email.messages :as messages]
[metabase.models.channel :as models.channel]
[metabase.models.notification :as models.notification]
- [metabase.notification.core :as notification]
[metabase.util :as u]
[metabase.util.i18n :refer [trs]]
[metabase.util.malli :as mu]
@@ -120,7 +119,7 @@
;; ------------------------------------------------------------------------------------------------;;
(defn- notification-recipients->emails
- [recipients payload]
+ [recipients notification-payload]
(into [] cat (for [recipient recipients
:let [details (:details recipient)
emails (case (:type recipient)
@@ -131,7 +130,7 @@
:notification-recipient/external-email
[(:email details)]
:notification-recipient/template
- [(not-empty (channel.params/substitute-params (:pattern details) payload :ignore-missing? (:is_optional details)))]
+ [(not-empty (channel.params/substitute-params (:pattern details) notification-payload :ignore-missing? (:is_optional details)))]
nil)]
:let [emails (filter some? emails)]
:when (seq emails)]
@@ -149,13 +148,12 @@
(mu/defmethod channel/render-notification
[:channel/email :notification/system-event]
[_channel-type
- notification-info :- notification/NotificationInfo
- template :- models.channel/ChannelTemplate
- recipients :- [:sequential models.notification/NotificationRecipient]]
+ notification-payload #_:- #_notification/NotificationPayload
+ template :- models.channel/ChannelTemplate
+ recipients :- [:sequential models.notification/NotificationRecipient]]
(assert (some? template) "Template is required for system event notifications")
- (let [payload (:payload notification-info)]
- [(construct-email (channel.params/substitute-params (-> template :details :subject) payload)
- (notification-recipients->emails recipients payload)
- [{:type "text/html; charset=utf-8"
- :content (render-body template payload)}]
- (-> template :details :recipient-type keyword))]))
+ [(construct-email (channel.params/substitute-params (-> template :details :subject) notification-payload)
+ (notification-recipients->emails recipients notification-payload)
+ [{:type "text/html; charset=utf-8"
+ :content (render-body template notification-payload)}]
+ (-> template :details :recipient-type keyword))])
diff --git a/src/metabase/channel/http.clj b/src/metabase/channel/impl/http.clj
similarity index 99%
rename from src/metabase/channel/http.clj
rename to src/metabase/channel/impl/http.clj
index 6c43843120d..c2201a9b820 100644
--- a/src/metabase/channel/http.clj
+++ b/src/metabase/channel/impl/http.clj
@@ -1,4 +1,4 @@
-(ns metabase.channel.http
+(ns metabase.channel.impl.http
(:require
[cheshire.core :as json]
[clj-http.client :as http]
diff --git a/src/metabase/channel/slack.clj b/src/metabase/channel/impl/slack.clj
similarity index 99%
rename from src/metabase/channel/slack.clj
rename to src/metabase/channel/impl/slack.clj
index 95a7f5a54b7..1d833797beb 100644
--- a/src/metabase/channel/slack.clj
+++ b/src/metabase/channel/impl/slack.clj
@@ -1,4 +1,4 @@
-(ns metabase.channel.slack
+(ns metabase.channel.impl.slack
(:require
[clojure.string :as str]
[metabase.channel.core :as channel]
diff --git a/src/metabase/core.clj b/src/metabase/core.clj
index a8cfd16f754..7ffcf096dfa 100644
--- a/src/metabase/core.clj
+++ b/src/metabase/core.clj
@@ -5,7 +5,6 @@
[environ.core :as env]
[java-time.api :as t]
[metabase.analytics.prometheus :as prometheus]
- [metabase.channel.core :as channel]
[metabase.config :as config]
[metabase.core.config-from-file :as config-from-file]
[metabase.core.initialization-status :as init-status]
@@ -166,8 +165,6 @@
(task/start-scheduler!)
;; In case we could not do this earlier (e.g. for DBs added via config file), because the scheduler was not up yet:
(database/check-and-schedule-tasks!)
- ;; load the channels
- (channel/find-and-load-metabase-channels!)
(init-status/set-complete!)
(let [start-time (.getStartTime (ManagementFactory/getRuntimeMXBean))
duration (- (System/currentTimeMillis) start-time)]
diff --git a/src/metabase/db/custom_migrations.clj b/src/metabase/db/custom_migrations.clj
index c029fbb7083..47132768338 100644
--- a/src/metabase/db/custom_migrations.clj
+++ b/src/metabase/db/custom_migrations.clj
@@ -1601,7 +1601,7 @@
{:name "User joined Email template"
:channel_type "channel/email"
:details (json/generate-string {:type "email/mustache-resource"
- :subject "{{context.extra.user-invited-email-subject}}"
+ :subject "{{payload.custom.user_invited_email_subject}}"
:path "metabase/email/new_user_invite.mustache"
:recipient-type :cc})
:created_at :%now
@@ -1614,7 +1614,7 @@
:channel_id nil
:template_id template-id
:recipients [{:type "notification-recipient/template"
- :details (json/generate-string {:pattern "{{event-info.object.email}}"})}]}])))
+ :details (json/generate-string {:pattern "{{payload.event_info.object.email}}"})}]}])))
(define-migration CreateSystemNotificationAlertCreated
(let [template-id (t2/insert-returning-pk!
@@ -1635,7 +1635,7 @@
:channel_id nil
:template_id template-id
:recipients [{:type "notification-recipient/template"
- :details (json/generate-string {:pattern "{{event-info.user.email}}"})}]}])))
+ :details (json/generate-string {:pattern "{{payload.event_info.user.email}}"})}]}])))
(define-migration CreateSystemNotificationSlackTokenError
(let [template-id (t2/insert-returning-pk!
@@ -1656,7 +1656,7 @@
:channel_id nil
:template_id template-id
:recipients [{:type "notification-recipient/template"
- :details (json/generate-string {:pattern "{{context.admin-email}}"
+ :details (json/generate-string {:pattern "{{context.admin_email}}"
:is_optional true})}
{:type "notification-recipient/group"
:permissions_group_id (t2/select-one-pk :permissions_group :name "Administrators")}]}])))
diff --git a/src/metabase/email/alert_new_confirmation.mustache b/src/metabase/email/alert_new_confirmation.mustache
index c4fed5832e2..cb3c9cb0118 100644
--- a/src/metabase/email/alert_new_confirmation.mustache
+++ b/src/metabase/email/alert_new_confirmation.mustache
@@ -1,4 +1,4 @@
{{> metabase/email/_header }}
- <p>This is just a confirmation that your alert about <a href="{{context.site-url}}/question/{{event-info.object.card.id}}">{{event-info.object.card.name}}</a> has been set up.</p>
- <p>This alert will be sent {{context.extra.alert-create-condition-description}}.</p>
+ <p>This is just a confirmation that your alert about <a href="{{context.site_url}}/question/{{payload.event_info.object.card.id}}">{{payload.event_info.object.card.name}}</a> has been set up.</p>
+ <p>This alert will be sent {{payload.custom.alert_create_condition_description}}.</p>
{{> metabase/email/_footer}}
diff --git a/src/metabase/email/new_user_invite.mustache b/src/metabase/email/new_user_invite.mustache
index 71c31accca2..0d4676edb03 100644
--- a/src/metabase/email/new_user_invite.mustache
+++ b/src/metabase/email/new_user_invite.mustache
@@ -1,6 +1,6 @@
{{> metabase/email/_header }}
<div style="padding-bottom: 2em; padding-top: 1em; text-align: center;">
- <img width="32" height="40" src="{{context.application-logo-url}}"/>
+ <img width="32" height="40" src="{{context.application_logo_url}}"/>
</div>
<div>
<div style="padding: 0.25em 0em .25em 0em; text-align: center; margin-left: auto; margin-right: auto; max-width: 400px; position: relative">
@@ -8,42 +8,42 @@
<tr><td colspan="3"><img src="http://static.metabase.com/email_graph_top.png" width="296" height="73" style="display:block" /></td></tr>
<tr>
<td height="15" width="60"><img src="http://static.metabase.com/email_graph_left.png" width="60" height="15" style="display:block" /></td>
- <td height="15" width="68" valign="middle" align="center" style="font-weight: bold; font-size: 0.6rem; line-height:15px;color: #fff; background-color:#333">{{{context.extra.user-invited-today}}}</td>
+ <td height="15" width="68" valign="middle" align="center" style="font-weight: bold; font-size: 0.6rem; line-height:15px;color: #fff; background-color:#333">{{{payload.custom.user_invited_today}}}</td>
<td height="15" width="168"><img src="http://static.metabase.com/email_graph_right.png" width="168" height="15" style="display:block" /></td></tr>
<tr><td colspan="3" height="46"><img src="http://static.metabase.com/email_graph_bottom.png" width="296" height="56" style="display:block" /></td></tr>
</table>
- <p style="line-height: 1.3rem; font-size: 12px; color: #949AAB;">{{event-info.object.first_name}}'s happiness and productivity over time</p>
+ <p style="line-height: 1.3rem; font-size: 12px; color: #949AAB;">{{payload.event_info.object.first_name}}'s happiness and productivity over time</p>
</div>
- {{#event-info.object.is_from_setup}}
+ {{#payload.event_info.object.is_from_setup}}
<div style="max-width: 500px; margin-left: auto; margin-right: auto">
- <h2 style="font-weight: bold; color: #4C5773; line-height: 2rem;">{{event-info.details.invitor.first_name}} could use your help setting up {{context.application-name}}</h2>
+ <h2 style="font-weight: bold; color: #4C5773; line-height: 2rem;">{{payload.event_info.details.invitor.first_name}} could use your help setting up {{context.application_name}}</h2>
</div>
<div style="max-width: 500px; margin-left: auto; margin-right: auto; line-height: 1.5rem; color: #4C5773;">
<p>{{context.application-name}} is a simple and powerful analytics tool that lets you easily <strong>ask questions</strong> and <strong>get answers</strong> about your company’s data.
- <p>Your {{context.application-name}} is up and running, but {{event-info.details.invitor.first_name}} needs you to connect your data. You'll probably need:
+ <p>Your {{context.application-name}} is up and running, but {{payload.event_info.details.invitor.first_name}} needs you to connect your data. You'll probably need:
<ul>
<li>Database type (Redshift, Snowflake, etc...)</li>
<li>Host and port</li>
<li>Username and password</li>
</ul>
</div>
- {{/event-info.object.is_from_setup}}
- {{^event-info.object.is_from_setup}}
+ {{/payload.event_info.object.is_from_setup}}
+ {{^payload.event_info.object.is_from_setup}}
<div style="text-align: center">
- <h2 style="font-weight: bold; color: #4C5773; line-height: 2rem;">{{event-info.details.invitor.first_name}} wants you to join them on {{context.application-name}}</h2>
+ <h2 style="font-weight: bold; color: #4C5773; line-height: 2rem;">{{payload.event_info.details.invitor.first_name}} wants you to join them on {{context.application_name}}</h2>
</div>
<div style="max-width: 500px; margin-left: auto; margin-right: auto; line-height: 1.5rem; color: #4C5773; text-align: center">
- <p>{{context.application-name}} is a simple and powerful analytics tool that lets you easily <strong>ask questions</strong> and <strong>get answers</strong> about your company’s data.
+ <p>{{context.application_name}} is a simple and powerful analytics tool that lets you easily <strong>ask questions</strong> and <strong>get answers</strong> about your company’s data.
</div>
- {{/event-info.object.is_from_setup}}
+ {{/payload.event_info.object.is_from_setup}}
<div style="padding: 1.25em 0 1em 0; text-align: center">
- <a style="display: inline-block; box-sizing: border-box; text-decoration: none; font-size: 1rem; padding: 0.8rem 2.25rem; background: #FBFCFD; color: #444; cursor: pointer; text-decoration: none; font-weight: bold; border-radius: 6px; background-color: #509EE3; color: #fff;" href="{{context.extra.user-invited-join-url}}">Join now</a>
+ <a style="display: inline-block; box-sizing: border-box; text-decoration: none; font-size: 1rem; padding: 0.8rem 2.25rem; background: #FBFCFD; color: #444; cursor: pointer; text-decoration: none; font-weight: bold; border-radius: 6px; background-color: #509EE3; color: #fff;" href="{{payload.custom.user_invited_join_url}}">Join now</a>
</div>
- {{#event-info.object.is_from_setup}}
+ {{#payload.event_info.object.is_from_setup}}
<div style="max-width: 500px; margin-left: auto; margin-right: auto; line-height: 1.5rem; color: #4C5773">
<p>P.S. If you run into any issues, check out our <a href="https://www.metabase.com/docs/latest/databases/connecting">connection</a> and <a href="https://www.metabase.com/docs/latest/troubleshooting-guide/db-connection">troubleshooting</a> guides</p>
</div>
- {{/event-info.object.is_from_setup}}
+ {{/payload.event_info.object.is_from_setup}}
</div>
</div>
</body>
diff --git a/src/metabase/email/slack_token_error.mustache b/src/metabase/email/slack_token_error.mustache
index 4cb5ff08715..f87f9613323 100644
--- a/src/metabase/email/slack_token_error.mustache
+++ b/src/metabase/email/slack_token_error.mustache
@@ -1,12 +1,12 @@
{{> metabase/email/_header }}
<div style="padding-bottom: 2em; padding-top: 1em; text-align: center;">
- <img width="32" height="40" src="{{context.application-logo-url}}"/>
+ <img width="32" height="40" src="{{context.application_logo_url}}"/>
</div>
<div style="text-align: center; margin: 0 auto; max-width: 500px">
<div style="text-align: left">
<h2 style="font-weight: normal; color: #4C545B; line-height: 34px;">Your Slack connection stopped working</h2>
<p style="line-height: 22px; margin-bottom: 40px">This may affect existing dashboard subscriptions. Follow the steps in settings to reconnect Slack and get things up and running again.</p>
</div>
- <a style="{{context.style.button}}" href="{{context.site-url}}/admin/settings/slack">Go to settings</a>
+ <a style="{{context.style.button}}" href="{{context.site_url}}/admin/settings/slack">Go to settings</a>
</div>
{{> metabase/email/_footer }}
diff --git a/src/metabase/events.clj b/src/metabase/events.clj
index 24860c295ae..f0f7b82aa29 100644
--- a/src/metabase/events.clj
+++ b/src/metabase/events.clj
@@ -19,7 +19,12 @@
[metabase.util.methodical.null-cache :as u.methodical.null-cache]
[metabase.util.methodical.unsorted-dispatcher
:as u.methodical.unsorted-dispatcher]
- [methodical.core :as methodical]))
+ [methodical.core :as methodical]
+ [potemkin :as p]))
+
+(p/import-vars
+ [events.schema
+ topic->schema])
(set! *warn-on-reflection* true)
diff --git a/src/metabase/events/notification.clj b/src/metabase/events/notification.clj
index d31422b6ec5..7bd30143932 100644
--- a/src/metabase/events/notification.clj
+++ b/src/metabase/events/notification.clj
@@ -1,18 +1,11 @@
(ns metabase.events.notification
(:require
- [java-time.api :as t]
[malli.core :as mc]
[malli.transform :as mtx]
- [metabase.email.messages :as messages]
[metabase.events :as events]
- [metabase.events.schema :as events.schema]
[metabase.models.notification :as models.notification]
[metabase.models.task-history :as task-history]
- [metabase.models.user :as user]
[metabase.notification.core :as notification]
- [metabase.public-settings :as public-settings]
- [metabase.pulse.core :as pulse]
- [metabase.util.i18n :as i18n :refer [trs]]
[metabase.util.log :as log]
[methodical.core :as methodical]
[toucan2.core :as t2]))
@@ -43,7 +36,7 @@
(defn- hydrate!
"Given a schema and value, hydrate the keys that are marked as to-hydrate.
- Hydrated keys have the :hydrate properties that can be added by [[events.schema/with-hydration]].
+ Hydrated keys have the :hydrate properties that can be added by [[metabase.events.schema/with-hydration]].
(hydrate! [:map
[:user_id {:hydrate {:key :user
@@ -54,55 +47,19 @@
[schema value]
(mc/decode schema value hydrate-transformer))
+(defn maybe-hydrate-event-info
+ "Hydrate event-info if the topic has a schema."
+ [topic event-info]
+ (cond->> event-info
+ (some? (events/topic->schema topic))
+ (hydrate! (events/topic->schema topic))))
+
(defn- notifications-for-topic
"Returns notifications for a given topic if it is supported and has notifications."
[topic]
(when (supported-topics topic)
(models.notification/notifications-for-event topic)))
-(defn- join-url
- [new-user]
- (let [reset-token (user/set-password-reset-token! (:id new-user))
- should-link-to-login-page (and (public-settings/sso-enabled?)
- (not (public-settings/enable-password-login)))]
- (if should-link-to-login-page
- (str (public-settings/site-url) "/auth/login")
- ;; NOTE: the new user join url is just a password reset with an indicator that this is a first time user
- (str (user/form-password-reset-url reset-token) "#new"))))
-
-(defn- extra-context
- "Returns a map of extra context for a given topic and event-info.
- Extra are set of contexts that are specific to a certain emails.
- Currently we need it to support usecases that our template engines doesn't support such as i18n,
- but ideally this should be part of the template."
- [topic event-info]
- (case topic
- :event/user-invited
- {:user-invited-today (t/format "MMM' 'dd,' 'yyyy" (t/zoned-date-time))
- :user-invited-email-subject (trs "You''re invited to join {0}''s {1}" (public-settings/site-name) (messages/app-name-trs))
- :user-invited-join-url (join-url (:object event-info))}
-
- :event/alert-create
- {:alert-create-condition-description (->> event-info :object
- messages/pulse->alert-condition-kwd
- (get messages/alert-condition-text))}
- {}))
-
-(defn- enriched-event-info
- [topic event-info]
- ;; DO NOT delete or rename these fields, they are used in the notification templates
- {:context {:application-name (public-settings/application-name)
- :application-logo-url (messages/logo-url)
- :site-name (public-settings/site-name)
- :site-url (public-settings/site-url)
- :admin-email (public-settings/admin-email)
- :style {:button (messages/button-style (pulse/primary-color))}
- :extra (extra-context topic event-info)}
- :event-info (cond->> event-info
- (some? (events.schema/topic->schema topic))
- (hydrate! (events.schema/topic->schema topic)))
- :event-topic topic})
-
(def ^:dynamic *skip-sending-notification?*
"Used as a hack for when we need to skip sending notifications for certain events.
@@ -117,10 +74,10 @@
:task_details {:trigger_type :notification-subscription/system-event
:event_name topic
:notification_ids (map :id notifications)}}
- (let [event-info (enriched-event-info topic event-info)]
- (log/infof "Found %d notifications for event: %s" (count notifications) topic)
- (doseq [notification notifications]
- (notification/*send-notification!* (assoc notification :payload event-info))))))))
+ (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})))))))
(methodical/defmethod events/publish-event! ::notification
[topic event-info]
diff --git a/src/metabase/models/notification.clj b/src/metabase/models/notification.clj
index aa70157c522..4c765b538b4 100644
--- a/src/metabase/models/notification.clj
+++ b/src/metabase/models/notification.clj
@@ -36,6 +36,7 @@
(def notification-types
"Set of valid notification types."
#{:notification/system-event
+ :notification/dashboard-subscription
;; for testing only
:notification/testing})
diff --git a/src/metabase/notification/core.clj b/src/metabase/notification/core.clj
index 58c2a6fa78f..02e8a304869 100644
--- a/src/metabase/notification/core.clj
+++ b/src/metabase/notification/core.clj
@@ -1,158 +1,20 @@
(ns metabase.notification.core
+ "Core functionality for notifications."
(:require
- [java-time.api :as t]
- [metabase.channel.core :as channel]
- [metabase.models.notification :as models.notification]
- [metabase.models.setting :as setting]
- [metabase.models.task-history :as task-history]
- [metabase.util :as u]
- [metabase.util.log :as log]
- [metabase.util.malli :as mu]
- [metabase.util.malli.schema :as ms]
- [metabase.util.retry :as retry]
- [toucan2.core :as t2])
- (:import
- (java.util.concurrent Callable Executors ExecutorService)
- (org.apache.commons.lang3.concurrent BasicThreadFactory$Builder)))
+ [metabase.notification.payload.core :as notification.payload]
+ [metabase.notification.send :as notification.send]
+ [potemkin :as p]))
-(set! *warn-on-reflection* true)
+;; ------------------------------------------------------------------------------------------------;;
+;; Public APIs ;;
+;; ------------------------------------------------------------------------------------------------;;
-(def ^:private Notification
- [:map {:closed true}
- [:payload_type (into [:enum] models.notification/notification-types)]
- [:id {:optional true} ms/PositiveInt]
- [:active {:optional true} :boolean]
- [:created_at {:optional true} :any]
- [:updated_at {:optional true} :any]])
-
-(def NotificationInfo
- "Schema for the notificaiton info."
- [:multi {:dispatch :payload_type}
- [:notification/system-event [:merge
- Notification
- [:map {:closed true}
- [:payload
- [:map {:closed true}
- ;; TODO: event-info schema for each event type
- [:event-topic [:fn #(= "event" (-> % keyword namespace))]]
- [:event-info [:maybe :map]]
- [:context :map]]]]]]
- ;; for testing only
- [:notification/testing :map]])
-
-(defn- hydrate-notification-handler
- [notification-handlers]
- (t2/hydrate notification-handlers
- :channel
- :template
- :recipients))
-
-(setting/defsetting notification-thread-pool-size
- "The size of the thread pool used to send notifications."
- :default 10
- :export? false
- :type :integer
- :visibility :internal)
-
-(defonce ^:private pool
- (delay (Executors/newFixedThreadPool
- (notification-thread-pool-size)
- (.build
- (doto (BasicThreadFactory$Builder.)
- (.namingPattern "send-notification-thread-pool-%d"))))))
-
-(def ^:private default-retry-config
- {:max-attempts 7
- :initial-interval-millis 500
- :multiplier 2.0
- :randomization-factor 0.1
- :max-interval-millis 30000})
-
-(defn- handler->channel-name
- [{:keys [channel_type channel_id]}]
- (if channel_id
- (str (u/qualified-name channel_type) " " channel_id)
- (u/qualified-name channel_type)))
-
-(defn- channel-send-retrying!
- [handler message]
- (try
- (let [notification-id (:notification_id handler)
- retry-config default-retry-config
- retry-errors (volatile! [])
- retry-report (fn []
- {:attempted_retries (count @retry-errors)
- ;; we want the last retry to be the most recent
- :retry_errors (reverse @retry-errors)})
- channel (or (:channel handler)
- {:type (:channel_type handler)})
- send! (fn []
- (try
- (channel/send! channel message)
- (catch Exception e
- (vswap! retry-errors conj {:message (u/strip-error e)
- :timestamp (t/offset-date-time)})
- (log/warnf e "[Notification %d] Failed to send to channel %s , retrying..." notification-id (handler->channel-name handler))
- (throw e))))
- retrier (retry/make retry-config)]
- (log/debugf "[Notification %d] Sending a message to channel %s" notification-id (handler->channel-name handler))
- (task-history/with-task-history {:task "channel-send"
- :on-success-info (fn [update-map _result]
- (cond-> update-map
- (seq @retry-errors)
- (update :task_details merge (retry-report))))
- :on-fail-info (fn [update-map _result]
- (update update-map :task_details merge (retry-report)))
- :task_details {:retry_config retry-config
- :channel_id (:id channel)
- :channel_type (:type channel)
- :template_id (:template_id handler)
- :notification_id notification-id
- :recipient_ids (map :id (:recipients handler))}}
- (retrier send!)
- (log/debugf "[Notification %d] Sent to channel %s with %d retries" notification-id (handler->channel-name handler) (count @retry-errors))))
- (catch Throwable e
- (log/errorf e "[Notification %d] Error sending notification!" (:notification_id handler)))))
-
-(mu/defn- send-notification-sync!
- "Send the notification to all handlers synchronously. Do not use this directly, use *send-notification!* instead."
- [notification-info :- NotificationInfo]
- (try
- (let [noti-handlers (hydrate-notification-handler (t2/select :model/NotificationHandler :notification_id (:id notification-info)))]
- (log/debugf "[Notification %d] Found %d handlers" (:id notification-info) (count noti-handlers))
- (task-history/with-task-history
- {:task "notification-send"
- :task_details {:notification_id (:id notification-info)
- :notification_handlers (map #(select-keys % [:id :channel_type :channel_id :template_id]) noti-handlers)}}
- (doseq [handler noti-handlers]
- (let [channel-type (:channel_type handler)
- messages (channel/render-notification
- channel-type
- notification-info
- (:template handler)
- (:recipients handler))]
- (log/debugf "[Notification %d] Got %d messages for channel %s with template %d"
- (:id notification-info) (count messages)
- (handler->channel-name handler)
- (-> handler :template :id))
- (doseq [message messages]
- (log/infof "[Notification %d] Sending message to channel %s"
- (:id notification-info) (:channel_type handler))
- (channel-send-retrying! handler message))))
- (log/infof "[Notification %d] Sent successfully" (:id notification-info))))
- (catch Exception e
- (log/errorf e "[Notification %d] Failed to send" (:id notification-info))
- (throw e)))
- nil)
-
-(defn- send-notification-async!
- "Send a notification asynchronously."
- [notification]
- (.submit ^ExecutorService @pool ^Callable
- (fn []
- (send-notification-sync! notification)))
- nil)
+(p/import-vars
+ [notification.payload
+ notification-payload
+ Notification
+ NotificationPayload])
(def ^:dynamic *send-notification!*
- "The function to send a notification. Defaults to `send-notification-async!`."
- send-notification-async!)
+ "The function to send a notification. Defaults to `notification.send/send-notification-async!`."
+ notification.send/send-notification-async!)
diff --git a/src/metabase/notification/payload/core.clj b/src/metabase/notification/payload/core.clj
new file mode 100644
index 00000000000..c9ba2b9bb03
--- /dev/null
+++ b/src/metabase/notification/payload/core.clj
@@ -0,0 +1,76 @@
+(ns metabase.notification.payload.core
+ (:require
+ [metabase.email.messages :as messages]
+ [metabase.models.notification :as models.notification]
+ [metabase.public-settings :as public-settings]
+ [metabase.pulse.core :as pulse]
+ [metabase.util :as u]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]))
+
+(def Notification
+ "Schema for the notification."
+ ;; TODO: how do we make this schema closed after :merge?
+ [:merge #_{:closed true}
+ [:map
+ [:payload_type (into [:enum] models.notification/notification-types)]
+ [:id {:optional true} ms/PositiveInt]
+ [:active {:optional true} :boolean]
+ [:created_at {:optional true} :any]
+ [:updated_at {:optional true} :any]]
+ [:multi {:dispatch :payload_type}
+ ;; system event is a bit special in that part of the payload comes from the event itself
+ [:notification/system-event [:map
+ [:payload
+ [:map {:closed true}
+ ;; TODO: event-info schema for each event type
+ [:event_topic [:fn #(= "event" (-> % keyword namespace))]]
+ [:event_info [:maybe :map]]]]]]
+
+ ;; for testing only
+ [:notification/testing :map]]])
+
+(def NotificationPayload
+ "Schema for the notification payload."
+ ;; TODO: how do we make this schema closed after :merge?
+ [:merge
+ Notification
+ [:map
+ [:context [:map]]]
+ [:multi {:dispatch :payload_type}
+ ;; override payload to add extra-context key
+ [:notification/system-event [:map
+ ;; override the payload with extra context
+ [:payload
+ [:map {:closed true}
+ [:event_topic [:fn #(= "event" (-> % keyword namespace))]]
+ [:event_info [:maybe :map]]
+ [:custom {:optional true} [:maybe :map]]]]]]
+ [:notification/testing :map]]])
+
+(defn- default-context
+ []
+ ;; DO NOT delete or rename these fields, they are used in the notification templates
+ {:application_name (public-settings/application-name)
+ :application_logo_url (messages/logo-url)
+ :site_name (public-settings/site-name)
+ :site_url (public-settings/site-url)
+ :admin_email (public-settings/admin-email)
+ :style {:button (messages/button-style (pulse/primary-color))}})
+
+(defmulti payload
+ "Given a notification info, return the notification payload."
+ :payload_type)
+
+(mu/defn notification-payload :- NotificationPayload
+ "Realize notification-info with :context and :payload."
+ [notification :- Notification]
+ (assoc notification
+ :payload (payload notification)
+ :context (default-context)))
+
+;; ------------------------------------------------------------------------------------------------;;
+;; Load the implementations ;;
+;; ------------------------------------------------------------------------------------------------;;
+(when-not *compile-files*
+ (u/find-and-load-namespaces! "metabase.notification.payload.impl"))
diff --git a/src/metabase/notification/payload/impl/system_event.clj b/src/metabase/notification/payload/impl/system_event.clj
new file mode 100644
index 00000000000..52472eba9e7
--- /dev/null
+++ b/src/metabase/notification/payload/impl/system_event.clj
@@ -0,0 +1,44 @@
+(ns metabase.notification.payload.impl.system-event
+ (:require
+ [java-time.api :as t]
+ [metabase.email.messages :as messages]
+ [metabase.models.user :as user]
+ [metabase.notification.payload.core :as notification.payload]
+ [metabase.public-settings :as public-settings]
+ [metabase.util.i18n :as i18n :refer [trs]]
+ [metabase.util.malli :as mu]))
+
+(defn- join-url
+ [new-user]
+ ;; TODO: the reset token should come from the event-info, not generated here!
+ (let [reset-token (user/set-password-reset-token! (:id new-user))
+ should-link-to-login-page (and (public-settings/sso-enabled?)
+ (not (public-settings/enable-password-login)))]
+ (if should-link-to-login-page
+ (str (public-settings/site-url) "/auth/login")
+ ;; NOTE: the new user join url is just a password reset with an indicator that this is a first time user
+ (str (user/form-password-reset-url reset-token) "#new"))))
+
+(defn- custom-payload
+ "Returns a map of custom payload for a given topic and event-info.
+ Custom are set of contexts that are specific to certain emails.
+ Currently we need it to support usecases that our template engines doesn't support such as i18n,
+ but ideally this should be part of the template."
+ [topic event-info]
+ (case topic
+ :event/user-invited
+ {:user_invited_today (t/format "MMM' 'dd,' 'yyyy" (t/zoned-date-time))
+ :user_invited_email_subject (trs "You''re invited to join {0}''s {1}" (public-settings/site-name) (messages/app-name-trs))
+ :user_invited_join_url (join-url (:object event-info))}
+
+ :event/alert-create
+ {:alert_create_condition_description (->> event-info :object
+ messages/pulse->alert-condition-kwd
+ (get messages/alert-condition-text))}
+ {}))
+
+(mu/defmethod notification.payload/payload :notification/system-event
+ [notification-info :- notification.payload/Notification]
+ (let [payload (:payload notification-info)
+ {:keys [event_topic event_info]} payload]
+ (assoc payload :custom (custom-payload event_topic event_info))))
diff --git a/src/metabase/notification/send.clj b/src/metabase/notification/send.clj
new file mode 100644
index 00000000000..e1a5ebc80d9
--- /dev/null
+++ b/src/metabase/notification/send.clj
@@ -0,0 +1,134 @@
+(ns metabase.notification.send
+ (:require
+ [java-time.api :as t]
+ [metabase.channel.core :as channel]
+ [metabase.models.setting :as setting]
+ [metabase.models.task-history :as task-history]
+ [metabase.notification.payload.core :as notification.payload]
+ [metabase.util :as u]
+ [metabase.util.log :as log]
+ [metabase.util.malli :as mu]
+ [metabase.util.retry :as retry]
+ [toucan2.core :as t2])
+ (:import
+ (java.util.concurrent Callable Executors ExecutorService)
+ (org.apache.commons.lang3.concurrent BasicThreadFactory$Builder)))
+
+(set! *warn-on-reflection* true)
+
+(defn- hydrate-notification-handler
+ [notification-handlers]
+ (t2/hydrate notification-handlers
+ :channel
+ :template
+ :recipients))
+
+(defn- handler->channel-name
+ [{:keys [channel_type channel_id]}]
+ (if channel_id
+ (str (u/qualified-name channel_type) " " channel_id)
+ (u/qualified-name channel_type)))
+
+(setting/defsetting notification-thread-pool-size
+ "The size of the thread pool used to send notifications."
+ :default 10
+ :export? false
+ :type :integer
+ :visibility :internal)
+
+(defonce ^:private pool
+ (delay (Executors/newFixedThreadPool
+ (notification-thread-pool-size)
+ (.build
+ (doto (BasicThreadFactory$Builder.)
+ (.namingPattern "send-notification-thread-pool-%d"))))))
+
+(def ^:private default-retry-config
+ {:max-attempts 7
+ :initial-interval-millis 500
+ :multiplier 2.0
+ :randomization-factor 0.1
+ :max-interval-millis 30000})
+
+(defn- channel-send-retrying!
+ [handler message]
+ (try
+ (let [notification-id (:notification_id handler)
+ retry-config default-retry-config
+ retry-errors (volatile! [])
+ retry-report (fn []
+ {:attempted_retries (count @retry-errors)
+ ;; we want the last retry to be the most recent
+ :retry_errors (reverse @retry-errors)})
+ channel (or (:channel handler)
+ {:type (:channel_type handler)})
+ send! (fn []
+ (try
+ (channel/send! channel message)
+ (catch Exception e
+ (vswap! retry-errors conj {:message (u/strip-error e)
+ :timestamp (t/offset-date-time)})
+ (log/warnf e "[Notification %d] Failed to send to channel %s , retrying..."
+ notification-id (handler->channel-name handler))
+ (throw e))))
+ retrier (retry/make retry-config)]
+ (log/debugf "[Notification %d] Sending a message to channel %s" notification-id (handler->channel-name handler))
+ (task-history/with-task-history {:task "channel-send"
+ :on-success-info (fn [update-map _result]
+ (cond-> update-map
+ (seq @retry-errors)
+ (update :task_details merge (retry-report))))
+ :on-fail-info (fn [update-map _result]
+ (update update-map :task_details merge (retry-report)))
+ :task_details {:retry_config retry-config
+ :channel_id (:id channel)
+ :channel_type (:type channel)
+ :template_id (:template_id handler)
+ :notification_id notification-id
+ :recipient_ids (map :id (:recipients handler))}}
+ (retrier send!)
+ (log/debugf "[Notification %d] Sent to channel %s with %d retries"
+ notification-id (handler->channel-name handler) (count @retry-errors))))
+ (catch Throwable e
+ (log/errorf e "[Notification %d] Error sending notification!" (:notification_id handler)))))
+
+(mu/defn send-notification-sync!
+ "Send the notification to all handlers synchronously. Do not use this directly, use *send-notification!* instead."
+ [notification-info :- notification.payload/Notification]
+ (try
+ (let [noti-handlers (hydrate-notification-handler
+ (t2/select :model/NotificationHandler :notification_id (:id notification-info)))
+ notification-payload (notification.payload/notification-payload notification-info)]
+ (log/debugf "[Notification %d] Found %d handlers" (:id notification-info) (count noti-handlers))
+ (task-history/with-task-history
+ {:task "notification-send"
+ :task_details {:notification_id (:id notification-info)
+ :notification_handlers (map #(select-keys % [:id :channel_type :channel_id :template_id]) noti-handlers)}}
+ (doseq [handler noti-handlers]
+ (let [channel-type (:channel_type handler)
+ messages (channel/render-notification
+ channel-type
+ notification-payload
+ (:template handler)
+ (:recipients handler))]
+ (log/debugf "[Notification %d] Got %d messages for channel %s with template %d"
+ (:id notification-info) (count messages)
+ (handler->channel-name handler)
+ (-> handler :template :id))
+ (doseq [message messages]
+ (log/infof "[Notification %d] Sending message to channel %s"
+ (:id notification-info) (:channel_type handler))
+ (channel-send-retrying! handler message))))
+ (log/infof "[Notification %d] Sent successfully" (:id notification-info))))
+ (catch Exception e
+ (log/errorf e "[Notification %d] Failed to send" (:id notification-info))
+ (throw e)))
+ nil)
+
+(mu/defn send-notification-async!
+ "Send a notification asynchronously."
+ [notification :- notification.payload/Notification]
+ (.submit ^ExecutorService @pool ^Callable
+ (fn []
+ (send-notification-sync! notification)))
+ nil)
diff --git a/src/metabase/pulse/send.clj b/src/metabase/pulse/send.clj
index 465d02a0221..062d1e39969 100644
--- a/src/metabase/pulse/send.clj
+++ b/src/metabase/pulse/send.clj
@@ -2,7 +2,6 @@
"Code related to sending Pulses (Alerts or Dashboard Subscriptions)."
(:require
[metabase.api.common :as api]
- [metabase.channel.core :as channel]
[metabase.events :as events]
[metabase.models.dashboard :as dashboard :refer [Dashboard]]
[metabase.models.dashboard-card :as dashboard-card]
@@ -285,6 +284,10 @@
(str (name type) " " id)
(name type)))
+(defn- channel-send!
+ [& args]
+ (apply (requiring-resolve 'metabase.channel.core/send!) args))
+
(defn- send-retrying!
[pulse-id channel message]
(try
@@ -297,7 +300,7 @@
:retry_errors @retry-errors})
send! (fn []
(try
- (channel/send! channel message)
+ (channel-send! channel message)
(catch Exception e
(vswap! retry-errors conj e)
;; Token errors have already been logged and we should not retry.
@@ -333,6 +336,10 @@
:when part]
part)))
+(defn- channel-render-notification
+ [& args]
+ (apply (requiring-resolve 'metabase.channel.core/render-notification) args))
+
(defn- pc->channel
"Given a pulse channel, return the channel object.
@@ -360,7 +367,7 @@
(u/prog1 (doseq [pulse-channel channels]
(try
(let [channel (pc->channel pulse-channel)
- messages (channel/render-notification (:type channel)
+ messages (channel-render-notification (:type channel)
(get-notification-info pulse parts pulse-channel)
nil
(channel-recipients pulse-channel))]
diff --git a/src/metabase/util.cljc b/src/metabase/util.cljc
index 6a6fee3072c..3d60ad1ca6c 100644
--- a/src/metabase/util.cljc
+++ b/src/metabase/util.cljc
@@ -66,7 +66,9 @@
with-timeout
with-us-locale]
[u.str
- build-sentence]))
+ build-sentence]
+ [u.ns
+ find-and-load-namespaces!]))
(defmacro or-with
"Like or, but determines truthiness with `pred`."
diff --git a/src/metabase/util/namespaces.clj b/src/metabase/util/namespaces.clj
index 8d6529cc022..2dbf34010c6 100644
--- a/src/metabase/util/namespaces.clj
+++ b/src/metabase/util/namespaces.clj
@@ -1,9 +1,15 @@
(ns metabase.util.namespaces
"Potemkin is Java-only, so here's a basic function-importing macro that works for both CLJS and CLJ."
+ #_{:clj-kondo/ignore [:discouraged-namespace]}
(:require
+ [metabase.plugins.classloader :as classloader]
+ [metabase.util.jvm :as u.jvm]
+ [metabase.util.log :as log]
[net.cgrand.macrovich :as macros]
[potemkin :as p]))
+(set! *warn-on-reflection* true)
+
(defn- redef [target sym]
(let [defn-name (or sym (symbol (name target)))]
`(def ~defn-name "docstring" (fn [& args#] (apply ~target args#)))))
@@ -35,3 +41,12 @@
target (symbol (name target-ns) (name target-sym))]]
(redef target new-sym)))
:clj `(p/import-vars ~@spaces)))
+
+(defn find-and-load-namespaces!
+ "Find and load all sub-namespaces of `root-ns` that are part of the Metabase channel system."
+ [root-ns]
+ (assert (string? root-ns) "root-ns must be a string")
+ (doseq [ns-symb u.jvm/metabase-namespace-symbols
+ :when (.startsWith (name ns-symb) root-ns)]
+ (log/infof "Loading namespace: %s" ns-symb)
+ (classloader/require ns-symb)))
diff --git a/test/metabase/api/channel_test.clj b/test/metabase/api/channel_test.clj
index 52c14afa3d0..faa85d9b5d0 100644
--- a/test/metabase/api/channel_test.clj
+++ b/test/metabase/api/channel_test.clj
@@ -8,8 +8,8 @@
[metabase.test :as mt]
[toucan2.core :as t2]))
-#_{:clj-kondo/ignore [:metabase/validate-deftest]}
-(use-fixtures :once (fn [& _args] (channel/find-and-load-metabase-channels!)))
+(comment
+ channel/keep-me)
(set! *warn-on-reflection* true)
diff --git a/test/metabase/channel/email_test.clj b/test/metabase/channel/impl/email_test.clj
similarity index 96%
rename from test/metabase/channel/email_test.clj
rename to test/metabase/channel/impl/email_test.clj
index 4bb3efd015a..7b76c9944f1 100644
--- a/test/metabase/channel/email_test.clj
+++ b/test/metabase/channel/impl/email_test.clj
@@ -1,4 +1,4 @@
-(ns metabase.channel.email-test
+(ns metabase.channel.impl.email-test
(:require
[clojure.test :refer :all]
[metabase.channel.core :as channel]
diff --git a/test/metabase/channel/slack_test.clj b/test/metabase/channel/slack_test.clj
index 7b250115b49..4988838dab7 100644
--- a/test/metabase/channel/slack_test.clj
+++ b/test/metabase/channel/slack_test.clj
@@ -1,7 +1,7 @@
(ns metabase.channel.slack-test
(:require
[clojure.test :refer :all]
- [metabase.channel.slack :as channel.slack]
+ [metabase.channel.impl.slack :as channel.slack]
[metabase.integrations.slack :as slack]))
(deftest create-and-upload-slack-attachments!-test
diff --git a/test/metabase/dashboard_subscription_test.clj b/test/metabase/dashboard_subscription_test.clj
index c6c899158bd..a6459db8997 100644
--- a/test/metabase/dashboard_subscription_test.clj
+++ b/test/metabase/dashboard_subscription_test.clj
@@ -2,7 +2,7 @@
(:require
[clojure.string :as str]
[clojure.test :refer :all]
- [metabase.channel.slack :as channel.slack]
+ [metabase.channel.impl.slack :as channel.slack]
[metabase.email.result-attachment :as email.result-attachment]
[metabase.models
:refer [Card
diff --git a/test/metabase/events/notification_test.clj b/test/metabase/events/notification_test.clj
index 926ed870358..162c812fffc 100644
--- a/test/metabase/events/notification_test.clj
+++ b/test/metabase/events/notification_test.clj
@@ -40,8 +40,8 @@
[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}}]
- [(:id n-2) {:event-info {::hi true}}]]
+ (is (=? [[(:id n-1) {:event_info {::hi true}}]
+ [(:id n-2) {:event_info {::hi true}}]]
(->> @sent-notis
(map (juxt :id :payload))
(sort-by first))))))))))
@@ -64,28 +64,6 @@
(events/publish-event! :event/unsupported-topic {::hi true})
(is (empty? @sent-notis))))))))
-(deftest enriched-event-info-settings-test
- (let [event-info {:foo :bar}]
- (testing "you shouldn't delete or rename these fields without 100% sure that it's not referenced
- in any channel_template.details or notification_recipient.details"
- (mt/with-additional-premium-features #{:whitelabel}
- (mt/with-temporary-setting-values
- [application-name "Metabase Test"
- site-name "Metabase Test"
- site-url "https://metabase.com"
- admin-email "ngoc@metabase.com"]
- (is (= {:event-info {:foo :bar}
- :event-topic :event/user-joined
- :context {:application-name "Metabase Test"
- :application-logo-url "http://static.metabase.com/email_logo.png"
- :site-name "Metabase Test"
- :site-url "https://metabase.com"
- :admin-email "ngoc@metabase.com"
- :style {:button true}
- :extra {}}}
- (-> (#'events.notification/enriched-event-info :event/user-joined event-info)
- (update-in [:context :style :button] string?)))))))))
-
(def user-hydra-model [:model/User :id :first_name])
(deftest hydrate-event-notifcation-test
diff --git a/test/metabase/models/notification_test.clj b/test/metabase/models/notification_test.clj
index 06f169dc457..dd05e5d634c 100644
--- a/test/metabase/models/notification_test.clj
+++ b/test/metabase/models/notification_test.clj
@@ -149,18 +149,19 @@
(deftest delete-template-set-null-on-existing-handlers-test
(testing "if a channel template is deleted, then set null on existing notification_handler"
- (mt/with-temp [:model/Channel chn-1 (assoc api.channel-test/default-test-channel :name "Channel 1")
- :model/ChannelTemplate tmpl-1 {:channel_type (:type chn-1)}]
- (let [noti (models.notification/create-notification!
- default-system-event-notification
- [default-user-invited-subscription]
- [{:channel_type (:type chn-1)
- :channel_id (:id chn-1)
- :template_id (:id tmpl-1)
- :recipients [{:type :notification-recipient/user
- :user_id (mt/user->id :rasta)}]}])]
- (t2/delete! :model/ChannelTemplate (:id tmpl-1))
- (is (=? {:template_id nil} (t2/select-one :model/NotificationHandler :notification_id (:id noti))))))))
+ (mt/with-model-cleanup [:model/Notification]
+ (mt/with-temp [:model/Channel chn-1 (assoc api.channel-test/default-test-channel :name "Channel 1")
+ :model/ChannelTemplate tmpl-1 {:channel_type (:type chn-1)}]
+ (let [noti (models.notification/create-notification!
+ default-system-event-notification
+ [default-user-invited-subscription]
+ [{:channel_type (:type chn-1)
+ :channel_id (:id chn-1)
+ :template_id (:id tmpl-1)
+ :recipients [{:type :notification-recipient/user
+ :user_id (mt/user->id :rasta)}]}])]
+ (t2/delete! :model/ChannelTemplate (:id tmpl-1))
+ (is (=? {:template_id nil} (t2/select-one :model/NotificationHandler :notification_id (:id noti)))))))))
(deftest cross-check-channel-type-and-template-type-test
(testing "can't create a handler with a template that has different channel type"
diff --git a/test/metabase/models/user_test.clj b/test/metabase/models/user_test.clj
index b33f79b30b8..15a19adb783 100644
--- a/test/metabase/models/user_test.clj
+++ b/test/metabase/models/user_test.clj
@@ -31,6 +31,7 @@
[metabase.server.middleware.session :as mw.session]
[metabase.test :as mt]
[metabase.test.data.users :as test.users]
+ [metabase.test.fixtures :as fixtures]
[metabase.test.integrations.ldap :as ldap.test]
[metabase.util :as u]
[metabase.util.password :as u.password]
@@ -39,6 +40,10 @@
(set! *warn-on-reflection* true)
+(use-fixtures
+ :once
+ (fixtures/initialize :test-users))
+
(comment
;; this has to be loaded for the Google Auth tests to work
metabase.integrations.google/keep-me)
diff --git a/test/metabase/notification/payload/core_test.clj b/test/metabase/notification/payload/core_test.clj
new file mode 100644
index 00000000000..3a73bc919f0
--- /dev/null
+++ b/test/metabase/notification/payload/core_test.clj
@@ -0,0 +1,29 @@
+(ns metabase.notification.payload.core-test
+ (:require
+ [clojure.test :refer :all]
+ [metabase.notification.payload.core :as notification.payload]
+ [metabase.test :as mt]))
+
+(deftest default-context-test
+ (testing "you shouldn't delete or rename these fields without 100% sure that it's not referenced
+ in any channel_template.details or notification_recipient.details"
+ (mt/with-additional-premium-features #{:whitelabel}
+ (mt/with-temporary-setting-values
+ [application-name "Metabase Test"
+ site-name "Metabase Test"
+ site-url "https://metabase.com"
+ admin-email "ngoc@metabase.com"]
+ (is (= {:payload_type :notification/system-event
+ :payload {:event_info {:foo :bar}
+ :event_topic :event/user-joined
+ :custom {}}
+ :context {:application_name "Metabase Test"
+ :application_logo_url "http://static.metabase.com/email_logo.png"
+ :site_name "Metabase Test"
+ :site_url "https://metabase.com"
+ :admin_email "ngoc@metabase.com"
+ :style {:button true}}}
+ (-> (notification.payload/notification-payload {:payload_type :notification/system-event
+ :payload {:event_topic :event/user-joined
+ :event_info {:foo :bar}}})
+ (update-in [:context :style :button] string?))))))))
diff --git a/test/metabase/notification/system_event_test.clj b/test/metabase/notification/payload/impl/system_event_test.clj
similarity index 63%
rename from test/metabase/notification/system_event_test.clj
rename to test/metabase/notification/payload/impl/system_event_test.clj
index 8c28ef1b737..8c6c164d6bf 100644
--- a/test/metabase/notification/system_event_test.clj
+++ b/test/metabase/notification/payload/impl/system_event_test.clj
@@ -1,4 +1,4 @@
-(ns metabase.notification.system-event-test
+(ns metabase.notification.payload.impl.system-event-test
(:require
[clojure.test :refer :all]
[metabase.events :as events]
@@ -23,79 +23,77 @@
(deftest system-event-e2e-test
(testing "a system event that sends to an email channel with a custom template to an user recipient"
- (mt/with-model-cleanup [:model/Notification]
- (notification.tu/with-send-notification-sync
- (mt/with-temp [:model/ChannelTemplate tmpl {:channel_type :channel/email
- :details {:type :email/mustache-text
- :subject "Welcome {{event-info.object.first_name}} to {{context.site-name}}"
- :body "Hello {{event-info.object.first_name}}! Welcome to {{context.site-name}}!"}}
- :model/User {user-id :id} {:email "ngoc@metabase.com"}
- :model/PermissionsGroup {group-id :id} {:name "Avengers"}
- :model/PermissionsGroupMembership _ {:group_id group-id
- :user_id user-id}]
- (let [rasta (mt/fetch-user :rasta)]
- (models.notification/create-notification!
- {:payload_type :notification/system-event}
- [{:type :notification-subscription/system-event
- :event_name :event/user-invited}]
- [{:channel_type :channel/email
- :template_id (:id tmpl)
- :recipients [{:type :notification-recipient/user
- :user_id (mt/user->id :crowberto)}
- {:type :notification-recipient/group
- :permissions_group_id group-id}
- {:type :notification-recipient/external-email
- :details {:email "hi@metabase.com"}}]}])
- (mt/with-temporary-setting-values
- [site-name "Metabase Test"]
- (mt/with-fake-inbox
- (publish-user-invited-event! rasta {:first_name "Ngoc" :email "ngoc@metabase.com"} false)
- (let [email {:from "notifications@metabase.com",
- :subject "Welcome Rasta to Metabase Test"
- :body [{:type "text/html; charset=utf-8"
- :content "Hello Rasta! Welcome to Metabase Test!"}]}]
- (is (=? {"crowberto@metabase.com" [email]
- "ngoc@metabase.com" [email]
- "hi@metabase.com" [email]}
- @mt/inbox)))))))))))
+ (notification.tu/with-notification-testing-setup
+ (mt/with-temp [:model/ChannelTemplate tmpl {:channel_type :channel/email
+ :details {:type :email/mustache-text
+ :subject "Welcome {{payload.event_info.object.first_name}} to {{context.site_name}}"
+ :body "Hello {{payload.event_info.object.first_name}}! Welcome to {{context.site_name}}!"}}
+ :model/User {user-id :id} {:email "ngoc@metabase.com"}
+ :model/PermissionsGroup {group-id :id} {:name "Avengers"}
+ :model/PermissionsGroupMembership _ {:group_id group-id
+ :user_id user-id}]
+ (let [rasta (mt/fetch-user :rasta)]
+ (models.notification/create-notification!
+ {:payload_type :notification/system-event}
+ [{:type :notification-subscription/system-event
+ :event_name :event/user-invited}]
+ [{:channel_type :channel/email
+ :template_id (:id tmpl)
+ :recipients [{:type :notification-recipient/user
+ :user_id (mt/user->id :crowberto)}
+ {:type :notification-recipient/group
+ :permissions_group_id group-id}
+ {:type :notification-recipient/external-email
+ :details {:email "hi@metabase.com"}}]}])
+ (mt/with-temporary-setting-values
+ [site-name "Metabase Test"]
+ (mt/with-fake-inbox
+ (publish-user-invited-event! rasta {:first_name "Ngoc" :email "ngoc@metabase.com"} false)
+ (let [email {:from "notifications@metabase.com",
+ :subject "Welcome Rasta to Metabase Test"
+ :body [{:type "text/html; charset=utf-8"
+ :content "Hello Rasta! Welcome to Metabase Test!"}]}]
+ (is (=? {"crowberto@metabase.com" [email]
+ "ngoc@metabase.com" [email]
+ "hi@metabase.com" [email]}
+ @mt/inbox))))))))))
(deftest system-event-resouce-template-test
(testing "a system event that sends to an email channel with a custom template to an user recipient"
- (mt/with-model-cleanup [:model/Notification]
- (notification.tu/with-send-notification-sync
- (mt/with-temp [:model/ChannelTemplate tmpl {:channel_type :channel/email
- :details {:type :email/mustache-resource
- :subject "Welcome {{event-info.object.first_name}} to {{context.site-name}}"
- :path "notification/channel_template/hello_world"}}
- :model/User {user-id :id} {:email "ngoc@metabase.com"}
- :model/PermissionsGroup {group-id :id} {:name "Avengers"}
- :model/PermissionsGroupMembership _ {:group_id group-id
- :user_id user-id}]
- (let [rasta (mt/fetch-user :rasta)]
- (models.notification/create-notification!
- {:payload_type :notification/system-event}
- [{:type :notification-subscription/system-event
- :event_name :event/user-invited}]
- [{:channel_type :channel/email
- :template_id (:id tmpl)
- :recipients [{:type :notification-recipient/user
- :user_id (mt/user->id :crowberto)}
- {:type :notification-recipient/group
- :permissions_group_id group-id}
- {:type :notification-recipient/external-email
- :details {:email "hi@metabase.com"}}]}])
- (mt/with-temporary-setting-values
- [site-name "Metabase Test"]
- (mt/with-fake-inbox
- (publish-user-invited-event! rasta {:first_name "Ngoc" :email "ngoc@metabase.com"} false)
- (let [email {:from "notifications@metabase.com",
- :subject "Welcome Rasta to Metabase Test"
- :body [{:type "text/html; charset=utf-8"
- :content "Hello Rasta! Welcome to Metabase Test!\n"}]}]
- (is (=? {"crowberto@metabase.com" [email]
- "ngoc@metabase.com" [email]
- "hi@metabase.com" [email]}
- @mt/inbox)))))))))))
+ (notification.tu/with-notification-testing-setup
+ (mt/with-temp [:model/ChannelTemplate tmpl {:channel_type :channel/email
+ :details {:type :email/mustache-resource
+ :subject "Welcome {{payload.event_info.object.first_name}} to {{context.site_name}}"
+ :path "notification/channel_template/hello_world"}}
+ :model/User {user-id :id} {:email "ngoc@metabase.com"}
+ :model/PermissionsGroup {group-id :id} {:name "Avengers"}
+ :model/PermissionsGroupMembership _ {:group_id group-id
+ :user_id user-id}]
+ (let [rasta (mt/fetch-user :rasta)]
+ (models.notification/create-notification!
+ {:payload_type :notification/system-event}
+ [{:type :notification-subscription/system-event
+ :event_name :event/user-invited}]
+ [{:channel_type :channel/email
+ :template_id (:id tmpl)
+ :recipients [{:type :notification-recipient/user
+ :user_id (mt/user->id :crowberto)}
+ {:type :notification-recipient/group
+ :permissions_group_id group-id}
+ {:type :notification-recipient/external-email
+ :details {:email "hi@metabase.com"}}]}])
+ (mt/with-temporary-setting-values
+ [site-name "Metabase Test"]
+ (mt/with-fake-inbox
+ (publish-user-invited-event! rasta {:first_name "Ngoc" :email "ngoc@metabase.com"} false)
+ (let [email {:from "notifications@metabase.com",
+ :subject "Welcome Rasta to Metabase Test"
+ :body [{:type "text/html; charset=utf-8"
+ :content "Hello Rasta! Welcome to Metabase Test!\n"}]}]
+ (is (=? {"crowberto@metabase.com" [email]
+ "ngoc@metabase.com" [email]
+ "hi@metabase.com" [email]}
+ @mt/inbox))))))))))
(deftest user-invited-event-send-email-test
(testing "publish an :user-invited event will send an email"
diff --git a/test/metabase/notification/core_test.clj b/test/metabase/notification/send_test.clj
similarity index 86%
rename from test/metabase/notification/core_test.clj
rename to test/metabase/notification/send_test.clj
index 8ee3cfb14dc..2de32b4959a 100644
--- a/test/metabase/notification/core_test.clj
+++ b/test/metabase/notification/send_test.clj
@@ -1,9 +1,9 @@
-(ns metabase.notification.core-test
+(ns metabase.notification.send-test
(:require
[clojure.test :refer :all]
[metabase.channel.core :as channel]
[metabase.models.notification :as models.notification]
- [metabase.notification.core :as notification]
+ [metabase.notification.send :as notification.send]
[metabase.notification.test-util :as notification.tu]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
@@ -29,13 +29,17 @@
:channel_id (:id chn-2)
:recipients [{:type :notification-recipient/user
:user_id (mt/user->id :rasta)}]}])
- notification-info (assoc n :payload {:event-info {:test true}
- :event-topic :event/test
- :context {:test true}})
+ notification-info (assoc n :payload {:event_info {:test true}
+ :event_topic :event/test})
+ expected-notification-payload (mt/malli=?
+ [:map
+ [:payload_type [:= :notification/system-event]]
+ [:context :map]
+ [:payload :map]])
renders (atom [])]
- (mt/with-dynamic-redefs [channel/render-notification (fn [channel-type notification template recipients]
+ (mt/with-dynamic-redefs [channel/render-notification (fn [channel-type notification-payload template recipients]
(swap! renders conj {:channel-type channel-type
- :notification notification
+ :notification-payload notification-payload
:template template
:recipients recipients})
;; rendered messages are recipients
@@ -44,15 +48,15 @@
(is (=? {:channel/metabase-test [{:type :notification-recipient/user :user_id (mt/user->id :crowberto)}
{:type :notification-recipient/user :user_id (mt/user->id :rasta)}]}
(notification.tu/with-captured-channel-send!
- (notification/*send-notification!* notification-info)))))
+ (notification.send/send-notification-sync! notification-info)))))
(testing "render-notification is called on all handlers with the correct channel and template"
(is (=? [{:channel-type (keyword notification.tu/test-channel-type)
- :notification notification-info
+ :notification-payload expected-notification-payload
:template tmpl
:recipients [{:type :notification-recipient/user :user_id (mt/user->id :crowberto)}]}
{:channel-type (keyword notification.tu/test-channel-type)
- :notification notification-info
+ :notification-payload expected-notification-payload
:template nil
:recipients [{:type :notification-recipient/user :user_id (mt/user->id :rasta)}]}]
@renders)))))))))
@@ -67,7 +71,7 @@
:channel_id (:id chn)
:recipients [{:type :notification-recipient/user :user_id (mt/user->id :crowberto)}]}])]
(t2/delete! :model/TaskHistory)
- (notification/*send-notification!* n)
+ (notification.send/send-notification-sync! n)
(is (=? [{:task "notification-send"
:task_details {:notification_id (:id n)
:notification_handlers [{:id (mt/malli=? :int)
@@ -105,7 +109,7 @@
(throw (Exception. "test-exception"))
(reset! send-args args)))]
(with-redefs [channel/send! send!]
- (notification/*send-notification!* n))
+ (notification.send/send-notification-sync! n))
(is (some? @send-args))
(is (=? {:task "channel-send"
:task_details {:attempted_retries 1
diff --git a/test/metabase/notification/test_util.clj b/test/metabase/notification/test_util.clj
index c5df6f2be4d..bf4628f30ef 100644
--- a/test/metabase/notification/test_util.clj
+++ b/test/metabase/notification/test_util.clj
@@ -5,6 +5,8 @@
[metabase.channel.core :as channel]
[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]))
@@ -29,11 +31,15 @@
[_channel-type notification-info _template _recipients]
[notification-info])
+(defmethod notification.payload/payload :notification/testing
+ [_notification]
+ {::payload? true})
+
#_{:clj-kondo/ignore [:metabase/test-helpers-use-non-thread-safe-functions]}
(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-notification-sync!]
+ `(binding [notification/*send-notification!* #'notification.send/send-notification-sync!]
~@body))
(defn do-with-captured-channel-send!
@@ -96,27 +102,9 @@
:return-value true}
:active true})
-;; :model/ChannelTemplate
-
(def channel-template-email-with-mustache-body
- "A :model/ChannelTemplate for email channels that has a :event/mustache template."
+ "A :model/ChannelTemplate for email channels that has a :event/mustache-text template."
{:channel_type :channel/email
:details {:type :email/mustache-text
- :subject "Welcome {{event-info.object.first_name}} to {{settings.site-name}}"
- :body "Hello {{event-info.object.first_name}}! Welcome to {{settings.site-name}}!"}})
-
-;; notification info
-(def notification-info-user-joined-event
- "A notification-info of the user-joined system event notification that can be used
- to test [[channel/render-notification]]."
- {:payload_type :notification/system-event
- :payload (#'events.notification/enriched-event-info
- :event/user-joined
- {:object
- {:email "rasta@metabase.com"
- :first_name "Rasta"
- :last_login nil
- :is_qbnewb true
- :is_superuser false
- :last_name "Toucan"
- :common_name "Rasta Toucan"}})})
+ :subject "Welcome {{payload.event_info.object.first_name}} to {{context.site_name}}"
+ :body "Hello {{payload.event_info.object.first_name}}! Welcome to {{context.site_name}}!"}})
diff --git a/test/metabase/test/initialize/plugins.clj b/test/metabase/test/initialize/plugins.clj
index 349a81431ea..fe29d5f26ec 100644
--- a/test/metabase/test/initialize/plugins.clj
+++ b/test/metabase/test/initialize/plugins.clj
@@ -2,7 +2,6 @@
(:require
[clojure.java.io :as io]
[clojure.tools.reader.edn :as edn]
- [metabase.channel.core :as channel]
[metabase.plugins :as plugins]
[metabase.plugins.initialize :as plugins.init]
[metabase.test.data.env.impl :as tx.env.impl]
@@ -62,8 +61,7 @@
(defn init! []
(plugins/load-plugins!)
- (load-plugin-manifests!)
- (channel/find-and-load-metabase-channels!))
+ (load-plugin-manifests!))
(defn init-test-drivers!
"Explicitly initialize the given test `drivers` via plugin manifests. These manifests can live in test_modules (having
diff --git a/test_resources/notification/channel_template/hello_world.mustache b/test_resources/notification/channel_template/hello_world.mustache
index 3fc51fda501..d3aa045159c 100644
--- a/test_resources/notification/channel_template/hello_world.mustache
+++ b/test_resources/notification/channel_template/hello_world.mustache
@@ -1 +1 @@
-Hello {{event-info.object.first_name}}! Welcome to {{context.site-name}}!
+Hello {{payload.event_info.object.first_name}}! Welcome to {{context.site_name}}!
--
GitLab