Skip to content
Snippets Groups Projects
Unverified Commit bd28194b authored by Braden Shepherdson's avatar Braden Shepherdson Committed by GitHub
Browse files

[MLv2] Full `:column`s for `:dimensions` in `available-drill-thrus` (#34810)

Previously only the `:column-name` was provided, and there is a
self-proclaimed "icky hack" to guess at the full columns. The FE
provides the (JSON) columns, so this PR converts and uses them.

The icky hack can be removed.

`:column-ref` is included because sometimes it's important that the refs
use the same UUID.
parent 841ec9ef
No related branches found
No related tags found
No related merge requests found
Showing
with 329 additions and 279 deletions
...@@ -181,6 +181,7 @@ const AGGREGATED_ORDERS_COLUMNS = { ...@@ -181,6 +181,7 @@ const AGGREGATED_ORDERS_COLUMNS = {
"temporal-unit": "month", "temporal-unit": "month",
}, },
], ],
unit: "month",
}), }),
count: createMockColumn({ count: createMockColumn({
......
...@@ -533,3 +533,30 @@ ...@@ -533,3 +533,30 @@
:legacy-ref legacy-ref :legacy-ref legacy-ref
:legacy-index->pMBQL-uuid *legacy-index->pMBQL-uuid*} :legacy-index->pMBQL-uuid *legacy-index->pMBQL-uuid*}
e)))))))) e))))))))
(defn- from-json [query-fragment]
#?(:cljs (if (object? query-fragment)
(js->clj query-fragment :keywordize-keys true)
query-fragment)
:clj query-fragment))
(defn js-legacy-query->pMBQL
"Given a JSON-formatted legacy MBQL query, transform it to pMBQL.
If you have only the inner query map (`{:source-table 2 ...}` or similar), call [[js-legacy-inner-query->pMBQL]]
instead."
[query-map]
(-> query-map
from-json
(u/assoc-default :type :query)
mbql.normalize/normalize
->pMBQL))
(defn js-legacy-inner-query->pMBQL
"Given a JSON-formatted *inner* query, transform it to pMBQL.
If you have a complete legacy query (`{:type :query, :query {...}}` or similar), call [[js-legacy-query->pMBQL]]
instead."
[inner-query]
(js-legacy-query->pMBQL {:type :query
:query (from-json inner-query)}))
(ns metabase.lib.drill-thru (ns metabase.lib.drill-thru
(:require (:require
[metabase.lib.aggregation :as lib.aggregation]
[metabase.lib.drill-thru.column-filter :as lib.drill-thru.column-filter] [metabase.lib.drill-thru.column-filter :as lib.drill-thru.column-filter]
[metabase.lib.drill-thru.common :as lib.drill-thru.common] [metabase.lib.drill-thru.common :as lib.drill-thru.common]
[metabase.lib.drill-thru.distribution :as lib.drill-thru.distribution] [metabase.lib.drill-thru.distribution :as lib.drill-thru.distribution]
...@@ -16,12 +15,9 @@ ...@@ -16,12 +15,9 @@
[metabase.lib.drill-thru.underlying-records :as lib.drill-thru.underlying-records] [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 :as lib.drill-thru.zoom]
[metabase.lib.drill-thru.zoom-in-timeseries :as lib.drill-thru.zoom-in-timeseries] [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.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.options :as lib.options]
[metabase.lib.schema :as lib.schema] [metabase.lib.schema :as lib.schema]
[metabase.lib.schema.drill-thru :as lib.schema.drill-thru] [metabase.lib.schema.drill-thru :as lib.schema.drill-thru]
[metabase.lib.util :as lib.util]
[metabase.util.malli :as mu])) [metabase.util.malli :as mu]))
(comment (comment
...@@ -39,28 +35,6 @@ ...@@ -39,28 +35,6 @@
;; TODO: ActionMode, PublicMode, MetabotMode need to be captured in the FE before calling `available-drill-thrus`. ;; TODO: ActionMode, PublicMode, MetabotMode need to be captured in the FE before calling `available-drill-thrus`.
(defn- icky-hack-add-source-uuid-to-aggregation-column-metadata
"This is an evil HACK -- the FE calls [[available-drill-thrus]] with query results metadata as produced
by [[metabase.query-processor.middleware.annotate]], which does not include the `:lib/source-uuid` for aggregation
columns (since it's still using legacy MBQL at this point), which
means [[metabase.lib.aggregation/column-metadata->aggregation-ref]] can't generate references (since it requires
`:lib/source-uuid`)... so we have to add it back in manually. I've added `:aggregation-index` to the annotate
results (see [[metabase.query-processor.middleware.annotate/cols-for-ags-and-breakouts]]), and we can use that to
determine the correct `:lib/source-uuid`.
There's probably a more general place we can be doing this, but it's escaping me, so I guess this will have to do
for now. It doesn't seem like the FE generally uses result metadata in most other places so this is not an issue
elsewhere.
Once we're using MLv2 queries everywhere and stop converting back to legacy we should be able to remove this ICKY
HACK."
[query stage-number {{:keys [aggregation-index], :as column} :column, :as context}]
(or (when (and aggregation-index
(not (:lib/source-uuid column)))
(when-let [ag (lib.aggregation/aggregation-at-index query stage-number aggregation-index)]
(assoc-in context [:column :lib/source-uuid] (lib.options/uuid ag))))
context))
;;; TODO: Missing drills: automatic insights, format. ;;; TODO: Missing drills: automatic insights, format.
(def ^:private available-drill-thru-fns (def ^:private available-drill-thru-fns
"Some drill thru functions are expected to return drills for just the specified `:column`; others are expected to "Some drill thru functions are expected to return drills for just the specified `:column`; others are expected to
...@@ -70,30 +44,22 @@ ...@@ -70,30 +44,22 @@
{:f #'lib.drill-thru.column-filter/column-filter-drill, :return-drills-for-dimensions? true} {:f #'lib.drill-thru.column-filter/column-filter-drill, :return-drills-for-dimensions? true}
{:f #'lib.drill-thru.foreign-key/foreign-key-drill, :return-drills-for-dimensions? false} {:f #'lib.drill-thru.foreign-key/foreign-key-drill, :return-drills-for-dimensions? false}
{:f #'lib.drill-thru.object-details/object-detail-drill, :return-drills-for-dimensions? false} {:f #'lib.drill-thru.object-details/object-detail-drill, :return-drills-for-dimensions? false}
{:f #'lib.drill-thru.pivot/pivot-drill, :return-drills-for-dimensions? true} {:f #'lib.drill-thru.pivot/pivot-drill, :return-drills-for-dimensions? false}
{:f #'lib.drill-thru.quick-filter/quick-filter-drill, :return-drills-for-dimensions? false} {:f #'lib.drill-thru.quick-filter/quick-filter-drill, :return-drills-for-dimensions? false}
{:f #'lib.drill-thru.sort/sort-drill, :return-drills-for-dimensions? true} {:f #'lib.drill-thru.sort/sort-drill, :return-drills-for-dimensions? true}
{:f #'lib.drill-thru.summarize-column/summarize-column-drill, :return-drills-for-dimensions? true} {:f #'lib.drill-thru.summarize-column/summarize-column-drill, :return-drills-for-dimensions? true}
{:f #'lib.drill-thru.summarize-column-by-time/summarize-column-by-time-drill, :return-drills-for-dimensions? true} {:f #'lib.drill-thru.summarize-column-by-time/summarize-column-by-time-drill, :return-drills-for-dimensions? true}
{:f #'lib.drill-thru.underlying-records/underlying-records-drill, :return-drills-for-dimensions? false} {:f #'lib.drill-thru.underlying-records/underlying-records-drill, :return-drills-for-dimensions? false}
{:f #'lib.drill-thru.zoom-in-timeseries/zoom-in-timeseries-drill, :return-drills-for-dimensions? true}]) {:f #'lib.drill-thru.zoom-in-timeseries/zoom-in-timeseries-drill, :return-drills-for-dimensions? false}])
(mu/defn ^:private dimension-contexts :- [:maybe [:sequential {:min 1} ::lib.schema.drill-thru/context]] (mu/defn ^:private dimension-contexts :- [:maybe [:sequential {:min 1} ::lib.schema.drill-thru/context]]
"Create new context maps (with updated `:column` and `:value` keys) for each of the `:dimensions` passed in. Some "Create new context maps (with updated `:column` and `:value` keys) for each of the `:dimensions` passed in. Some
drill thru functions are expected to return drills for each of these columns, while others are expected to ignore drill thru functions are expected to return drills for each of these columns, while others are expected to ignore
them. Why? Who knows." them. Why? Who knows."
[query :- ::lib.schema/query [{:keys [dimensions], :as context} :- ::lib.schema.drill-thru/context]
stage-number :- :int (not-empty
{:keys [dimensions], :as context} :- ::lib.schema.drill-thru/context] (for [dimension dimensions]
(when (seq dimensions) (merge context dimension))))
(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]] (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. "Get a list (possibly empty) of available drill-thrus for a column, or a column + value pair.
...@@ -109,13 +75,12 @@ ...@@ -109,13 +75,12 @@
stage-number :- :int stage-number :- :int
context :- ::lib.schema.drill-thru/context] context :- ::lib.schema.drill-thru/context]
(try (try
(let [primary-context (icky-hack-add-source-uuid-to-aggregation-column-metadata query stage-number context) (let [dim-contexts (dimension-contexts context)]
dim-contexts (dimension-contexts query stage-number primary-context)]
(into [] (into []
(for [{:keys [f return-drills-for-dimensions?]} available-drill-thru-fns (for [{:keys [f return-drills-for-dimensions?]} available-drill-thru-fns
context (if (and return-drills-for-dimensions? (seq dim-contexts)) context (if (and return-drills-for-dimensions? dim-contexts)
dim-contexts dim-contexts
[primary-context]) [context])
:let [drill (f query stage-number context)] :let [drill (f query stage-number context)]
:when drill] :when drill]
drill))) drill)))
......
(ns metabase.lib.drill-thru.object-details (ns metabase.lib.drill-thru.object-details
(:require (:require
[medley.core :as m]
[metabase.lib.aggregation :as lib.aggregation] [metabase.lib.aggregation :as lib.aggregation]
[metabase.lib.drill-thru.common :as lib.drill-thru.common] [metabase.lib.drill-thru.common :as lib.drill-thru.common]
[metabase.lib.metadata.calculation :as lib.metadata.calculation] [metabase.lib.metadata.calculation :as lib.metadata.calculation]
...@@ -24,8 +25,7 @@ ...@@ -24,8 +25,7 @@
(empty? (lib.aggregation/aggregations query stage-number))) (empty? (lib.aggregation/aggregations query stage-number)))
(let [[pk-column] (lib.metadata.calculation/primary-keys query) ; Already know there's only one. (let [[pk-column] (lib.metadata.calculation/primary-keys query) ; Already know there's only one.
pk-value (->> row pk-value (->> row
(filter #(= (:column-name %) (:name pk-column))) (m/find-first #(-> % :column :name (= (:name pk-column))))
first
:value)] :value)]
(when (and pk-value (when (and pk-value
;; Only recurse if this is a different column - otherwise it's an infinite loop. ;; Only recurse if this is a different column - otherwise it's an infinite loop.
......
...@@ -52,7 +52,7 @@ ...@@ -52,7 +52,7 @@
(when (and (lib.drill-thru.common/mbql-stage? query stage-number) (when (and (lib.drill-thru.common/mbql-stage? query stage-number)
;; (editable? query stage-number) ;; (editable? query stage-number)
column column
(some? value) (some? value) ; Deliberately allows value :null, only a missing value should fail this test.
(not (lib.types.isa/primary-key? column)) (not (lib.types.isa/primary-key? column))
(not (lib.types.isa/foreign-key? column))) (not (lib.types.isa/foreign-key? column)))
;; for aggregate columns, we want to introduce a new stage when applying the drill-thru, `:new-stage?` is used to ;; for aggregate columns, we want to introduce a new stage when applying the drill-thru, `:new-stage?` is used to
......
(ns metabase.lib.drill-thru.zoom-in-timeseries (ns metabase.lib.drill-thru.zoom-in-timeseries
(:require (:require
[medley.core :as m]
[metabase.lib.breakout :as lib.breakout] [metabase.lib.breakout :as lib.breakout]
[metabase.lib.drill-thru.common :as lib.drill-thru.common] [metabase.lib.drill-thru.common :as lib.drill-thru.common]
[metabase.lib.equality :as lib.equality]
[metabase.lib.filter :as lib.filter] [metabase.lib.filter :as lib.filter]
[metabase.lib.join.util :as lib.join.util]
[metabase.lib.metadata :as lib.metadata] [metabase.lib.metadata :as lib.metadata]
[metabase.lib.remove-replace :as lib.remove-replace] [metabase.lib.remove-replace :as lib.remove-replace]
[metabase.lib.schema :as lib.schema] [metabase.lib.schema :as lib.schema]
...@@ -25,26 +24,18 @@ ...@@ -25,26 +24,18 @@
(zipmap (drop-last valid-current-units) (zipmap (drop-last valid-current-units)
(drop 1 valid-current-units))) (drop 1 valid-current-units)))
(defn- is-ref-for-source-column? [a-ref column] (mu/defn ^:private matching-breakout-dimension :- [:maybe ::lib.schema.drill-thru/context.row.value]
(and (lib.util/clause-of-type? a-ref :field)
(let [[_field _opts id-or-name] a-ref]
(if (integer? id-or-name)
(= id-or-name (:id column))
(and (if-let [join-alias (lib.join.util/current-join-alias a-ref)]
(= join-alias (lib.join.util/current-join-alias column))
true)
(= id-or-name (:lib/source-column-alias column)))))))
(mu/defn ^:private matching-breakout-ref :- [:maybe :mbql.clause/field]
[query :- ::lib.schema/query [query :- ::lib.schema/query
stage-number :- :int stage-number :- :int
column :- lib.metadata/ColumnMetadata] dimensions :- [:sequential ::lib.schema.drill-thru/context.row.value]]
(let [breakouts (lib.breakout/breakouts query stage-number)] (first (for [breakout (lib.breakout/breakouts query stage-number)
(m/find-first (fn [breakout] :when (and (lib.util/clause-of-type? breakout :field)
(and (is-ref-for-source-column? breakout column) (lib.temporal-bucket/temporal-bucket breakout))
(= (lib.temporal-bucket/temporal-bucket breakout) {:keys [column] :as dimension} dimensions
(lib.temporal-bucket/temporal-bucket column)))) :when (and (lib.equality/find-matching-column breakout [column])
breakouts))) (= (lib.temporal-bucket/temporal-bucket breakout)
(lib.temporal-bucket/temporal-bucket column)))]
(assoc dimension :column-ref breakout))))
(mu/defn ^:private next-breakout-unit :- [:maybe ::lib.schema.temporal-bucketing/unit.date-time.truncate] (mu/defn ^:private next-breakout-unit :- [:maybe ::lib.schema.temporal-bucketing/unit.date-time.truncate]
[column :- lib.metadata/ColumnMetadata] [column :- lib.metadata/ColumnMetadata]
...@@ -69,27 +60,28 @@ ...@@ -69,27 +60,28 @@
This is different from the `:drill-thru/zoom` type, which is for showing the details of a single object." This is different from the `:drill-thru/zoom` type, which is for showing the details of a single object."
;; TODO: This naming is confusing. Fix it? ;; TODO: This naming is confusing. Fix it?
[query :- ::lib.schema/query [query :- ::lib.schema/query
stage-number :- :int stage-number :- :int
{:keys [column value]} :- ::lib.schema.drill-thru/context] {:keys [column dimensions value]} :- ::lib.schema.drill-thru/context]
(when (and (lib.drill-thru.common/mbql-stage? query stage-number) (when (and (lib.drill-thru.common/mbql-stage? query stage-number)
column column
(some? value) (not-empty dimensions)
(matching-breakout-ref query stage-number column)) (some? value))
(when-let [next-unit (next-breakout-unit column)] (when-let [dimension (matching-breakout-dimension query stage-number dimensions)]
{:lib/type :metabase.lib.drill-thru/drill-thru (when-let [next-unit (next-breakout-unit (:column dimension))]
:display-name (describe-next-unit next-unit) {:lib/type :metabase.lib.drill-thru/drill-thru
:type :drill-thru/zoom-in.timeseries :display-name (describe-next-unit next-unit)
:column column :type :drill-thru/zoom-in.timeseries
:value value :dimension dimension
:next-unit next-unit}))) :next-unit next-unit}))))
(mu/defmethod lib.drill-thru.common/drill-thru-method :drill-thru/zoom-in.timeseries (mu/defmethod lib.drill-thru.common/drill-thru-method :drill-thru/zoom-in.timeseries
[query :- ::lib.schema/query [query :- ::lib.schema/query
stage-number :- :int stage-number :- :int
{:keys [column value next-unit]} :- ::lib.schema.drill-thru/drill-thru.zoom-in.timeseries] {:keys [dimension next-unit]} :- ::lib.schema.drill-thru/drill-thru.zoom-in.timeseries]
(let [breakout (matching-breakout-ref query stage-number column) (let [{:keys [column value]} dimension
new-breakout (lib.temporal-bucket/with-temporal-bucket breakout next-unit)] old-breakout (:column-ref dimension)
new-breakout (lib.temporal-bucket/with-temporal-bucket old-breakout next-unit)]
(-> query (-> query
(lib.filter/filter stage-number (lib.filter/= column value)) (lib.filter/filter stage-number (lib.filter/= column value))
(lib.remove-replace/replace-clause stage-number breakout new-breakout)))) (lib.remove-replace/replace-clause stage-number old-breakout new-breakout))))
...@@ -23,7 +23,6 @@ ...@@ -23,7 +23,6 @@
[metabase.lib.stage :as lib.stage] [metabase.lib.stage :as lib.stage]
[metabase.lib.util :as lib.util] [metabase.lib.util :as lib.util]
[metabase.mbql.js :as mbql.js] [metabase.mbql.js :as mbql.js]
[metabase.mbql.normalize :as mbql.normalize]
[metabase.shared.util.time :as shared.ut] [metabase.shared.util.time :as shared.ut]
[metabase.util :as u] [metabase.util :as u]
[metabase.util.log :as log])) [metabase.util.log :as log]))
...@@ -69,15 +68,6 @@ ...@@ -69,15 +68,6 @@
[query] [query]
(lib.core/suggested-name query)) (lib.core/suggested-name query))
(defn- pMBQL [query-map]
(as-> query-map <>
(js->clj <> :keywordize-keys true)
(if (:type <>)
<>
(assoc <> :type :query))
(mbql.normalize/normalize <>)
(lib.convert/->pMBQL <>)))
(defn ^:export metadataProvider (defn ^:export metadataProvider
"Convert metadata to a metadata provider if it is not one already." "Convert metadata to a metadata provider if it is not one already."
[database-id metadata] [database-id metadata]
...@@ -88,7 +78,7 @@ ...@@ -88,7 +78,7 @@
(defn ^:export query (defn ^:export query
"Coerce a plain map `query` to an actual query object that you can use with MLv2." "Coerce a plain map `query` to an actual query object that you can use with MLv2."
[database-id metadata query-map] [database-id metadata query-map]
(let [query-map (pMBQL query-map)] (let [query-map (lib.convert/js-legacy-query->pMBQL query-map)]
(log/debugf "query map: %s" (pr-str query-map)) (log/debugf "query map: %s" (pr-str query-map))
(lib.core/query (metadataProvider database-id metadata) query-map))) (lib.core/query (metadataProvider database-id metadata) query-map)))
...@@ -853,16 +843,31 @@ ...@@ -853,16 +843,31 @@
[a-query stage-number join-condition bucketing-option] [a-query stage-number join-condition bucketing-option]
(lib.core/join-condition-update-temporal-bucketing a-query stage-number join-condition bucketing-option)) (lib.core/join-condition-update-temporal-bucketing a-query stage-number join-condition bucketing-option))
(defn- fix-column-with-ref [a-ref column]
(cond-> column
;; Sometimes the FE has result metadata from the QP, without the required :lib/source-uuid on it.
;; We have the UUID for the aggregation in its ref, so use that here.
(some-> a-ref first (= :aggregation)) (assoc :lib/source-uuid (last a-ref))))
(defn- js-cells-by (defn- js-cells-by
"Given a `col-fn`, returns a function that will extract a JS object like "Given a `col-fn`, returns a function that will extract a JS object like
`{col: {name: \"ID\", ...}, value: 12}` into a CLJS map like `{:column-name \"ID\", :value 12}`. `{col: {name: \"ID\", ...}, value: 12}` into a CLJS map like
```
{:column {:lib/type :metadata/column ...}
:column-ref [:field ...]
:value 12}
```
The spelling of the column key differs between multiple JS objects of this same general shape The spelling of the column key differs between multiple JS objects of this same general shape
(`col` on data rows, `column` on dimensions), etc., hence the abstraction." (`col` on data rows, `column` on dimensions), etc., hence the abstraction."
[col-fn] [col-fn]
(fn [^js cell] (fn [^js cell]
{:column-name (.-name (col-fn cell)) (let [column (js.metadata/parse-column (col-fn cell))
:value (.-value cell)})) column-ref (when-let [a-ref (:field-ref column)]
(legacy-ref->pMBQL a-ref))]
{:column (fix-column-with-ref column-ref column)
:column-ref column-ref
:value (.-value cell)})))
(def ^:private row-cell (js-cells-by #(.-col ^js %))) (def ^:private row-cell (js-cells-by #(.-col ^js %)))
(def ^:private dimension-cell (js-cells-by #(.-column ^js %))) (def ^:private dimension-cell (js-cells-by #(.-column ^js %)))
...@@ -874,15 +879,19 @@ ...@@ -874,15 +879,19 @@
- Nullable data row (the array of `{col, value}` pairs from `clicked.data`) - Nullable data row (the array of `{col, value}` pairs from `clicked.data`)
- Nullable dimensions list (`{column, value}` pairs from `clicked.dimensions`)" - Nullable dimensions list (`{column, value}` pairs from `clicked.dimensions`)"
[a-query stage-number column value row dimensions] [a-query stage-number column value row dimensions]
(->> (merge {:column (js.metadata/parse-column column) (lib.convert/with-aggregation-list (lib.core/aggregations a-query stage-number)
:value (cond (let [column-ref (when-let [a-ref (.-field_ref ^js column)]
(undefined? value) nil ; Missing a value, ie. a column click (legacy-ref->pMBQL a-ref))]
(nil? value) :null ; Provided value is null, ie. database NULL (->> (merge {:column (fix-column-with-ref column-ref (js.metadata/parse-column column))
:else value)} :column-ref column-ref
(when row {:row (mapv row-cell row)}) :value (cond
(when (not-empty dimensions) {:dimensions (mapv dimension-cell dimensions)})) (undefined? value) nil ; Missing a value, ie. a column click
(lib.core/available-drill-thrus a-query stage-number) (nil? value) :null ; Provided value is null, ie. database NULL
to-array)) :else value)}
(when row {:row (mapv row-cell row)})
(when (not-empty dimensions) {:dimensions (mapv dimension-cell dimensions)}))
(lib.core/available-drill-thrus a-query stage-number)
to-array))))
(defn ^:export drill-thru (defn ^:export drill-thru
"Applies the given `drill-thru` to the specified query and stage. Returns the updated query. "Applies the given `drill-thru` to the specified query and stage. Returns the updated query.
......
...@@ -211,7 +211,8 @@ ...@@ -211,7 +211,8 @@
(defmethod rename-key-fn :field (defmethod rename-key-fn :field
[_object-type] [_object-type]
{:source :lib/source}) {:source :lib/source
:unit :metabase.lib.field/temporal-unit})
(defn- parse-field-id (defn- parse-field-id
[id] [id]
...@@ -224,19 +225,20 @@ ...@@ -224,19 +225,20 @@
[_object-type] [_object-type]
(fn [k v] (fn [k v]
(case k (case k
:base-type (keyword v) :base-type (keyword v)
:coercion-strategy (keyword v) :coercion-strategy (keyword v)
:effective-type (keyword v) :effective-type (keyword v)
:fingerprint (if (map? v) :fingerprint (if (map? v)
(walk/keywordize-keys v) (walk/keywordize-keys v)
(js->clj v :keywordize-keys true)) (js->clj v :keywordize-keys true))
:has-field-values (keyword v) :has-field-values (keyword v)
:lib/source (if (= v "aggregation") :lib/source (if (= v "aggregation")
:source/aggregations :source/aggregations
(keyword "source" v)) (keyword "source" v))
:semantic-type (keyword v) :metabase.lib.field/temporal-unit (keyword v)
:visibility-type (keyword v) :semantic-type (keyword v)
:id (parse-field-id v) :visibility-type (keyword v)
:id (parse-field-id v)
v))) v)))
(defmethod parse-objects :field (defmethod parse-objects :field
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
[metabase.lib.schema.id :as lib.schema.id] [metabase.lib.schema.id :as lib.schema.id]
[metabase.lib.schema.metadata :as lib.schema.metadata] [metabase.lib.schema.metadata :as lib.schema.metadata]
[metabase.lib.schema.order-by :as lib.schema.order-by] [metabase.lib.schema.order-by :as lib.schema.order-by]
[metabase.lib.schema.ref :as lib.schema.ref]
[metabase.lib.schema.temporal-bucketing [metabase.lib.schema.temporal-bucketing
:as lib.schema.temporal-bucketing] :as lib.schema.temporal-bucketing]
[metabase.util.malli.registry :as mr])) [metabase.util.malli.registry :as mr]))
...@@ -157,8 +158,7 @@ ...@@ -157,8 +158,7 @@
::drill-thru.common ::drill-thru.common
[:map [:map
[:type [:= :drill-thru/zoom-in.timeseries]] [:type [:= :drill-thru/zoom-in.timeseries]]
[:column [:ref ::lib.schema.metadata/column]] [:dimension [:ref ::context.row.value]]
[:value some?]
[:next-unit [:ref ::drill-thru.zoom-in.timeseries.next-unit]]]]) [:next-unit [:ref ::drill-thru.zoom-in.timeseries.next-unit]]]])
(mr/def ::drill-thru (mr/def ::drill-thru
...@@ -182,34 +182,19 @@ ...@@ -182,34 +182,19 @@
[:drill-thru/automatic-insights ::drill-thru.automatic-insights] [:drill-thru/automatic-insights ::drill-thru.automatic-insights]
[:drill-thru/zoom-in.timeseries ::drill-thru.zoom-in.timeseries]]]) [: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 (mr/def ::context.row.value
[:map [:map
[:column-name string?] [:column [:ref ::lib.schema.metadata/column]]
[:value :any]]) [:column-ref [:ref ::lib.schema.ref/ref]]
[:value :any]])
(mr/def ::context.row (mr/def ::context.row
[:sequential [:ref ::context.row.value]]) [:sequential [:ref ::context.row.value]])
(mr/def ::context (mr/def ::context
[:map [:map
[:column [:ref ::lib.schema.metadata/column]] [:column [:ref ::lib.schema.metadata/column]]
[:value [:maybe :any]] [:column-ref [:ref ::lib.schema.ref/ref]]
[:value [:maybe :any]]
[:row {:optional true} [:ref ::context.row]] [:row {:optional true} [:ref ::context.row]]
[:dimensions {:optional true} [:maybe [:ref ::context.row]]]]) [:dimensions {:optional true} [:maybe [:ref ::context.row]]]])
...@@ -18,8 +18,9 @@ ...@@ -18,8 +18,9 @@
count-col (m/find-first (fn [col] count-col (m/find-first (fn [col]
(= (:display-name col) "Count")) (= (:display-name col) "Count"))
(lib/returned-columns query)) (lib/returned-columns query))
context {:column count-col context {:column count-col
:value nil}] :column-ref (lib/ref count-col)
:value nil}]
(is (some? count-col)) (is (some? count-col))
(is (nil? (lib.drill-thru.distribution/distribution-drill query -1 context)))))) (is (nil? (lib.drill-thru.distribution/distribution-drill query -1 context))))))
......
...@@ -16,8 +16,9 @@ ...@@ -16,8 +16,9 @@
(let [query (lib/query meta/metadata-provider (meta/table-metadata :orders)) (let [query (lib/query meta/metadata-provider (meta/table-metadata :orders))
drill (lib.drill-thru.sort/sort-drill query drill (lib.drill-thru.sort/sort-drill query
-1 -1
{:column (meta/field-metadata :orders :id) {:column (meta/field-metadata :orders :id)
:value nil})] :column-ref (lib/ref (meta/field-metadata :orders :id))
:value nil})]
(is (=? {:type :drill-thru/sort (is (=? {:type :drill-thru/sort
:column {:id (meta/id :orders :id)} :column {:id (meta/id :orders :id)}
:sort-directions [:asc :desc]} :sort-directions [:asc :desc]}
...@@ -55,8 +56,9 @@ ...@@ -55,8 +56,9 @@
count-col (m/find-first (fn [col] count-col (m/find-first (fn [col]
(= (:display-name col) "Count")) (= (:display-name col) "Count"))
(lib/returned-columns query)) (lib/returned-columns query))
context {:column count-col context {:column count-col
:value nil}] :column-ref (lib/ref count-col)
:value nil}]
(is (some? count-col)) (is (some? count-col))
(let [drill (lib.drill-thru.sort/sort-drill query -1 context)] (let [drill (lib.drill-thru.sort/sort-drill query -1 context)]
(is (=? {:lib/type :metabase.lib.drill-thru/drill-thru (is (=? {:lib/type :metabase.lib.drill-thru/drill-thru
...@@ -79,8 +81,10 @@ ...@@ -79,8 +81,10 @@
(let [query (-> (lib/query meta/metadata-provider (meta/table-metadata :orders)) (let [query (-> (lib/query meta/metadata-provider (meta/table-metadata :orders))
(lib/order-by (meta/field-metadata :orders :user-id)) (lib/order-by (meta/field-metadata :orders :user-id))
(lib/order-by (meta/field-metadata :orders :id))) (lib/order-by (meta/field-metadata :orders :id)))
context {:column (meta/field-metadata :orders :user-id) user-id (meta/field-metadata :orders :user-id)
:value nil} context {:column user-id
:column-ref (lib/ref user-id)
:value nil}
drill (lib.drill-thru.sort/sort-drill query -1 context)] drill (lib.drill-thru.sort/sort-drill query -1 context)]
(is (=? {:stages (is (=? {:stages
[{:order-by [[:asc {} [:field {} (meta/id :orders :user-id)]] [{:order-by [[:asc {} [:field {} (meta/id :orders :user-id)]]
......
...@@ -19,8 +19,9 @@ ...@@ -19,8 +19,9 @@
count-col (m/find-first (fn [col] count-col (m/find-first (fn [col]
(= (:display-name col) "Count")) (= (:display-name col) "Count"))
(lib/returned-columns query)) (lib/returned-columns query))
context {:column count-col context {:column count-col
:value nil}] :column-ref (lib/ref count-col)
:value nil}]
(is (some? count-col)) (is (some? count-col))
(is (nil? (lib.drill-thru.summarize-column-by-time/summarize-column-by-time-drill query -1 context)))))) (is (nil? (lib.drill-thru.summarize-column-by-time/summarize-column-by-time-drill query -1 context))))))
......
...@@ -85,9 +85,9 @@ ...@@ -85,9 +85,9 @@
row :- Row row :- Row
{:keys [column-name click-type query-type], :as _test-case} :- TestCase] {:keys [column-name click-type query-type], :as _test-case} :- TestCase]
(let [cols (lib/returned-columns query -1 (lib.util/query-stage query -1)) (let [cols (lib/returned-columns query -1 (lib.util/query-stage query -1))
col (m/find-first (fn [col] by-name (m/index-by :name cols)
(= (:name col) column-name)) col (get by-name column-name)
cols) refs (update-vals by-name lib/ref)
_ (assert col (lib.util/format "No column found named %s; found: %s" _ (assert col (lib.util/format "No column found named %s; found: %s"
(pr-str column-name) (pr-str column-name)
(pr-str (map :name cols)))) (pr-str (map :name cols))))
...@@ -98,14 +98,20 @@ ...@@ -98,14 +98,20 @@
(for [col cols (for [col cols
:when (and (= (:lib/source col) :source/breakouts) :when (and (= (:lib/source col) :source/breakouts)
(not= (:name col) column-name))] (not= (:name col) column-name))]
{:column-name (:name col), :value (get row (:name col))}))] {:column col
:column-ref (get refs (:name col))
:value (get row (:name col))}))]
(merge (merge
{:column col {:column col
:value nil} :column-ref (get refs column-name)
:value nil}
(when (= click-type :cell) (when (= click-type :cell)
{:value value {:value value
:row (for [[column-name value] row] :row (for [[column-name value] row
{:column-name column-name, :value value}) :let [column (by-name column-name)]]
{:column column
:column-ref (get refs column-name)
:value value})
:dimensions dimensions})))) :dimensions dimensions}))))
(def ^:private AvailableDrillsTestCase (def ^:private AvailableDrillsTestCase
......
...@@ -20,18 +20,26 @@ ...@@ -20,18 +20,26 @@
:breakout [[:field {:temporal-unit :day} (meta/id :orders :created-at)] :breakout [[:field {:temporal-unit :day} (meta/id :orders :created-at)]
[:field {:temporal-unit :year} (meta/id :orders :created-at)]]}]} [:field {:temporal-unit :year} (meta/id :orders :created-at)]]}]}
query)) query))
(let [created-at (m/find-first #(and (= (:id %) (meta/id :orders :created-at)) (let [columns (lib/returned-columns query)
created-at (m/find-first #(and (= (:id %) (meta/id :orders :created-at))
(= (lib.temporal-bucket/raw-temporal-bucket %) :year)) (= (lib.temporal-bucket/raw-temporal-bucket %) :year))
(lib/returned-columns query)) (lib/returned-columns query))
_ (assert created-at) _ (assert created-at)
drill (lib.drill-thru.zoom-in-timeseries/zoom-in-timeseries-drill query count-col (m/find-first #(= (:name %) "count") columns)
-1 _ (assert count-col)
{:column created-at drill (lib.drill-thru.zoom-in-timeseries/zoom-in-timeseries-drill
:value 2022})] query -1
{:column count-col
:column-ref (lib/ref count-col)
:value 200
:dimensions [{:column created-at
:column-ref (lib/ref created-at)
:value 2022}]})]
(is (=? {:type :drill-thru/zoom-in.timeseries (is (=? {:type :drill-thru/zoom-in.timeseries
:column {:id (meta/id :orders :created-at) :dimension {:column {:id (meta/id :orders :created-at)
:metabase.lib.field/temporal-unit :year} :metabase.lib.field/temporal-unit :year}
:value 2022 :column-ref [:field {} (meta/id :orders :created-at)]
:value 2022}
:next-unit :quarter :next-unit :quarter
:display-name "See this year by quarter"} :display-name "See this year by quarter"}
drill)) drill))
...@@ -46,18 +54,24 @@ ...@@ -46,18 +54,24 @@
[:field {:temporal-unit :year} (meta/id :orders :created-at)] [:field {:temporal-unit :year} (meta/id :orders :created-at)]
2022]]}]} 2022]]}]}
query')) query'))
(let [created-at (m/find-first #(and (= (:id %) (meta/id :orders :created-at)) (let [columns (lib/returned-columns query')
created-at (m/find-first #(and (= (:id %) (meta/id :orders :created-at))
(= (lib.temporal-bucket/raw-temporal-bucket %) :quarter)) (= (lib.temporal-bucket/raw-temporal-bucket %) :quarter))
(lib/returned-columns query')) columns)
_ (assert created-at) _ (assert created-at)
drill (lib.drill-thru.zoom-in-timeseries/zoom-in-timeseries-drill query' drill (lib.drill-thru.zoom-in-timeseries/zoom-in-timeseries-drill
-1 query' -1
{:column created-at {:column count-col
:value "2022-04-01T00:00:00"})] :column-ref (lib/ref count-col)
:value 19
:dimensions [{:column created-at
:column-ref (lib/ref created-at)
:value "2022-04-01T00:00:00"}]})]
(is (=? {:type :drill-thru/zoom-in.timeseries (is (=? {:type :drill-thru/zoom-in.timeseries
:column {:id (meta/id :orders :created-at) :dimension {:column {:id (meta/id :orders :created-at)
:metabase.lib.field/temporal-unit :quarter} :metabase.lib.field/temporal-unit :quarter}
:value "2022-04-01T00:00:00" :column-ref [:field {} (meta/id :orders :created-at)]
:value "2022-04-01T00:00:00"}
:next-unit :month :next-unit :month
:display-name "See this quarter by month"} :display-name "See this quarter by month"}
drill)) drill))
......
This diff is collapsed.
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment