From e12e36044e588761a3fb9aed10900a9d506d7fd6 Mon Sep 17 00:00:00 2001 From: Noah Moss <32746338+noahmoss@users.noreply.github.com> Date: Fri, 27 Sep 2024 15:38:05 -0400 Subject: [PATCH] Fix non-bool value in CSV upload availability check for stats ping (#48167) --- src/metabase/analytics/stats.clj | 12 +++++------ test/metabase/analytics/stats_test.clj | 29 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/metabase/analytics/stats.clj b/src/metabase/analytics/stats.clj index 55b7543f47e..257d7f47a10 100644 --- a/src/metabase/analytics/stats.clj +++ b/src/metabase/analytics/stats.clj @@ -588,12 +588,12 @@ (defn- csv-upload-available? "Is CSV upload currently available to be used on this instance?" [] - (let [major-version (config/current-major-version) - minor-version (config/current-minor-version) - engines (t2/select-fn-set :engine :model/Database - {:where [:in :engine (map name (keys csv-upload-version-availability))]})] - (when (and major-version minor-version) - (boolean + (boolean + (let [major-version (config/current-major-version) + minor-version (config/current-minor-version) + engines (t2/select-fn-set :engine :model/Database + {:where [:in :engine (map name (keys csv-upload-version-availability))]})] + (when (and major-version minor-version) (some (fn [engine] (when-let [[required-major required-minor] (csv-upload-version-availability engine)] diff --git a/test/metabase/analytics/stats_test.clj b/test/metabase/analytics/stats_test.clj index 3173b75e505..e5c4fd89bc0 100644 --- a/test/metabase/analytics/stats_test.clj +++ b/test/metabase/analytics/stats_test.clj @@ -4,6 +4,7 @@ [clojure.test :refer :all] [java-time.api :as t] [metabase.analytics.stats :as stats :refer [legacy-anonymous-usage-stats]] + [metabase.config :as config] [metabase.core :as mbc] [metabase.db :as mdb] [metabase.email :as email] @@ -392,6 +393,34 @@ (mt/with-temp [:model/QueryExecution _ query-execution-defaults] (is (true? (@#'stats/sufficient-queries? 1))))))) +(deftest csv-upload-available-test + (mt/with-temp-empty-app-db [_conn :h2] + (mdb/setup-db! :create-sample-content? true) + + (testing "csv-upload-available? currently detects upload availability based on the current MB version" + (mt/with-temp [:model/Database _ {:engine :postgres}] + (with-redefs [config/current-major-version (constantly 46) + config/current-minor-version (constantly 0)] + (is false? (@#'stats/csv-upload-available?))) + + (with-redefs [config/current-major-version (constantly 47) + config/current-minor-version (constantly 1)] + (is true? (@#'stats/csv-upload-available?)))) + + (mt/with-temp [:model/Database _ {:engine :redshift}] + (with-redefs [config/current-major-version (constantly 49) + config/current-minor-version (constantly 5)] + (is false? (@#'stats/csv-upload-available?))) + + (with-redefs [config/current-major-version (constantly 49) + config/current-minor-version (constantly 6)] + (is true? (@#'stats/csv-upload-available?)))) + + ;; If we can't detect the MB version, return nil + (with-redefs [config/current-major-version (constantly nil) + config/current-minor-version (constantly nil)] + (is false? (@#'stats/csv-upload-available?)))))) + (def ^:private excluded-features "Set of features intentionally excluded from the daily stats ping. If you add a new feature, either add it to the stats ping or to this set, so that [[every-feature-is-accounted-for-test]] passes." -- GitLab