Expect broken field refs in filter drills (#46691)

* Handle column matching errors

* Fix test

* Fix test

* Fix test

* Fix test

* Fix test

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."

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]}]

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]

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