[MLv2] Handle nested "Zoom In" drills on binned columns (#40965)

The first time you zoom in on a bin in a binned query, it works fine.

For example, Orders COUNT by SUBTOTAL auto-binned has a bin for 40-60.
If you zoom in on that, it correctly shows the range 40 to 60 broken
down into smaller bins which are 2.5 units wide. If you then try to
zoom in on one of these smaller bins, say 50-52.5, there are two
problems:

1. The filters from the first zoom (`>= 40` and `< 60`) are still
   there, and so are the new ones (`>= 50` and `< 52.5`).
2. The new filter is actually wrong; it uses the big width (20)
   from the original bins, not the width of the nested bins (2.5),
   so the new filters are `>= 50` and `< 70`.

This PR fixes both issues, so nested bins (1) remove the old filters,
and (2) use the correct width for the nested bins.

src/metabase/lib/binning.cljc

@@ -147,15 +147,25 @@
   [metadata-providerable :- ::lib.schema.metadata/metadata-providerable
    column-metadata       :- ::lib.schema.metadata/column
    value                 :- number?]
+  ;; TODO: I think this function is taking the wrong approach. It uses the (global) :fingerprint for all cases, and if
+  ;; we're looking at nested bins (eg. bin a query, then zoom in on one of those bins) we have tighter min and max
+  ;; bounds on the column's own `binning-options`. We should be using those bounds everywhere if they exist, and falling
+  ;; back on the fingerprint only if they're not defined.
   (when-let [binning-options (binning column-metadata)]
     (case (:strategy binning-options)
       :num-bins
-      (when-let [{min-value :min, max-value :max, :as _number-fingerprint} (get-in column-metadata [:fingerprint :type :type/Number])]
-        (let [{:keys [num-bins]} binning-options
-              bin-width          (lib.binning.util/nicer-bin-width min-value max-value num-bins)]
-          {:bin-width bin-width
-           :min-value value
-           :max-value (+ value bin-width)}))
+      (or ;; If the column is already binned, compute the width of this single bin based on its bounds and width.
+          (when-let [bin-width (:bin-width binning-options)]
+            {:bin-width bin-width
+             :min-value value
+             :max-value (+ value bin-width)})
+          ;; Otherwise use the fingerprint.
+          (when-let [{min-value :min, max-value :max, :as _number-fingerprint} (get-in column-metadata [:fingerprint :type :type/Number])]
+            (let [{:keys [num-bins]} binning-options
+                  bin-width          (lib.binning.util/nicer-bin-width min-value max-value num-bins)]
+              {:bin-width bin-width
+               :min-value value
+               :max-value (+ value bin-width)})))
 
       :bin-width
       (let [{:keys [bin-width]} binning-options]

src/metabase/lib/drill_thru/zoom_in_bins.cljc

@@ -19,8 +19,9 @@
   - Remove breakouts for `dimensions`. Please note that with regular cells and pivot cells it would mean removing all
     breakouts; but with legend item clicks it would remove the breakout for the legend item column only.
 
-  - Add a filter based on columns and values from `dimensions`. Take temporal units and binning strategies into
-    account
+  - Remove any existing filters for this column.
+
+  - Add new filters limiting this column to the range defined by the clicked bin.
     https://github.com/metabase/metabase/blob/0624d8d0933f577cc70c03948f4b57f73fe13ada/frontend/src/metabase-lib/queries/utils/actions.js#L99
 
   - Add a breakout based on the numeric column (from requirements). For location columns, use the binning strategy
@@ -71,6 +72,7 @@
    [metabase.lib.binning :as lib.binning]
    [metabase.lib.breakout :as lib.breakout]
    [metabase.lib.drill-thru.common :as lib.drill-thru.common]
+   [metabase.lib.equality :as lib.equality]
    [metabase.lib.filter :as lib.filter]
    [metabase.lib.remove-replace :as lib.remove-replace]
    [metabase.lib.schema :as lib.schema]
@@ -128,7 +130,11 @@
   [query                                        :- ::lib.schema/query
    stage-number                                 :- :int
    {:keys [column min-value max-value new-binning]} :- ::lib.schema.drill-thru/drill-thru.zoom-in.binning]
-  (-> query
-      (lib.filter/filter stage-number (lib.filter/>= column min-value))
-      (lib.filter/filter stage-number (lib.filter/< column max-value))
-      (update-breakout stage-number column new-binning)))
+  (let [old-filters (filter (fn [[operator _opts filter-column]]
+                              (and (#{:>= :<} operator)
+                                   (lib.equality/find-matching-column filter-column [column])))
+                            (lib.filter/filters query stage-number))]
+    (-> (reduce lib.remove-replace/remove-clause query old-filters)
+        (lib.filter/filter stage-number (lib.filter/>= column min-value))
+        (lib.filter/filter stage-number (lib.filter/< column max-value))
+        (update-breakout stage-number column new-binning))))

test/metabase/lib/drill_thru/zoom_in_bins_test.cljc

@@ -255,3 +255,41 @@
                         :dimensions [discount-dim]}
           drills       (set (map :type (lib/available-drill-thrus query context)))]
       (is (not (drills :drill-thru/zoom-in.binning))))))
+
+(deftest ^:parallel nested-zoom-test
+  (testing "repeatedly zooming in on smaller bins should work"
+    (let [query (as-> (lib/query meta/metadata-provider (meta/table-metadata :orders)) $q
+                  ;; Filtering like we'd already zoomed in once, on the 40-60 bin.
+                  (lib/filter $q (lib/>= (meta/field-metadata :orders :subtotal) 40))
+                  (lib/filter $q (lib/<  (meta/field-metadata :orders :subtotal) 60))
+                  (lib/aggregate $q (lib/count))
+                  (lib/breakout $q (lib/with-binning (meta/field-metadata :orders :subtotal)
+                                     {:strategy  :num-bins
+                                      :num-bins  8
+                                      :bin-width 2.5
+                                      :min-value 40
+                                      :max-value 60})))]
+      (lib.drill-thru.tu/test-drill-application
+        {:click-type     :cell
+         :query-type     :aggregated
+         :custom-query   query
+         :custom-row     {"count" 100
+                          "SUBTOTAL" 50} ;; Clicking the 50-52.5 bin
+         :column-name    "count"
+         :drill-type     :drill-thru/zoom-in.binning
+         :expected       {:type        :drill-thru/zoom-in.binning
+                          :column      {:name "SUBTOTAL"}
+                          :min-value   50
+                          :max-value   52.5
+                          :new-binning {:strategy :default}}
+         :expected-query {:stages [{:source-table (meta/id :orders)
+                                    :aggregation  [[:count {}]]
+                                    :breakout     [[:field
+                                                    {:binning {:strategy :default}}
+                                                    (meta/id :orders :subtotal)]]
+                                    :filters      [[:>= {}
+                                                    [:field {} (meta/id :orders :subtotal)]
+                                                    50]
+                                                   [:< {}
+                                                    [:field {} (meta/id :orders :subtotal)]
+                                                    52.5]]}]}}))))