Skip to content
Snippets Groups Projects
Unverified Commit f16f2def authored by metamben's avatar metamben Committed by GitHub
Browse files

Remove deprecated column rendering limit (#21696)

parent d2fe4563
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/pulse/render/body.clj
+ 19
− 52
View file @ f16f2def
......@@ -53,11 +53,6 @@
"Maximum number of rows to render in a Pulse image."
10)
(def cols-limit
"Maximum number of columns to render in a Pulse image. Set to infinity, so that columns are not truncated.
TODO: we should eventually remove the column limiting logic if it's not used anywhere."
##Inf)
;; NOTE: hiccup does not escape content by default so be sure to use "h" to escape any user-controlled content :-/
;;; +----------------------------------------------------------------------------------------------------------------+
......@@ -70,12 +65,6 @@
(and (not (isa? semantic_type :type/Description))
(not (contains? #{:details-only :retired :sensitive} visibility_type))))
(defn- count-displayed-columns
"Return a count of the number of columns to be included in a table display"
[cols]
(count (filter show-in-table? cols)))
;;; --------------------------------------------------- Formatting ---------------------------------------------------
(s/defn ^:private format-cell
......@@ -178,15 +167,14 @@
(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 data column-limit]
(prep-for-html-rendering timezone-id card data column-limit {}))
([timezone-id :- (s/maybe s/Str) card {:keys [cols rows viz-settings]} column-limit
([timezone-id :- (s/maybe s/Str) card data]
(prep-for-html-rendering timezone-id card data {}))
([timezone-id :- (s/maybe s/Str) card {:keys [cols rows viz-settings]}
{:keys [bar-column] :as data-attributes}]
(let [remapping-lookup (create-remapping-lookup cols)
limited-cols (take column-limit cols)]
(let [remapping-lookup (create-remapping-lookup cols)]
(cons
(query-results->header-row remapping-lookup card limited-cols bar-column)
(query-results->row-seq timezone-id remapping-lookup limited-cols
(query-results->header-row remapping-lookup card cols bar-column)
(query-results->row-seq timezone-id remapping-lookup cols
(take rows-limit rows)
viz-settings
data-attributes)))))
......@@ -195,42 +183,21 @@
[:strong {:style (style/style {:color style/color-gray-3})} (h (common/format-number number))])
(defn- render-truncation-warning
[col-limit col-count row-limit row-count]
(let [over-row-limit (> row-count row-limit)
over-col-limit (> col-count col-limit)]
(when (or over-row-limit over-col-limit)
[row-limit row-count]
(let [over-row-limit (> row-count row-limit)]
(when over-row-limit
[:div {:style (style/style {:padding-top :16px})}
(cond
(and over-row-limit over-col-limit)
[:div {:style (style/style {:color style/color-gray-2
:padding-bottom :10px})}
"Showing " (strong-limit-text row-limit)
" of " (strong-limit-text row-count)
" rows and " (strong-limit-text col-limit)
" of " (strong-limit-text col-count)
" columns."]
over-row-limit
[:div {:style (style/style {:color style/color-gray-2
:padding-bottom :10px})}
"Showing " (strong-limit-text row-limit)
" of " (strong-limit-text row-count)
" rows."]
over-col-limit
[:div {:style (style/style {:color style/color-gray-2
:padding-bottom :10px})}
"Showing " (strong-limit-text col-limit)
" of " (strong-limit-text col-count)
" columns."])])))
[:div {:style (style/style {:color style/color-gray-2
:padding-bottom :10px})}
"Showing " (strong-limit-text row-limit)
" of " (strong-limit-text row-count)
" rows."]])))
(defn- attached-results-text
"Returns hiccup structures to indicate truncated results are available as an attachment"
[render-type cols cols-limit rows rows-limit]
[render-type rows rows-limit]
(when (and (not= :inline render-type)
(or (< cols-limit (count-displayed-columns cols))
(< rows-limit (count rows))))
(< rows-limit (count rows)))
[:div {:style (style/style {:color style/color-gray-2
:margin-bottom :16px})}
(trs "More results have been included as a file attachment")]))
......@@ -251,13 +218,13 @@
(table/render-table
(color/make-color-selector data (:visualization_settings card))
(mapv :name (:cols data))
(prep-for-html-rendering timezone-id card data cols-limit))
(render-truncation-warning cols-limit (count-displayed-columns cols) rows-limit (count rows))]]
(prep-for-html-rendering timezone-id card data))
(render-truncation-warning rows-limit (count rows))]]
{:attachments
nil
:content
(if-let [results-attached (attached-results-text render-type cols cols-limit rows rows-limit)]
(if-let [results-attached (attached-results-text render-type rows rows-limit)]
(list results-attached table-body)
(list table-body))}))
......
test/metabase/pulse/render/body_test.clj
+ 14
− 58
View file @ f16f2def
......@@ -55,7 +55,7 @@
(defn- prep-for-html-rendering'
[cols rows bar-column min-value max-value]
(let [results (#'body/prep-for-html-rendering pacific-tz {} {:cols cols :rows rows} (count cols)
(let [results (#'body/prep-for-html-rendering pacific-tz {} {:cols cols :rows rows}
{:bar-column bar-column :min-value min-value :max-value max-value})]
[(first results)
(col-counts results)]))
......@@ -134,16 +134,14 @@
:bar-width nil}
(first (#'body/prep-for-html-rendering pacific-tz
card
{:cols cols :rows []}
(count test-columns)))))
{:cols cols :rows []}))))
;; card does not contain custom column names
(is (= {:row ["Last Login" "Name"]
:bar-width nil}
(first (#'body/prep-for-html-rendering pacific-tz
{}
{:cols cols :rows []}
(count test-columns))))))))
{:cols cols :rows []})))))))
;; When including a bar column, bar-width is 99%
(deftest bar-width
......@@ -162,14 +160,14 @@
(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} (count test-columns))))))
(rest (#'body/prep-for-html-rendering pacific-tz {} {:cols test-columns :rows test-data})))))
;; 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} (count test-columns)
(rest (#'body/prep-for-html-rendering pacific-tz {} {:cols test-columns :rows test-data}
{:bar-column second, :min-value 0, :max-value 40})))))
(defn- add-rating
......@@ -214,27 +212,18 @@
[(number "3") (number "34.05") "Good" "Aug 1, 2014" "The Gorbals"]]
(map :row (rest (#'body/prep-for-html-rendering pacific-tz
{}
{:cols test-columns-with-remapping :rows test-data-with-remapping}
(count test-columns-with-remapping)))))))
{:cols test-columns-with-remapping :rows test-data-with-remapping}))))))
;; There should be no truncation warning if the number of rows/cols is fewer than the row/column limit
(deftest no-truncation-warnig
(is (= ""
(html (#'body/render-truncation-warning 100 10 100 10)))))
(html (#'body/render-truncation-warning 100 10)))))
;; When there are more rows than the limit, check to ensure a truncation warning is present
(deftest truncation-warning-when-rows-exceed-max
(is (= [true false]
(let [html-output (html (#'body/render-truncation-warning 100 10 10 100))]
[(boolean (re-find #"Showing.*10.*of.*100.*rows" html-output))
(boolean (re-find #"Showing .* of .* columns" html-output))]))))
;; When there are more columns than the limit, check to ensure a truncation warning is present
(deftest truncation-warning-when-cols-exceed-max
(is (= [true false]
(let [html-output (html (#'body/render-truncation-warning 10 100 100 10))]
[(boolean (re-find #"Showing.*10.*of.*100.*columns" html-output))
(boolean (re-find #"Showing .* of .* rows" html-output))]))))
(is (= true
(let [html-output (html (#'body/render-truncation-warning 10 100))]
(boolean (re-find #"Showing.*10.*of.*100.*rows" html-output))))))
(def ^:private test-columns-with-date-semantic-type
(update test-columns 2 merge {:base_type :type/Text
......@@ -247,8 +236,7 @@
{: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-with-date-semantic-type :rows test-data}
(count test-columns))))))
{:cols test-columns-with-date-semantic-type :rows test-data})))))
(deftest error-test
(testing "renders error"
......@@ -363,46 +351,14 @@
(comp replace-style-maps #'body/render-truncation-warning))
(deftest no-truncation-warnig-for-style
(is (nil? (render-truncation-warning' 10 5 20 10))))
(is (nil? (render-truncation-warning' 10 5))))
(deftest renders-truncation-style-1
(is (= [:div :style-map
[:div :style-map
"Showing " [:strong :style-map "10"] " of "
[:strong :style-map "11"] " columns."]]
(render-truncation-warning' 10 11 20 10))))
(deftest renders-truncation-style-2
(deftest renders-truncation
(is (= [:div
:style-map
[:div :style-map "Showing "
[:strong :style-map "20"] " of " [:strong :style-map "21"] " rows."]]
(render-truncation-warning' 10 5 20 21))))
(deftest renders-truncation-style-3
(is (= [:div
:style-map
[:div
:style-map
"Showing "
[:strong :style-map "20"]
" of "
[:strong :style-map "21"]
" rows and "
[:strong :style-map "10"]
" of "
[:strong :style-map "11"]
" columns."]]
(render-truncation-warning' 10 11 20 21))))
(deftest counts-displayed-columns
(is (= 4
(#'body/count-displayed-columns test-columns))))
(deftest counts-displayed-columns-excludes-undisplayed
(is (= 4
(#'body/count-displayed-columns
(concat test-columns [description-col detail-col sensitive-col retired-col])))))
(render-truncation-warning' 20 21))))
;; Test rendering a bar graph
;;
......
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