Add aggregation-position to available-metrics (#34867)

src/metabase/lib/metric.cljc

@@ -9,7 +9,6 @@
    [metabase.lib.ref :as lib.ref]
    [metabase.lib.schema :as lib.schema]
    [metabase.lib.schema.expression :as lib.schema.expression]
-   [metabase.lib.schema.id :as lib.schema.id]
    [metabase.lib.util :as lib.util]
    [metabase.mbql.normalize :as mbql.normalize]
    [metabase.shared.util.i18n :as i18n]
@@ -71,26 +70,11 @@
         (lib.metadata.calculation/display-name query stage-number metric-metadata style))
       (fallback-display-name)))
 
-(mu/defn ^:private aggregating-by-metric? :- :boolean
-  "Whether a given stage of a query currently includes a `:metric` ref clause in its aggregations."
-  [query        :- ::lib.schema/query
-   stage-number :- :int
-   metric-id    :- ::lib.schema.id/metric]
-  (let [{aggregations :aggregation} (lib.util/query-stage query stage-number)]
-    (boolean
-     (some (fn [[tag :as clause]]
-             (when (= tag :metric)
-               (let [[_tag _opts id] clause]
-                 (= id metric-id))))
-           aggregations))))
-
 (defmethod lib.metadata.calculation/display-info-method :metadata/metric
-  [query stage-number {:keys [id description], :as metric-metadata}]
+  [query stage-number metric-metadata]
   (merge
    ((get-method lib.metadata.calculation/display-info-method :default) query stage-number metric-metadata)
-   {:description description}
-   (when (aggregating-by-metric? query stage-number id)
-     {:selected true})))
+   (select-keys metric-metadata [:description :aggregation-position])))
 
 (defmethod lib.metadata.calculation/display-info-method :metric
   [query stage-number [_tag _opts metric-id-or-name]]
@@ -109,6 +93,22 @@
 (mu/defn available-metrics :- [:maybe [:sequential {:min 1} lib.metadata/MetricMetadata]]
   "Get a list of Metrics that you may consider using as aggregations for a query. Only Metrics that have the same
   `table-id` as the `source-table` for this query will be suggested."
-  [query :- ::lib.schema/query]
-  (when-let [source-table-id (lib.util/source-table-id query)]
-    (not-empty (lib.metadata.protocols/metrics (lib.metadata/->metadata-provider query) source-table-id))))
+  ([query]
+   (available-metrics query -1))
+  ([query :- ::lib.schema/query
+    stage-number :- :int]
+   (when-let [source-table-id (lib.util/source-table-id query)]
+     (let [metrics (lib.metadata.protocols/metrics (lib.metadata/->metadata-provider query) source-table-id)
+           metric-aggregations (into {}
+                                     (keep-indexed (fn [index aggregation-clause]
+                                                     (when (lib.util/clause-of-type? aggregation-clause :metric)
+                                                       [(get aggregation-clause 2) index])))
+                                     (:aggregation (lib.util/query-stage query stage-number)))]
+       (cond
+         (empty? metrics)             nil
+         (empty? metric-aggregations) (vec metrics)
+         :else                        (mapv (fn [metric-metadata]
+                                              (let [aggregation-pos (-> metric-metadata :id metric-aggregations)]
+                                                (cond-> metric-metadata
+                                                  aggregation-pos (assoc :aggregation-position aggregation-pos))))
+                                            metrics))))))

test/metabase/lib/metric_test.cljc

@@ -68,8 +68,7 @@
                      :display-name      "Sum of Cans"
                      :long-display-name "Sum of Cans"
                      :effective-type    :type/Integer
-                     :description       "Number of toucans plus number of pelicans"
-                     :selected          true}
+                     :description       "Number of toucans plus number of pelicans"}
                     (lib/display-info query-with-metric metric))
     metric-clause
     metric-metadata))
@@ -97,14 +96,28 @@
          (lib/type-of query-with-metric [:metric {} 1]))))
 
 (deftest ^:parallel available-metrics-test
-  (testing "Should return Metrics with the same Table ID as query's `:source-table`"
-    (is (=? [{:lib/type    :metadata/metric
-              :id          metric-id
-              :name        "Sum of Cans"
-              :table-id    (meta/id :venues)
-              :definition  metric-definition
-              :description "Number of toucans plus number of pelicans"}]
-            (lib/available-metrics (lib/query metadata-provider (meta/table-metadata :venues))))))
+  (let [expected-metric-metadata {:lib/type    :metadata/metric
+                                  :id          metric-id
+                                  :name        "Sum of Cans"
+                                  :table-id    (meta/id :venues)
+                                  :definition  metric-definition
+                                  :description "Number of toucans plus number of pelicans"}]
+    (testing "Should return Metrics with the same Table ID as query's `:source-table`"
+      (is (=? [expected-metric-metadata]
+              (lib/available-metrics (lib/query metadata-provider (meta/table-metadata :venues))))))
+    (testing "Should return the position in the list of aggregations"
+      (let [metrics (lib/available-metrics query-with-metric)]
+        (is (=? [(assoc expected-metric-metadata :aggregation-position 0)]
+                metrics))
+        (testing "Display info should contains aggregation-position"
+          (is (=? [{:name                 "sum_of_cans",
+                    :display-name         "Sum of Cans",
+                    :long-display-name    "Sum of Cans",
+                    :effective-type       :type/Integer,
+                    :description          "Number of toucans plus number of pelicans",
+                    :aggregation-position 0}]
+                  (map #(lib/display-info query-with-metric %)
+                       metrics)))))))
   (testing "query with different Table -- don't return Metrics"
     (is (nil? (lib/available-metrics (lib/query metadata-provider (meta/table-metadata :orders)))))))
 
@@ -130,8 +143,7 @@
                       :display-name      "Sum of Cans"
                       :long-display-name "Sum of Cans"
                       :effective-type    :type/Integer
-                      :description       "Number of toucans plus number of pelicans"
-                      :selected          true}]
+                      :description       "Number of toucans plus number of pelicans"}]
                     (map (partial lib/display-info query')
                          (lib/aggregations query'))))))))))