From af54379afa513713d0601fdd6ff11364209075ff Mon Sep 17 00:00:00 2001 From: "Mahatthana (Kelvin) Nomsawadi" <me@bboykelvin.dev> Date: Wed, 3 Apr 2024 17:21:55 +0700 Subject: [PATCH] [MS2] 1. Analytics (#40681) * Remove old config + fix typo * Add new analytics for 4 new illustration settings --- .../serialization.md | 1 - .../metabase-enterprise/settings/selectors.ts | 8 +- .../src/metabase-enterprise/settings/types.ts | 2 +- .../IllustrationWidget/IllustrationWidget.tsx | 6 +- .../IllustrationWidget.unit.spec.tsx | 6 +- .../src/metabase-types/api/mocks/settings.ts | 1 - frontend/src/metabase-types/api/settings.ts | 1 - frontend/src/metabase/auth/selectors.ts | 4 - frontend/src/metabase/home/selectors.ts | 4 - src/metabase/analytics/stats.clj | 51 ++++++----- src/metabase/public_settings.clj | 9 -- test/metabase/analytics/stats_test.clj | 91 ++++++++++--------- 12 files changed, 88 insertions(+), 96 deletions(-) diff --git a/docs/installation-and-operation/serialization.md b/docs/installation-and-operation/serialization.md index c71c30c1467..f917db71ab4 100644 --- a/docs/installation-and-operation/serialization.md +++ b/docs/installation-and-operation/serialization.md @@ -252,7 +252,6 @@ site-name application-font-files loading-message report-timezone -show-lighthouse-illustration persisted-models-enabled enable-content-management? subscription-allowed-domains diff --git a/enterprise/frontend/src/metabase-enterprise/settings/selectors.ts b/enterprise/frontend/src/metabase-enterprise/settings/selectors.ts index f4706c66296..51310d202c4 100644 --- a/enterprise/frontend/src/metabase-enterprise/settings/selectors.ts +++ b/enterprise/frontend/src/metabase-enterprise/settings/selectors.ts @@ -55,7 +55,7 @@ export function getLoginPageIllustration( isDefault: true, }; - case "no-illustration": + case "none": return null; case "custom": @@ -81,7 +81,7 @@ export function getLandingPageIllustration( isDefault: true, }; - case "no-illustration": + case "none": return null; case "custom": @@ -102,7 +102,7 @@ export function getNoDataIllustration(state: EnterpriseState): string | null { case "default": return noResultsSource; - case "no-illustration": + case "none": return null; case "custom": @@ -120,7 +120,7 @@ export function getNoObjectIllustration(state: EnterpriseState): string | null { case "default": return noResultsSource; - case "no-illustration": + case "none": return null; case "custom": diff --git a/enterprise/frontend/src/metabase-enterprise/settings/types.ts b/enterprise/frontend/src/metabase-enterprise/settings/types.ts index b93e58dc699..c9b2bce0dba 100644 --- a/enterprise/frontend/src/metabase-enterprise/settings/types.ts +++ b/enterprise/frontend/src/metabase-enterprise/settings/types.ts @@ -9,7 +9,7 @@ interface EnterpriseSettingsState extends SettingsState { values: EnterpriseSettings; } -export type IllustrationSettingValue = "default" | "no-illustration" | "custom"; +export type IllustrationSettingValue = "default" | "none" | "custom"; export interface EnterpriseSettings extends Settings { "application-colors"?: Record<string, string>; diff --git a/enterprise/frontend/src/metabase-enterprise/whitelabel/components/IllustrationWidget/IllustrationWidget.tsx b/enterprise/frontend/src/metabase-enterprise/whitelabel/components/IllustrationWidget/IllustrationWidget.tsx index f2efae4eb5f..fe0e83e26e7 100644 --- a/enterprise/frontend/src/metabase-enterprise/whitelabel/components/IllustrationWidget/IllustrationWidget.tsx +++ b/enterprise/frontend/src/metabase-enterprise/whitelabel/components/IllustrationWidget/IllustrationWidget.tsx @@ -52,12 +52,12 @@ interface SelectOption { const SELECT_OPTIONS: Record<IllustrationType, SelectOption[]> = { background: [ { label: t`Lighthouse`, value: "default" }, - { label: t`No illustration`, value: "no-illustration" }, + { label: t`No illustration`, value: "none" }, { label: t`Custom`, value: "custom" }, ], icon: [ { label: t`Sailboat`, value: "default" }, - { label: t`No illustration`, value: "no-illustration" }, + { label: t`No illustration`, value: "none" }, { label: t`Custom`, value: "custom" }, ], }; @@ -240,7 +240,7 @@ function getPreviewImage({ return PREVIEW_ELEMENTS[defaultPreviewType]; } - if (value === "no-illustration") { + if (value === "none") { return null; } diff --git a/enterprise/frontend/src/metabase-enterprise/whitelabel/components/IllustrationWidget/IllustrationWidget.unit.spec.tsx b/enterprise/frontend/src/metabase-enterprise/whitelabel/components/IllustrationWidget/IllustrationWidget.unit.spec.tsx index b9bc1b6d2db..45d154c0544 100644 --- a/enterprise/frontend/src/metabase-enterprise/whitelabel/components/IllustrationWidget/IllustrationWidget.unit.spec.tsx +++ b/enterprise/frontend/src/metabase-enterprise/whitelabel/components/IllustrationWidget/IllustrationWidget.unit.spec.tsx @@ -89,7 +89,7 @@ describe("IllustrationWidget", () => { it("should allow setting 'No illustration' option", async () => { const noIllustrationOption = { label: "No illustration", - value: "no-illustration", + value: "none", }; const { onChange } = setup({ @@ -126,7 +126,7 @@ describe("IllustrationWidget", () => { it("should not remove the custom uploaded image after changing the option to 'No illustration'", async () => { const noIllustrationOption = { label: "No illustration", - value: "no-illustration", + value: "none", }; const setting = { ...defaultSetting, @@ -230,7 +230,7 @@ describe("IllustrationWidget", () => { it("should not call callbacks when selecting 'No illustration' option twice", async () => { const setting = { ...defaultSetting, - value: "no-illustration", + value: "none", } as const; const { onChange, onChangeSetting } = setup({ setting, diff --git a/frontend/src/metabase-types/api/mocks/settings.ts b/frontend/src/metabase-types/api/mocks/settings.ts index 53e8a9e89c5..9696e9de5b8 100644 --- a/frontend/src/metabase-types/api/mocks/settings.ts +++ b/frontend/src/metabase-types/api/mocks/settings.ts @@ -205,7 +205,6 @@ export const createMockSettings = ( "show-homepage-data": false, "show-homepage-pin-message": false, "show-homepage-xrays": false, - "show-lighthouse-illustration": true, "show-metabase-links": true, "show-metabot": true, "site-locale": "en", diff --git a/frontend/src/metabase-types/api/settings.ts b/frontend/src/metabase-types/api/settings.ts index f8c32f923e5..3c2aee20688 100644 --- a/frontend/src/metabase-types/api/settings.ts +++ b/frontend/src/metabase-types/api/settings.ts @@ -283,7 +283,6 @@ interface PublicSettings { "report-timezone-short": string; "session-cookies": boolean | null; "setup-token": string | null; - "show-lighthouse-illustration": boolean; "show-metabase-links": boolean; "show-metabot": boolean; "site-locale": string; diff --git a/frontend/src/metabase/auth/selectors.ts b/frontend/src/metabase/auth/selectors.ts index 7a16dc74f44..a8b333322d2 100644 --- a/frontend/src/metabase/auth/selectors.ts +++ b/frontend/src/metabase/auth/selectors.ts @@ -31,10 +31,6 @@ export const getHasSessionCookies = (state: State) => { return getSetting(state, "session-cookies") ?? false; }; -export const getHasIllustration = (state: State) => { - return getSetting(state, "show-lighthouse-illustration"); -}; - export const getSiteLocale = (state: State) => { return getSetting(state, "site-locale"); }; diff --git a/frontend/src/metabase/home/selectors.ts b/frontend/src/metabase/home/selectors.ts index ee7b31996f3..26eb9468cff 100644 --- a/frontend/src/metabase/home/selectors.ts +++ b/frontend/src/metabase/home/selectors.ts @@ -16,10 +16,6 @@ export const getHasMetabotLogo = (state: State) => { return getSetting(state, "show-metabot"); }; -export const getHasIllustration = (state: State) => { - return getSetting(state, "show-lighthouse-illustration"); -}; - export const getCustomHomePageDashboardId = createSelector( [getUser], user => user?.custom_homepage?.dashboard_id || null, diff --git a/src/metabase/analytics/stats.clj b/src/metabase/analytics/stats.clj index 86b7b8a0abe..8c3a870910b 100644 --- a/src/metabase/analytics/stats.clj +++ b/src/metabase/analytics/stats.clj @@ -126,31 +126,34 @@ (defn- instance-settings "Figure out global info about this instance" [] - {:version (config/mb-version-info :tag) - :running_on (environment-type) - :startup_time_millis (public-settings/startup-time-millis) - :application_database (config/config-str :mb-db-type) - :check_for_updates (public-settings/check-for-updates) - :report_timezone (driver/report-timezone) + {:version (config/mb-version-info :tag) + :running_on (environment-type) + :startup_time_millis (public-settings/startup-time-millis) + :application_database (config/config-str :mb-db-type) + :check_for_updates (public-settings/check-for-updates) + :report_timezone (driver/report-timezone) ; We deprecated advanced humanization but have this here anyways - :friendly_names (= (humanization/humanization-strategy) "advanced") - :email_configured (email/email-configured?) - :slack_configured (slack/slack-configured?) - :sso_configured (google/google-auth-enabled) - :instance_started (snowplow/instance-creation) - :has_sample_data (t2/exists? Database, :is_sample true) - :enable_embedding (embed.settings/enable-embedding) - :embedding_app_origin_set (boolean (embed.settings/embedding-app-origin)) - :appearance_site_name (not= (public-settings/site-name) "Metabase") - :appearance_help_link (public-settings/help-link) - :appearance_logo (not= (public-settings/application-logo-url) "app/assets/img/logo.svg") - :appareance_favicon (not= (public-settings/application-favicon-url) "app/assets/img/favicon.ico") - :apperance_loading_message (not= (public-settings/loading-message) :doing-science) - :appearance_metabot_greeting (not (public-settings/show-metabot)) - :apparerance_lighthouse_illustration (not (public-settings/show-lighthouse-illustration)) - :appearance_ui_colors (appearance-ui-colors-changed?) - :appearance_chart_colors (appearance-chart-colors-changed?) - :appearance_show_mb_links (not (public-settings/show-metabase-links))}) + :friendly_names (= (humanization/humanization-strategy) "advanced") + :email_configured (email/email-configured?) + :slack_configured (slack/slack-configured?) + :sso_configured (google/google-auth-enabled) + :instance_started (snowplow/instance-creation) + :has_sample_data (t2/exists? Database, :is_sample true) + :enable_embedding (embed.settings/enable-embedding) + :embedding_app_origin_set (boolean (embed.settings/embedding-app-origin)) + :appearance_site_name (not= (public-settings/site-name) "Metabase") + :appearance_help_link (public-settings/help-link) + :appearance_logo (not= (public-settings/application-logo-url) "app/assets/img/logo.svg") + :appearance_favicon (not= (public-settings/application-favicon-url) "app/assets/img/favicon.ico") + :appearance_loading_message (not= (public-settings/loading-message) :doing-science) + :appearance_metabot_greeting (not (public-settings/show-metabot)) + :appearance_login_page_illustration (public-settings/login-page-illustration) + :appearance_landing_page_illustration (public-settings/landing-page-illustration) + :appearance_no_data_illustration (public-settings/no-data-illustration) + :appearance_no_object_illustration (public-settings/no-object-illustration) + :appearance_ui_colors (appearance-ui-colors-changed?) + :appearance_chart_colors (appearance-chart-colors-changed?) + :appearance_show_mb_links (not (public-settings/show-metabase-links))}) (defn- user-metrics "Get metrics based on user records. diff --git a/src/metabase/public_settings.clj b/src/metabase/public_settings.clj index 68d80304080..5d76f858c06 100644 --- a/src/metabase/public_settings.clj +++ b/src/metabase/public_settings.clj @@ -435,15 +435,6 @@ :feature :whitelabel :default true) -(defsetting show-lighthouse-illustration - (deferred-tru "Display the lighthouse illustration on the home and login pages.") - :visibility :public - :export? true - :type :boolean - :audit :getter - :feature :whitelabel - :default true) - (defsetting login-page-illustration (deferred-tru "Options for displaying the illustration on the login page.") :visibility :public diff --git a/test/metabase/analytics/stats_test.clj b/test/metabase/analytics/stats_test.clj index 74daba4dee7..f75dcf41af2 100644 --- a/test/metabase/analytics/stats_test.clj +++ b/test/metabase/analytics/stats_test.clj @@ -78,26 +78,29 @@ enable-embedding false] (t2.with-temp/with-temp [:model/Database _ {:is_sample true}] (let [stats (anonymous-usage-stats)] - (is (partial= {:running_on :unknown - :check_for_updates true - :startup_time_millis 1234.0 - :friendly_names false - :email_configured false - :slack_configured false - :sso_configured false - :has_sample_data true - :enable_embedding false - :embedding_app_origin_set false - :appearance_site_name false - :appearance_help_link :metabase - :appearance_logo false - :appareance_favicon false - :apperance_loading_message false - :appearance_metabot_greeting false - :apparerance_lighthouse_illustration false - :appearance_ui_colors false - :appearance_chart_colors false - :appearance_show_mb_links false} + (is (partial= {:running_on :unknown + :check_for_updates true + :startup_time_millis 1234.0 + :friendly_names false + :email_configured false + :slack_configured false + :sso_configured false + :has_sample_data true + :enable_embedding false + :embedding_app_origin_set false + :appearance_site_name false + :appearance_help_link :metabase + :appearance_logo false + :appearance_favicon false + :appearance_loading_message false + :appearance_metabot_greeting false + :appearance_login_page_illustration "default" + :appearance_landing_page_illustration "default" + :appearance_no_data_illustration "default" + :appearance_no_object_illustration "default" + :appearance_ui_colors false + :appearance_chart_colors false + :appearance_show_mb_links false} stats)) (is (malli= [:map-of :string ms/IntGreaterThanOrEqualToZero] (-> stats :stats :database :dbms_versions)))))))) @@ -117,31 +120,37 @@ application-favicon-url "http://example.com/favicon.ico" loading-message :running-query show-metabot false - show-lighthouse-illustration false + login-page-illustration "default" + landing-page-illustration "custom" + no-data-illustration "none" + no-object-illustration "custom" application-colors {:brand "#123456"} show-metabase-links false] (t2.with-temp/with-temp [:model/Database _ {:is_sample true}] (let [stats (anonymous-usage-stats)] - (is (partial= {:running_on :unknown - :check_for_updates true - :startup_time_millis 1234.0 - :friendly_names false - :email_configured false - :slack_configured false - :sso_configured false - :has_sample_data true - :enable_embedding true - :embedding_app_origin_set false - :appearance_site_name true - :appearance_help_link :hidden - :appearance_logo true - :appareance_favicon true - :apperance_loading_message true - :appearance_metabot_greeting true - :apparerance_lighthouse_illustration true - :appearance_ui_colors true - :appearance_chart_colors false - :appearance_show_mb_links true} + (is (partial= {:running_on :unknown + :check_for_updates true + :startup_time_millis 1234.0 + :friendly_names false + :email_configured false + :slack_configured false + :sso_configured false + :has_sample_data true + :enable_embedding true + :embedding_app_origin_set false + :appearance_site_name true + :appearance_help_link :hidden + :appearance_logo true + :appearance_favicon true + :appearance_loading_message true + :appearance_metabot_greeting true + :appearance_login_page_illustration "default" + :appearance_landing_page_illustration "custom" + :appearance_no_data_illustration "none" + :appearance_no_object_illustration "custom" + :appearance_ui_colors true + :appearance_chart_colors false + :appearance_show_mb_links true} stats)) (is (malli= [:map-of :string ms/IntGreaterThanOrEqualToZero] (-> stats :stats :database :dbms_versions))))))))) -- GitLab