From f12601a5d02e375e696909dda096a01f7cadde15 Mon Sep 17 00:00:00 2001
From: metamben <103100869+metamben@users.noreply.github.com>
Date: Fri, 20 Oct 2023 16:36:32 +0300
Subject: [PATCH] Add aggregation-position to available-metrics (#34867)
---
src/metabase/lib/metric.cljc | 42 +++++++++++++++---------------
test/metabase/lib/metric_test.cljc | 36 ++++++++++++++++---------
2 files changed, 45 insertions(+), 33 deletions(-)
diff --git a/src/metabase/lib/metric.cljc b/src/metabase/lib/metric.cljc
index ebd788ae54c..674f54acae1 100644
--- a/src/metabase/lib/metric.cljc
+++ b/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))))))
diff --git a/test/metabase/lib/metric_test.cljc b/test/metabase/lib/metric_test.cljc
index da7afa42284..24ae80c2287 100644
--- a/test/metabase/lib/metric_test.cljc
+++ b/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'))))))))))
--
GitLab