Merge pull request #3252 from metabase/minor-cleanup

Minor settings cleanup 

src/metabase/api/session.clj

@@ -79,7 +79,7 @@
   ;; Don't leak whether the account doesn't exist, just pretend everything is ok
   (when-let [{user-id :id, google-auth? :google_auth} (db/select-one ['User :id :google_auth] :email email)]
     (let [reset-token        (set-user-password-reset-token! user-id)
-          password-reset-url (str ((resolve 'metabase.core/site-url) request) "/auth/reset_password/" reset-token)]
+          password-reset-url (str (public-settings/site-url request) "/auth/reset_password/" reset-token)]
       (email/send-password-reset-email email google-auth? server-name password-reset-url)
       (log/info password-reset-url))))
 

src/metabase/api/setting.clj

@@ -2,7 +2,7 @@
   "/api/setting endpoints"
   (:require [compojure.core :refer [GET PUT DELETE]]
             [metabase.api.common :refer :all]
-            (metabase.models [setting :as setting])))
+            [metabase.models.setting :as setting]))
 
 (defendpoint GET "/"
   "Get all `Settings` and their values. You must be a superuser to do this."

src/metabase/api/setup.clj

 (ns metabase.api.setup
   (:require [compojure.core :refer [GET POST]]
+            [medley.core :as m]
             (metabase.api [common :refer :all]
                           [database :refer [annotation:DBEngine]])
             (metabase [db :as db]
@@ -11,6 +12,7 @@
                              [session :refer [Session]]
                              [setting :as setting]
                              [user :refer [User set-user-password!]])
+            [metabase.public-settings :as public-settings]
             [metabase.setup :as setup]
             [metabase.util :as u]))
 
@@ -30,8 +32,8 @@
    last_name  [Required NonEmptyString]
    email      [Required Email]
    password   [Required ComplexPassword]}
-  ;; Call (metabase.core/site-url request) to set the Site URL setting if it's not already set
-  (@(ns-resolve 'metabase.core 'site-url) request)
+  ;; Call (public-settings/site-url request) to set the Site URL setting if it's not already set
+  (public-settings/site-url request)
   ;; Now create the user
   (let [session-id (str (java.util.UUID/randomUUID))
         new-user   (db/insert! User
@@ -43,9 +45,11 @@
     ;; this results in a second db call, but it avoids redundant password code so figure it's worth it
     (set-user-password! (:id new-user) password)
     ;; set a couple preferences
-    (setting/set! :site-name site_name)
-    (setting/set! :admin-email email)
-    (setting/set! :anon-tracking-enabled (or allow_tracking true))
+    (public-settings/site-name site_name)
+    (public-settings/admin-email email)
+    (public-settings/anon-tracking-enabled (if (m/boolean? allow_tracking)
+                                             allow_tracking
+                                             true)) ; default to `true` if allow_tracking isn't specified
     ;; setup database (if needed)
     (when (driver/is-engine? engine)
       (->> (db/insert! Database
@@ -57,7 +61,7 @@
                              true))
            (events/publish-event :database-create)))
     ;; clear the setup token now, it's no longer needed
-    (setup/token-clear)
+    (setup/clear-token!)
     ;; then we create a session right away because we want our new user logged in to continue the setup process
     (db/insert! Session
       :id      session-id

src/metabase/core.clj

 ;; -*- comment-column: 35; -*-
 (ns metabase.core
   (:gen-class)
-  (:require [clojure.string :as s]
-            [clojure.tools.logging :as log]
+  (:require [clojure.tools.logging :as log]
             [ring.adapter.jetty :as ring-jetty]
             (ring.middleware [cookies :refer [wrap-cookies]]
                              [gzip :refer [wrap-gzip]]
@@ -29,41 +28,6 @@
 
 ;;; CONFIG
 
-;; TODO - Should the be moved to `metabase.public-settings` ?
-
-(defsetting site-name
-  "The name used for this instance of Metabase."
-  :default "Metabase")
-
-(defsetting -site-url
-  "The base URL of this Metabase instance, e.g. \"http://metabase.my-company.com\"")
-
-(defsetting admin-email
-  "The email address users should be referred to if they encounter a problem.")
-
-(defsetting anon-tracking-enabled
-  "Enable the collection of anonymous usage data in order to help Metabase improve."
-  :type   :boolean
-  :default true)
-
-(defsetting google-maps-api-key
-  "A Google Maps API key is required to enable certain map visualizations.")
-
-(defn site-url
-  "Fetch the site base URL that should be used for password reset emails, etc.
-   This strips off any trailing slashes that may have been added.
-
-   The first time this function is called, we'll set the value of the setting `-site-url` with the value of
-   the ORIGIN header (falling back to HOST if needed, i.e. for unit tests) of some API request.
-   Subsequently, the site URL can only be changed via the admin page."
-  {:arglists '([request])}
-  [{{:strs [origin host]} :headers}]
-  {:pre  [(or origin host)]
-   :post [(string? %)]}
-  (or (some-> (-site-url)
-              (s/replace #"/$" "")) ; strip off trailing slash if one was included
-      (-site-url (or origin host))))
-
 (def app
   "The primary entry point to the HTTP server"
   (-> routes/routes
@@ -106,7 +70,7 @@
 (defn- -init-create-setup-token
   "Create and set a new setup token and log it."
   []
-  (let [setup-token (setup/token-create)                    ; we need this here to create the initial token
+  (let [setup-token (setup/create-token!)                    ; we need this here to create the initial token
         hostname    (or (config/config-str :mb-jetty-host) "localhost")
         port        (config/config-int :mb-jetty-port)
         setup-url   (str "http://"

src/metabase/public_settings.clj

 (ns metabase.public-settings
-  (:require (metabase [config :as config]
+  (:require [clojure.string :as s]
+            (metabase [config :as config]
                       [db :as db])
             (metabase.models [common :as common]
                              [setting :refer [defsetting], :as setting])
-            [metabase.setup :as setup]
             [metabase.util.password :as password])
   (:import java.util.TimeZone))
 
@@ -12,12 +12,45 @@
   :type    :boolean
   :default true)
 
-;; TODO - define a JSON type ?
 (defsetting version-info
   "Information about available versions of Metabase."
   :type    :json
   :default {})
 
+(defsetting site-name
+  "The name used for this instance of Metabase."
+  :default "Metabase")
+
+(defsetting -site-url
+  "The base URL of this Metabase instance, e.g. \"http://metabase.my-company.com\"")
+
+(defsetting admin-email
+  "The email address users should be referred to if they encounter a problem.")
+
+(defsetting anon-tracking-enabled
+  "Enable the collection of anonymous usage data in order to help Metabase improve."
+  :type   :boolean
+  :default true)
+
+(defsetting google-maps-api-key
+  "A Google Maps API key is required to enable certain map visualizations.")
+
+(defn site-url
+  "Fetch the site base URL that should be used for password reset emails, etc.
+   This strips off any trailing slashes that may have been added.
+
+   The first time this function is called, we'll set the value of the setting `-site-url` with the value of
+   the ORIGIN header (falling back to HOST if needed, i.e. for unit tests) of some API request.
+   Subsequently, the site URL can only be changed via the admin page."
+  {:arglists '([request])}
+  [{{:strs [origin host]} :headers}]
+  {:pre  [(or origin host)]
+   :post [(string? %)]}
+  (or (some-> (-site-url)
+              (s/replace #"/$" "")) ; strip off trailing slash if one was included
+      (-site-url (or origin host))))
+
+
 
 (defn- short-timezone-name*
   "Get a short display name (e.g. `PST`) for `report-timezone`, or fall back to the System default if it's not set."
@@ -38,13 +71,13 @@
    :timezones             common/timezones
    :version               config/mb-version-info
    :engines               ((resolve 'metabase.driver/available-drivers))
-   :setup_token           (setup/token-value)
-   :anon_tracking_enabled (setting/get :anon-tracking-enabled)
-   :site_name             (setting/get :site-name)
+   :setup_token           ((resolve 'metabase.setup/token-value))
+   :anon_tracking_enabled (anon-tracking-enabled)
+   :site_name             (site-name)
    :email_configured      ((resolve 'metabase.email/email-configured?))
-   :admin_email           (setting/get :admin-email)
+   :admin_email           (admin-email)
    :report_timezone       (setting/get :report-timezone)
    :timezone_short        (short-timezone-name (setting/get :report-timezone))
    :has_sample_dataset    (db/exists? 'Database, :is_sample true)
    :google_auth_client_id (setting/get :google-auth-client-id)
-   :google_maps_api_key   (setting/get :google-maps-api-key)})
+   :google_maps_api_key   (google-maps-api-key)})

src/metabase/setup.clj

@@ -15,13 +15,13 @@
   {:pre [(string? token)]}
   (= token @setup-token))
 
-(defn token-create
+(defn create-token!
   "Create and set a new `@setup-token`.
    Returns the newly created token."
   []
   (reset! setup-token (str (java.util.UUID/randomUUID))))
 
-(defn token-clear
+(defn clear-token!
   "Clear the `@setup-token` if it exists and reset it to nil."
   []
   (reset! setup-token nil))

src/metabase/task/follow_up_emails.clj

 (ns metabase.task.follow-up-emails
   "Tasks which follow up with Metabase users."
-  (:require [clj-time.coerce :as c]
-            [clj-time.core :as t]
-            [clojure.tools.logging :as log]
+  (:require [clojure.tools.logging :as log]
+            (clj-time [coerce :as c]
+                      [core :as t])
             (clojurewerkz.quartzite [jobs :as jobs]
                                     [triggers :as triggers])
             [clojurewerkz.quartzite.schedule.cron :as cron]
-            [metabase.db :as db]
-            [metabase.email :as email]
+            (metabase [db :as db]
+                      [email :as email])
             [metabase.email.messages :as messages]
-            [metabase.models.activity :as activity]
-            [metabase.models.setting :as setting]
-            [metabase.models.user :as user]
-            [metabase.models.view-log :as view-log]
+            (metabase.models [activity :as activity]
+                             [setting :as setting]
+                             [user :as user]
+                             [view-log :as view-log])
             [metabase.task :as task]))
 
 

test/metabase/api/setup_test.clj

@@ -6,7 +6,8 @@
             (metabase.models [session :refer [Session]]
                              [setting :as setting]
                              [user :refer [User]])
-            [metabase.setup :as setup]
+            (metabase [public-settings :as public-settings]
+                      [setup :as setup])
             (metabase.test [data :refer :all]
                            [util :refer [match-$ random-name], :as tu])
             [metabase.util :as u]))
@@ -19,7 +20,7 @@
   (expect
     [true
      email]
-    [(tu/is-uuid-string? (:id (http/client :post 200 "setup" {:token (setup/token-create)
+    [(tu/is-uuid-string? (:id (http/client :post 200 "setup" {:token (setup/create-token!)
                                                               :prefs {:site_name "Metabase Test"}
                                                               :user  {:first_name user-name
                                                                       :last_name  user-name
@@ -27,8 +28,8 @@
                                                                       :password   "anythingUP12!!"}})))
      (do
        ;; reset our setup token
-       (setup/token-create)
-       (setting/get :admin-email))]))
+       (setup/create-token!)
+       (public-settings/admin-email))]))
 
 
 ;; Test input validations