diff --git a/src/metabase/lib/drill_thru/column_extract.cljc b/src/metabase/lib/drill_thru/column_extract.cljc index 0cdf5a6c964f0e936b8744f3b4033f8bbf99e767..3f517d34beeb6c24e472186c64ee1404dcaa7749 100644 --- a/src/metabase/lib/drill_thru/column_extract.cljc +++ b/src/metabase/lib/drill_thru/column_extract.cljc @@ -45,11 +45,12 @@ (nil? value) (lib.drill-thru.common/mbql-stage? query stage-number)) (when-let [drill (column-extract-drill-for-column query column)] - (merge drill - {:lib/type :metabase.lib.drill-thru/drill-thru - :type :drill-thru/column-extract} - (lib.drill-thru.column-filter/prepare-query-for-drill-addition - query stage-number column column-ref :expression))))) + (when-let [drill-details (lib.drill-thru.column-filter/prepare-query-for-drill-addition + query stage-number column column-ref :expression)] + (merge drill + drill-details + {:lib/type :metabase.lib.drill-thru/drill-thru + :type :drill-thru/column-extract}))))) (mu/defn extractions-for-drill :- [:sequential ::lib.schema.extraction/extraction] "Returns the extractions from a given drill." diff --git a/src/metabase/lib/drill_thru/column_filter.cljc b/src/metabase/lib/drill_thru/column_filter.cljc index af775246b36ee87fbbac79cfadddfe58ae3a4d62..13e4704b371631f62cdeea6bb7e7cfd864122259 100644 --- a/src/metabase/lib/drill_thru/column_filter.cljc +++ b/src/metabase/lib/drill_thru/column_filter.cljc @@ -39,10 +39,10 @@ [metabase.lib.util :as lib.util] [metabase.util.malli :as mu])) -(mu/defn prepare-query-for-drill-addition :- [:map - [:query ::lib.schema/query] - [:stage-number :int] - [:column lib.filter/ColumnWithOperators]] +(mu/defn prepare-query-for-drill-addition :- [:maybe [:map + [:query ::lib.schema/query] + [:stage-number :int] + [:column lib.filter/ColumnWithOperators]]] "If the column we're filtering on is an aggregation, the filtering must happen in a later stage. This function returns a map with that possibly-updated `:query` and `:stage-number`, plus the `:column` for filtering in that stage (with filter operators, as returned by [[lib.filter/filterable-columns]]). @@ -85,7 +85,9 @@ (and (:lib/source-uuid column) (m/find-first #(= (:lib/source-uuid %) (:lib/source-uuid column)) columns)))] - (assoc base :column filter-column))) + ;; If we cannot find the matching column, don't allow to drill + (when filter-column + (assoc base :column filter-column)))) (mu/defn column-filter-drill :- [:maybe ::lib.schema.drill-thru/drill-thru.column-filter] "Filtering at the column level, based on its type. Displays a submenu of eg. \"Today\", \"This Week\", etc. for date @@ -96,22 +98,21 @@ [query :- ::lib.schema/query stage-number :- :int {:keys [column column-ref value]} :- ::lib.schema.drill-thru/context] - ;; Note: original code uses an addition `clicked.column.field_ref != null` condition. (when (and (lib.drill-thru.common/mbql-stage? query stage-number) column (nil? value) (not (lib.types.isa/structured? column))) - (let [initial-op (when-not (lib.types.isa/temporal? column) ; Date fields have special handling in the FE. - (-> (lib.filter.operator/filter-operators column) - first - (assoc :lib/type :operator/filter)))] - - (merge - {:lib/type :metabase.lib.drill-thru/drill-thru - :type :drill-thru/column-filter - :initial-op initial-op} - ;; When the column we would be filtering on is an aggregation, it can't be filtered without adding a stage. - (prepare-query-for-drill-addition query stage-number column column-ref :filter))))) + ;; When the column we would be filtering on is an aggregation, it can't be filtered without adding a stage. + (when-let [drill-details (prepare-query-for-drill-addition query stage-number column column-ref :filter)] + (let [initial-op (when-not (lib.types.isa/temporal? column) ; Date fields have special handling in the FE. + (-> (lib.filter.operator/filter-operators column) + first + (assoc :lib/type :operator/filter)))] + (merge + drill-details + {:lib/type :metabase.lib.drill-thru/drill-thru + :type :drill-thru/column-filter + :initial-op initial-op}))))) (defmethod lib.drill-thru.common/drill-thru-info-method :drill-thru/column-filter [_query _stage-number {:keys [initial-op]}] diff --git a/src/metabase/lib/drill_thru/quick_filter.cljc b/src/metabase/lib/drill_thru/quick_filter.cljc index 2ddb662c782f44edd0ec1392fe58102cfd23f979..b954d0e217bf965e708145e6d086960606d8b65b 100644 --- a/src/metabase/lib/drill_thru/quick_filter.cljc +++ b/src/metabase/lib/drill_thru/quick_filter.cljc @@ -120,13 +120,13 @@ (not (lib.types.isa/foreign-key? column))) ;; For aggregate columns, we want to introduce a new stage when applying the drill-thru. ;; [[lib.drill-thru.column-filter/prepare-query-for-drill-addition]] handles this. (#34346) - (let [adjusted (lib.drill-thru.column-filter/prepare-query-for-drill-addition - query stage-number column column-ref :filter)] - (merge {:lib/type :metabase.lib.drill-thru/drill-thru + (when-let [drill-details (lib.drill-thru.column-filter/prepare-query-for-drill-addition + query stage-number column column-ref :filter)] + (merge drill-details + {:lib/type :metabase.lib.drill-thru/drill-thru :type :drill-thru/quick-filter - :operators (operators-for (:column adjusted) value) - :value value} - adjusted)))) + :operators (operators-for (:column drill-details) value) + :value value})))) (defmethod lib.drill-thru.common/drill-thru-info-method :drill-thru/quick-filter [_query _stage-number drill-thru] diff --git a/test/metabase/lib/drill_thru/column_filter_test.cljc b/test/metabase/lib/drill_thru/column_filter_test.cljc index 17644cd276739176dcb0e04f20adbddfc18c3526..b94a3af4cc41d5da083995a3768bea67f4ec1b46 100644 --- a/test/metabase/lib/drill_thru/column_filter_test.cljc +++ b/test/metabase/lib/drill_thru/column_filter_test.cljc @@ -231,6 +231,17 @@ (lib/available-drill-thrus query -1) (m/find-first #(= (:type %) :drill-thru/column-filter)))))))) +(deftest ^:parallel column-filter-unavailable-for-broken-ref-test + (testing "do not return column filter drill when the corresponding filterable column cannot be found" + (let [query (lib/query meta/metadata-provider (meta/table-metadata :orders)) + subtotal (m/find-first #(= (:name %) "SUBTOTAL") (lib/returned-columns query))] + (is (nil? + (->> {:column subtotal + :column-ref [:field {:lib/uuid (str (random-uuid))} 999] + :value nil} + (lib/available-drill-thrus query -1) + (m/find-first #(= (:type %) :drill-thru/column-filter)))))))) + (deftest ^:parallel native-models-with-renamed-columns-test (testing "Generate sane queries for native query models with renamed columns (#22715 #36583)" (let [metadata-provider (lib.tu/mock-metadata-provider