From a634189ae6c719e4086994fa975fb70adce7622e Mon Sep 17 00:00:00 2001
From: Jerry Huang <34140255+qwef@users.noreply.github.com>
Date: Tue, 20 Feb 2024 13:56:42 -0500
Subject: [PATCH] Add Creator Sentiment Email task (#38787)
* Add new email function and send email to creator
* remove debug stuff
* add tests, address comments
* actually add tests
* Update src/metabase/email/messages.clj
Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com>
* address comments
---------
Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com>
---
.clj-kondo/hooks/metabase/models/setting.clj | 1 +
src/metabase/analytics/stats.clj | 18 +--
.../email/creator_sentiment_email.mustache | 38 ++++++
src/metabase/email/messages.clj | 33 ++++++
.../public_settings/premium_features.clj | 2 +-
.../task/creator_sentiment_emails.clj | 110 ++++++++++++++++++
.../task/creator_sentiment_emails_test.clj | 93 +++++++++++++++
7 files changed, 279 insertions(+), 16 deletions(-)
create mode 100644 src/metabase/email/creator_sentiment_email.mustache
create mode 100644 src/metabase/task/creator_sentiment_emails.clj
create mode 100644 test/metabase/task/creator_sentiment_emails_test.clj
diff --git a/.clj-kondo/hooks/metabase/models/setting.clj b/.clj-kondo/hooks/metabase/models/setting.clj
index 4c64701a4bb..9e2aa9fdbb0 100644
--- a/.clj-kondo/hooks/metabase/models/setting.clj
+++ b/.clj-kondo/hooks/metabase/models/setting.clj
@@ -92,6 +92,7 @@
metabot-prompt-generator-token-limit
multi-setting-read-only
notification-link-base-url
+ no-surveys
num-metabot-choices
openai-api-key
openai-available-models
diff --git a/src/metabase/analytics/stats.clj b/src/metabase/analytics/stats.clj
index 1f80f79971b..93ceb3b147e 100644
--- a/src/metabase/analytics/stats.clj
+++ b/src/metabase/analytics/stats.clj
@@ -16,21 +16,9 @@
[metabase.integrations.google :as google]
[metabase.integrations.slack :as slack]
[metabase.models
- :refer [Card
- Collection
- Dashboard
- DashboardCard
- Database
- Field
- Metric
- PermissionsGroup
- Pulse
- PulseCard
- PulseChannel
- QueryCache
- Segment
- Table
- User]]
+ :refer [Card Collection Dashboard DashboardCard Database Field Metric
+ PermissionsGroup Pulse PulseCard PulseChannel QueryCache Segment
+ Table User]]
[metabase.models.humanization :as humanization]
[metabase.public-settings :as public-settings]
[metabase.util :as u]
diff --git a/src/metabase/email/creator_sentiment_email.mustache b/src/metabase/email/creator_sentiment_email.mustache
new file mode 100644
index 00000000000..98be30bcbd0
--- /dev/null
+++ b/src/metabase/email/creator_sentiment_email.mustache
@@ -0,0 +1,38 @@
+{{> metabase/email/_header }}
+ <div class="card-container" style="color: {{colorTextMedium}};">
+ {{#self-hosted}}
+ <p style="font-weight: 500; font-size: 0.875em; line-height: 1.375em; color: {{colorTextMedium}};">
+ <i>(This email is sent directly from {{self-hosted}}, it doesn't go through any external services)</i>
+ </p>
+ {{/self-hosted}}
+
+ <p style="font-weight: 500; line-height: 1.375em; color: {{colorTextMedium}};">
+ Hi {{first-name}},
+ </p>
+
+ <p style="font-weight: 500; line-height: 1.375em; color: {{colorTextMedium}};">
+ We at Metabase are working hard to make the experience of people like you, who create lots of content (questions, dashboards, etc).
+ </p>
+
+ <p style="font-weight: 600; line-height: 1.375em; color: {{colorTextMedium}};">
+ We’d like to know how we’re doing.<br>Can you spare 30 seconds to fill out this NPS survey?
+ </p>
+
+ <div style="text-align: center;">
+ <a
+ href="{{link}}"
+ style="display: inline-block; padding: 0.75rem 1.125em; background-color: {{applicationColor}}; color: #FFF; border-radius: 6px; font-size: 0.875em; font-weight: 700; text-decoration: none;"
+ >
+ Answer Now
+ </a>
+ </div>
+
+ <p style="font-weight: 500; line-height: 1.375em; color: {{colorTextMedium}};">
+ Thank you,
+ </p>
+
+ <p style="font-weight: 500; line-height: 1.375em; color: {{colorTextMedium}};">
+ - The Metabase Product Team
+ </p>
+ </div>
+{{> metabase/email/_footer }}
diff --git a/src/metabase/email/messages.clj b/src/metabase/email/messages.clj
index c99c2ab70b6..80cb761f1c0 100644
--- a/src/metabase/email/messages.clj
+++ b/src/metabase/email/messages.clj
@@ -291,6 +291,39 @@
:message (stencil/render-file "metabase/email/follow_up_email" context)}]
(email/send-message! email)))
+(defn- creator-sentiment-blob
+ "Create a blob of instance/user data to be sent to the creator sentiment survey."
+ [instance-data created_at num_dashboards num_questions num_models]
+ (-> {:instance instance-data
+ :user {:created_at created_at
+ :num_dashboards num_dashboards
+ :num_questions num_questions
+ :num_models num_models}}
+ json/generate-string
+ .getBytes
+ codecs/bytes->b64-str))
+
+(defn send-creator-sentiment-email!
+ "Format and send an email to the system admin following up on the installation."
+ [{:keys [email created_at first_name num_dashboards num_questions num_models]} instance-data]
+ {:pre [(u/email? email)]}
+ (let [blob (when (public-settings/anon-tracking-enabled)
+ (creator-sentiment-blob instance-data created_at num_dashboards num_questions num_models))
+ context (merge (common-context)
+ {:emailType "notification"
+ :logoHeader true
+ :first-name first_name
+ :link (if (public-settings/anon-tracking-enabled)
+ (str "https://metabase.com/feedback/creator?context=" blob)
+ "https://metabase.com/feedback/creator")}
+ (when-not (premium-features/is-hosted?)
+ {:self-hosted (public-settings/site-url)}))
+ message {:subject "Metabase would love your take on something"
+ :recipients [email]
+ :message-type :html
+ :message (stencil/render-file "metabase/email/creator_sentiment_email" context)}]
+ (email/send-message! message)))
+
(defn- make-message-attachment [[content-id url]]
{:type :inline
:content-id content-id
diff --git a/src/metabase/public_settings/premium_features.clj b/src/metabase/public_settings/premium_features.clj
index 842b9a1b655..ef86a464543 100644
--- a/src/metabase/public_settings/premium_features.clj
+++ b/src/metabase/public_settings/premium_features.clj
@@ -249,7 +249,7 @@
(cached-logger (premium-embedding-token) e)
#{}))))
-(defn- has-any-features?
+(defn has-any-features?
"True if we have a valid premium features token with ANY features."
[]
(boolean (seq (*token-features*))))
diff --git a/src/metabase/task/creator_sentiment_emails.clj b/src/metabase/task/creator_sentiment_emails.clj
new file mode 100644
index 00000000000..b8242222d92
--- /dev/null
+++ b/src/metabase/task/creator_sentiment_emails.clj
@@ -0,0 +1,110 @@
+(ns metabase.task.creator-sentiment-emails
+ (:require
+ [clojurewerkz.quartzite.jobs :as jobs]
+ [clojurewerkz.quartzite.schedule.cron :as cron]
+ [clojurewerkz.quartzite.triggers :as triggers]
+ [java-time.api :as t]
+ [metabase.analytics.snowplow :as snowplow]
+ [metabase.config :as config]
+ [metabase.db :as mdb]
+ [metabase.driver.sql.query-processor :as sql.qp]
+ [metabase.email :as email]
+ [metabase.email.messages :as messages]
+ [metabase.models.setting :as setting]
+ [metabase.public-settings :as public-settings]
+ [metabase.public-settings.premium-features :as premium-features]
+ [metabase.task :as task]
+ [metabase.util.i18n :refer [deferred-tru]]
+ [metabase.util.log :as log]
+ [toucan2.core :as t2]))
+
+(set! *warn-on-reflection* true)
+
+(setting/defsetting ^:private no-surveys
+ (deferred-tru "Enable or disable creator sentiment emails")
+ :type :boolean
+ :default false
+ :visibility :internal
+ :audit :getter)
+
+(defn- fetch-creators
+ "Fetch the creators who are eligible for a creator sentiment email. Which are users who, in the past 2 months:
+ - Created at least 10 questions total
+ - Created at least 2 SQL questions
+ - Created at least 1 dashboard
+ - Only admins if whitelabeling is enabled"
+ [has-whitelabelling?]
+ (t2/query {:select [[:u.email :email]
+ [:u.date_joined :created_at]
+ [:u.first_name :first_name]
+ [[:count [:distinct [:case [:= :d.archived false] :d.id]]] :num_dashboards]
+ [[:count [:distinct [:case [:and [:= :rc.type "question"] [:= :rc.archived false]] :rc.id]]] :num_questions]
+ [[:count [:distinct [:case [:and [:= :rc.type "model"] [:= :rc.archived false]] :rc.id]]] :num_models]]
+ :from [[:core_user :u]]
+ :join [[:report_card :rc] [:= :rc.creator_id :u.id]
+ [:report_dashboard :d] [:= :d.creator_id :u.id]]
+ :where [:and
+ [:>= :rc.created_at (sql.qp/add-interval-honeysql-form (mdb/db-type) :%now -2 :month)]
+ [:>= :d.created_at (sql.qp/add-interval-honeysql-form (mdb/db-type) :%now -2 :month)]
+ [:= :u.is_active true]
+ [:= :u.type "personal"]
+ (when has-whitelabelling? [:= :u.is_superuser true])]
+ :group-by [:u.id]
+ :having [:and
+ [:>= [:count [:distinct :rc.id]] 10]
+ [:>= [:count [:distinct [:case [:= :rc.query_type "native"] :rc.id]]] 2]
+ [:>= [:count [:distinct :d.id]] 1]]}))
+
+(defn fetch-plan-info
+ "Figure out what plan this Metabase instance is on."
+ []
+ (cond
+ (and config/ee-available? (premium-features/is-hosted?) (premium-features/has-any-features?)) "pro-cloud/enterprise-cloud"
+ (and config/ee-available? (premium-features/is-hosted?) (not (premium-features/has-any-features?))) "starter"
+ (and config/ee-available? (not (premium-features/is-hosted?))) "pro-self-hosted/enterprise-self-hosted"
+ (not config/ee-available?) "oss"
+ :else "unknown"))
+
+(defn- fetch-instance-data []
+ {:created_at (snowplow/instance-creation)
+ :plan (fetch-plan-info)
+ :verison (config/mb-version-info :tag)
+ :num_users (t2/count :model/User :is_active true, :type "personal")
+ :num_dashboards (t2/count :model/Dashboard :archived false)
+ :num_questions (t2/count :model/Card :archived false :type "question")
+ :num_models (t2/count :model/Card :archived false :type "model")})
+
+(defn- send-creator-sentiment-emails!
+ "Send an email to the instance admin following up on their experience with Metabase thus far."
+ []
+ ;; we need access to email AND the instance must have surveys enabled.
+ (when (and (email/email-configured?)
+ (not (no-surveys)))
+ (let [instance-data (when (public-settings/anon-tracking-enabled) (fetch-instance-data))
+ creators (fetch-creators (premium-features/enable-whitelabeling?))
+ month (- (.getValue (t/month)) 1)]
+ (doseq [creator creators]
+ ;; Send the email if the creator's email hash matches the current month
+ (when (= (-> creator :email hash (mod 12)) month)
+ (try
+ (messages/send-creator-sentiment-email! creator instance-data)
+ (catch Throwable e
+ (log/error "Problem sending creator sentiment email:" e))))))))
+
+(jobs/defjob ^{:doc "Sends out a monthly survey to a portion of the creators."} CreatorSentimentEmail [_]
+ (send-creator-sentiment-emails!))
+
+(def ^:private creator-sentiment-emails-job-key "metabase.task.creator-sentiment-emails.job")
+(def ^:private creator-sentiment-emails-trigger-key "metabase.task.creator-sentiment-emails.trigger")
+
+(defmethod task/init! ::SendCreatorSentimentEmails [_]
+ (let [job (jobs/build
+ (jobs/of-type CreatorSentimentEmail)
+ (jobs/with-identity (jobs/key creator-sentiment-emails-job-key)))
+ trigger (triggers/build
+ (triggers/with-identity (triggers/key creator-sentiment-emails-trigger-key))
+ (triggers/start-now)
+ (triggers/with-schedule
+ ;; Fire at 9:15am on the 1st day of every month
+ (cron/cron-schedule "0 15 9 1 * ?")))]
+ (task/schedule-task! job trigger)))
diff --git a/test/metabase/task/creator_sentiment_emails_test.clj b/test/metabase/task/creator_sentiment_emails_test.clj
new file mode 100644
index 00000000000..feae64aa4d4
--- /dev/null
+++ b/test/metabase/task/creator_sentiment_emails_test.clj
@@ -0,0 +1,93 @@
+(ns ^:mb/once metabase.task.creator-sentiment-emails-test
+ (:require
+ [clojure.test :refer :all]
+ [java-time.api :as t]
+ [metabase.email-test :as et :refer [inbox]]
+ [metabase.public-settings :as public-settings]
+ [metabase.public-settings.premium-features :as premium-features]
+ [metabase.task.creator-sentiment-emails :as creator-sentiment-emails]
+ [metabase.test :as mt]
+ [toucan2.tools.with-temp :as t2.with-temp]))
+
+(deftest send-creator-sentiment-emails!-test
+ (mt/with-fake-inbox
+ (testing "Make sure we only send emails when no surveys is false."
+ (with-redefs [creator-sentiment-emails/no-surveys (constantly true)]
+ (#'creator-sentiment-emails/send-creator-sentiment-emails!)
+ (is (= 0
+ (-> @inbox vals first count)))))
+
+ (testing "Make sure that send-creator-sentiment-emails! only sends emails to creators with the correct month hash."
+ (with-redefs [creator-sentiment-emails/no-surveys (constantly false)
+ creator-sentiment-emails/fetch-creators (fn [_] [{:email "a@metabase.com"} ;; mods to 1
+ {:email "b@metabase.com"} ;; mods to 4
+ {:email "c@metabase.com"}]) ;; mods to 2
+ t/month (constantly (t/month 2))]
+ (#'creator-sentiment-emails/send-creator-sentiment-emails!)
+ (is (= 1
+ (-> @inbox vals first count)))))
+
+ (mt/reset-inbox!)
+ (testing "Make sure context is included when anon tracking is enabled"
+ (with-redefs [public-settings/anon-tracking-enabled (constantly true)
+ creator-sentiment-emails/no-surveys (constantly false)
+ creator-sentiment-emails/fetch-creators (fn [_] [{:email "a@metabase.com"}])
+ t/month (constantly (t/month 2))]
+ (#'creator-sentiment-emails/send-creator-sentiment-emails!)
+ (is (= 1
+ (count (et/regex-email-bodies #"creator\?context="))))))
+
+ (mt/reset-inbox!)
+ (testing "Make sure context isn't included when anon tracking is enabled"
+ (with-redefs [public-settings/anon-tracking-enabled (constantly false)
+ creator-sentiment-emails/no-surveys (constantly false)
+ creator-sentiment-emails/fetch-creators (fn [_] [{:email "a@metabase.com"}])
+ t/month (constantly (t/month 2))]
+ (#'creator-sentiment-emails/send-creator-sentiment-emails!)
+ (is (= 0
+ (count (et/regex-email-bodies #"creator\?context="))))))
+
+ (mt/reset-inbox!)
+ (testing "Make sure external services message is included when is self hosted"
+ (with-redefs [premium-features/is-hosted? (constantly false)
+ creator-sentiment-emails/no-surveys (constantly false)
+ creator-sentiment-emails/fetch-creators (fn [_] [{:email "a@metabase.com"}])
+ t/month (constantly (t/month 2))]
+ (#'creator-sentiment-emails/send-creator-sentiment-emails!)
+ (is (= 1
+ (count (et/regex-email-bodies #"external services"))))))
+
+ (mt/reset-inbox!)
+ (testing "Make sure external services isn't included when not self hosted"
+ (with-redefs [premium-features/is-hosted? (constantly true)
+ creator-sentiment-emails/no-surveys (constantly false)
+ creator-sentiment-emails/fetch-creators (fn [_] [{:email "a@metabase.com"}])
+ t/month (constantly (t/month 2))]
+ (#'creator-sentiment-emails/send-creator-sentiment-emails!)
+ (is (= 0
+ (count (et/regex-email-bodies #"external services"))))))))
+
+(deftest fetch-creators-test
+ (let [creator-id 33
+ creator-email "creator@metabase.com"]
+ (t2.with-temp/with-temp [:model/User _ {:email creator-email :id creator-id}
+ :model/User _ {:email "noncreator@metabase.com"}
+ :model/Dashboard _ {:creator_id creator-id}
+ :model/Card _ {:creator_id creator-id :query_type "native"}
+ :model/Card _ {:creator_id creator-id :query_type "native"}
+ :model/Card _ {:creator_id creator-id}
+ :model/Card _ {:creator_id creator-id}
+ :model/Card _ {:creator_id creator-id}
+ :model/Card _ {:creator_id creator-id}
+ :model/Card _ {:creator_id creator-id}
+ :model/Card _ {:creator_id creator-id}
+ :model/Card _ {:creator_id creator-id}
+ :model/Card _ {:creator_id creator-id}]
+ (testing "Test we only fetch creators with the correct number of questions and dashboards."
+ (let [creators (#'creator-sentiment-emails/fetch-creators false)]
+ (is (= 1 (count creators)))
+ (is (= creator-email (-> creators first :email)))))
+
+ (testing "Whitelabelling only fetches superusers (doesn't fetch anyone)."
+ (let [creators (#'creator-sentiment-emails/fetch-creators true)]
+ (is (= 0 (count creators))))))))
--
GitLab