diff --git a/modules/drivers/bigquery/test/metabase/driver/bigquery/query_processor_test.clj b/modules/drivers/bigquery/test/metabase/driver/bigquery/query_processor_test.clj index 530cd7dbab7e9a34c2644c7746ddcde6226872d8..d53b0042cdcc066b7c579e9dbcf96d4d6261bd4a 100644 --- a/modules/drivers/bigquery/test/metabase/driver/bigquery/query_processor_test.clj +++ b/modules/drivers/bigquery/test/metabase/driver/bigquery/query_processor_test.clj @@ -29,9 +29,9 @@ (mt/rows (qp/process-query (mt/native-query - {:query (str "SELECT `v2_test_data.venues`.`id` " - "FROM `v2_test_data.venues` " - "ORDER BY `v2_test_data.venues`.`id` DESC " + {:query (str "SELECT `v3_test_data.venues`.`id` " + "FROM `v3_test_data.venues` " + "ORDER BY `v3_test_data.venues`.`id` DESC " "LIMIT 2;")}))))) (testing (str "make sure that BigQuery native queries maintain the column ordering specified in the SQL -- " @@ -53,10 +53,10 @@ :field_ref [:field-literal "checkins_id" :type/Integer]}] (qp.test/cols (qp/process-query - {:native {:query (str "SELECT `v2_test_data.checkins`.`venue_id` AS `venue_id`, " - " `v2_test_data.checkins`.`user_id` AS `user_id`, " - " `v2_test_data.checkins`.`id` AS `checkins_id` " - "FROM `v2_test_data.checkins` " + {:native {:query (str "SELECT `v3_test_data.checkins`.`venue_id` AS `venue_id`, " + " `v3_test_data.checkins`.`user_id` AS `user_id`, " + " `v3_test_data.checkins`.`id` AS `checkins_id` " + "FROM `v3_test_data.checkins` " "LIMIT 2")} :type :native :database (mt/id)}))))) @@ -106,11 +106,11 @@ rows)))) (testing "let's make sure we're generating correct HoneySQL + SQL for aggregations" - (is (= {:select [[(hx/identifier :field "v2_test_data.venues" "price") + (is (= {:select [[(hx/identifier :field "v3_test_data.venues" "price") (hx/identifier :field-alias "price")] - [(hsql/call :avg (hx/identifier :field "v2_test_data.venues" "category_id")) + [(hsql/call :avg (hx/identifier :field "v3_test_data.venues" "category_id")) (hx/identifier :field-alias "avg")]] - :from [(hx/identifier :table "v2_test_data.venues")] + :from [(hx/identifier :table "v3_test_data.venues")] :group-by [(hx/identifier :field-alias "price")] :order-by [[(hx/identifier :field-alias "avg") :asc]]} (mt/with-everything-store @@ -121,9 +121,9 @@ :breakout [$price] :order-by [[:asc [:aggregation 0]]]}))))) - (is (= {:query (str "SELECT `v2_test_data.venues`.`price` AS `price`," - " avg(`v2_test_data.venues`.`category_id`) AS `avg` " - "FROM `v2_test_data.venues` " + (is (= {:query (str "SELECT `v3_test_data.venues`.`price` AS `price`," + " avg(`v3_test_data.venues`.`category_id`) AS `avg` " + "FROM `v3_test_data.venues` " "GROUP BY `price` " "ORDER BY `avg` ASC, `price` ASC") :params nil @@ -137,9 +137,9 @@ (mt/test-driver :bigquery (is (= (str "SELECT `categories__via__category_id`.`name` AS `name`," " count(*) AS `count` " - "FROM `v2_test_data.venues` " - "LEFT JOIN `v2_test_data.categories` `categories__via__category_id`" - " ON `v2_test_data.venues`.`category_id` = `categories__via__category_id`.`id` " + "FROM `v3_test_data.venues` " + "LEFT JOIN `v3_test_data.categories` `categories__via__category_id`" + " ON `v3_test_data.venues`.`category_id` = `categories__via__category_id`.`id` " "GROUP BY `name` " "ORDER BY `name` ASC") ;; normally for test purposes BigQuery doesn't support foreign keys so override the function that checks @@ -209,13 +209,13 @@ (is (= (str "-- Metabase:: userID: 1000 queryType: MBQL queryHash: 01020304\n" "SELECT" - " `v2_test_data.venues`.`id` AS `id`," - " `v2_test_data.venues`.`name` AS `name`," - " `v2_test_data.venues`.`category_id` AS `category_id`," - " `v2_test_data.venues`.`latitude` AS `latitude`," - " `v2_test_data.venues`.`longitude` AS `longitude`," - " `v2_test_data.venues`.`price` AS `price` " - "FROM `v2_test_data.venues` " + " `v3_test_data.venues`.`id` AS `id`," + " `v3_test_data.venues`.`name` AS `name`," + " `v3_test_data.venues`.`category_id` AS `category_id`," + " `v3_test_data.venues`.`latitude` AS `latitude`," + " `v3_test_data.venues`.`longitude` AS `longitude`," + " `v3_test_data.venues`.`price` AS `price` " + "FROM `v3_test_data.venues` " "LIMIT 1") (query->native {:database (mt/id) @@ -232,9 +232,9 @@ (qp.test/rows (qp/process-query (mt/native-query - {:query (str "SELECT `v2_test_data.venues`.`name` AS `name` " - "FROM `v2_test_data.venues` " - "WHERE `v2_test_data.venues`.`name` = ?") + {:query (str "SELECT `v3_test_data.venues`.`name` AS `name` " + "FROM `v3_test_data.venues` " + "WHERE `v3_test_data.venues`.`name` = ?") :params ["Red Medicine"]})))) (str "Do we properly unprepare, and can we execute, queries that still have parameters for one reason or " "another? (EE #277)")))) @@ -403,7 +403,7 @@ (t/local-date "2019-11-12")])))) (mt/test-driver :bigquery (mt/with-everything-store - (let [expected ["WHERE `v2_test_data.checkins`.`date` BETWEEN ? AND ?" + (let [expected ["WHERE `v3_test_data.checkins`.`date` BETWEEN ? AND ?" (t/local-date "2019-11-11") (t/local-date "2019-11-12")]] (testing "Should be able to get temporal type from a FieldInstance" @@ -419,7 +419,7 @@ (t/local-date "2019-11-11") (t/local-date "2019-11-12")])))) (testing "Should be able to get temporal type from a wrapped field-id" - (is (= (cons "WHERE date_trunc(`v2_test_data.checkins`.`date`, day) BETWEEN ? AND ?" + (is (= (cons "WHERE date_trunc(`v3_test_data.checkins`.`date`, day) BETWEEN ? AND ?" (rest expected)) (between->sql [:between [:datetime-field [:field-id (mt/id :checkins :date)] :day] @@ -450,14 +450,14 @@ (mt/with-temp-copy-of-db (try (bigquery.tx/execute! - (format "CREATE TABLE `v2_test_data.%s` ( ts TIMESTAMP, dt DATETIME )" table-name)) + (format "CREATE TABLE `v3_test_data.%s` ( ts TIMESTAMP, dt DATETIME )" table-name)) (bigquery.tx/execute! - (format "INSERT INTO `v2_test_data.%s` (ts, dt) VALUES (TIMESTAMP \"2020-01-01 00:00:00 UTC\", DATETIME \"2020-01-01 00:00:00\")" + (format "INSERT INTO `v3_test_data.%s` (ts, dt) VALUES (TIMESTAMP \"2020-01-01 00:00:00 UTC\", DATETIME \"2020-01-01 00:00:00\")" table-name)) (sync/sync-database! (mt/db)) (f table-name) (finally - (bigquery.tx/execute! "DROP TABLE IF EXISTS `v2_test_data.%s`" table-name))))))) + (bigquery.tx/execute! "DROP TABLE IF EXISTS `v3_test_data.%s`" table-name))))))) (deftest filter-by-datetime-timestamp-test (mt/test-driver :bigquery @@ -498,7 +498,7 @@ (qp/process-query {:database (mt/id) :type :native - :native {:query "SELECT count(*) FROM `v2_attempted_murders.attempts` WHERE {{d}}" + :native {:query "SELECT count(*) FROM `v3_attempted_murders.attempts` WHERE {{d}}" :template-tags {"d" {:name "d" :display-name "Date" :type :dimension @@ -651,5 +651,5 @@ (mt/formatted-rows [int] (qp/process-query (mt/native-query - {:query "SELECT count(*) AS `count` FROM `v2_test_data.venues` WHERE `v2_test_data.venues`.`name` = ?" + {:query "SELECT count(*) AS `count` FROM `v3_test_data.venues` WHERE `v3_test_data.venues`.`name` = ?" :params ["x\\\\' OR 1 = 1 -- "]}))))))))) diff --git a/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj b/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj index e3986cb4eda8db5de17db567a5020450c78bbe66..d26937779252d938f003375ca2085b2bdd66d9bb 100644 --- a/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj +++ b/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj @@ -62,11 +62,11 @@ (mt/with-temp-copy-of-db (try (bigquery.tx/execute! - (str "CREATE VIEW `v2_test_data.%s` " + (str "CREATE VIEW `v3_test_data.%s` " "AS " "SELECT v.id AS id, v.name AS venue_name, c.name AS category_name " - "FROM `%s.v2_test_data.venues` v " - "LEFT JOIN `%s.v2_test_data.categories` c " + "FROM `%s.v3_test_data.venues` v " + "LEFT JOIN `%s.v3_test_data.categories` c " "ON v.category_id = c.id " "ORDER BY v.id ASC " "LIMIT 3") @@ -75,7 +75,7 @@ (bigquery.tx/project-id)) (f view-name) (finally - (bigquery.tx/execute! "DROP VIEW IF EXISTS `v2_test_data.%s`" view-name))))))) + (bigquery.tx/execute! "DROP VIEW IF EXISTS `v3_test_data.%s`" view-name))))))) (defmacro ^:private with-view [[view-name-binding] & body] `(do-with-view (fn [~(or view-name-binding '_)] ~@body))) diff --git a/modules/drivers/bigquery/test/metabase/test/data/bigquery.clj b/modules/drivers/bigquery/test/metabase/test/data/bigquery.clj index 401af34311dc9893edddae46fdefe7ab7999aba4..bd5871873c9eb7b7f3a73cec18092a5c35dbdbcf 100644 --- a/modules/drivers/bigquery/test/metabase/test/data/bigquery.clj +++ b/modules/drivers/bigquery/test/metabase/test/data/bigquery.clj @@ -40,7 +40,7 @@ (defn- normalize-name ^String [db-or-table identifier] (let [s (str/replace (name identifier) "-" "_")] (case db-or-table - :db (str "v2_" s) + :db (str "v3_" s) :table s))) (def ^:private details diff --git a/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj b/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj index b198ce8f20dc8cc029d9fc6c5e63f129823a9f92..2bf34cc3f3d0a41b6aa5c71099e35c3d2b93e16e 100644 --- a/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj +++ b/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj @@ -1,9 +1,9 @@ (ns metabase.driver.snowflake-test - (:require [clojure.java.jdbc :as jdbc] - [clojure + (:require [clojure [set :as set] [string :as str] [test :refer :all]] + [clojure.java.jdbc :as jdbc] [metabase [driver :as driver] [models :refer [Table]] @@ -11,47 +11,43 @@ [sync :as sync] [test :as mt] [util :as u]] - [metabase.driver.sql-jdbc - [connection :as sql-jdbc.conn] - [execute :as sql-jdbc.execute]] - [metabase.models - [database :refer [Database]]] + [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn] + [metabase.models.database :refer [Database]] [metabase.test.data [dataset-definitions :as dataset-defs] [sql :as sql.tx]] [metabase.test.data.sql.ddl :as ddl] [toucan.db :as db])) -;; (deftest ddl-statements-test (testing "make sure we didn't break the code that is used to generate DDL statements when we add new test datasets" (testing "Create DB DDL statements" - (is (= "DROP DATABASE IF EXISTS \"v2_test-data\"; CREATE DATABASE \"v2_test-data\";" + (is (= "DROP DATABASE IF EXISTS \"v3_test-data\"; CREATE DATABASE \"v3_test-data\";" (sql.tx/create-db-sql :snowflake (mt/get-dataset-definition dataset-defs/test-data))))) (testing "Create Table DDL statements" (is (= (map #(str/replace % #"\s+" " ") - ["DROP TABLE IF EXISTS \"v2_test-data\".\"PUBLIC\".\"users\";" - "CREATE TABLE \"v2_test-data\".\"PUBLIC\".\"users\" (\"id\" INTEGER AUTOINCREMENT, \"name\" TEXT, + ["DROP TABLE IF EXISTS \"v3_test-data\".\"PUBLIC\".\"users\";" + "CREATE TABLE \"v3_test-data\".\"PUBLIC\".\"users\" (\"id\" INTEGER AUTOINCREMENT, \"name\" TEXT, \"last_login\" TIMESTAMP_LTZ, \"password\" TEXT, PRIMARY KEY (\"id\")) ;" - "DROP TABLE IF EXISTS \"v2_test-data\".\"PUBLIC\".\"categories\";" - "CREATE TABLE \"v2_test-data\".\"PUBLIC\".\"categories\" (\"id\" INTEGER AUTOINCREMENT, \"name\" TEXT, + "DROP TABLE IF EXISTS \"v3_test-data\".\"PUBLIC\".\"categories\";" + "CREATE TABLE \"v3_test-data\".\"PUBLIC\".\"categories\" (\"id\" INTEGER AUTOINCREMENT, \"name\" TEXT, PRIMARY KEY (\"id\")) ;" - "DROP TABLE IF EXISTS \"v2_test-data\".\"PUBLIC\".\"venues\";" - "CREATE TABLE \"v2_test-data\".\"PUBLIC\".\"venues\" (\"id\" INTEGER AUTOINCREMENT, \"name\" TEXT, + "DROP TABLE IF EXISTS \"v3_test-data\".\"PUBLIC\".\"venues\";" + "CREATE TABLE \"v3_test-data\".\"PUBLIC\".\"venues\" (\"id\" INTEGER AUTOINCREMENT, \"name\" TEXT, \"category_id\" INTEGER, \"latitude\" FLOAT, \"longitude\" FLOAT, \"price\" INTEGER, PRIMARY KEY (\"id\")) ;" - "DROP TABLE IF EXISTS \"v2_test-data\".\"PUBLIC\".\"checkins\";" - "CREATE TABLE \"v2_test-data\".\"PUBLIC\".\"checkins\" (\"id\" INTEGER AUTOINCREMENT, \"date\" DATE, + "DROP TABLE IF EXISTS \"v3_test-data\".\"PUBLIC\".\"checkins\";" + "CREATE TABLE \"v3_test-data\".\"PUBLIC\".\"checkins\" (\"id\" INTEGER AUTOINCREMENT, \"date\" DATE, \"user_id\" INTEGER, \"venue_id\" INTEGER, PRIMARY KEY (\"id\")) ;" - "ALTER TABLE \"v2_test-data\".\"PUBLIC\".\"venues\" ADD CONSTRAINT \"gory_id_categories_-1524018980\" - FOREIGN KEY (\"category_id\") REFERENCES \"v2_test-data\".\"PUBLIC\".\"categories\" (\"id\");" - "ALTER TABLE \"v2_test-data\".\"PUBLIC\".\"checkins\" ADD CONSTRAINT \"ckins_user_id_users_-230440067\" - FOREIGN KEY (\"user_id\") REFERENCES \"v2_test-data\".\"PUBLIC\".\"users\" (\"id\");" - "ALTER TABLE \"v2_test-data\".\"PUBLIC\".\"checkins\" ADD CONSTRAINT \"kins_venue_id_venues_621212269\" - FOREIGN KEY (\"venue_id\") REFERENCES \"v2_test-data\".\"PUBLIC\".\"venues\" (\"id\");"]) + "ALTER TABLE \"v3_test-data\".\"PUBLIC\".\"venues\" ADD CONSTRAINT \"egory_id_categories_-740504465\" + FOREIGN KEY (\"category_id\") REFERENCES \"v3_test-data\".\"PUBLIC\".\"categories\" (\"id\");" + "ALTER TABLE \"v3_test-data\".\"PUBLIC\".\"checkins\" ADD CONSTRAINT \"ckins_user_id_users_1638713823\" + FOREIGN KEY (\"user_id\") REFERENCES \"v3_test-data\".\"PUBLIC\".\"users\" (\"id\");" + "ALTER TABLE \"v3_test-data\".\"PUBLIC\".\"checkins\" ADD CONSTRAINT \"ins_venue_id_venues_-833167948\" + FOREIGN KEY (\"venue_id\") REFERENCES \"v3_test-data\".\"PUBLIC\".\"venues\" (\"id\");"]) (ddl/create-db-tables-ddl-statements :snowflake (-> (mt/get-dataset-definition dataset-defs/test-data) - (update :database-name #(str "v2_" %))))))))) + (update :database-name #(str "v3_" %))))))))) ;; TODO -- disabled because these are randomly failing, will figure out when I'm back from vacation. I think it's a ;; bug in the JDBC driver -- Cam @@ -167,7 +163,7 @@ {:database (mt/id) :type :native :native {:query (str "SELECT {{filter_date}}, \"last_login\" " - "FROM \"v2_test-data\".\"PUBLIC\".\"users\" " + "FROM \"v3_test-data\".\"PUBLIC\".\"users\" " "WHERE date_trunc('day', CAST(\"last_login\" AS timestamp))" " = date_trunc('day', CAST({{filter_date}} AS timestamp))") :template-tags {:filter_date {:name "filter_date" diff --git a/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj b/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj index 8b621ee740a70cbb8b881bf001f4e142deb616bb..d11625aa9a7249ba8f908f67008d72d36c3566bd 100644 --- a/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj +++ b/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj @@ -35,9 +35,9 @@ "Prepend `database-name` with a version number so we can create new versions without breaking existing tests." [database-name] ;; try not to qualify the database name twice! - (if (str/starts-with? database-name "v2_") + (if (str/starts-with? database-name "v3_") database-name - (str "v2_" database-name))) + (str "v3_" database-name))) (defmethod tx/dbdef->connection-details :snowflake [_ context {:keys [database-name]}] diff --git a/src/metabase/public_settings.clj b/src/metabase/public_settings.clj index 7b7c1352da1cd656832c2f0e4e017fe9201f7705..400439fcb92a2888225e3eac9310dea0ff3a0807 100644 --- a/src/metabase/public_settings.clj +++ b/src/metabase/public_settings.clj @@ -226,6 +226,11 @@ [] (or (:brand (setting/get-json :application-colors)) "#509EE3")) +(defn secondary-chart-color + "The first 'Additional chart color'" + [] + (or (:accent3 (setting/get-json :application-colors)) "#EF8C8C")) + (defsetting application-logo-url (deferred-tru "For best results, use an SVG file with a transparent background.") :visibility :public diff --git a/src/metabase/pulse/render/body.clj b/src/metabase/pulse/render/body.clj index 330cbeda22ae2fa119be652b197269c6f2eeee98..3e099547cef10ed78e5da0f490d9c2fc67f702e7 100644 --- a/src/metabase/pulse/render/body.clj +++ b/src/metabase/pulse/render/body.clj @@ -93,13 +93,21 @@ col-name)) :bar-width (when include-bar? 99)}) +(defn- normalize-bar-value + "Normalizes bar-value into a value between 0 and 100, where 0 corresponds to `min-value` and 100 to `max-value`" + [bar-value min-value max-value] + (float + (/ + (* (- (double bar-value) min-value) + 100) + (- max-value min-value)))) + (s/defn ^:private query-results->row-seq "Returns a seq of stringified formatted rows that can be rendered into HTML" - [timezone-id :- (s/maybe s/Str) remapping-lookup cols rows bar-column max-value] + [timezone-id :- (s/maybe s/Str) remapping-lookup cols rows {:keys [bar-column min-value max-value]}] (for [row rows] - {:bar-width (when-let [bar-value (and bar-column (bar-column row))] - ;; cast to double to avoid "Non-terminating decimal expansion" errors - (float (* 100 (/ (double bar-value) max-value)))) + {:bar-width (some-> (and bar-column (bar-column row)) + (normalize-bar-value min-value max-value)) :row (for [[maybe-remapped-col maybe-remapped-row-cell] (map vector cols row) :when (and (not (:remapped_from maybe-remapped-col)) (show-in-table? maybe-remapped-col)) @@ -112,12 +120,15 @@ (s/defn ^:private prep-for-html-rendering "Convert the query results (`cols` and `rows`) into a formatted seq of rows (list of strings) that can be rendered as HTML" - [timezone-id :- (s/maybe s/Str) card {:keys [cols rows]} bar-column max-value column-limit] - (let [remapping-lookup (create-remapping-lookup cols) - limited-cols (take column-limit cols)] - (cons - (query-results->header-row remapping-lookup card limited-cols bar-column) - (query-results->row-seq timezone-id remapping-lookup limited-cols (take rows-limit rows) bar-column max-value)))) + ([timezone-id :- (s/maybe s/Str) card data column-limit] + (prep-for-html-rendering timezone-id card data column-limit {})) + ([timezone-id :- (s/maybe s/Str) card {:keys [cols rows]} column-limit + {:keys [bar-column min-value max-value] :as data-attributes}] + (let [remapping-lookup (create-remapping-lookup cols) + limited-cols (take column-limit cols)] + (cons + (query-results->header-row remapping-lookup card limited-cols bar-column) + (query-results->row-seq timezone-id remapping-lookup limited-cols (take rows-limit rows) data-attributes))))) (defn- strong-limit-text [number] [:strong {:style (style/style {:color style/color-gray-3})} (h (common/format-number number))]) @@ -179,7 +190,7 @@ (table/render-table (color/make-color-selector data (:visualization_settings card)) (mapv :name (:cols data)) - (prep-for-html-rendering timezone-id card data nil nil cols-limit)) + (prep-for-html-rendering timezone-id card data cols-limit)) (render-truncation-warning cols-limit (count-displayed-columns cols) rows-limit (count rows))]] {:attachments nil @@ -193,15 +204,20 @@ [_ _ timezone-id :- (s/maybe s/Str) card {:keys [cols] :as data}] (let [[x-axis-rowfn y-axis-rowfn] (common/graphing-column-row-fns card data) rows (common/non-nil-rows x-axis-rowfn y-axis-rowfn (:rows data)) - max-value (apply max (map y-axis-rowfn rows))] + row-values (map y-axis-rowfn rows) + min-value (min 0 (apply min row-values)) + max-value (apply max row-values)] {:attachments nil :content [:div (table/render-table (color/make-color-selector data (:visualization_settings card)) + (normalize-bar-value 0 min-value max-value) (mapv :name cols) - (prep-for-html-rendering timezone-id card data y-axis-rowfn max-value 2)) + (prep-for-html-rendering timezone-id card data 2 {:bar-column y-axis-rowfn + :min-value min-value + :max-value max-value})) (render-truncation-warning 2 (count-displayed-columns cols) rows-limit (count rows))]})) (s/defmethod render :scalar :- common/RenderedPulseCard diff --git a/src/metabase/pulse/render/style.clj b/src/metabase/pulse/render/style.clj index 40b291f0914e95b976be0f77c3e53f1b4aced8d5..357056164fe824be475783d74b98ddbb30b5ae12 100644 --- a/src/metabase/pulse/render/style.clj +++ b/src/metabase/pulse/render/style.clj @@ -71,6 +71,11 @@ [] (public-settings/application-color)) +(defn secondary-color + "Secondary color to use in Pulse charts; normally red, but customizable when whitelabeling is enabled." + [] + (public-settings/secondary-chart-color)) + (defn font-style "Font family to use in rendered Pulses." [] diff --git a/src/metabase/pulse/render/table.clj b/src/metabase/pulse/render/table.clj index 3a8e315ad1a4f4a59e98b348f8626216f245471f..0865dd54bce0f547ee703d5117eeee9a86de47d1 100644 --- a/src/metabase/pulse/render/table.clj +++ b/src/metabase/pulse/render/table.clj @@ -22,6 +22,8 @@ :padding-top :20px :padding-bottom :5px})) +(def ^:private max-bar-width 106) + (defn- bar-td-style [] (merge (style/font-style) @@ -31,7 +33,6 @@ :color style/color-text-dark :border-bottom (str "1px solid " style/color-body-row-border) :height :36px - :width :106px :padding-right :0.5em :padding-left :0.5em})) @@ -41,6 +42,25 @@ (defn- bar-td-style-numeric [] (merge (style/font-style) (bar-td-style) {:text-align :right})) +(defn- render-bar-component + ([color positive? width-in-pixels] + (render-bar-component color positive? width-in-pixels 0)) + ([color positive? width-in-pixels offset] + [:div + {:style (style/style + (merge + {:width (format "%spx" width-in-pixels) + :background-color color + :max-height :10px + :height :10px + :margin-top :3px} + (if positive? + {:border-radius "0px 2px 2px 0px"} + {:border-radius "2px 0px 0px 2px" + ;; `float: right` would be nice instead of the `margin-left` hack, but CSSBox puts in an erroneous 2px gap with it + :margin-left (format "%spx" (- max-bar-width width-in-pixels))})))} + " "])) + (defn- heading-style-for-type [cell] (if (instance? NumericWrapper cell) @@ -53,6 +73,10 @@ (bar-td-style-numeric) (bar-td-style))) +(defn- normalized-score->pixels + [score] + (int (* (/ score 100.0) max-bar-width))) + (defn- render-table-head [{:keys [bar-width row]}] [:thead [:tr @@ -62,13 +86,31 @@ (when bar-width [:th {:style (style/style (bar-td-style) (bar-th-style) {:width (str bar-width "%")})}])]]) +(defn- render-bar + [bar-width normalized-zero] + (if (< bar-width normalized-zero) + (list + [:td {:style (style/style (bar-td-style) {:width :99%, :border-right "1px solid black", :padding-right 0})} + (render-bar-component (style/secondary-color) + false + (normalized-score->pixels (- normalized-zero bar-width)) + (normalized-score->pixels bar-width))] + [:td {:style (style/style (bar-td-style) {:width :99%})}]) + (list + (when-not (zero? normalized-zero) + [:td {:style (style/style (bar-td-style) {:width :99%, :border-right "1px solid black"})}]) + [:td {:style (style/style (bar-td-style) {:width :99%, :padding-left 0})} + (render-bar-component (style/primary-color) + true + (normalized-score->pixels (- bar-width normalized-zero)))]))) + (defn- render-table-body "Render Hiccup `<tbody>` of a `<table>`. `get-background-color` is a function that returned the background color for the current cell; it is invoked like (get-background-color cell-value column-name row-index)" - [get-background-color column-names rows] + [get-background-color normalized-zero column-names rows] [:tbody (for [[row-idx {:keys [row bar-width]}] (m/indexed rows)] [:tr {:style (style/style {:color style/color-gray-3})} @@ -79,23 +121,22 @@ (when (and bar-width (= col-idx 1)) {:font-weight 700}))} (h cell)]) - (when bar-width - [:td {:style (style/style (bar-td-style) {:width :99%})} - [:div {:style (style/style {:background-color (style/primary-color) - :max-height :10px - :height :10px - :border-radius :2px - :width (str (max bar-width 0) "%")})} - " "]])])]) + (some-> bar-width (render-bar normalized-zero))])]) (defn render-table "This function returns the HTML data structure for the pulse table. `color-selector` is a function that returns the background color for a given cell. `column-names` is different from the header in `header+rows` as the header is the display_name (i.e. human friendly. `header+rows` includes the text contents of the table we're about ready to - create." - [^JSObject color-selector, column-names [header & rows]] - [:table {:style (style/style {:max-width (str "100%"), :white-space :nowrap, :padding-bottom :8px, :border-collapse :collapse}) - :cellpadding "0" - :cellspacing "0"} - (render-table-head header) - (render-table-body (partial color/get-background-color color-selector) column-names rows)]) + create. If `normalized-zero` is set (defaults to 0), render values less than it as negative" + ([^JSObject color-selector, column-names, [header & rows :as contents]] + (render-table color-selector 0 column-names contents)) + ([^JSObject color-selector, normalized-zero, column-names, [header & rows]] + [:table {:style (style/style {:max-width "100%" + :white-space :nowrap + :padding-bottom :8px + :border-collapse :collapse + :width "1%"}) + :cellpadding "0" + :cellspacing "0"} + (render-table-head header) + (render-table-body (partial color/get-background-color color-selector) normalized-zero column-names rows)])) diff --git a/test/metabase/pulse/render/body_test.clj b/test/metabase/pulse/render/body_test.clj index 091d40cd84e64cda645a6ef0a52ff75b4bee4a44..d7566faf0f3888e1ebebd3cc453c62e353c6f24e 100644 --- a/test/metabase/pulse/render/body_test.clj +++ b/test/metabase/pulse/render/body_test.clj @@ -49,8 +49,9 @@ #{4}]) (defn- prep-for-html-rendering' - [cols rows bar-column max-value] - (let [results (#'body/prep-for-html-rendering pacific-tz {} {:cols cols :rows rows} bar-column max-value (count cols))] + [cols rows bar-column min-value max-value] + (let [results (#'body/prep-for-html-rendering pacific-tz {} {:cols cols :rows rows} (count cols) + {:bar-column bar-column :min-value min-value :max-value max-value})] [(first results) (col-counts results)])) @@ -80,31 +81,31 @@ ;; Testing the format of headers (deftest header-result (is (= default-header-result - (prep-for-html-rendering' test-columns test-data nil nil)))) + (prep-for-html-rendering' test-columns test-data nil nil nil)))) (deftest header-result-2 (let [cols-with-desc (conj test-columns description-col) data-with-desc (mapv #(conj % "Desc") test-data)] (is (= default-header-result - (prep-for-html-rendering' cols-with-desc data-with-desc nil nil))))) + (prep-for-html-rendering' cols-with-desc data-with-desc nil nil nil))))) (deftest header-result-3 (let [cols-with-details (conj test-columns detail-col) data-with-details (mapv #(conj % "Details") test-data)] (is (= default-header-result - (prep-for-html-rendering' cols-with-details data-with-details nil nil))))) + (prep-for-html-rendering' cols-with-details data-with-details nil nil nil))))) (deftest header-result-4 (let [cols-with-sensitive (conj test-columns sensitive-col) data-with-sensitive (mapv #(conj % "Sensitive") test-data)] (is (= default-header-result - (prep-for-html-rendering' cols-with-sensitive data-with-sensitive nil nil))))) + (prep-for-html-rendering' cols-with-sensitive data-with-sensitive nil nil nil))))) (deftest header-result-5 (let [columns-with-retired (conj test-columns retired-col) data-with-retired (mapv #(conj % "Retired") test-data)] (is (= default-header-result - (prep-for-html-rendering' columns-with-retired data-with-retired nil nil))))) + (prep-for-html-rendering' columns-with-retired data-with-retired nil nil nil))))) (deftest prefers-col-visualization-settings-for-header (testing "Users can give columns custom names. Use those if they exist." @@ -129,8 +130,6 @@ (first (#'body/prep-for-html-rendering pacific-tz card {:cols cols :rows []} - nil - nil (count test-columns))))) ;; card does not contain custom column names @@ -139,35 +138,34 @@ (first (#'body/prep-for-html-rendering pacific-tz {} {:cols cols :rows []} - nil - nil (count test-columns)))))))) ;; When including a bar column, bar-width is 99% (deftest bar-width (is (= (assoc-in default-header-result [0 :bar-width] 99) - (prep-for-html-rendering' test-columns test-data second 40.0)))) + (prep-for-html-rendering' test-columns test-data second 0 40.0)))) ;; When there are too many columns, #'body/prep-for-html-rendering show narrow it (deftest narrow-the-columns (is (= [{:row [(number "ID") (number "Latitude")] :bar-width 99} #{2}] - (prep-for-html-rendering' (subvec test-columns 0 2) test-data second 40.0 )))) + (prep-for-html-rendering' (subvec test-columns 0 2) test-data second 0 40.0)))) ;; Basic test that result rows are formatted correctly (dates, floating point numbers etc) (deftest format-result-rows (is (= [{:bar-width nil, :row [(number "1") (number "34.10") "Apr 1, 2014" "Stout Burgers & Beers"]} {:bar-width nil, :row [(number "2") (number "34.04") "Dec 5, 2014" "The Apple Pan"]} {:bar-width nil, :row [(number "3") (number "34.05") "Aug 1, 2014" "The Gorbals"]}] - (rest (#'body/prep-for-html-rendering pacific-tz {} {:cols test-columns :rows test-data} nil nil (count test-columns)))))) + (rest (#'body/prep-for-html-rendering pacific-tz {} {:cols test-columns :rows test-data} (count test-columns)))))) ;; Testing the bar-column, which is the % of this row relative to the max of that column (deftest bar-column (is (= [{:bar-width (float 85.249), :row [(number "1") (number "34.10") "Apr 1, 2014" "Stout Burgers & Beers"]} {:bar-width (float 85.1015), :row [(number "2") (number "34.04") "Dec 5, 2014" "The Apple Pan"]} {:bar-width (float 85.1185), :row [(number "3") (number "34.05") "Aug 1, 2014" "The Gorbals"]}] - (rest (#'body/prep-for-html-rendering pacific-tz {} {:cols test-columns :rows test-data} second 40 (count test-columns)))))) + (rest (#'body/prep-for-html-rendering pacific-tz {} {:cols test-columns :rows test-data} (count test-columns) + {:bar-column second, :min-value 0, :max-value 40}))))) (defn- add-rating "Injects `RATING-OR-COL` and `DESCRIPTION-OR-COL` into `COLUMNS-OR-ROW`" @@ -202,7 +200,7 @@ (is (= [{:row [(number "ID") (number "Latitude") "Rating Desc" "Last Login" "Name"] :bar-width nil} #{5}] - (prep-for-html-rendering' test-columns-with-remapping test-data-with-remapping nil nil)))) + (prep-for-html-rendering' test-columns-with-remapping test-data-with-remapping nil nil nil)))) ;; Result rows should include only the remapped column value, not the original (deftest include-only-remapped-column-name @@ -212,8 +210,6 @@ (map :row (rest (#'body/prep-for-html-rendering pacific-tz {} {:cols test-columns-with-remapping :rows test-data-with-remapping} - nil - nil (count test-columns-with-remapping))))))) ;; There should be no truncation warning if the number of rows/cols is fewer than the row/column limit @@ -246,8 +242,6 @@ (rest (#'body/prep-for-html-rendering pacific-tz {} {:cols test-columns-with-date-special-type :rows test-data} - nil - nil (count test-columns)))))) (defn- render-scalar-value [results] diff --git a/test/metabase/pulse/render/table_test.clj b/test/metabase/pulse/render/table_test.clj index a8275e6681ce2316555004ed08bfddefb0111575..f51233ce3e1c0bc6d220f3a6fcd4b2aec3c48149 100644 --- a/test/metabase/pulse/render/table_test.clj +++ b/test/metabase/pulse/render/table_test.clj @@ -64,6 +64,6 @@ [4 5 6] [7 8 9]]}] (-> (color/make-color-selector query-results (:visualization_settings render.tu/test-card)) - (#'table/render-table ["a" "b" "c"] (query-results->header+rows query-results)) + (#'table/render-table 0 ["a" "b" "c"] (query-results->header+rows query-results)) find-table-body cell-value->background-color))) diff --git a/test/metabase/query_processor_test/implicit_joins_test.clj b/test/metabase/query_processor_test/implicit_joins_test.clj index 9117f70b092aa1191dc978fe0b10a955715bba84..047cb530b4d3f0f519d1b69d68c8210f35bcea8a 100644 --- a/test/metabase/query_processor_test/implicit_joins_test.clj +++ b/test/metabase/query_processor_test/implicit_joins_test.clj @@ -149,4 +149,4 @@ (mt/run-mbql-query messages {:aggregation [[:count]] :breakout [$sender_id->users.name] - :filter [:= $reciever_id->users.name "Rasta Toucan"]}))))) + :filter [:= $receiver_id->users.name "Rasta Toucan"]}))))) diff --git a/test/metabase/test/data/dataset_definitions/avian-singles.edn b/test/metabase/test/data/dataset_definitions/avian-singles.edn index f18e5eec2905cbde7c82739986f24d1a2f77c877..06c9a122077a10362226df5122d2f2ae8b2390ab 100644 --- a/test/metabase/test/data/dataset_definitions/avian-singles.edn +++ b/test/metabase/test/data/dataset_definitions/avian-singles.edn @@ -11,7 +11,7 @@ ["messages" [{:field-name "sender_id" :base-type :type/Integer :fk :users} - {:field-name "reciever_id" + {:field-name "receiver_id" :base-type :type/Integer :fk :users} {:field-name "text"