Add (legacy) expression conversion tests (#37826)

Part of #37173.

e2e/test/scenarios/embedding/embedding-downloads-questions.cy.spec.js

@@ -103,7 +103,10 @@ describeEE("scenarios > embedding > questions > downloads", () => {
       cy.get("@questionId").then(questionId => {
         visitQuestion(questionId);
 
-        openStaticEmbeddingModal({ activeTab: "appearance", acceptTerms: false });
+        openStaticEmbeddingModal({
+          activeTab: "appearance",
+          acceptTerms: false,
+        });
 
         cy.log("Disable downloads");
         cy.findByLabelText("Enable users to download data from this embed?")

e2e/test/scenarios/embedding/embedding-smoketests.cy.spec.js

@@ -340,7 +340,7 @@ function visitAndEnableSharing(object, acceptTerms = true) {
     visitDashboard(ORDERS_DASHBOARD_ID);
   }
 
-  openStaticEmbeddingModal({acceptTerms});
+  openStaticEmbeddingModal({ acceptTerms });
 }
 
 function sidebar() {

e2e/test/scenarios/sharing/reproductions/26988-embedding-dynamic-settings.cy.spec.js

@@ -37,7 +37,7 @@ describeEE("issue 26988", () => {
     openStaticEmbeddingModal({
       activeTab: "appearance",
       previewMode: "preview",
-      acceptTerms: false
+      acceptTerms: false,
     });
 
     cy.wait("@dashboard");

src/metabase/lib/metric.cljc

@@ -2,6 +2,7 @@
   "A Metric is a saved MBQL query stage snippet with EXACTLY ONE `:aggregation` and optionally a `:filter` (boolean)
   expression. Can be passed into the `:aggregation`s list."
   (:require
+   [metabase.lib.aggregation :as lib.aggregation]
    [metabase.lib.convert :as lib.convert]
    [metabase.lib.metadata :as lib.metadata]
    [metabase.lib.metadata.calculation :as lib.metadata.calculation]
@@ -99,17 +100,17 @@
     stage-number :- :int]
    (when (zero? (lib.util/canonical-stage-index query stage-number))
      (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)))))))
+       (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])))
+                                       (lib.aggregation/aggregations 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/js_test.cljs

@@ -190,8 +190,11 @@
       (testing "from pMBQL expression"
         (is (= (js->clj legacy-expr) (js->clj legacy-expr'))))
       (testing "from legacy filter"
-        (is (=? [:< {} [:field {} (meta/id :venues :price)] 100]
-                pmbql-filter)))
+        (let [filter-expr [:< {} [:field {} (meta/id :venues :price)] 100]]
+          (is (=? filter-expr pmbql-filter))
+          (testing "created expression can be used to add a filter to a query (#37173)"
+            (is (=? {:stages [{:filters [filter-expr]}]}
+                    (lib/filter query pmbql-filter))))))
       (testing "from pMBQL filter"
         (is (= (js->clj legacy-filter) (js->clj legacy-filter'))))))
   (testing "conversion drops aggregation-options (#36120)"
@@ -219,7 +222,30 @@
           legacy-expr-from-query (lib.js/legacy-expression-for-expression-clause query-with-expr 0 expr-from-query)
           named-expr (lib/with-expression-name expr "named")]
       (is (= legacy-expr expr legacy-expr' legacy-expr-from-query))
-      (is (= "named" (lib/display-name query named-expr))))))
+      (is (= "named" (lib/display-name query named-expr)))))
+  (testing "simple expressions can be converted properly (#37173)"
+    (let [query lib.tu/venues-query
+          legacy-expr #js ["+" 1 2]
+          expr (lib.js/expression-clause-for-legacy-expression query 0 legacy-expr)
+          legacy-expr' (lib.js/legacy-expression-for-expression-clause query 0 expr)
+          query-with-expr (lib/expression query 0 "expr" expr)
+          expr-from-query (first (lib/expressions query-with-expr 0))
+          legacy-expr-from-query (lib.js/legacy-expression-for-expression-clause query-with-expr 0 expr-from-query)]
+      (is (=? [:+ {} 1 2] expr))
+      (is (= (js->clj legacy-expr) (js->clj legacy-expr') (js->clj legacy-expr-from-query)))
+      (testing "created expression can be aggregated in a query (#37173)"
+        (is (=? {:stages [{:aggregation [[:+ {} 1 2]]}]}
+                (lib/aggregate query -1 expr))))
+      (testing "created expression can be added as an expression to a query (#37173)"
+        (is (=? {:stages [{:expressions [[:+ {:lib/expression-name "expr"} 1 2]]}]}
+                (lib/expression query -1 "expr" expr))))))
+  (testing "filters from queries can be converted to legacy clauses (#37173)"
+    (let [query (lib/filter lib.tu/venues-query (lib/< (meta/field-metadata :venues :price) 3))
+          expr (first (lib/filters query))
+          legacy-expr (lib.js/legacy-expression-for-expression-clause query 0 expr)
+          price-id (meta/id :venues :price)]
+      (is (=? [:< {} [:field {:base-type :type/Integer, :effective-type :type/Integer} price-id] 3] expr))
+      (is (= ["<" ["field" price-id {"base-type" "Integer"}] 3] (js->clj legacy-expr))))))
 
 (deftest ^:parallel filter-drill-details-test
   (testing ":value field on the filter drill"
@@ -275,12 +301,20 @@
               ["field" (meta/id :venues :longitude) {"base-type" "type/Float"}]
               ["field" (meta/id :venues :price) {"base-type" "type/Integer"}]]
              (->> query lib/returned-columns (map to-legacy-refs)))))
-    (testing "segment refs come without options"
-      (is (= [["segment" segment-id]]
-             (->> query lib/available-segments (map to-legacy-refs)))))
-    (testing "metric refs come without options"
-      (is (= [["metric" metric-id]]
-             (->> query lib/available-metrics (map to-legacy-refs)))))))
+    (let [legacy-refs (->> query lib/available-segments (map lib.js/legacy-ref))]
+      (testing "legacy segment refs come without options"
+        (is (= [["segment" segment-id]] (map array-checker legacy-refs))))
+      (testing "segment legacy ref can be converted to an expression and back (#37173)"
+        (let [segment-expr (lib.js/expression-clause-for-legacy-expression query -1 (first legacy-refs))]
+          (is (=? [:segment {} segment-id] segment-expr))
+          (is (= ["segment" segment-id] (js->clj (lib.js/legacy-expression-for-expression-clause query -1 segment-expr)))))))
+    (let [legacy-refs (->> query lib/available-metrics (map lib.js/legacy-ref))]
+      (testing "metric refs come without options"
+        (is (= [["metric" metric-id]] (map array-checker legacy-refs))))
+      (testing "metric legacy ref can be converted to an expression and back (#37173)"
+        (let [metric-expr (lib.js/expression-clause-for-legacy-expression query -1 (first legacy-refs))]
+          (is (=? [:metric {} metric-id] metric-expr))
+          (is (= ["metric" metric-id] (js->clj (lib.js/legacy-expression-for-expression-clause query -1 metric-expr)))))))))
 
 (deftest ^:parallel source-table-or-card-id-test
   (testing "returns the table-id as a number"