diff --git a/shared/src/metabase/shared/util/i18n.cljs b/shared/src/metabase/shared/util/i18n.cljs
index 7d45853075c65012d2ab011aad58ac158efbac43..6a1dafeef28f506b07b6a8a03651e7fe34a18fcb 100644
--- a/shared/src/metabase/shared/util/i18n.cljs
+++ b/shared/src/metabase/shared/util/i18n.cljs
@@ -8,19 +8,39 @@
(comment metabase.shared.util.i18n/keep-me
ttag/keep-me)
+(defn- escape-format-string
+ "Converts `''` to `'` inside the string; that's `java.text.MessageFormat` escaping that isn't needed in JS."
+ [format-string]
+ (str/replace format-string #"''" "'"))
+
(defn js-i18n
- "Format an i18n `format-string` with `args` with a translated string in the user locale."
+ "Format an i18n `format-string` with `args` with a translated string in the user locale.
+
+ The strings are formatted in `java.test.MessageFormat` style. That's used directly in JVM Clojure, but in CLJS we have
+ to adapt to ttag, which doesn't have the same escaping rules.
+ - 'xyz' single quotes wrap literal text which should not be interpolated, and could contain literal '{0}'.
+ - A literal single quote is written with two single quotes: `''`
+ The first part is not supported at all. `''` is converted to a single `'`."
[format-string & args]
- (let [strings (str/split format-string #"\{\d+\}")]
+ (let [strings (-> format-string
+ escape-format-string
+ (str/split #"\{\d+\}"))]
(apply ttag/t (clj->js strings) (clj->js args))))
+(def ^:private re-param-zero #"\{0\}")
+
(defn js-i18n-n
- "Format an i18n `format-string` with the appropritae plural form based on the value `n`.
+ "Format an i18n `format-string` with the appropriate plural form based on the value `n`.
Allows `n` to be interpolated into the string using {0}."
[format-string format-string-pl n]
- (let [strings (str/split format-string #"\{0\}")
- strings (if (= (count strings) 1) [format-string ""] strings)
- has-n? (re-find #".*\{0\}.*" format-string)]
+ (let [format-string-esc (escape-format-string format-string)
+ strings (str/split format-string-esc re-param-zero)
+ strings (if (= (count strings) 1)
+ [format-string-esc ""]
+ strings)
+ has-n? (re-find #".*\{0\}.*" format-string-esc)]
(ttag/ngettext (ttag/msgid (clj->js strings) (if has-n? n ""))
- format-string-pl
+ (-> format-string-pl
+ escape-format-string
+ (str/replace re-param-zero (str n)))
n)))
diff --git a/shared/test/metabase/shared/i18n_test.cljc b/shared/test/metabase/shared/i18n_test.cljc
new file mode 100644
index 0000000000000000000000000000000000000000..b5bb8f1215240a8f410521fbe71b4af469dc4f6e
--- /dev/null
+++ b/shared/test/metabase/shared/i18n_test.cljc
@@ -0,0 +1,28 @@
+(ns metabase.shared.i18n-test
+ (:require
+ [clojure.test :refer [are deftest is testing]]
+ [metabase.shared.util.i18n :as i18n]))
+
+(deftest ^:parallel tru-test
+ (testing "basic strings"
+ (is (= "some text here" (i18n/tru "some text here"))))
+ (testing "escaping single quotes"
+ (is (= "Where there's life there's hope, and need of vittles."
+ (i18n/tru "Where there''s life there''s hope, and need of vittles.")))))
+
+(deftest ^:parallel trun-test
+ (testing "basic"
+ (are [n exp] (= exp (i18n/trun "{0} cat" "{0} cats" n))
+ 0 "0 cats"
+ 1 "1 cat"
+ 7 "7 cats"))
+ (testing "escaping in singular"
+ (are [n exp] (= exp (i18n/trun "{0} cat''s food" "{0} cats worth of food" n))
+ 0 "0 cats worth of food"
+ 1 "1 cat's food"
+ 7 "7 cats worth of food"))
+ (testing "escaping in both"
+ (are [n exp] (= exp (i18n/trun "{0} cat''s food" "{0} cats'' food" n))
+ 0 "0 cats' food"
+ 1 "1 cat's food"
+ 7 "7 cats' food")))
diff --git a/src/metabase/api/table.clj b/src/metabase/api/table.clj
index be028e5201af9293ddb60010d3c725a56142d7cd..27ff6393577435d74ba6f4d528234fa37742e4f8 100644
--- a/src/metabase/api/table.clj
+++ b/src/metabase/api/table.clj
@@ -270,6 +270,7 @@
(defn- supports-numeric-binning? [db]
(and db (driver/database-supports? (:engine db) :binning db)))
+;; TODO: Remove all this when the FE is fully ported to [[metabase.lib.binning/available-binning-strategies]].
(defn- assoc-field-dimension-options [{:keys [base_type semantic_type fingerprint] :as field} db]
(let [{min_value :min, max_value :max} (get-in fingerprint [:type :type/Number])
[default-option all-options] (cond
diff --git a/src/metabase/lib/binning.cljc b/src/metabase/lib/binning.cljc
new file mode 100644
index 0000000000000000000000000000000000000000..c077f6fb870cee5449333eb19cf2532fb5517740
--- /dev/null
+++ b/src/metabase/lib/binning.cljc
@@ -0,0 +1,131 @@
+(ns metabase.lib.binning
+ (:require
+ [metabase.lib.dispatch :as lib.dispatch]
+ [metabase.lib.hierarchy :as lib.hierarchy]
+ [metabase.lib.metadata.calculation :as lib.metadata.calculation]
+ [metabase.lib.schema :as lib.schema]
+ [metabase.lib.schema.binning :as lib.schema.binning]
+ [metabase.shared.util.i18n :as i18n]
+ [metabase.util.malli :as mu]))
+
+(defmulti with-binning-method
+ "Implementation for [[with-binning]]. Implement this to tell [[with-binning]] how to add binning to a particular MBQL
+ clause."
+ {:arglists '([x binning])}
+ (fn [x _binning]
+ (lib.dispatch/dispatch-value x)) :hierarchy lib.hierarchy/hierarchy)
+
+(defmethod with-binning-method :dispatch-type/fn
+ [f binning]
+ (fn [query stage-number]
+ (let [x (f query stage-number)]
+ (with-binning-method x binning))))
+
+(mu/defn with-binning
+ "Add binning to an MBQL clause or something that can be converted to an MBQL clause.
+ Eg. for a Field or Field metadata or `:field` clause, this might do something like this:
+
+ (with-binning some-field (bin-by :num-bins 4))
+
+ =>
+
+ [:field {:binning {:strategy :num-bins :num-bins 4}} 1]
+
+ Pass `nil` `binning` to remove any binning."
+ [x binning :- [:maybe [:or ::lib.schema.binning/binning ::lib.schema.binning/binning-option]]]
+ (with-binning-method x (if (contains? binning :mbql)
+ (:mbql binning)
+ binning)))
+
+(defmulti binning-method
+ "Implementation of [[binning]]. Return the current binning options associated with `x`."
+ {:arglists '([x])}
+ lib.dispatch/dispatch-value
+ :hierarchy lib.hierarchy/hierarchy)
+
+(defmethod binning-method :default
+ [_x]
+ nil)
+
+(mu/defn binning :- [:maybe ::lib.schema.binning/binning]
+ "Get the current binning options associated with `x`, if any."
+ [x]
+ (binning-method x))
+
+(defmulti available-binning-strategies-method
+ "Implementation for [[available-binning-strategies]]. Return a set of binning strategies from
+ `:metabase.lib.schema.binning/binning-strategies` that are allowed to be used with `x`."
+ {:arglists '([query stage-number x])}
+ (fn [_query _stage-number x]
+ (lib.dispatch/dispatch-value x))
+ :hierarchy lib.hierarchy/hierarchy)
+
+(defmethod available-binning-strategies-method :default
+ [_query _stage-number _x]
+ nil)
+
+(mu/defn available-binning-strategies :- [:sequential [:ref ::lib.schema.binning/binning-option]]
+ "Get a set of available binning strategies for `x`. Returns nil if none are available."
+ ([query x]
+ (available-binning-strategies query -1 x))
+
+ ([query :- ::lib.schema/query
+ stage-number :- :int
+ x]
+ (available-binning-strategies-method query stage-number x)))
+
+(defn- default-auto-bin []
+ {:display-name (i18n/tru "Auto bin")
+ :default true
+ :mbql {:strategy :default}})
+
+(defn- dont-bin []
+ {:display-name (i18n/tru "Don''t bin")
+ :mbql nil})
+
+(defn- with-binning-option-type [m]
+ (assoc m :lib/type ::binning-option))
+
+(def ^:private *numeric-binning-strategies
+ (delay (mapv with-binning-option-type
+ [(default-auto-bin)
+ {:display-name (i18n/tru "10 bins") :mbql {:strategy :num-bins :num-bins 10}}
+ {:display-name (i18n/tru "50 bins") :mbql {:strategy :num-bins :num-bins 50}}
+ {:display-name (i18n/tru "100 bins") :mbql {:strategy :num-bins :num-bins 100}}
+ (dont-bin)])))
+
+(defn numeric-binning-strategies
+ "List of binning options for numeric fields. These split the data evenly into a fixed number of bins."
+ []
+ @*numeric-binning-strategies)
+
+(def ^:private *coordinate-binning-strategies
+ (delay
+ (mapv with-binning-option-type
+ [(default-auto-bin)
+ {:display-name (i18n/tru "Bin every 0.1 degrees") :mbql {:strategy :bin-width :bin-width 0.1}}
+ {:display-name (i18n/tru "Bin every 1 degree") :mbql {:strategy :bin-width :bin-width 1.0}}
+ {:display-name (i18n/tru "Bin every 10 degrees") :mbql {:strategy :bin-width :bin-width 10.0}}
+ {:display-name (i18n/tru "Bin every 20 degrees") :mbql {:strategy :bin-width :bin-width 20.0}}
+ (dont-bin)])))
+
+(defn coordinate-binning-strategies
+ "List of binning options for coordinate fields (ie. latitude and longitude). These split the data into as many
+ ranges as necessary, with each range being a certain number of degrees wide."
+ []
+ @*coordinate-binning-strategies)
+
+(defmethod lib.metadata.calculation/display-info-method ::binning-option
+ [_query _stage-number binning-option]
+ (select-keys binning-option [:display-name :default]))
+
+(defn binning-display-name
+ "This is implemented outside of [[lib.metadata.calculation/display-name]] because it needs access to the field type.
+ It's called directly by `:field` or `:metadata/field`'s [[lib.metadata.calculation/display-name]]."
+ [{:keys [bin-width num-bins strategy] :as binning-options} field-metadata]
+ (when binning-options
+ (case strategy
+ :num-bins (i18n/trun "{0} bin" "{0} bins" num-bins)
+ :bin-width (str bin-width (when (isa? (:semantic-type field-metadata) :type/Coordinate)
+ "°"))
+ :default (i18n/tru "Auto binned"))))
diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc
index 1309c52548bc4f943f810dddd84b0b287e63dbfa..151a49111b517490581ee002d7f1442a623205d5 100644
--- a/src/metabase/lib/core.cljc
+++ b/src/metabase/lib/core.cljc
@@ -5,6 +5,7 @@
+ - * / time abs concat replace ref var])
(:require
[metabase.lib.aggregation :as lib.aggregation]
+ [metabase.lib.binning :as lib.binning]
[metabase.lib.breakout :as lib.breakout]
[metabase.lib.card :as lib.card]
[metabase.lib.column-group :as lib.column-group]
@@ -29,6 +30,7 @@
[metabase.shared.util.namespaces :as shared.ns]))
(comment lib.aggregation/keep-me
+ lib.binning/keep-me
lib.breakout/keep-me
lib.card/keep-me
lib.column-group/keep-me
@@ -67,6 +69,10 @@
sum
sum-where
var]
+ [lib.binning
+ available-binning-strategies
+ binning
+ with-binning]
[lib.breakout
breakout
breakoutable-columns
diff --git a/src/metabase/lib/field.cljc b/src/metabase/lib/field.cljc
index e0104a96d204a723d4c1dd2dd2484d9920f02a5d..46c00ee9f443fc637b6db375286685be92028fc8 100644
--- a/src/metabase/lib/field.cljc
+++ b/src/metabase/lib/field.cljc
@@ -2,11 +2,13 @@
(:require
[medley.core :as m]
[metabase.lib.aggregation :as lib.aggregation]
+ [metabase.lib.binning :as lib.binning]
[metabase.lib.expression :as lib.expression]
[metabase.lib.join :as lib.join]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.normalize :as lib.normalize]
+ [metabase.lib.options :as lib.options]
[metabase.lib.ref :as lib.ref]
[metabase.lib.schema :as lib.schema]
[metabase.lib.schema.common :as lib.schema.common]
@@ -17,6 +19,7 @@
[metabase.lib.temporal-bucket :as lib.temporal-bucket]
[metabase.lib.util :as lib.util]
[metabase.shared.util.i18n :as i18n]
+ [metabase.util :as u]
[metabase.util.humanization :as u.humanization]
[metabase.util.log :as log]
[metabase.util.malli :as mu]))
@@ -145,7 +148,9 @@
;;; TODO -- base type should be affected by `temporal-unit`, right?
(defmethod lib.metadata.calculation/metadata-method :field
- [query stage-number [_tag {:keys [source-field effective-type base-type temporal-unit join-alias], :as opts} :as field-ref]]
+ [query
+ stage-number
+ [_tag {:keys [base-type binning effective-type join-alias source-field temporal-unit], :as opts} :as field-ref]]
(let [field-metadata (resolve-field-metadata query stage-number field-ref)
metadata (merge
{:lib/type :metadata/field}
@@ -158,6 +163,8 @@
{:base-type base-type})
(when temporal-unit
{::temporal-unit temporal-unit})
+ (when binning
+ {::binning binning})
(when join-alias
{::join-alias join-alias})
(when source-field
@@ -171,10 +178,11 @@
[query stage-number {field-display-name :display-name
field-name :name
temporal-unit :unit
+ binning :binning
join-alias :source_alias
fk-field-id :fk-field-id
table-id :table-id
- :as _field-metadata} style]
+ :as field-metadata} style]
(let [field-display-name (or field-display-name
(u.humanization/name->human-readable-name :simple field-name))
join-display-name (when (= style :long)
@@ -188,18 +196,20 @@
display-name (if join-display-name
(str join-display-name " → " field-display-name)
field-display-name)]
- (if temporal-unit
- (lib.util/format "%s (%s)" display-name (name temporal-unit))
- display-name)))
+ (cond
+ temporal-unit (lib.util/format "%s (%s)" display-name (name temporal-unit))
+ binning (lib.util/format "%s: %s" display-name (lib.binning/binning-display-name binning field-metadata))
+ :else display-name)))
(defmethod lib.metadata.calculation/display-name-method :field
[query
stage-number
- [_tag {:keys [join-alias temporal-unit source-field], :as _opts} _id-or-name, :as field-clause]
+ [_tag {:keys [binning join-alias temporal-unit source-field], :as _opts} _id-or-name, :as field-clause]
style]
(if-let [field-metadata (cond-> (resolve-field-metadata query stage-number field-clause)
join-alias (assoc :source_alias join-alias)
temporal-unit (assoc :unit temporal-unit)
+ binning (assoc :binning binning)
source-field (assoc :fk-field-id source-field))]
(lib.metadata.calculation/display-name query stage-number field-metadata style)
;; mostly for the benefit of JS, which does not enforce the Malli schemas.
@@ -227,6 +237,7 @@
(when-let [card (lib.metadata/card query card-id)]
{:table {:name (:name card), :display-name (:name card)}})))))
+;;; ---------------------------------- Temporal Bucketing ----------------------------------------
(defmethod lib.temporal-bucket/temporal-bucket-method :field
[[_tag opts _id-or-name]]
(:temporal-unit opts))
@@ -281,6 +292,43 @@
(isa? effective-type :type/Time) lib.temporal-bucket/time-bucket-options
:else [])))
+;;; ---------------------------------------- Binning ---------------------------------------------
+(defmethod lib.binning/binning-method :field
+ [field-clause]
+ (-> field-clause lib.options/options :binning))
+
+(defmethod lib.binning/binning-method :metadata/field
+ [metadata]
+ (::binning metadata))
+
+(defmethod lib.binning/with-binning-method :field
+ [field-clause binning]
+ (lib.options/update-options field-clause u/assoc-dissoc :binning binning))
+
+(defmethod lib.binning/with-binning-method :metadata/field
+ [metadata binning]
+ (u/assoc-dissoc metadata ::binning binning))
+
+(defmethod lib.binning/available-binning-strategies-method :field
+ [query stage-number field-ref]
+ (lib.binning/available-binning-strategies query stage-number (resolve-field-metadata query stage-number field-ref)))
+
+(defmethod lib.binning/available-binning-strategies-method :metadata/field
+ [query _stage-number {:keys [effective-type fingerprint semantic-type] :as _field-metadata}]
+ (let [binning? (some-> query lib.metadata/database :features (contains? :binning))
+ {min-value :min max-value :max} (get-in fingerprint [:type :type/Number])]
+ (cond
+ ;; TODO: Include the time and date binning strategies too; see metabase.api.table/assoc-field-dimension-options.
+ (and binning? min-value max-value
+ (isa? semantic-type :type/Coordinate))
+ (lib.binning/coordinate-binning-strategies)
+
+ (and binning? min-value max-value
+ (isa? effective-type :type/Number)
+ (not (isa? semantic-type :Relation/*)))
+ (lib.binning/numeric-binning-strategies))))
+
+;;; -------------------------------------- Join Alias --------------------------------------------
(defmethod lib.join/current-join-alias-method :field
[[_tag opts]]
(get opts :join-alias))
@@ -316,10 +364,10 @@
{:join-alias join-alias})
(when-let [temporal-unit (::temporal-unit metadata)]
{:temporal-unit temporal-unit})
+ (when-let [binning (::binning metadata)]
+ {:binning binning})
(when-let [source-field-id (:fk-field-id metadata)]
- {:source-field source-field-id})
- ;; TODO -- binning options.
- )
+ {:source-field source-field-id}))
always-use-name? (#{:source/card :source/native :source/previous-stage} (:lib/source metadata))]
[:field options (if always-use-name?
(:name metadata)
diff --git a/src/metabase/lib/options.cljc b/src/metabase/lib/options.cljc
index e83ff49ac13e564e9ea59c294acd44748e45b09f..790472edc44c3b5dc8390999b9d309e132d20a8b 100644
--- a/src/metabase/lib/options.cljc
+++ b/src/metabase/lib/options.cljc
@@ -1,6 +1,7 @@
(ns metabase.lib.options
(:require
[metabase.shared.util.i18n :as i18n]
+ [metabase.util :as u]
[metabase.util.malli :as mu]))
;;; TODO -- not 100% sure we actually need all of this stuff anymore.
@@ -40,7 +41,9 @@
(mu/defn with-options
"Update `x` so its [[options]] are `new-options`. If the clause or map already has options, this will
- *replace* the old options; if it does not, this will the new options.
+ *replace* the old options; if it does not, this will set the new options.
+
+ If `x` is a map with `:lib/options` and `new-options` is `empty?`, this will drop `:lib/options` entirely.
You should probably prefer [[update-options]] to using this directly, so you don't stomp over existing stuff
unintentionally. Implement this if you need to teach Metabase lib how to support something that doesn't follow the
@@ -48,7 +51,7 @@
[x new-options :- [:maybe map?]]
(cond
(map? x)
- (assoc x :lib/options new-options)
+ (u/assoc-dissoc x :lib/options (not-empty new-options))
(mbql-clause? x)
(if ((some-fn nil? map?) (second x))
diff --git a/src/metabase/lib/schema/binning.cljc b/src/metabase/lib/schema/binning.cljc
new file mode 100644
index 0000000000000000000000000000000000000000..29097c9edfe99de033d444c8f73012a970b40ac5
--- /dev/null
+++ b/src/metabase/lib/schema/binning.cljc
@@ -0,0 +1,30 @@
+(ns metabase.lib.schema.binning
+ "Malli schema for binning of a column's values.
+
+ There are two approaches to binning, selected by `:strategy`:
+ - `{:strategy :bin-width :bin-width 10}` makes 1 or more bins that are 10 wide;
+ - `{:strategy :num-bins :num-bins 12}` splits the column into 12 bins."
+ (:require
+ [metabase.lib.schema.common :as lib.schema.common]
+ [metabase.util.malli.registry :as mr]))
+
+(mr/def ::binning-strategies
+ [:enum :bin-width :default :num-bins])
+
+(mr/def ::binning
+ [:and
+ [:map
+ [:strategy [:ref ::binning-strategies]]
+ [:bin-width {:optional true} pos?]
+ [:num-bins {:optional true} ::lib.schema.common/int-greater-than-zero]]
+ [:fn {:error/message "if :strategy is not :default, the matching key :bin-width or :num-bins must also be set"}
+ #(when-let [strat (:strategy %)]
+ (or (= strat :default)
+ (contains? % strat)))]])
+
+(mr/def ::binning-option
+ [:map
+ [:lib/type [:= :metabase.lib.binning/binning-option]]
+ [:display-name :string]
+ [:mbql [:maybe ::binning]]
+ [:default {:optional true} :boolean]])
diff --git a/src/metabase/util.cljc b/src/metabase/util.cljc
index 255f1b555e9b69b3d8b6fb9fb531d83eac408c4e..8e012a9e47da2bfea8c5ad7ed1d621fa6b8aa867 100644
--- a/src/metabase/util.cljc
+++ b/src/metabase/util.cljc
@@ -773,3 +773,14 @@
(regexp? x) :dispatch-type/regex
;; we should add more mappings here as needed
:else :dispatch-type/*))
+
+(defn assoc-dissoc
+ "Called like `(assoc m k v)`, this does [[assoc]] if `(some? v)`, and [[dissoc]] if not.
+
+ Put another way: `k` will either be set to `v`, or removed.
+
+ Note that if `v` is `false`, it will be handled with [[assoc]]; only `nil` causes a [[dissoc]]."
+ [m k v]
+ (if (some? v)
+ (assoc m k v)
+ (dissoc m k)))
diff --git a/test/metabase/lib/field_test.cljc b/test/metabase/lib/field_test.cljc
index 7c4521b04697474d6b3001025fe8659aadea4942..b0231f3a54a87d489ad76c1a874004e47aad815c 100644
--- a/test/metabase/lib/field_test.cljc
+++ b/test/metabase/lib/field_test.cljc
@@ -2,6 +2,7 @@
(:require
[clojure.test :refer [are deftest is testing]]
[medley.core :as m]
+ [metabase.lib.binning :as lib.binning]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
@@ -228,6 +229,80 @@
(lib/available-temporal-buckets (:query temporal-bucketing-mock-metadata)
(lib/with-temporal-bucket x :month-of-year))))))))))
+(deftest ^:parallel unresolved-lib-field-with-binning-test
+ (let [query (lib/query-for-table-name meta/metadata-provider "ORDERS")
+ binning {:strategy :num-bins
+ :num-bins 10}
+ f (lib/with-binning (lib/field (meta/id :orders :subtotal)) binning)]
+ (is (fn? f))
+ (let [field (f query -1)]
+ (is (=? [:field {:binning binning} (meta/id :orders :subtotal)]
+ field))
+ (testing "(lib/binning <column-metadata>)"
+ (is (= binning
+ (lib/binning (lib.metadata.calculation/metadata query -1 field)))))
+ (testing "(lib/binning <field-ref>)"
+ (is (= binning
+ (lib/binning field))))
+ #?(:clj
+ ;; i18n/trun doesn't work in the CLJS tests, only in proper FE, so this test is JVM-only.
+ (is (= "Subtotal: 10 bins"
+ (lib.metadata.calculation/display-name query -1 field)))))))
+
+(deftest ^:parallel with-binning-test
+ (doseq [[binning1 binning2] (partition 2 1 [{:strategy :default}
+ {:strategy :num-bins :num-bins 10}
+ {:strategy :bin-width :bin-width 1.0}
+ {:strategy :default}])
+ :let [field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "ORDERS" "SUBTOTAL")]
+ [what x] {"column metadata" field-metadata
+ "field ref" (lib/ref field-metadata)}
+ :let [x' (lib/with-binning x binning1)]]
+ (testing (str what " strategy = " (:strategy binning2) "\n\n" (u/pprint-to-str x') "\n")
+ (testing "lib/binning should return the binning settings"
+ (is (= binning1
+ (lib/binning x'))))
+ (testing "should generate a :field ref with correct :binning"
+ (is (=? [:field
+ {:lib/uuid string?
+ :binning binning1}
+ integer?]
+ (lib/ref x'))))
+ (testing "remove the binning setting"
+ (let [x'' (lib/with-binning x' nil)]
+ (is (nil? (lib/binning x'')))
+ (is (= x
+ x''))))
+ (testing "change the binning setting, THEN remove it"
+ (let [x'' (lib/with-binning x' binning2)
+ x''' (lib/with-binning x'' nil)]
+ (is (= binning2
+ (lib/binning x'')))
+ (is (nil? (lib/binning x''')))
+ (is (= x
+ x''')))))))
+
+(deftest ^:parallel available-binning-strategies-test
+ (doseq [{:keys [expected-options field-metadata query]}
+ [{:query (lib/query-for-table-name meta/metadata-provider "ORDERS")
+ :field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "ORDERS" "SUBTOTAL")
+ :expected-options (lib.binning/numeric-binning-strategies)}
+ {:query (lib/query-for-table-name meta/metadata-provider "PEOPLE")
+ :field-metadata (lib.metadata/field meta/metadata-provider "PUBLIC" "PEOPLE" "LATITUDE")
+ :expected-options (lib.binning/coordinate-binning-strategies)}]]
+ (testing (str (:semantic-type field-metadata) " Field")
+ (doseq [[what x] [["column metadata" field-metadata]
+ ["field ref" (lib/ref field-metadata)]]]
+ (testing (str what "\n\n" (u/pprint-to-str x))
+ (is (= expected-options
+ (lib/available-binning-strategies query x)))
+ (testing "when binned, should still return the same available units"
+ (let [binned (lib/with-binning x (second expected-options))]
+ (is (= (-> expected-options second :mbql)
+ (lib/binning binned)))
+ (is (= expected-options
+ (lib/available-binning-strategies query binned))))))))))
+
(deftest ^:parallel joined-field-column-name-test
(let [card {:dataset-query {:database (meta/id)
:type :query
diff --git a/test/metabase/util_test.cljc b/test/metabase/util_test.cljc
index a782ada2dc1229b825941b5a9a34efd854721ff9..bbe534b68540b55a9943ba5cd148b94facfdc622 100644
--- a/test/metabase/util_test.cljc
+++ b/test/metabase/util_test.cljc
@@ -381,3 +381,13 @@
:dispatch-type/regex
:dispatch-type/fn
:dispatch-type/*)))
+
+(deftest ^:parallel assoc-dissoc-test
+ (testing `lib.options/with-option-value
+ (is (= {:foo "baz"}
+ (u/assoc-dissoc {:foo "bar"} :foo "baz")))
+ (is (= {}
+ (u/assoc-dissoc {:foo "bar"} :foo nil)))
+ (is (= {:foo false}
+ (u/assoc-dissoc {:foo "bar"} :foo false))
+ "false should be assoc'd")))