Fix binning of nested questions (#12627)

* Fix binning of nested questions
* Fix insight calculation

src/metabase/api/table.clj

@@ -153,8 +153,7 @@
                :type "type/Coordinate"})))))
 
 (def ^:private dimension-options-for-response
-  (m/map-kv (fn [k v]
-              [(str k) v]) dimension-options))
+  (m/map-keys str dimension-options))
 
 (defn- create-dim-index-seq [dim-type]
   (->> dimension-options
@@ -244,11 +243,11 @@
         (assoc-dimension-options driver)
         format-fields-for-response
         (update :fields
-                (let [hidden (Boolean/parseBoolean include_hidden_fields)
+                (let [hidden    (Boolean/parseBoolean include_hidden_fields)
                       sensitive (Boolean/parseBoolean include_sensitive_fields)]
                   (partial filter (fn [{:keys [visibility_type]}]
                                     (case (keyword visibility_type)
-                                      :hidden hidden
+                                      :hidden    hidden
                                       :sensitive sensitive
                                       true))))))))
 
@@ -274,12 +273,12 @@
       (-> col
           (update :base_type keyword)
           (assoc
-              :table_id     (str "card__" card-id)
-              :id           [:field-literal (:name col) (or (:base_type col) :type/*)]
-              ;; Assoc special_type at least temprorarily. We need the correct special type in place to make decisions
-              ;; about what kind of dimension options should be added. PK/FK values will be removed after we've added
-              ;; the dimension options
-              :special_type (keyword (:special_type col)))
+           :table_id     (str "card__" card-id)
+           :id           [:field-literal (:name col) (or (:base_type col) :type/*)]
+           ;; Assoc special_type at least temprorarily. We need the correct special type in place to make decisions
+           ;; about what kind of dimension options should be added. PK/FK values will be removed after we've added
+           ;; the dimension options
+           :special_type (keyword (:special_type col)))
           add-field-dimension-options))))
 
 (defn root-collection-schema-name

src/metabase/query_processor/middleware/binning.clj

@@ -74,7 +74,7 @@
 (s/defn ^:private resolve-default-strategy :- [(s/one (s/enum :bin-width :num-bins) "strategy")
                                                (s/one {:bin-width s/Num, :num-bins su/IntGreaterThanZero} "opts")]
   "Determine the approprate strategy & options to use when `:default` strategy was specified."
-  [metadata :- {:special_type (s/maybe su/FieldType), s/Any s/Any}, min-value :- s/Num, max-value :- s/Num]
+  [metadata :- {(s/optional-key :special_type) (s/maybe su/FieldType), s/Any s/Any}, min-value :- s/Num, max-value :- s/Num]
   (if (isa? (:special_type metadata) :type/Coordinate)
     (let [bin-width (public-settings/breakout-bin-width)]
       [:bin-width

src/metabase/sync/analyze/fingerprint/insights.clj

@@ -202,10 +202,11 @@
                                (stats/simple-linear-regression xfn yfn)
                                (best-fit xfn yfn))))
                 (fn [[[y-previous y-current] [x-previous x-current] [offset slope] best-fit]]
-                  (let [unit         (if (or (nil? (:unit datetime))
-                                             (->> datetime :unit mbql.u/normalize-token (= :default)))
-                                       (infer-unit x-previous x-current)
-                                       (:unit datetime))
+                  (let [unit         (let [unit (some-> datetime :unit mbql.u/normalize-token)]
+                                       (if (or (nil? unit)
+                                               (= unit :default))
+                                         (infer-unit x-previous x-current)
+                                         unit))
                         show-change? (valid-period? x-previous x-current unit)]
                     (f/robust-map
                      :last-value     y-current

test/metabase/query_processor/middleware/binning_test.clj

 (ns metabase.query-processor.middleware.binning-test
-  (:require [expectations :refer [expect]]
+  (:require [clojure.test :refer :all]
+            [expectations :refer [expect]]
             [metabase
              [test :as mt]
              [util :as u]]
-            [metabase.models.field :as field :refer [Field]]
+            [metabase.models
+             [card :refer [Card]]
+             [field :as field :refer [Field]]]
             [metabase.query-processor.middleware.binning :as binning]
             [metabase.query-processor.test-util :as qp.test-util]
             [metabase.test.data :as data]
@@ -156,3 +159,17 @@
                    :breakout     [[:binning-strategy [:field-id (u/get-id field)] :default]]}}
        :type     :query
        :database (data/id)}))))
+
+(deftest bining-nested-questions-test
+  (mt/with-temp Card [{card-id :id} {:dataset_query {:database (mt/id)
+                                                     :type     :query
+                                                     :query    {:source-table (mt/id :venues)}}}]
+    (is (= [[1 22]
+            [2 59]
+            [3 13]
+            [4 6]]
+         (->> (mt/run-mbql-query nil
+                {:source-table (str "card__" card-id)
+                 :breakout     [[:binning-strategy [:field-literal "PRICE" :type/Float] :default]]
+                 :aggregation  [[:count]]})
+              (mt/formatted-rows [int int]))))))