diff --git a/e2e/test/scenarios/embedding/embedding-downloads-questions.cy.spec.js b/e2e/test/scenarios/embedding/embedding-downloads-questions.cy.spec.js index 40fd5f0b5503bea44a384f3754c790389c09561b..4422503e59c930d9b05acad444ebf0f71c0ac445 100644 --- a/e2e/test/scenarios/embedding/embedding-downloads-questions.cy.spec.js +++ b/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?") diff --git a/e2e/test/scenarios/embedding/embedding-smoketests.cy.spec.js b/e2e/test/scenarios/embedding/embedding-smoketests.cy.spec.js index 2e5b79646661b621195d2a54045e62afd30f8860..3ffd4abe2419ba2872b49b24433e1f131234d84a 100644 --- a/e2e/test/scenarios/embedding/embedding-smoketests.cy.spec.js +++ b/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() { diff --git a/e2e/test/scenarios/sharing/reproductions/26988-embedding-dynamic-settings.cy.spec.js b/e2e/test/scenarios/sharing/reproductions/26988-embedding-dynamic-settings.cy.spec.js index aa768c7049edec7b43d88e774f1bbf87e48dda44..9c573eaccf6a56d629e62651851a75a450438fc5 100644 --- a/e2e/test/scenarios/sharing/reproductions/26988-embedding-dynamic-settings.cy.spec.js +++ b/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"); diff --git a/src/metabase/lib/metric.cljc b/src/metabase/lib/metric.cljc index 1af2b831973524acc41077a27d717a868662d5de..4ae3a618d37cae862bb84ec944b9b440e234eb38 100644 --- a/src/metabase/lib/metric.cljc +++ b/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))))))) diff --git a/test/metabase/lib/js_test.cljs b/test/metabase/lib/js_test.cljs index c806854b9630f1cda492efb7fa5fd375e7a6f9b5..3b367eecdb43fbe792daa4ca0c37fba9bbae99a3 100644 --- a/test/metabase/lib/js_test.cljs +++ b/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"