Misc `zoom-in-geographic` drill fixes (#36448)

frontend/src/metabase-lib/tests/drills-zoom-in-binning.unit.spec.ts

+import type { BinningMetadata } from "metabase-types/api";
 import {
   createOrdersQuantityDatasetColumn,
   createPeopleLatitudeDatasetColumn,
@@ -15,8 +16,7 @@ import {
   createNotEditableQuery,
 } from "./drills-common";
 
-// eslint-disable-next-line jest/no-disabled-tests
-describe.skip("drill-thru/zoom-in.binning (metabase#36177)", () => {
+describe("drill-thru/zoom-in.binning (metabase#36177)", () => {
   const drillType = "drill-thru/zoom-in.binning";
   const stageIndex = 0;
   const aggregationColumn = createCountDatasetColumn();
@@ -26,24 +26,50 @@ describe.skip("drill-thru/zoom-in.binning (metabase#36177)", () => {
       name: "numeric",
       tableName: "ORDERS",
       breakoutColumn: createOrdersQuantityDatasetColumn({ source: "breakout" }),
-      binningStrategies: ["Auto bin", "10 bins", "50 bins", "100 bins"],
+      binningStrategies: [
+        { name: "Auto bin", metadata: { binning_strategy: "default" } },
+        {
+          name: "10 bins",
+          metadata: { binning_strategy: "num-bins", num_bins: 10 },
+        },
+        {
+          name: "50 bins",
+          metadata: { binning_strategy: "num-bins", num_bins: 50 },
+        },
+        {
+          name: "100 bins",
+          metadata: { binning_strategy: "num-bins", num_bins: 100 },
+        },
+      ],
     },
     {
       name: "location",
       tableName: "PEOPLE",
       breakoutColumn: createPeopleLatitudeDatasetColumn({ source: "breakout" }),
       binningStrategies: [
-        "Auto bin",
-        "Bin every 0.1 degrees",
-        "Bin every 1 degree",
-        "Bin every 10 degrees",
-        "Bin every 20 degrees",
+        { name: "Auto bin", metadata: { binning_strategy: "default" } },
+        {
+          name: "Bin every 0.1 degrees",
+          metadata: { binning_strategy: "bin-width", bin_width: 0.1 },
+        },
+        {
+          name: "Bin every 1 degree",
+          metadata: { binning_strategy: "bin-width", bin_width: 1 },
+        },
+        {
+          name: "Bin every 10 degrees",
+          metadata: { binning_strategy: "bin-width", bin_width: 10 },
+        },
+        {
+          name: "Bin every 20 degrees",
+          metadata: { binning_strategy: "bin-width", bin_width: 20 },
+        },
       ],
     },
   ])("$name", ({ tableName, breakoutColumn, binningStrategies }) => {
     it.each(binningStrategies)(
       'should drill thru an aggregated cell with "%s" binning strategy',
-      binningStrategy => {
+      ({ name: binningStrategy, metadata: binningMetadata }) => {
         const query = createQueryWithClauses({
           aggregations: [{ operatorName: "count" }],
           breakouts: [
@@ -62,7 +88,10 @@ describe.skip("drill-thru/zoom-in.binning (metabase#36177)", () => {
           },
           breakouts: [
             {
-              column: breakoutColumn,
+              column: {
+                ...breakoutColumn,
+                binning_info: binningMetadata as BinningMetadata,
+              },
               value: 20,
             },
           ],
@@ -119,7 +148,8 @@ describe.skip("drill-thru/zoom-in.binning (metabase#36177)", () => {
       expect(drill).toBeNull();
     });
 
-    it("should not drill thru a non-editable query", () => {
+    // eslint-disable-next-line jest/no-disabled-tests
+    it.skip("should not drill thru a non-editable query (#36125)", () => {
       const query = createNotEditableQuery(
         createQueryWithClauses({
           aggregations: [{ operatorName: "count" }],
@@ -139,7 +169,10 @@ describe.skip("drill-thru/zoom-in.binning (metabase#36177)", () => {
         },
         breakouts: [
           {
-            column: breakoutColumn,
+            column: {
+              ...breakoutColumn,
+              binning_info: { binning_strategy: "default" },
+            },
             value: 20,
           },
         ],

frontend/src/metabase-lib/tests/drills-zoom-in-geographic.unit.spec.ts

@@ -27,8 +27,7 @@ import {
   createPeopleCountryField,
 } from "./drills-common";
 
-// eslint-disable-next-line jest/no-disabled-tests
-describe.skip("drill-thru/zoom-in.geographic (metabase#36247)", () => {
+describe("drill-thru/zoom-in.geographic (metabase#36247)", () => {
   const drillType = "drill-thru/zoom-in.geographic";
   const stageIndex = 0;
   const defaultQuery = Lib.withDifferentTable(createQuery(), PEOPLE_ID);

frontend/src/metabase-types/api/dataset.ts

@@ -10,6 +10,12 @@ import type { TableId } from "./table";
 export type RowValue = string | number | null | boolean;
 export type RowValues = RowValue[];
 
+export type BinningMetadata = {
+  binning_strategy?: "default" | "bin-width" | "num-bins";
+  bin_width?: number;
+  num_bins?: number;
+};
+
 export interface DatasetColumn {
   id?: FieldId;
   name: string;
@@ -29,9 +35,7 @@ export interface DatasetColumn {
   remapped_from?: string;
   remapped_to?: string;
   effective_type?: string;
-  binning_info?: {
-    bin_width?: number;
-  };
+  binning_info?: BinningMetadata | null;
   settings?: Record<string, any>;
   fingerprint?: FieldFingerprint | null;
 

src/metabase/lib/convert.cljc

@@ -259,13 +259,17 @@
   (let [result (get m k ::not-found)]
     (if-not (= result ::not-found)
       result
-      (throw (ex-info (str "Unable to find " (pr-str k) " in map.")
+      (throw (ex-info (str "Unable to find key " (pr-str k) " in map.")
                       {:m m
                        :k k})))))
 
 (defmethod ->pMBQL :aggregation
-  [[tag value opts]]
-  (lib.options/ensure-uuid [tag opts (get-or-throw! *legacy-index->pMBQL-uuid* value)]))
+  [[tag aggregation-index opts, :as clause]]
+  (lib.options/ensure-uuid
+   [tag opts (or (get *legacy-index->pMBQL-uuid* aggregation-index)
+                 (throw (ex-info (str "Error converting :aggregation reference: no aggregation at index "
+                                      aggregation-index)
+                                 {:clause clause})))]))
 
 (defmethod ->pMBQL :aggregation-options
   [[_tag aggregation options]]

src/metabase/lib/drill_thru.cljc

@@ -55,7 +55,7 @@
    {:f #'lib.drill-thru.summarize-column-by-time/summarize-column-by-time-drill, :return-drills-for-dimensions? true}
    {:f #'lib.drill-thru.underlying-records/underlying-records-drill,             :return-drills-for-dimensions? false}
    {:f #'lib.drill-thru.zoom-in-timeseries/zoom-in-timeseries-drill,             :return-drills-for-dimensions? false}
-   {:f #'lib.drill-thru.zoom-in-geographic/zoom-in-geographic-drill,             :return-drills-for-dimensions? false}
+   {:f #'lib.drill-thru.zoom-in-geographic/zoom-in-geographic-drill,             :return-drills-for-dimensions? true}
    {:f #'lib.drill-thru.zoom-in-bins/zoom-in-binning-drill,                      :return-drills-for-dimensions? true}])
 
 (mu/defn ^:private dimension-contexts :- [:maybe [:sequential {:min 1} ::lib.schema.drill-thru/context]]

src/metabase/lib/drill_thru/zoom_in_geographic.cljc

@@ -24,11 +24,11 @@
 
      If we have binned breakouts on latitude and longitude:
 
-     2a. With binning `:bin-width` >= 20°, replace them with `:bin-width` of 10° and add `:between` filters for the
+     2a. With binning `:bin-width` >= 20°, replace them with `:bin-width` of 10° and add `:>=`/`:<` filters for the
          clicked latitude/longitude values.
 
      2b. Otherwise if `:bin-width` is < 20°, replace them with the current `:bin-width` divided by 10, and add
-         `:between` filters for the clicked latitude/longitude values."
+         `:>=`/`:<` filters for the clicked latitude/longitude values."
   (:require
    [medley.core :as m]
    [metabase.lib.binning :as lib.binning]
@@ -38,6 +38,7 @@
    [metabase.lib.metadata.calculation :as lib.metadata.calculation]
    [metabase.lib.remove-replace :as lib.remove-replace]
    [metabase.lib.schema :as lib.schema]
+   [metabase.lib.schema.binning :as lib.schema.binning]
    [metabase.lib.schema.drill-thru :as lib.schema.drill-thru]
    [metabase.lib.schema.metadata :as lib.schema.metadata]
    [metabase.lib.types.isa :as lib.types.isa]
@@ -82,7 +83,7 @@
 
 (mu/defn ^:private country-state-city->binned-lat-lon-drill :- [:maybe ::lib.schema.drill-thru/drill-thru.zoom-in.geographic.country-state-city->binned-lat-lon]
   [{:keys [column value lat-column lon-column], :as _context} :- ContextWithLatLon
-   lat-lon-bin-width                                          :- number?]
+   lat-lon-bin-width                                          :- ::lib.schema.binning/bin-width]
   (when value
     {:lib/type  :metabase.lib.drill-thru/drill-thru
      :type      :drill-thru/zoom-in.geographic
@@ -110,11 +111,12 @@
     (country-state-city->binned-lat-lon-drill context 0.1)))
 
 (mu/defn ^:private binned-lat-lon->binned-lat-lon-drill :- [:maybe ::lib.schema.drill-thru/drill-thru.zoom-in.geographic.binned-lat-lon->binned-lat-lon]
-  [{:keys [lat-column lon-column lat-value lon-value], :as _context} :- ContextWithLatLon]
+  [metadata-providerable                                             :- ::lib.schema.metadata/metadata-providerable
+   {:keys [lat-column lon-column lat-value lon-value], :as _context} :- ContextWithLatLon]
   (when (and lat-value
              lon-value)
-    (when-let [lat-bin-width (:bin-width (lib.binning/binning lat-column))]
-      (when-let [lon-bin-width (:bin-width (lib.binning/binning lon-column))]
+    (when-let [{lat-bin-width :bin-width} (lib.binning/resolve-bin-width metadata-providerable lat-column lat-value)]
+      (when-let [{lon-bin-width :bin-width} (lib.binning/resolve-bin-width metadata-providerable lon-column lon-value)]
         (let [[new-lat-bin-width new-lon-bin-width] (if (and (>= lat-bin-width 20)
                                                              (>= lon-bin-width 20))
                                                       [10 10]
@@ -146,7 +148,7 @@
             [country->binned-lat-lon-drill
              state->binned-lat-lon-drill
              city->binned-lat-lon-drill
-             binned-lat-lon->binned-lat-lon-drill]))))
+             (partial binned-lat-lon->binned-lat-lon-drill query)]))))
 
 ;;;
 ;;; Application
@@ -191,8 +193,10 @@
     :as drill} :- ::lib.schema.drill-thru/drill-thru.zoom-in.geographic.binned-lat-lon->binned-lat-lon]
   (-> query
       ;; TODO -- remove/update existing filters on these columns?
-      (lib.filter/filter stage-number (lib.filter/between lat lat-min lat-max))
-      (lib.filter/filter stage-number (lib.filter/between lon lon-min lon-max))
+      (lib.filter/filter stage-number (lib.filter/>= lat lat-min))
+      (lib.filter/filter stage-number (lib.filter/<  lat lat-max))
+      (lib.filter/filter stage-number (lib.filter/>= lon lon-min))
+      (lib.filter/filter stage-number (lib.filter/<  lon lon-max))
       (add-or-update-lat-lon-binning stage-number drill)))
 
 (mu/defmethod lib.drill-thru.common/drill-thru-method :drill-thru/zoom-in.geographic :- ::lib.schema/query