diff --git a/src/metabase/lib/binning.cljc b/src/metabase/lib/binning.cljc
index 0f42e88235e7bb45f08d6f9cbfa5b005714f12e6..241bb7b4fad1834c33badb7335fdb3f8b7f751f8 100644
--- a/src/metabase/lib/binning.cljc
+++ b/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]
diff --git a/src/metabase/lib/drill_thru/zoom_in_bins.cljc b/src/metabase/lib/drill_thru/zoom_in_bins.cljc
index 140776767b0556515ff6dfbdd9cbd5c078cba13c..c99d31667a83cc30354b04c700e73f54a0d8de6a 100644
--- a/src/metabase/lib/drill_thru/zoom_in_bins.cljc
+++ b/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))))
diff --git a/test/metabase/lib/drill_thru/zoom_in_bins_test.cljc b/test/metabase/lib/drill_thru/zoom_in_bins_test.cljc
index ed15742854698158033d55154e11445a5048edbc..0e7ab7e672dfa2d748c0e04dfd64de3457f4a9e7 100644
--- a/test/metabase/lib/drill_thru/zoom_in_bins_test.cljc
+++ b/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]]}]}}))))