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

Consider drill thru `:dimensions` when calculating `available-drill-thrus` (#34347)

parent 52d9eec9
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/lib/drill_thru.cljc
+ 47
− 25
View file @ 171d3d2d
......@@ -16,10 +16,12 @@
[metabase.lib.drill-thru.underlying-records :as lib.drill-thru.underlying-records]
[metabase.lib.drill-thru.zoom :as lib.drill-thru.zoom]
[metabase.lib.drill-thru.zoom-in-timeseries :as lib.drill-thru.zoom-in-timeseries]
[metabase.lib.field :as lib.field]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.options :as lib.options]
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.drill-thru :as lib.schema.drill-thru]
[metabase.lib.util :as lib.util]
[metabase.util.malli :as mu]))
(comment
......@@ -27,6 +29,10 @@
lib.drill-thru.pk/keep-me
lib.drill-thru.zoom/keep-me)
(defmethod lib.metadata.calculation/display-info-method ::drill-thru
[query stage-number drill-thru]
(lib.drill-thru.common/drill-thru-info-method query stage-number drill-thru))
;; TODO: Different ways to apply drill-thru to a query.
;; So far:
;; - :filter on each :operators of :drill-thru/quick-filter applied with (lib/filter query stage filter-clause)
......@@ -55,9 +61,34 @@
(assoc-in context [:column :lib/source-uuid] (lib.options/uuid ag))))
context))
(defmethod lib.metadata.calculation/display-info-method ::drill-thru
[query stage-number drill-thru]
(lib.drill-thru.common/drill-thru-info-method query stage-number drill-thru))
;;; TODO: Missing drills: automatic insights, format.
(def ^:private available-drill-thru-fns
[#'lib.drill-thru.distribution/distribution-drill
#'lib.drill-thru.column-filter/column-filter-drill
#'lib.drill-thru.foreign-key/foreign-key-drill
#'lib.drill-thru.object-details/object-detail-drill
#'lib.drill-thru.pivot/pivot-drill
#'lib.drill-thru.quick-filter/quick-filter-drill
#'lib.drill-thru.sort/sort-drill
#'lib.drill-thru.summarize-column/summarize-column-drill
#'lib.drill-thru.summarize-column-by-time/summarize-column-by-time-drill
#'lib.drill-thru.underlying-records/underlying-records-drill
#'lib.drill-thru.zoom-in-timeseries/zoom-in-timeseries-drill])
(mu/defn ^:private all-contexts :- [:sequential {:min 1} ::lib.schema.drill-thru/context]
[query :- ::lib.schema/query
stage-number :- :int
{:keys [dimensions], :as context} :- ::lib.schema.drill-thru/context]
(if (empty? dimensions)
[context]
(let [stage (lib.util/query-stage query stage-number)
returned-columns (lib.metadata.calculation/returned-columns query stage-number stage)]
(for [{:keys [column-name value]} dimensions
:let [col (lib.field/resolve-column-name-in-metadata column-name returned-columns)]
:when col]
(assoc context
:column col
:value value)))))
(mu/defn available-drill-thrus :- [:sequential [:ref ::lib.schema.drill-thru/drill-thru]]
"Get a list (possibly empty) of available drill-thrus for a column, or a column + value pair.
......@@ -71,28 +102,19 @@
([query :- ::lib.schema/query
stage-number :- :int
context :- ::lib.schema.drill-thru/context]
(let [context (icky-hack-add-source-uuid-to-aggregation-column-metadata query stage-number context)]
(try
(into []
(keep #(% query stage-number context))
;; TODO: Missing drills: automatic insights, format.
[lib.drill-thru.distribution/distribution-drill
lib.drill-thru.column-filter/column-filter-drill
lib.drill-thru.foreign-key/foreign-key-drill
lib.drill-thru.object-details/object-detail-drill
lib.drill-thru.pivot/pivot-drill
lib.drill-thru.quick-filter/quick-filter-drill
lib.drill-thru.sort/sort-drill
lib.drill-thru.summarize-column/summarize-column-drill
lib.drill-thru.summarize-column-by-time/summarize-column-by-time-drill
lib.drill-thru.underlying-records/underlying-records-drill
lib.drill-thru.zoom-in-timeseries/zoom-in-timeseries-drill])
(catch #?(:clj Throwable :cljs :default) e
(throw (ex-info (str "Error getting available drill thrus for query: " (ex-message e))
{:query query
:stage-number stage-number
:context context}
e)))))))
(try
(into []
(mapcat (fn [context]
(let [context (icky-hack-add-source-uuid-to-aggregation-column-metadata query stage-number context)]
(keep #(% query stage-number context)
available-drill-thru-fns))))
(all-contexts query stage-number context))
(catch #?(:clj Throwable :cljs :default) e
(throw (ex-info (str "Error getting available drill thrus for query: " (ex-message e))
{:query query
:stage-number stage-number
:context context}
e))))))
(mu/defn drill-thru :- ::lib.schema/query
"`(drill-thru query stage-number drill-thru)`
......
src/metabase/lib/equality.cljc
+ 2
− 2
View file @ 171d3d2d
......@@ -268,8 +268,8 @@
Throws if there are multiple, ambiguous matches.
Returns the matching ref, or nil if no plausible matches are found."
[column :- lib.metadata/ColumnMetadata
refs :- [:sequential ::lib.schema.ref/ref]]
[column :- lib.metadata/ColumnMetadata
refs :- [:sequential ::lib.schema.ref/ref]]
(let [ref-tails (group-by ref-id-or-name refs)
matches (or (some->> column :lib/source-uuid (get ref-tails) not-empty)
(not-empty (get ref-tails (:id column)))
......
src/metabase/lib/field.cljc
+ 6
− 5
View file @ 171d3d2d
......@@ -47,13 +47,14 @@
[[tag opts id-or-name]]
[(keyword tag) (normalize-field-options opts) id-or-name])
(mu/defn ^:private resolve-column-name-in-metadata :- [:maybe lib.metadata/ColumnMetadata]
(mu/defn resolve-column-name-in-metadata :- [:maybe lib.metadata/ColumnMetadata]
"Find the column with `column-name` in a sequence of `column-metadatas`."
[column-name :- ::lib.schema.common/non-blank-string
column-metadatas :- [:sequential lib.metadata/ColumnMetadata]]
(or (m/find-first #(= (:lib/desired-column-alias %) column-name)
column-metadatas)
(m/find-first #(= (:name %) column-name)
column-metadatas)
(or (some (fn [k]
(m/find-first #(= (get % k) column-name)
column-metadatas))
[:lib/desired-column-alias :name])
(do
(log/warn (i18n/tru "Invalid :field clause: column {0} does not exist. Found: {1}"
(pr-str column-name)
......
src/metabase/lib/schema/drill_thru.cljc
+ 27
− 3
View file @ 171d3d2d
......@@ -172,10 +172,34 @@
[:drill-thru/automatic-insights ::drill-thru.automatic-insights]
[:drill-thru/zoom-in.timeseries ::drill-thru.zoom-in.timeseries]]])
;;; Frontend passes in something that looks like this. Why this shape? Who knows.
(comment
{:column {:lib/type :metadata/column
:remapped-from-index nil
:base-type :type/BigInteger
:semantic-type :type/Quantity
:name "count"
:lib/source :source/aggregations
:aggregation-index 0
:effective-type :type/BigInteger
:display-name "Count"
:remapping nil}
:value 457
:row [{:column-name "CREATED_AT", :value "2024-01-01T00:00:00Z"}
{:column-name "count", :value 457}]
:dimensions [{:column-name "CREATED_AT", :value "2024-01-01T00:00:00Z"}]})
(mr/def ::context.row.value
[:map
[:column-name string?]
[:value :any]])
(mr/def ::context.row
[:sequential [:ref ::context.row.value]])
(mr/def ::context
[:map
[:column [:ref ::lib.schema.metadata/column]]
[:value [:maybe :any]]
[:row {:optional true} [:sequential [:map
[:column-name string?]
[:value :any]]]]])
[:row {:optional true} [:ref ::context.row]]
[:dimensions {:optional true} [:maybe [:ref ::context.row]]]])
test/metabase/lib/drill_thru_test.cljc
+ 48
− 24
View file @ 171d3d2d
......@@ -72,8 +72,13 @@
([query context depth]
(doseq [drill (lib/available-drill-thrus query -1 context)
args (drill-thru-test-args drill)]
(if (= (:type drill) :drill-thru/pivot)
(condp = (:type drill)
:drill-thru/pivot
(log/warnf "drill-thru-method is not yet implemented for :drill-thru/pivot (#33559)")
:drill-thru/underlying-records
(log/warnf "drill-thru-method is not yet implemented for :drill-thru/underlying-records (#34233)")
(testing (str "\ndrill =\n" (u/pprint-to-str drill)
"\nargs =\n" (u/pprint-to-str args))
(try
......@@ -354,29 +359,48 @@
(deftest ^:parallel timeseries-breakout-table-view-available-drill-thrus-test
(testing "ORDERS + count aggregation + breakout on CREATED_AT by month query"
(testing "options for CREATED_AT column + value"
(let [query orders-count-aggregation-breakout-on-created-at-by-month-query
column (m/find-first #(= (:name %) "CREATED_AT")
(lib/returned-columns orders-count-aggregation-breakout-on-created-at-by-month-query))
_ (assert column)
context {:column column
:value "2018-05"
:row [{:column-name "CREATED_AT", :value "2018-05"}
{:column-name "count", :value 10}]}]
(is (=? [{:lib/type :metabase.lib.drill-thru/drill-thru
:type :drill-thru/quick-filter
:operators [{:name "<"}
{:name ">"}
{:name "="}
{:name "≠"}]}
{:lib/type :metabase.lib.drill-thru/drill-thru
:display-name "See this month by week"
:type :drill-thru/zoom-in.timeseries
:column {:name "CREATED_AT"
:metabase.lib.field/temporal-unit :month}
:value "2018-05"
:next-unit :week}]
(lib/available-drill-thrus query -1 context)))
(test-drill-applications query context)))))
(let [query orders-count-aggregation-breakout-on-created-at-by-month-query
count-column (m/find-first #(= (:name %) "count")
(lib/returned-columns orders-count-aggregation-breakout-on-created-at-by-month-query))
_ (assert count-column)
created-at-column (m/find-first #(= (:name %) "CREATED_AT")
(lib/returned-columns orders-count-aggregation-breakout-on-created-at-by-month-query))
_ (assert created-at-column)
row [{:column-name "CREATED_AT", :value "2018-05-01T00:00:00Z"}
{:column-name "count", :value 457}]
expected-drills {:quick-filter {:lib/type :metabase.lib.drill-thru/drill-thru
:type :drill-thru/quick-filter
:operators [{:name "<"}
{:name ">"}
{:name "="}
{:name "≠"}]}
:underlying-records {:lib/type :metabase.lib.drill-thru/drill-thru
:type :drill-thru/underlying-records
:row-count 2
:table-name "Orders"}
:zoom-in.timeseries {:lib/type :metabase.lib.drill-thru/drill-thru
:display-name "See this month by week"
:type :drill-thru/zoom-in.timeseries
:column {:name "CREATED_AT"
:metabase.lib.field/temporal-unit :month}
:value "2018-05-01T00:00:00Z"
:next-unit :week}}]
(let [context {:column created-at-column
:value "2018-05-01T00:00:00Z"
:row row}]
(testing (str "\ncontext =\n" (u/pprint-to-str context))
(is (=? (map expected-drills [:quick-filter :zoom-in.timeseries])
(lib/available-drill-thrus query -1 context)))
(test-drill-applications query context)))
(testing "with :dimensions"
(let [context {:column count-column
:value 457
:row row
:dimensions [{:column-name "CREATED_AT", :value "2018-05-01T00:00:00Z"}]}]
(testing (str "\ncontext =\n" (u/pprint-to-str context))
(is (=? (map expected-drills [:quick-filter :underlying-records :zoom-in.timeseries])
(lib/available-drill-thrus query -1 context)))
(test-drill-applications query context))))))))
(deftest ^:parallel count-aggregation-table-view-available-drill-thrus-test
(testing "ORDERS + count aggregation + breakout on CREATED_AT by month query"
......
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