Skip to content
Snippets Groups Projects
Unverified Commit 51757c4a authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

Return row count: 2 instead of negative row count (#36389)

parent 117fee84
Branches
Tags
No related merge requests found
......@@ -23,9 +23,9 @@
There is another quite different case: clicking the legend of a chart with multiple bars or lines broken out by
category. Then `column` is nil!"
[query :- ::lib.schema/query
stage-number :- :int
{:keys [column column-ref dimensions value]} :- ::lib.schema.drill-thru/context]
[query :- ::lib.schema/query
stage-number :- :int
{:keys [column column-ref dimensions value], :as _context} :- ::lib.schema.drill-thru/context]
;; Clicking on breakouts is weird. Clicking on Count(People) by State: Minnesota yields a FE `clicked` with:
;; - column is COUNT
;; - row[0] has col: STATE, value: "Minnesota"
......@@ -50,7 +50,10 @@
:type :drill-thru/underlying-records
;; TODO: This is a bit confused for non-COUNT aggregations. Perhaps it should just always be 10 or something?
;; Note that some languages have different plurals for exactly 2, or for 1, 2-5, and 6+.
:row-count (if (number? value) value 2)
:row-count (if (and (number? value)
(not (neg? value)))
value
2)
:table-name (when-let [table-or-card (or (some->> query lib.util/source-table-id (lib.metadata/table query))
(some->> query lib.util/source-card-id (lib.metadata/card query)))]
(lib.metadata.calculation/display-name query stage-number table-or-card))
......
......@@ -12,7 +12,10 @@
[metabase.lib.test-util.metadata-providers.mock :as providers.mock]
[metabase.util :as u]
#?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal])
:clj ([java-time.api :as jt]))))
:clj ([java-time.api :as jt]
[metabase.util.malli.fn :as mu.fn]))))
#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
(deftest ^:parallel returns-underlying-records-test-1
(lib.drill-thru.tu/test-returns-drill
......@@ -68,9 +71,7 @@
drills))))))))))
#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
(def last-month
(def ^:private last-month
#?(:cljs (let [now (js/Date.)
year (.getFullYear now)
month (.getMonth now)]
......@@ -301,3 +302,42 @@
:breakout (symbol "nil #_\"key is not present.\"")
:fields (symbol "nil #_\"key is not present.\"")}]}
(lib.drill-thru/drill-thru query -1 drill))))))
(deftest ^:parallel negative-aggregation-values-display-info-test
(testing "should use the default row count for aggregations with negative values (#36143)"
(let [query (-> (lib/query meta/metadata-provider (meta/table-metadata :orders))
(lib/aggregate (lib/count))
(lib/breakout (lib.metadata/field lib.tu/metadata-provider-with-mock-cards
(meta/id :orders :created-at))))
count-col (m/find-first #(= (:name %) "count")
(lib/returned-columns query))
_ (is (some? count-col))
context {:column count-col
:column-ref (lib/ref count-col)
:value -10,
:row [{:column (meta/field-metadata :orders :created-at)
:column-ref (lib/ref (meta/field-metadata :orders :created-at))
:value "2020-01-01"}
{:column count-col
:column-ref (lib/ref count-col)
:value -10}]
:dimensions [{:column (meta/field-metadata :orders :created-at)
:column-ref (lib/ref (meta/field-metadata :orders :created-at)),
:value "2020-01-01"}]}
drill (m/find-first #(= (:type %) :drill-thru/underlying-records)
(lib/available-drill-thrus query -1 context))]
(is (=? {:lib/type :metabase.lib.drill-thru/drill-thru
:type :drill-thru/underlying-records
:row-count 2
:table-name "Orders"
:dimensions [{:column {:name "CREATED_AT"}
:column-ref [:field {} (meta/id :orders :created-at)]
:value "2020-01-01"}]
:column-ref [:aggregation {} string?]}
drill))
;; display info currently doesn't include a `display-name` for drill thrus... we can fix this later.
(binding #?(:clj [mu.fn/*enforce* false] :cljs [])
(is (=? {:type :drill-thru/underlying-records
:row-count 2
:table-name "Orders"}
(lib/display-info query -1 drill)))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment