Fix some bad code

src/metabase/task/follow_up_emails.clj

@@ -9,10 +9,11 @@
             (metabase [db :as db]
                       [email :as email])
             [metabase.email.messages :as messages]
-            (metabase.models [activity :as activity]
+            (metabase.models [activity :refer [Activity]]
                              [setting :as setting]
-                             [user :as user]
-                             [view-log :as view-log])
+                             [user :as user, :refer [User]]
+                             [view-log :refer [ViewLog]])
+            [metabase.public-settings :as public-settings]
             [metabase.task :as task]))
 
 
@@ -23,8 +24,8 @@
 (defonce ^:private follow-up-emails-job (atom nil))
 (defonce ^:private follow-up-emails-trigger (atom nil))
 
-(setting/defsetting follow-up-email-sent
-  "have we sent a follow up email to the instance admin?"
+(setting/defsetting ^:private follow-up-email-sent
+  "Have we sent a follow up email to the instance admin?"
   :type      :boolean
   :default   false
   :internal? true)
@@ -35,41 +36,39 @@
 (defonce ^:private abandonment-emails-job (atom nil))
 (defonce ^:private abandonment-emails-trigger (atom nil))
 
-(setting/defsetting abandonment-email-sent
-  "have we sent an abandonment email to the instance admin?"
+(setting/defsetting ^:private abandonment-email-sent
+  "Have we sent an abandonment email to the instance admin?"
   :type      :boolean
   :default   false
   :internal? true)
 
 ;; 2 weeks of inactivity after 30 days of total install
-;;
 
 ;; this sends out a general 2 week email follow up email
 (jobs/defjob FollowUpEmail
              [ctx]
              ;; if we've already sent the follow-up email then we are done
-             (when-not (= "true" (follow-up-email-sent))
+             (when-not (follow-up-email-sent)
                ;; figure out when we consider the instance created
                (when-let [instance-created (user/instance-created-at)]
                  ;; we need to be 2+ weeks (14 days) from creation to send the follow up
                  (when (< (* 14 24 60 60 1000)
-                          (- (System/currentTimeMillis) (.getTime ^java.sql.Timestamp instance-created)))
+                          (- (System/currentTimeMillis) (.getTime instance-created)))
                    (send-follow-up-email!)))))
 
 ;; this sends out an email any time after 30 days if the instance has stopped being used for 14 days
 (jobs/defjob AbandonmentEmail
              [ctx]
              ;; if we've already sent the abandonment email then we are done
-             (when-not (= "true" (abandonment-email-sent))
+             (when-not (abandonment-email-sent)
                ;; figure out when we consider the instance created
                (when-let [instance-created (user/instance-created-at)]
                  ;; we need to be 4+ weeks (30 days) from creation to send the follow up
                  (when (< (* 30 24 60 60 1000)
-                          (- (System/currentTimeMillis) (.getTime ^java.sql.Timestamp instance-created)))
+                          (- (System/currentTimeMillis) (.getTime instance-created)))
                    ;; we need access to email AND the instance must be opted into anonymous tracking
                    (when (and (email/email-configured?)
-                              (let [tracking? (setting/get :anon-tracking-enabled)]
-                                (or (nil? tracking?) (= "true" tracking?))))
+                              (public-settings/anon-tracking-enabled))
                      (send-abandonment-email!))))))
 
 (defn task-init
@@ -108,26 +107,25 @@
   (try
     ;; we need access to email AND the instance must be opted into anonymous tracking
     (when (and (email/email-configured?)
-               (let [tracking? (setting/get :anon-tracking-enabled)]
-                 (or (nil? tracking?) (= "true" tracking?))))
+               (public-settings/anon-tracking-enabled))
       ;; grab the oldest admins email address, that's who we'll send to
-      (when-let [admin (user/User :is_superuser true, {:order-by [:date_joined]})]
+      (when-let [admin (User :is_superuser true, {:order-by [:date_joined]})]
         (messages/send-follow-up-email (:email admin) "follow-up")))
     (catch Throwable t
       (log/error "Problem sending follow-up email" t))
     (finally
-      (setting/set! :follow-up-email-sent true))))
+      (follow-up-email-sent true))))
 
 (defn- send-abandonment-email!
   "Send an email to the instance admin about why Metabase usage has died down."
   []
   ;; grab the oldest admins email address, that's who we'll send to
-  (when-let [admin (user/User :is_superuser true, {:order-by [:date_joined]})]
+  (when-let [admin (User :is_superuser true, {:order-by [:date_joined]})]
     ;; inactive = no users created, no activity created, no dash/card views (past 7 days)
-    (let [last-user      (c/from-sql-time (db/select-one-field :date_joined user/User,       {:order-by [[:date_joined :desc]]}))
-          last-activity  (c/from-sql-time (db/select-one-field :timestamp activity/Activity, {:order-by [[:timestamp :desc]]}))
-          last-view      (c/from-sql-time (db/select-one-field :timestamp view-log/ViewLog,  {:order-by [[:timestamp :desc]]}))
-          two-weeks-ago  (t/minus (t/now) (t/days 14))]
+    (let [last-user     (c/from-sql-time (db/select-one-field :date_joined User,   {:order-by [[:date_joined :desc]]}))
+          last-activity (c/from-sql-time (db/select-one-field :timestamp Activity, {:order-by [[:timestamp :desc]]}))
+          last-view     (c/from-sql-time (db/select-one-field :timestamp ViewLog,  {:order-by [[:timestamp :desc]]}))
+          two-weeks-ago (t/minus (t/now) (t/days 14))]
       (when (and (t/before? last-user two-weeks-ago)
                  (t/before? last-activity two-weeks-ago)
                  (t/before? last-view two-weeks-ago))
@@ -136,4 +134,4 @@
           (catch Throwable t
             (log/error "Problem sending abandonment email" t))
           (finally
-            (setting/set! :abandonment-email-sent "true")))))))
+            (abandonment-email-sent true)))))))