Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Enterprise settings (metabase#23441)
* Allow for disabling settings
Disabled settings will return their default value (else nil if no
default is set). This allows us to have enterprise override settings and
use them from regular OSS code without classloaders, extra vars,
remembering to check if the feature is enabled, etc.
Motivating examples are the appearance settings. We allow
`application-font` setting to change the font of the application. This
is an enterprise feature, but anyone can post to
`api/setting/application-font` and set a new value or startup as
`MB_APPLICATION_FONT=comic-sans java -jar metabase.jar` and have the
functionality.
Same thing for application colors in static viz. The calling code just
calls `(settings/application-colors)` and uses them but doesn't check if
the enterprise settings are enabled. To do this correctly, you have to
remember to implement the following onerous procedure:
A whole namespace for a setting
```clojure
(ns metabase-enterprise.embedding.utils
(:require [metabase.models.setting :as setting :refer [defsetting]]
[metabase.public-settings :as public-settings]
[metabase.public-settings.premium-features :as premium-features]
[metabase.util.i18n :refer [deferred-tru]]))
(defsetting notification-link-base-url
(deferred-tru "By default \"Site Url\" is used in notification links, but can be overridden.")
:visibility :internal
:getter (fn []
(when (premium-features/hide-embed-branding?)
(or (setting/get-value-of-type :string :notification-link-base-url)
(public-settings/site-url)))))
```
And then in the calling code you have to do the procedure to
conditionally require it and put it behind a var that can handle it
being nil:
```clojure
;; we want to load this at the top level so the Setting the namespace defines gets loaded
(def ^:private site-url*
(or (u/ignore-exceptions
(classloader/require 'metabase-enterprise.embedding.utils)
(resolve 'metabase-enterprise.embedding.utils/notification-link-base-url))
(constantly nil)))
;; and then the usage
(defn- site-url
"Return the Notification Link Base URL if set by enterprise env var, or Site URL."
[]
(or (site-url*) (public-settings/site-url)))
```
Far nicer to just place the following into the regular public-settings
namespace:
```clojure
(defsetting notification-link-base-url
(deferred-tru "By default \"Site Url\" is used in notification links, but can be overridden.")
:visibility :internal
:enabled? premium-features/hide-embed-branding?)
```
Then no need for a custom namespace to hold this setting, no need to
have an extra var to point to the setting else a fallback value.
Note that this feature is not required on every enterprise feature we
have. We a namespace `metabase-enterprise.sso.integrations.sso-settings`
that has 24 settings in it, all of which are enterprise features. But
these features are used in our enterprise sso offerings and are directly
referenced from the enterprise features. No need for the extra var to
point to them and the flag checks happen in other parts.
* Mark the UI/UX customization settings as requiring whitelabeling
Mark the following settings as requiring
premium-settings/enable-whitelabeling? (aka token check)
- application-name
- loading-message (override of "doing science")
- show-metabot (override of showing our friendly metabot)
- application-colors
- application-font
- application-logo-url
- application-favicon-url
Updates the helper functions for colors to use the setting rather than
feeling entitled to use a lower level `setting/get-value-of-type`. We
need the higher level api so it takes into account if its enabled or
not.
* Move notification-link-base-url into regular settings with enabled?
* Cleanup ns- Loading branch information
