Skip to content
Snippets Groups Projects
Unverified Commit ba57ab66 authored by Tim Macdonald's avatar Tim Macdonald Committed by GitHub
Browse files

Render negative values in pulse bar charts (#13646) 

[Fixes #7044] Render pulse bar charts with  negative values well

Also fixes misspelled column name in Avian Singles test dataset

* Bump DB prefixes so data gets reloaded to reflect col name change [ci drivers]

* Bump test DB version from v2 to v3

[ci drivers]

* More BigQuery test fixes :wrench:

 [ci drivers]

* Fix Snowflake [ci snowflake]

Co-authored-by: default avatarCam Saul <github@camsaul.com>
parent f665ef1e
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 175 additions and 118 deletions
modules/drivers/bigquery/test/metabase/driver/bigquery/query_processor_test.clj
+ 33
− 33
View file @ ba57ab66
......@@ -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 -- "]})))))))))
modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
+ 4
− 4
View file @ ba57ab66
......@@ -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)))
......
modules/drivers/bigquery/test/metabase/test/data/bigquery.clj
+ 1
− 1
View file @ ba57ab66
......@@ -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
......
modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj
+ 21
− 25
View file @ ba57ab66
(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"
......
modules/drivers/snowflake/test/metabase/test/data/snowflake.clj
+ 2
− 2
View file @ ba57ab66
......@@ -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]}]
......
src/metabase/public_settings.clj
+ 5
− 0
View file @ ba57ab66
......@@ -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
......
src/metabase/pulse/render/body.clj
+ 29
− 13
View file @ ba57ab66
......@@ -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
......
src/metabase/pulse/render/style.clj
+ 5
− 0
View file @ ba57ab66
......@@ -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."
[]
......
src/metabase/pulse/render/table.clj
+ 58
− 17
View file @ ba57ab66
......@@ -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))})))}
"&#160;"]))
(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) "%")})}
"&#160;"]])])])
(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)]))
test/metabase/pulse/render/body_test.clj
+ 14
− 20
View file @ ba57ab66
......@@ -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]
......
test/metabase/pulse/render/table_test.clj
+ 1
− 1
View file @ ba57ab66
......@@ -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)))
test/metabase/query_processor_test/implicit_joins_test.clj
+ 1
− 1
View file @ ba57ab66
......@@ -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"]})))))
test/metabase/test/data/dataset_definitions/avian-singles.edn
+ 1
− 1
View file @ ba57ab66
......@@ -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"
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment