diff --git a/src/metabase/lib/drill_thru.cljc b/src/metabase/lib/drill_thru.cljc index abcea1bc0b49830f4ffff4af612588d6ef23b85a..885dac08545e717df3c7de17c661ec03ffd5b536 100644 --- a/src/metabase/lib/drill_thru.cljc +++ b/src/metabase/lib/drill_thru.cljc @@ -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)` diff --git a/src/metabase/lib/equality.cljc b/src/metabase/lib/equality.cljc index f8f59a7b1eac87b0bf8872ce440c2f96363b3dfa..58dca3c585ad8b01978bab7130e813b4a53d42d1 100644 --- a/src/metabase/lib/equality.cljc +++ b/src/metabase/lib/equality.cljc @@ -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))) diff --git a/src/metabase/lib/field.cljc b/src/metabase/lib/field.cljc index abd94c21bd26fb2783aa809a9bd5d7a0aa42e423..f35b71a1aa7063d989082da571f04d22788a58f4 100644 --- a/src/metabase/lib/field.cljc +++ b/src/metabase/lib/field.cljc @@ -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) diff --git a/src/metabase/lib/schema/drill_thru.cljc b/src/metabase/lib/schema/drill_thru.cljc index 67965ea70ad4e7754ea297e180be87132b73290c..ce3594e7c3fff1b93a70bdbb1ce06e906271d24f 100644 --- a/src/metabase/lib/schema/drill_thru.cljc +++ b/src/metabase/lib/schema/drill_thru.cljc @@ -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]]]]) diff --git a/test/metabase/lib/drill_thru_test.cljc b/test/metabase/lib/drill_thru_test.cljc index f4eac9f4d763c33296541273a12c2dd6c721a8c3..f9821bf5e23433f920c09753128ec6bfade97661 100644 --- a/test/metabase/lib/drill_thru_test.cljc +++ b/test/metabase/lib/drill_thru_test.cljc @@ -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"