From 9f94b7a60305e3ac699d77853d0316acc051b909 Mon Sep 17 00:00:00 2001
From: Ngoc Khuat <qn.khuat@gmail.com>
Date: Thu, 19 Jan 2023 19:17:18 +0700
Subject: [PATCH] Custom list for cards (#27403)
---
.clj-kondo/config.edn | 1 +
.../serialization/v2/extract_test.clj | 166 +++++++++++++---
.../serialization/v2/load_test.clj | 96 +++++++++-
src/metabase/api/card.clj | 68 +++++++
src/metabase/api/dashboard.clj | 38 +---
src/metabase/api/embed.clj | 73 +++++++-
src/metabase/api/field.clj | 2 +-
src/metabase/api/public.clj | 17 ++
src/metabase/models/card.clj | 47 +++--
src/metabase/models/dashboard.clj | 37 ++--
src/metabase/models/parameter_card.clj | 35 ++--
src/metabase/models/params.clj | 10 +-
src/metabase/models/params/card_values.clj | 11 +-
src/metabase/models/params/static_values.clj | 28 +++
src/metabase/models/serialization/base.clj | 65 +++----
src/metabase/models/serialization/util.clj | 27 ++-
test/metabase/api/card_test.clj | 146 +++++++++++++++
test/metabase/api/dashboard_test.clj | 101 +++-------
test/metabase/api/embed_test.clj | 52 +++++
test/metabase/api/public_test.clj | 177 +++++++++++++-----
test/metabase/models/card_test.clj | 82 +++++++-
test/metabase/models/dashboard_test.clj | 31 +++
22 files changed, 1027 insertions(+), 283 deletions(-)
create mode 100644 src/metabase/models/params/static_values.clj
diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
index 9bc9fb6452e..70ae863e0e2 100644
--- a/.clj-kondo/config.edn
+++ b/.clj-kondo/config.edn
@@ -451,6 +451,7 @@
metabase.api.common/defendpoint-async-schema hooks.metabase.api.common/defendpoint
metabase.api.common/defendpoint-schema hooks.metabase.api.common/defendpoint
metabase.api.dashboard-test/with-chain-filter-fixtures hooks.common/let-one-with-optional-value
+ metabase.api.card-test/with-card-param-values-fixtures hooks.common/let-one-with-optional-value
metabase.api.embed-test/do-response-formats hooks.common/with-two-bindings
metabase.api.embed-test/with-chain-filter-fixtures hooks.common/let-one-with-optional-value
metabase.api.embed-test/with-temp-card hooks.common/let-one-with-optional-value
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
index 69201fd9468..6b8519526b8 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/extract_test.clj
@@ -184,6 +184,18 @@
:collection_id dave-coll-id
:creator_id mark-id
:parameters []}]
+ Dashboard [{param-dash-id :id
+ param-dash :entity_id} {:name "Dave's Dash with parameters"
+ :collection_id dave-coll-id
+ :creator_id mark-id
+ :parameters [{:id "abc"
+ :type "category"
+ :name "CATEGORY"
+ :values_source_type "card"
+ ;; card_id is in a different collection with dashboard's collection
+ :values_source_config {:card_id c1-id
+ :value_field [:field field-id nil]}}]}]
+
DashboardCard [_ {:card_id c1-id
:dashboard_id dash-id
:parameter_mappings
@@ -378,37 +390,79 @@
[{:model "Collection" :id dave-coll-eid}]}
(set (serdes.base/serdes-dependencies ser)))))))
- (testing "collection filtering based on :user option"
- (testing "only unowned collections are returned with no user"
- (is (= ["Some Collection"]
- (->> (serdes.base/extract-all "Collection" {:collection-set #{coll-id}})
- (into [])
- (map :name)))))
- (testing "unowned collections and the personal one with a user"
- (is (= #{coll-eid mark-coll-eid}
- (->> {:collection-set (extract/collection-set-for-user mark-id)}
- (serdes.base/extract-all "Collection")
- (by-model "Collection"))))
- (is (= #{coll-eid dave-coll-eid}
- (->> {:collection-set (extract/collection-set-for-user dave-id)}
- (serdes.base/extract-all "Collection")
- (by-model "Collection"))))))
-
- (testing "dashboards are filtered based on :user"
- (testing "dashboards in unowned collections are always returned"
- (is (= #{dash-eid}
- (->> {:collection-set #{coll-id}}
- (serdes.base/extract-all "Dashboard")
- (by-model "Dashboard"))))
- (is (= #{dash-eid}
- (->> {:collection-set (extract/collection-set-for-user mark-id)}
- (serdes.base/extract-all "Dashboard")
- (by-model "Dashboard")))))
- (testing "dashboards in personal collections are returned for the :user"
- (is (= #{dash-eid other-dash}
- (->> {:collection-set (extract/collection-set-for-user dave-id)}
- (serdes.base/extract-all "Dashboard")
- (by-model "Dashboard")))))))))
+ (testing "Dashboards with parameters where the source is a card"
+ (let [ser (serdes.base/extract-one "Dashboard" {} (db/select-one 'Dashboard :id param-dash-id))]
+ (is (schema= {:parameters
+ (s/eq [{:id "abc"
+ :name "CATEGORY"
+ :type :category
+ :values_query_type "list"
+ :values_source_config {:card_id c1-eid
+ :value_field [:field
+ ["My Database" nil "Schemaless Table" "Some Field"]
+ nil]},
+ :values_source_type "card"}])
+ s/Keyword s/Any}
+ ser))
+ (is (= #{[{:model "Collection" :id dave-coll-eid}]
+ [{:model "Card" :id c1-eid}]
+ [{:model "Database", :id "My Database"}
+ {:model "Table", :id "Schemaless Table"}
+ {:model "Field", :id "Some Field"}]}
+ (set (serdes.base/serdes-dependencies ser))))))
+
+ (testing "Cards with parameters where the source is a card"
+ (let [ser (serdes.base/extract-one "Dashboard" {} (db/select-one 'Dashboard :id param-dash-id))]
+ (is (schema= {:parameters
+ (s/eq [{:id "abc"
+ :name "CATEGORY"
+ :type :category
+ :values_query_type "list"
+ :values_source_config {:card_id c1-eid
+ :value_field [:field
+ ["My Database" nil "Schemaless Table" "Some Field"]
+ nil]},
+ :values_source_type "card"}])
+ s/Keyword s/Any}
+ ser))
+ (is (= #{[{:model "Collection" :id dave-coll-eid}]
+ [{:model "Card" :id c1-eid}]
+ [{:model "Database", :id "My Database"}
+ {:model "Table", :id "Schemaless Table"}
+ {:model "Field", :id "Some Field"}]}
+ (set (serdes.base/serdes-dependencies ser))))))
+
+ (testing "collection filtering based on :user option"
+ (testing "only unowned collections are returned with no user"
+ (is (= ["Some Collection"]
+ (->> (serdes.base/extract-all "Collection" {:collection-set #{coll-id}})
+ (into [])
+ (map :name)))))
+ (testing "unowned collections and the personal one with a user"
+ (is (= #{coll-eid mark-coll-eid}
+ (->> {:collection-set (extract/collection-set-for-user mark-id)}
+ (serdes.base/extract-all "Collection")
+ (by-model "Collection"))))
+ (is (= #{coll-eid dave-coll-eid}
+ (->> {:collection-set (extract/collection-set-for-user dave-id)}
+ (serdes.base/extract-all "Collection")
+ (by-model "Collection"))))))
+
+ (testing "dashboards are filtered based on :user"
+ (testing "dashboards in unowned collections are always returned"
+ (is (= #{dash-eid}
+ (->> {:collection-set #{coll-id}}
+ (serdes.base/extract-all "Dashboard")
+ (by-model "Dashboard"))))
+ (is (= #{dash-eid}
+ (->> {:collection-set (extract/collection-set-for-user mark-id)}
+ (serdes.base/extract-all "Dashboard")
+ (by-model "Dashboard")))))
+ (testing "dashboards in personal collections are returned for the :user"
+ (is (= #{dash-eid other-dash param-dash}
+ (->> {:collection-set (extract/collection-set-for-user dave-id)}
+ (serdes.base/extract-all "Dashboard")
+ (by-model "Dashboard")))))))))
(deftest dimensions-test
(ts/with-empty-h2-app-db
@@ -894,6 +948,56 @@
[{:model "Dashboard" :id dash-eid}]}
(set (serdes.base/serdes-dependencies ser))))))))))
+(deftest cards-test
+ (ts/with-empty-h2-app-db
+ (ts/with-temp-dpc
+ [User [{mark-id :id} {:first_name "Mark"
+ :last_name "Knopfler"
+ :email "mark@direstrai.ts"}]
+ Database [{db-id :id} {:name "My Database"}]
+ Table [{table-id :id} {:name "Schemaless Table" :db_id db-id}]
+ Field [{field-id :id} {:name "A Field" :table_id table-id}]
+ Collection [{coll-id-1 :id} {:name "1st collection"}]
+ Collection [{coll-id-2 :id
+ coll-eid-2 :entity_id} {:name "2nd collection"}]
+
+ Card [{card-id-1 :id
+ card-eid-1 :entity_id} {:name "Source question"
+ :database_id db-id
+ :table_id table-id
+ :collection_id coll-id-1
+ :creator_id mark-id}]
+ Card [{card-id-2 :id} {:name "Card 2"
+ :database_id db-id
+ :table_id table-id
+ :collection_id coll-id-2
+ :creator_id mark-id
+ :parameters [{:id "abc"
+ :type "category"
+ :name "CATEGORY"
+ :values_source_type "card"
+ ;; card_id is in a different collection with dashboard's collection
+ :values_source_config {:card_id card-id-1
+ :value_field [:field field-id nil]}}]}]]
+ (testing "Cards with parameter's source is another question"
+ (let [ser (serdes.base/extract-one "Card" {} (db/select-one Card :id card-id-2))]
+ (is (= [{:id "abc",
+ :type :category,
+ :name "CATEGORY",
+ :values_source_type "card",
+ :values_source_config {:card_id card-eid-1,
+ :value_field [:field ["My Database" nil "Schemaless Table" "A Field"] nil]}}]
+ (:parameters ser)))
+ (is (= #{[{:model "Database" :id "My Database"}]
+ [{:model "Collection" :id coll-eid-2}]
+ [{:model "Database" :id "My Database"}
+ {:model "Table" :id "Schemaless Table"}]
+ [{:model "Card" :id card-eid-1}]
+ [{:model "Database" :id "My Database"}
+ {:model "Table" :id "Schemaless Table"}
+ {:model "Field" :id "A Field"}]}
+ (set (serdes.base/serdes-dependencies ser)))))))))
+
(deftest selective-serialization-basic-test
(ts/with-empty-h2-app-db
(ts/with-temp-dpc [User [{mark-id :id} {:first_name "Mark"
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
index 2acdf2b49f3..6fb5c4fa993 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/load_test.clj
@@ -201,7 +201,6 @@
table2d (atom nil)
field2d (atom nil)]
-
(ts/with-source-and-dest-dbs
(testing "serializing the original database, table, field and card"
(ts/with-source-db
@@ -940,3 +939,98 @@
:template-tags
(get "snippet: things")
:snippet-id)))))))))
+
+(deftest card-has-parameter-with-source-card-test
+ (let [db (atom nil)
+ table (atom nil)
+ field (atom nil)
+ card (atom nil)
+ card2 (atom nil)
+ extracted (atom nil)]
+ (testing "Card that has parameter with source card must be deserialization"
+ (ts/with-empty-h2-app-db
+ (reset! db (ts/create! Database :name "my-db"))
+ (reset! table (ts/create! Table :name "CUSTOMERS" :db_id (:id @db)))
+ (reset! field (ts/create! Field :name "NAME" :table_id (:id @table)))
+ (reset! card (ts/create! Card :name "source card"))
+ (reset! card2 (ts/create! Card
+ :name "the card"
+ :parameters [{:id "abc"
+ :type "category"
+ :name "CATEGORY"
+ :values_source_type "card"
+ ;; card_id is in a different collection with dashboard's collection
+ :values_source_config {:card_id (:id @card)
+ :value_field [:field (:id @field) nil]}}]))
+
+ (testing "on extraction"
+ (reset! extracted (serdes.base/extract-one "Card" {} @card2))
+ (is (= [{:id "abc",
+ :name "CATEGORY",
+ :type :category,
+ :values_source_config {:card_id (:entity_id @card),
+ :value_field [:field
+ ["my-db" nil "CUSTOMERS" "NAME"]
+ nil]},
+ :values_source_type "card"}]
+ (:parameters @extracted))))
+
+ (testing "when loading"
+ (let [new-eid (u/generate-nano-id)
+ ingestion (ingestion-in-memory [(assoc @extracted :entity_id new-eid)])]
+ (is (some? (serdes.load/load-metabase ingestion)))
+ (is (= {:card_id (:id @card),
+ :value_field [:field (:id @field) nil]}
+ (-> (db/select-one-field :parameters Card :entity_id new-eid)
+ first
+ :values_source_config)))))))))
+
+(deftest dashboard-has-parameter-with-source-card-test
+ (let [db (atom nil)
+ table (atom nil)
+ field (atom nil)
+ card (atom nil)
+ dash (atom nil)
+ coll (atom nil)
+ extracted (atom nil)]
+ (testing "Card that has parameter with source card must be deserialization"
+ (ts/with-empty-h2-app-db
+ (reset! db (ts/create! Database :name "my-db"))
+ (reset! coll (ts/create! Collection :name "pop! minis"))
+ (reset! table (ts/create! Table :name "CUSTOMERS" :db_id (:id @db)))
+ (reset! field (ts/create! Field :name "NAME" :table_id (:id @table)))
+ (reset! card (ts/create! Card :name "source card"))
+ (reset! dash (ts/create! Dashboard
+ :name "A dashboard"
+ :collection_id (:id @coll)
+ :parameters [{:id "abc"
+ :type "category"
+ :name "CATEGORY"
+ :values_source_type "card"
+ ;; card_id is in a different collection with dashboard's collection
+ :values_source_config {:card_id (:id @card)
+ :value_field [:field (:id @field) nil]}}]))
+
+
+ (testing "on extraction"
+ (reset! extracted (serdes.base/extract-one "Dashboard" {} @dash))
+ (is (= [{:id "abc",
+ :name "CATEGORY",
+ :type :category,
+ :values_query_type "list"
+ :values_source_config {:card_id (:entity_id @card),
+ :value_field [:field
+ ["my-db" nil "CUSTOMERS" "NAME"]
+ nil]},
+ :values_source_type "card"}]
+ (:parameters @extracted))))
+
+ (testing "when loading"
+ (let [new-eid (u/generate-nano-id)
+ ingestion (ingestion-in-memory [(assoc @extracted :entity_id new-eid)])]
+ (is (some? (serdes.load/load-metabase ingestion)))
+ (is (= {:card_id (:id @card),
+ :value_field [:field (:id @field) nil]}
+ (-> (db/select-one-field :parameters Dashboard :entity_id new-eid)
+ first
+ :values_source_config)))))))))
diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj
index cd6612560cd..2dcde4dd8ca 100644
--- a/src/metabase/api/card.clj
+++ b/src/metabase/api/card.clj
@@ -4,6 +4,7 @@
[cheshire.core :as json]
[clojure.core.async :as a]
[clojure.data :as data]
+ [clojure.string :as str]
[clojure.tools.logging :as log]
[clojure.walk :as walk]
[compojure.core :refer [DELETE GET POST PUT]]
@@ -11,17 +12,20 @@
[metabase.api.common :as api]
[metabase.api.common.validation :as validation]
[metabase.api.dataset :as api.dataset]
+ [metabase.api.field :as api.field]
[metabase.api.timeline :as api.timeline]
[metabase.db.query :as mdb.query]
[metabase.driver :as driver]
[metabase.email.messages :as messages]
[metabase.events :as events]
[metabase.mbql.normalize :as mbql.normalize]
+ [metabase.mbql.util :as mbql.u]
[metabase.models
:refer [Card
CardBookmark
Collection
Database
+ Field
PersistedInfo
Pulse
Table
@@ -29,6 +33,9 @@
[metabase.models.collection :as collection]
[metabase.models.interface :as mi]
[metabase.models.moderation-review :as moderation-review]
+ [metabase.models.params :as params]
+ [metabase.models.params.card-values :as params.card-values]
+ [metabase.models.params.static-values :as params.static-values]
[metabase.models.persisted-info :as persisted-info]
[metabase.models.pulse :as pulse]
[metabase.models.query :as query]
@@ -908,4 +915,65 @@ saved later when it is ready."
(persisted-info/mark-for-pruning! {:id (:id persisted-info)} "off")
api/generic-204-no-content)))
+(defn mapping->field-values
+ "Get param values for the \"old style\" parameters. This mimic's the api/dashboard version except we don't have
+ chain-filter issues or dashcards to worry about."
+ [card param query]
+ (when-let [field-clause (params/param-target->field-clause (:target param) card)]
+ (when-let [field-id (mbql.u/match-one field-clause [:field (id :guard integer?) _] id)]
+ (if (str/blank? query)
+ (api.field/check-perms-and-return-field-values field-id)
+ (let [field (api/check-404 (db/select-one Field :id field-id))]
+ ;; matching the output of the other params. [["Foo" "Foo"] ["Bar" "Bar"]] -> [["Foo"] ["Bar"]]. This shape
+ ;; is what the return-field-values returns above
+ {:values (map (comp vector first) (api.field/search-values field field query))
+ ;; assume there are more
+ :has_more_values true
+ :field_id field-id})))))
+
+(s/defn param-values
+ "Fetch values for a parameter.
+
+ The source of values could be:
+ - static-list: user defined values list
+ - card: values is result of running a card"
+ ([card param-key]
+ (param-values card param-key nil))
+
+ ([card :- su/Map
+ param-key :- su/NonBlankString
+ query :- (s/maybe su/NonBlankString)]
+ (let [param (get (m/index-by :id (:parameters card)) param-key)
+ source-type (:values_source_type param)]
+ (when-not param
+ (throw (ex-info (tru "Card does not have a parameter with the ID {0}" (pr-str param-key))
+ {:status-code 400})))
+ (case source-type
+ "static-list" (params.static-values/param->values param query)
+ "card" (params.card-values/param->values param query)
+ nil (mapping->field-values card param query)
+ (throw (ex-info (tru "Invalid values-source-type: {0}" (pr-str source-type))
+ {:values-source-type source-type
+ :status-code 400}))))))
+
+(api/defendpoint GET "/:card-id/params/:param-key/values"
+ "Fetch possible values of the parameter whose ID is `:param-key`.
+
+ ;; fetch values for Card 1 parameter 'abc' that are possible
+ GET /api/card/1/params/abc/values"
+ [card-id param-key]
+ (let [card (api/read-check Card card-id)]
+ (param-values card param-key)))
+
+(api/defendpoint GET "/:card-id/params/:param-key/search/:query"
+ "Fetch possible values of the parameter whose ID is `:param-key` that contain `:query`.
+
+ ;; fetch values for Card 1 parameter 'abc' that contain 'Orange';
+ GET /api/card/1/params/abc/search/Orange
+
+ Currently limited to first 1000 results."
+ [card-id param-key query]
+ (let [card (api/read-check Card card-id)]
+ (param-values card param-key query)))
+
(api/define-routes)
diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index 258622a0e73..5e46b92d475 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -3,7 +3,6 @@
(:require
[cheshire.core :as json]
[clojure.set :as set]
- [clojure.string :as str]
[clojure.tools.logging :as log]
[compojure.core :refer [DELETE GET POST PUT]]
[medley.core :as m]
@@ -27,6 +26,7 @@
[metabase.models.params :as params]
[metabase.models.params.card-values :as params.card-values]
[metabase.models.params.chain-filter :as chain-filter]
+ [metabase.models.params.static-values :as params.static-values]
[metabase.models.query :as query :refer [Query]]
[metabase.models.query.permissions :as query-perms]
[metabase.models.revision :as revision]
@@ -38,7 +38,6 @@
[metabase.query-processor.pivot :as qp.pivot]
[metabase.query-processor.util :as qp.util]
[metabase.related :as related]
- [metabase.search.util :as search]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
[metabase.util.schema :as su]
@@ -439,7 +438,7 @@
api/generic-204-no-content)
(defn- param-target->field-id [target query]
- (when-let [field-clause (params/param-target->field-clause target {:card {:dataset_query query}})]
+ (when-let [field-clause (params/param-target->field-clause target {:dataset_query query})]
(mbql.u/match-one field-clause [:field (id :guard integer?) _] id)))
;; TODO -- should we only check *new* or *modified* mappings?
@@ -685,7 +684,8 @@
(s/defn ^:private mappings->field-ids :- (s/maybe #{su/IntGreaterThanZero})
[parameter-mappings :- (s/maybe (s/cond-pre #{dashboard-card/ParamMapping} [dashboard-card/ParamMapping]))]
(set (for [param parameter-mappings
- :let [field-clause (params/param-target->field-clause (:target param) (:dashcard param))]
+ :let [field-clause (params/param-target->field-clause (:target param)
+ (-> param :dashcard :card))]
:when field-clause
:let [field-id (mbql.u/match-one field-clause [:field (id :guard integer?) _] id)]
:when field-id]
@@ -751,32 +751,6 @@
(api/throw-403 e)
(throw e)))))))
-(defn- query-matches
- "Filter the values according to the `search-term`.
-
- Values could have 2 shapes
- - [value1, value2]
- - [[value1, label1], [value2, label2]] - we search using label in this case"
- [query values]
- (let [normalized-query (search/normalize query)]
- (filter #(str/includes? (search/normalize (if (string? %)
- %
- ;; search by label
- (second %)))
- normalized-query) values)))
-
-(defn- static-parameter-values
- [{values-source-options :values_source_config :as _param} query]
- (when-let [values (:values values-source-options)]
- {:values (if query
- (query-matches query values)
- values)
- :has_more_values false}))
-
-(defn- card-parameter-values
- [{config :values_source_config :as _param} query]
- (params.card-values/values-from-card (:card_id config) (:value_field config) query))
-
(s/defn param-values
"Fetch values for a parameter.
@@ -798,8 +772,8 @@
{:resolved-params (keys (:resolved-params dashboard))
:status-code 400})))
(case (:values_source_type param)
- "static-list" (static-parameter-values param query)
- "card" (card-parameter-values param query)
+ "static-list" (params.static-values/param->values param query)
+ "card" (params.card-values/param->values param query)
nil (chain-filter dashboard param-key constraint-param-key->value query)))))
#_{:clj-kondo/ignore [:deprecated-var]}
diff --git a/src/metabase/api/embed.clj b/src/metabase/api/embed.clj
index a0df238431c..a263c76aee7 100644
--- a/src/metabase/api/embed.clj
+++ b/src/metabase/api/embed.clj
@@ -20,6 +20,7 @@
[clojure.tools.logging :as log]
[compojure.core :refer [GET]]
[medley.core :as m]
+ [metabase.api.card :as api.card]
[metabase.api.common :as api]
[metabase.api.common.validation :as validation]
[metabase.api.dashboard :as api.dashboard]
@@ -552,7 +553,48 @@
(into {} (for [[slug value] merged-slug->value]
[(get slug->id (name slug)) value]))))
-(defn- param-values [token searched-param-id prefix id-query-params]
+(defn card-param-values
+ "Search for card parameter values. Does security checks to ensure the parameter is on the card and then gets param
+ values according to [[api.card/param-values]]."
+ [{:keys [unsigned-token card param-key search-prefix]}]
+ (let [slug-token-params (embed/get-in-unsigned-token-or-throw unsigned-token [:params])
+ id->slug (into {} (map (juxt :id :slug) (:parameters card)))
+ slug->id (into {} (map (juxt :slug :id) (:parameters card)))
+ searched-param-slug (get id->slug param-key)
+ embedding-params (:embedding_params card)]
+ (try
+ (when-not (= (get embedding-params (keyword searched-param-slug)) "enabled")
+ (throw (ex-info (tru "Cannot search for values: {0} is not an enabled parameter."
+ (pr-str searched-param-slug))
+ {:status-code 400})))
+ (when (get slug-token-params (keyword searched-param-slug))
+ (throw (ex-info (tru "You can''t specify a value for {0} if it's already set in the JWT." (pr-str searched-param-slug))
+ {:status-code 400})))
+ (try
+ (binding [api/*current-user-permissions-set* (atom #{"/"})]
+ (api.card/param-values card param-key search-prefix))
+ (catch Throwable e
+ (throw (ex-info (.getMessage e)
+ {:card-id (u/the-id card)
+ :param-key param-key
+ :search-prefix search-prefix}
+ e))))
+ (catch Throwable e
+ (let [e (ex-info (.getMessage e)
+ {:card-id (u/the-id card)
+ :card-params (:parametres card)
+ :allowed-param-slugs embedding-params
+ :slug->id slug->id
+ :id->slug id->slug
+ :param-id param-key
+ :param-slug searched-param-slug
+ :token-params slug-token-params}
+ e)]
+ (log/errorf e "embedded card-param-values error\n%s"
+ (u/pprint-to-str (u/all-ex-data e)))
+ (throw e))))))
+
+(defn- dashboard-param-values [token searched-param-id prefix id-query-params]
(let [unsigned-token (embed/unsign token)
dashboard-id (embed/get-in-unsigned-token-or-throw unsigned-token [:resource :dashboard])
_ (check-embedding-enabled-for-dashboard dashboard-id)
@@ -597,13 +639,38 @@
(api/defendpoint-schema GET "/dashboard/:token/params/:param-key/values"
"Embedded version of chain filter values endpoint."
[token param-key :as {:keys [query-params]}]
- (param-values token param-key nil query-params))
+ (dashboard-param-values token param-key nil query-params))
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema GET "/dashboard/:token/params/:param-key/search/:prefix"
"Embedded version of chain filter search endpoint."
[token param-key prefix :as {:keys [query-params]}]
- (param-values token param-key prefix query-params))
+ (dashboard-param-values token param-key prefix query-params))
+
+#_{:clj-kondo/ignore [:deprecated-var]}
+(api/defendpoint-schema GET "/card/:token/params/:param-key/values"
+ "Embedded version of api.card filter values endpoint."
+ [token param-key]
+ (let [unsigned (embed/unsign token)
+ card-id (embed/get-in-unsigned-token-or-throw unsigned [:resource :question])
+ card (db/select-one Card :id card-id)]
+ (check-embedding-enabled-for-card card-id)
+ (card-param-values {:unsigned-token unsigned
+ :card card
+ :param-key param-key})))
+
+#_{:clj-kondo/ignore [:deprecated-var]}
+(api/defendpoint-schema GET "/card/:token/params/:param-key/search/:prefix"
+ "Embedded version of chain filter search endpoint."
+ [token param-key prefix]
+ (let [unsigned (embed/unsign token)
+ card-id (embed/get-in-unsigned-token-or-throw unsigned [:resource :question])
+ card (db/select-one Card :id card-id)]
+ (check-embedding-enabled-for-card card-id)
+ (card-param-values {:unsigned-token unsigned
+ :card card
+ :param-key param-key
+ :search-prefix prefix})))
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema ^:streaming GET "/pivot/card/:token/query"
diff --git a/src/metabase/api/field.clj b/src/metabase/api/field.clj
index d41e539ecfb..539972343f2 100644
--- a/src/metabase/api/field.clj
+++ b/src/metabase/api/field.clj
@@ -219,7 +219,7 @@
:has_more_values has_more_values}
(params.field-values/get-or-create-field-values-for-current-user! (api/check-404 field))))
-(defn- check-perms-and-return-field-values
+(defn check-perms-and-return-field-values
"Impl for `GET /api/field/:id/values` endpoint; check whether current user has read perms for Field with `id`, and, if
so, return its values."
[field-id]
diff --git a/src/metabase/api/public.clj b/src/metabase/api/public.clj
index 34b02aa71c9..82207edf420 100644
--- a/src/metabase/api/public.clj
+++ b/src/metabase/api/public.clj
@@ -6,6 +6,7 @@
[compojure.core :refer [GET]]
[medley.core :as m]
[metabase.actions.execution :as actions.execution]
+ [metabase.api.card :as api.card]
[metabase.api.common :as api]
[metabase.api.common.validation :as validation]
[metabase.api.dashboard :as api.dashboard]
@@ -470,6 +471,22 @@
;;; ------------------------------------------------ Param Values -------------------------------------------------
+#_{:clj-kondo/ignore [:deprecated-var]}
+(api/defendpoint-schema GET "/card/:uuid/params/:param-key/values"
+ "Fetch values for a parameter on a public card."
+ [uuid param-key]
+ (validation/check-public-sharing-enabled)
+ (let [card (db/select-one Card :public_uuid uuid, :archived false)]
+ (api.card/param-values card param-key)))
+
+#_{:clj-kondo/ignore [:deprecated-var]}
+(api/defendpoint-schema GET "/card/:uuid/params/:param-key/search/:query"
+ "Fetch values for a parameter on a public card containing `query`."
+ [uuid param-key query]
+ (validation/check-public-sharing-enabled)
+ (let [card (db/select-one Card :public_uuid uuid, :archived false)]
+ (api.card/param-values card param-key query)))
+
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema GET "/dashboard/:uuid/params/:param-key/values"
"Fetch filter values for dashboard parameter `param-key`."
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index 301d77c2959..e8803ee60da 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -11,6 +11,7 @@
[metabase.models.collection :as collection]
[metabase.models.field-values :as field-values]
[metabase.models.interface :as mi]
+ [metabase.models.parameter-card :as parameter-card :refer [ParameterCard]]
[metabase.models.params :as params]
[metabase.models.permissions :as perms]
[metabase.models.query :as query]
@@ -46,7 +47,7 @@
"Return the number of dashboard/card filters and other widgets that use this card to populate their available
values (via ParameterCards)"
[{:keys [id]}]
- (db/count 'ParameterCard, :card_id id))
+ (db/count ParameterCard, :card_id id))
(mi/define-simple-hydration-method average-query-time
:average_query_time
@@ -223,15 +224,15 @@
(let [defaults {:parameters []
:parameter_mappings []}
card (merge defaults card)]
- (u/prog1 card
- ;; make sure this Card doesn't have circular source query references
- (check-for-circular-source-query-references card)
- (check-field-filter-fields-are-from-correct-database card)
- ;; TODO: add a check to see if all id in :parameter_mappings are in :parameters
- (assert-valid-model card)
- (params/assert-valid-parameters card)
- (params/assert-valid-parameter-mappings card)
- (collection/check-collection-namespace Card (:collection_id card)))))
+ (u/prog1 card
+ ;; make sure this Card doesn't have circular source query references
+ (check-for-circular-source-query-references card)
+ (check-field-filter-fields-are-from-correct-database card)
+ ;; TODO: add a check to see if all id in :parameter_mappings are in :parameters
+ (assert-valid-model card)
+ (params/assert-valid-parameters card)
+ (params/assert-valid-parameter-mappings card)
+ (collection/check-collection-namespace Card (:collection_id card)))))
(defn- post-insert [card]
;; if this Card has any native template tag parameters we need to update FieldValues for any Fields that are
@@ -239,7 +240,8 @@
(u/prog1 card
(when-let [field-ids (seq (params/card->template-tag-field-ids card))]
(log/info "Card references Fields in params:" field-ids)
- (field-values/update-field-values-for-on-demand-dbs! field-ids))))
+ (field-values/update-field-values-for-on-demand-dbs! field-ids))
+ (parameter-card/upsert-or-delete-from-parameters! "card" (:id card) (:parameters card))))
(defonce
^{:doc "Atom containing a function used to check additional sandboxing constraints for Metabase Enterprise Edition.
@@ -291,12 +293,17 @@
(collection/check-collection-namespace Card (:collection_id changes))
(params/assert-valid-parameters changes)
(params/assert-valid-parameter-mappings changes)
+ (parameter-card/upsert-or-delete-from-parameters! "card" id (:parameters changes))
;; additional checks (Enterprise Edition only)
(@pre-update-check-sandbox-constraints changes)
(assert-valid-model (merge old-card-info changes)))))
;; Cards don't normally get deleted (they get archived instead) so this mostly affects tests
(defn- pre-delete [{:keys [id]}]
+ ;; delete any ParameterCard that the parameters on this card linked to
+ (parameter-card/delete-all-for-parameterized-object! "card" id)
+ ;; delete any ParameterCard linked to this card
+ (db/delete! ParameterCard :card_id id)
(db/delete! 'ModerationReview :moderated_item_type "card", :moderated_item_id id)
(db/delete! 'Revision :model "Card", :model_id id))
@@ -377,6 +384,7 @@
(update :creator_id serdes.util/export-user)
(update :made_public_by_id serdes.util/export-user)
(update :dataset_query serdes.util/export-mbql)
+ (update :parameters serdes.util/export-parameters)
(update :parameter_mappings serdes.util/export-parameter-mappings)
(update :visualization_settings serdes.util/export-visualization-settings)
(update :result_metadata export-result-metadata))
@@ -393,14 +401,17 @@
(update :made_public_by_id serdes.util/import-user)
(update :collection_id serdes.util/import-fk 'Collection)
(update :dataset_query serdes.util/import-mbql)
+ (update :parameters serdes.util/import-parameters)
(update :parameter_mappings serdes.util/import-parameter-mappings)
(update :visualization_settings serdes.util/import-visualization-settings)
(update :result_metadata import-result-metadata)))
(defmethod serdes.base/serdes-dependencies "Card"
- [{:keys [collection_id database_id dataset_query parameter_mappings result_metadata table_id visualization_settings]}]
+ [{:keys [collection_id database_id dataset_query parameters parameter_mappings
+ result_metadata table_id visualization_settings]}]
(->> (map serdes.util/mbql-deps parameter_mappings)
(reduce set/union)
+ (set/union (serdes.util/parameters-deps parameters))
(set/union #{[{:model "Database" :id database_id}]})
; table_id and collection_id are nullable.
(set/union (when table_id #{(serdes.util/table->path table_id)}))
@@ -411,10 +422,11 @@
vec))
(defmethod serdes.base/serdes-descendants "Card" [_model-name id]
- (let [card (db/select-one Card :id id)
- source-table (some-> card :dataset_query :query :source-table)
- template-tags (some->> card :dataset_query :native :template-tags vals (keep :card-id))
- snippets (some->> card :dataset_query :native :template-tags vals (keep :snippet-id))]
+ (let [card (db/select-one Card :id id)
+ source-table (some-> card :dataset_query :query :source-table)
+ template-tags (some->> card :dataset_query :native :template-tags vals (keep :card-id))
+ parameters-card-id (some->> card :parameters (keep (comp :card_id :values_source_config)))
+ snippets (some->> card :dataset_query :native :template-tags vals (keep :snippet-id))]
(set/union
(when (and (string? source-table)
(str/starts-with? source-table "card__"))
@@ -422,6 +434,9 @@
(when (seq template-tags)
(set (for [card-id template-tags]
["Card" card-id])))
+ (when (seq parameters-card-id)
+ (set (for [card-id parameters-card-id]
+ ["Card" card-id])))
(when (seq snippets)
(set (for [snippet-id snippets]
["NativeQuerySnippet" snippet-id]))))))
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index e40f0a0e49f..43c27e87419 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -91,12 +91,12 @@
(defn- post-insert
[dashboard]
(u/prog1 dashboard
- (parameter-card/upsert-or-delete-for-dashboard! dashboard)))
+ (parameter-card/upsert-or-delete-from-parameters! "dashboard" (:id dashboard) (:parameters dashboard))))
(defn- pre-update [dashboard]
(u/prog1 dashboard
(params/assert-valid-parameters dashboard)
- (parameter-card/upsert-or-delete-for-dashboard! dashboard)
+ (parameter-card/upsert-or-delete-from-parameters! "dashboard" (:id dashboard) (:parameters dashboard))
(collection/check-collection-namespace Dashboard (:collection_id dashboard))))
(defn- update-dashboard-subscription-pulses!
@@ -456,6 +456,7 @@
(hydrate dash :ordered_cards))]
(-> (serdes.base/extract-one-basics "Dashboard" dash)
(update :ordered_cards #(mapv extract-dashcard %))
+ (update :parameters serdes.util/export-parameters)
(update :collection_id serdes.util/export-fk Collection)
(update :creator_id serdes.util/export-user)
(update :made_public_by_id serdes.util/export-user))))
@@ -466,6 +467,7 @@
serdes.base/load-xform-basics
;; Deliberately not doing anything to :ordered_cards - they get handled by load-insert! and load-update! below.
(update :collection_id serdes.util/import-fk Collection)
+ (update :parameters serdes.util/import-parameters)
(update :creator_id serdes.util/import-user)
(update :made_public_by_id serdes.util/import-user)))
@@ -490,19 +492,26 @@
set))
(defmethod serdes.base/serdes-dependencies "Dashboard"
- [{:keys [collection_id ordered_cards]}]
- (->> ordered_cards
- (map serdes-deps-dashcard)
- (reduce set/union #{[{:model "Collection" :id collection_id}]})))
+ [{:keys [collection_id ordered_cards parameters]}]
+ (->> (map serdes-deps-dashcard ordered_cards)
+ (reduce set/union)
+ (set/union #{[{:model "Collection" :id collection_id}]})
+ (set/union (serdes.util/parameters-deps parameters))))
(defmethod serdes.base/serdes-descendants "Dashboard" [_model-name id]
- ;; DashboardCards are inlined into Dashboards, but we need to capture what those those DashboardCards rely on
- ;; here. So their cards, both direct and mentioned in their parameters.
- (set (for [{:keys [card_id parameter_mappings]} (db/select ['DashboardCard :card_id :parameter_mappings]
- :dashboard_id id)
- ;; Capture all card_ids in the parameters, plus this dashcard's card_id if non-nil.
- card-id (cond-> (set (keep :card_id parameter_mappings))
- card_id (conj card_id))]
- ["Card" card-id])))
+ (let [dashcards (db/select ['DashboardCard :card_id :parameter_mappings]
+ :dashboard_id id)
+ dashboard (db/select-one Dashboard :id id)]
+ (set/union
+ ;; DashboardCards are inlined into Dashboards, but we need to capture what those those DashboardCards rely on
+ ;; here. So their cards, both direct and mentioned in their parameters.
+ (set (for [{:keys [card_id parameter_mappings]} dashcards
+ ;; Capture all card_ids in the parameters, plus this dashcard's card_id if non-nil.
+ card-id (cond-> (set (keep :card_id parameter_mappings))
+ card_id (conj card_id))]
+ ["Card" card-id]))
+ ;; parameter with values_source_type = "card" will depend on a card
+ (set (for [card-id (some->> dashboard :parameters (keep (comp :card_id :values_source_config)))]
+ ["Card" card-id])))))
(serdes.base/register-ingestion-path! "Dashboard" (serdes.base/ingestion-matcher-collected "collections" "Dashboard"))
diff --git a/src/metabase/models/parameter_card.clj b/src/metabase/models/parameter_card.clj
index 166a297b518..54b65ddedb0 100644
--- a/src/metabase/models/parameter_card.clj
+++ b/src/metabase/models/parameter_card.clj
@@ -3,19 +3,21 @@
[metabase.models.interface :as mi]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
[toucan.db :as db]
[toucan.models :as models]))
(models/defmodel ParameterCard :parameter_card)
(defonce ^{:doc "Set of valid parameterized_object_type for a ParameterCard"}
- valid-parameterized_object_type #{"dashboard"})
+ valid-parameterized-object-type #{"dashboard" "card"})
(defn- validate-parameterized-object-type
[{:keys [parameterized_object_type] :as _parameter-card}]
- (when-not (valid-parameterized_object_type parameterized_object_type)
+ (when-not (valid-parameterized-object-type parameterized_object_type)
(throw (ex-info (tru "invalid parameterized_object_type")
- {:allowed-types valid-parameterized_object_type}))))
+ {:allowed-types valid-parameterized-object-type}))))
;;; ----------------------------------------------- Entity & Lifecycle -----------------------------------------------
@@ -45,27 +47,30 @@
([parameterized-object-type parameterized-object-id parameter-ids-still-in-use]
(db/delete! ParameterCard
:parameterized_object_type parameterized-object-type
- :parameterized_object_id parameterized-object-id
+ :parameterized_object_id parameterized-object-id
(if (empty? parameter-ids-still-in-use)
- {}
- {:where [:not-in :parameter_id parameter-ids-still-in-use]}))))
+ {}
+ {:where [:not-in :parameter_id parameter-ids-still-in-use]}))))
-(defn- upsert-for-dashboard!
- [dashboard-id parameters]
+(defn- upsert-from-parameters!
+ [parameterized-object-type parameterized-object-id parameters]
(doseq [{:keys [values_source_config id]} parameters]
(let [card-id (:card_id values_source_config)
- conditions {:parameterized_object_id dashboard-id
- :parameterized_object_type "dashboard"
+ conditions {:parameterized_object_id parameterized-object-id
+ :parameterized_object_type parameterized-object-type
:parameter_id id}]
(or (db/update-where! ParameterCard conditions :card_id card-id)
(db/insert! ParameterCard (merge conditions {:card_id card-id}))))))
-(defn upsert-or-delete-for-dashboard!
- "Create, update, or delete appropriate ParameterCards for each parameter in the dashboard"
- [{dashboard-id :id parameters :parameters}]
+(mu/defn upsert-or-delete-from-parameters!
+ "From a parameters list on card or dashboard, create, update,
+ or delete appropriate ParameterCards for each parameter in the dashboard"
+ [parameterized-object-type :- ms/NonBlankString
+ parameterized-object-id :- ms/IntGreaterThanZero
+ parameters :- [:maybe [:sequential ms/Parameter]]]
(let [upsertable? (fn [{:keys [values_source_type values_source_config id]}]
(and values_source_type id (:card_id values_source_config)
(= values_source_type "card")))
upsertable-parameters (filter upsertable? parameters)]
- (upsert-for-dashboard! dashboard-id upsertable-parameters)
- (delete-all-for-parameterized-object! "dashboard" dashboard-id (map :id upsertable-parameters))))
+ (upsert-from-parameters! parameterized-object-type parameterized-object-id upsertable-parameters)
+ (delete-all-for-parameterized-object! parameterized-object-type parameterized-object-id (map :id upsertable-parameters))))
diff --git a/src/metabase/models/params.clj b/src/metabase/models/params.clj
index 09b07f30b53..bb6df696bba 100644
--- a/src/metabase/models/params.clj
+++ b/src/metabase/models/params.clj
@@ -61,20 +61,20 @@
"Fetch the `:field` clause from `dashcard` referenced by `template-tag`.
(template-tag->field-form [:template-tag :company] some-dashcard) ; -> [:field 100 nil]"
- [[_ tag] dashcard]
- (get-in dashcard [:card :dataset_query :native :template-tags (u/qualified-name tag) :dimension]))
+ [[_ tag] card]
+ (get-in card [:dataset_query :native :template-tags (u/qualified-name tag) :dimension]))
(s/defn param-target->field-clause :- (s/maybe mbql.s/field)
"Parse a Card parameter `target` form, which looks something like `[:dimension [:field-id 100]]`, and return the Field
ID it references (if any)."
- [target dashcard]
+ [target card]
(let [target (mbql.normalize/normalize-tokens target :ignore-path)]
(when (mbql.u/is-clause? :dimension target)
(let [[_ dimension] target]
(try
(unwrap-field-clause
(if (mbql.u/is-clause? :template-tag dimension)
- (template-tag->field-form dimension dashcard)
+ (template-tag->field-form dimension card)
dimension))
(catch Throwable e
(log/error e (tru "Could not find matching Field ID for target:") target)))))))
@@ -182,7 +182,7 @@
[dashboard]
(when-let [fields (seq (for [dashcard (:ordered_cards dashboard)
param (:parameter_mappings dashcard)
- :let [field-clause (param-target->field-clause (:target param) dashcard)]
+ :let [field-clause (param-target->field-clause (:target param) (:card dashcard))]
:when field-clause]
field-clause))]
(set fields)))
diff --git a/src/metabase/models/params/card_values.clj b/src/metabase/models/params/card_values.clj
index ba2b51a0f84..de0321e0183 100644
--- a/src/metabase/models/params/card_values.clj
+++ b/src/metabase/models/params/card_values.clj
@@ -10,9 +10,9 @@
(def ^:dynamic *max-rows*
"Maximum number of rows returned when running a card.
- It's 2000 because this is the limit we use when viewing a question.
- Maybe we should lower it for the sake of display a parameter dropdown."
- 2000)
+ It's 1000 because it matches with the limit for chain-filter.
+ Maybe we should lower it for the sake of displaying a parameter dropdown."
+ 1000)
(defn- values-from-card-query
[card-id value-field query]
@@ -52,3 +52,8 @@
{:values (map first (get-in result [:data :rows]))
;; if the row_count returned = the limit we specified, then it's probably has more than that
:has_more_values (= (:row_count result) query-limit)})))
+
+(defn param->values
+ "Given a param and query returns the values."
+ [{config :values_source_config :as _param} query]
+ (values-from-card (:card_id config) (:value_field config) query))
diff --git a/src/metabase/models/params/static_values.clj b/src/metabase/models/params/static_values.clj
new file mode 100644
index 00000000000..a172868ae2f
--- /dev/null
+++ b/src/metabase/models/params/static_values.clj
@@ -0,0 +1,28 @@
+(ns metabase.models.params.static-values
+ "Code related to getting values for a parameter where its source values is a card."
+ (:require
+ [clojure.string :as str]
+ [metabase.search.util :as search]))
+
+(defn- query-matches
+ "Filter the values according to the `search-term`.
+
+ Values could have 2 shapes
+ - [value1, value2]
+ - [[value1, label1], [value2, label2]] - we search using label in this case"
+ [query values]
+ (let [normalized-query (search/normalize query)]
+ (filter #(str/includes? (search/normalize (if (string? %)
+ %
+ ;; search by label
+ (second %)))
+ normalized-query) values)))
+
+(defn param->values
+ "Given a param return the values"
+ [{values-source-options :values_source_config :as _param} query]
+ (when-let [values (:values values-source-options)]
+ {:values (if query
+ (query-matches query values)
+ values)
+ :has_more_values false}))
diff --git a/src/metabase/models/serialization/base.clj b/src/metabase/models/serialization/base.clj
index 61a7032c960..486f2cce981 100644
--- a/src/metabase/models/serialization/base.clj
+++ b/src/metabase/models/serialization/base.clj
@@ -264,6 +264,39 @@
(defmethod extract-one :default [model-name _opts entity]
(extract-one-basics model-name entity))
+(defmulti serdes-descendants
+ "Captures the notion that eg. a dashboard \"contains\" its cards.
+ Returns a set, possibly empty or nil, of `[model-name database-id]` pairs for all entities that this entity contains
+ or requires to be executed.
+ Dispatches on the model name.
+
+ For example:
+ - a `Collection` contains 0 or more other `Collection`s plus many `Card`s and `Dashboard`s;
+ - a `Dashboard` contains its `DashboardCard`s;
+ - each `DashboardCard` contains its `Card`;
+ - a `Card` might stand alone, or it might require `NativeQuerySnippet`s or other `Card`s as inputs; and
+ - a `NativeQuerySnippet` similarly might derive from others;
+
+ A transitive closure over [[serdes-descendants]] should thus give a complete \"subtree\", such as a complete
+ `Collection` and all its contents.
+
+ A typical implementation will run a query or two to collect eg. all `DashboardCard`s that are part of this
+ `Dashboard`, and return them as pairs like `[\"DashboardCard\" 17]`.
+
+ What about [[serdes-dependencies]]?
+ Despite the similar-sounding names, this differs crucially from [[serdes-dependencies]]. [[serdes-descendants]] finds
+ all entities that are \"part\" of the given entity.
+
+ [[serdes-dependencies]] finds all entities that need to be loaded into appdb before this one can be, generally because
+ this has a foreign key to them. The arrow \"points the other way\": [[serdes-dependencies]] points *up* -- from a
+ `Dashboard` to its containing `Collection`, `Collection` to its parent, from a `DashboardCard` to its `Dashboard` and
+ `Card`. [[serdes-descendants]] points *down* to contents, children, and components."
+ {:arglists '([model-name db-id])}
+ (fn [model-name _] model-name))
+
+(defmethod serdes-descendants :default [_ _]
+ nil)
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Deserialization Process |
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -446,38 +479,6 @@
(load-insert! model adjusted)
(load-update! model adjusted maybe-local)))))
-(defmulti serdes-descendants
- "Captures the notion that eg. a dashboard \"contains\" its cards.
- Returns a set, possibly empty or nil, of `[model-name database-id]` pairs for all entities that this entity contains
- or requires to be executed.
- Dispatches on the model name.
-
- For example:
- - a `Collection` contains 0 or more other `Collection`s plus many `Card`s and `Dashboard`s;
- - a `Dashboard` contains its `DashboardCard`s;
- - each `DashboardCard` contains its `Card`;
- - a `Card` might stand alone, or it might require `NativeQuerySnippet`s or other `Card`s as inputs; and
- - a `NativeQuerySnippet` similarly might derive from others;
-
- A transitive closure over [[serdes-descendants]] should thus give a complete \"subtree\", such as a complete
- `Collection` and all its contents.
-
- A typical implementation will run a query or two to collect eg. all `DashboardCard`s that are part of this
- `Dashboard`, and return them as pairs like `[\"DashboardCard\" 17]`.
-
- What about [[serdes-dependencies]]?
- Despite the similar-sounding names, this differs crucially from [[serdes-dependencies]]. [[serdes-descendants]] finds
- all entities that are \"part\" of the given entity.
-
- [[serdes-dependencies]] finds all entities that need to be loaded into appdb before this one can be, generally because
- this has a foreign key to them. The arrow \"points the other way\": [[serdes-dependencies]] points *up* -- from a
- `Dashboard` to its containing `Collection`, `Collection` to its parent, from a `DashboardCard` to its `Dashboard` and
- `Card`. [[serdes-descendants]] points *down* to contents, children, and components."
- {:arglists '([model-name db-id])}
- (fn [model-name _] model-name))
-
-(defmethod serdes-descendants :default [_ _]
- nil)
(defn entity-id?
"Checks if the given string is a 21-character NanoID. Useful for telling entity IDs apart from identity hashes."
diff --git a/src/metabase/models/serialization/util.clj b/src/metabase/models/serialization/util.clj
index 6a5ebe5bc8b..aa69adc0417 100644
--- a/src/metabase/models/serialization/util.clj
+++ b/src/metabase/models/serialization/util.clj
@@ -254,7 +254,6 @@
:database :card_id :card-id :source-table :breakout :aggregation :filter :segment
::mb.viz/param-mapping-source :snippet-id))))))
-;(ids->fully-qualified-names {:aggregation [[:sum [:field 277405 nil]]]})
(defn export-mbql
"Given an MBQL expression, convert it to an EDN structure and turn the non-portable Database, Table and Field IDs
@@ -405,6 +404,32 @@
(map mbql-fully-qualified-names->ids)
(map #(m/update-existing % :card_id import-fk 'Card))))
+(defn export-parameters
+ "Given the :parameter field of a `Card` or `Dashboard`, as a vector of maps, converts
+ it to a portable form with the CardIds/FieldIds replaced with `[db schema table field]` references."
+ [parameters]
+ (map ids->fully-qualified-names parameters))
+
+(defn import-parameters
+ "Given the :parameter field as exported by serialization convert its field references
+ (`[db schema table field]`) back into raw IDs."
+ [parameters]
+ (for [param parameters]
+ (-> param
+ mbql-fully-qualified-names->ids
+ (m/update-existing-in [:values_source_config :card_id] import-fk 'Card))))
+
+(defn parameters-deps
+ "Given the :parameters (possibly nil) for an entity, return any embedded serdes-deps as a set.
+ Always returns an empty set even if the input is nil."
+ [parameters]
+ (reduce set/union #{}
+ (for [parameter parameters
+ :when (= "card" (:values_source_type parameter))
+ :let [config (:values_source_config parameter)]]
+ (set/union #{[{:model "Card" :id (:card_id config)}]}
+ (mbql-deps-vector (:value_field config))))))
+
(defn- export-visualizations [entity]
(mbql.u/replace
entity
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index e9067d7da57..29cfbce4eac 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -2,6 +2,7 @@
"Tests for /api/card endpoints."
(:require
[cheshire.core :as json]
+ [clojure.set :as set]
[clojure.string :as str]
[clojure.test :refer :all]
[clojure.tools.macro :as tools.macro]
@@ -2300,3 +2301,148 @@
(is (not (contains? (task.persist-refresh/job-info-for-individual-refresh)
(u/the-id parchived)))
"Scheduled refresh of archived model"))))))
+
+(defn- param-values-url
+ ([card-or-id param-key]
+ (param-values-url card-or-id param-key nil))
+ ([card-or-id param-key query]
+ (if query
+ (format "card/%d/params/%s/search/%s" (u/the-id card-or-id) (name param-key) query)
+ (format "card/%d/params/%s/values" (u/the-id card-or-id) (name param-key)))))
+
+(defn do-with-card-param-values-fixtures
+ "Impl of `with-card-param-values-fixtures` macro."
+ ([f]
+ (do-with-card-param-values-fixtures nil f))
+
+ ([card-values f]
+ (mt/with-temp*
+ [Card [source-card {:database_id (mt/id)
+ :table_id (mt/id :venues)
+ :dataset_query (mt/mbql-query venues {:limit 5})}]
+ Card [field-filter-card {:dataset_query
+ {:database (mt/id)
+ :type :native
+ :native {:query "SELECT COUNT(*) FROM VENUES WHERE {{NAME}}"
+ :template-tags {"NAME" {:id "name_param_id"
+ :name "NAME"
+ :display_name "Name"
+ :type :dimension
+ :dimension [:field (mt/id :venues :name) nil]
+ :required true}}}}
+ :name "native card with field filter"
+ :parameters [{:id "name_param_id",
+ :type :string/=,
+ :target [:dimension [:template-tag "NAME"]],
+ :name "Name",
+ :slug "NAME"}]}]
+ Card [card (merge
+ {:database_id (mt/id)
+ :dataset_query (mt/mbql-query venues)
+ :parameters [{:name "Static Category",
+ :slug "static_category"
+ :id "_STATIC_CATEGORY_",
+ :type "category",
+ :values_source_type "static-list"
+ :values_source_config {:values ["African" "American" "Asian"]}}
+ {:name "Static Category label",
+ :slug "static_category_label"
+ :id "_STATIC_CATEGORY_LABEL_",
+ :type "category",
+ :values_source_type "static-list"
+ :values_source_config {:values [["African" "Af"] ["American" "Am"] ["Asian" "As"]]}}
+ {:name "Card as source"
+ :slug "card"
+ :id "_CARD_"
+ :type "category"
+ :values_source_type "card"
+ :values_source_config {:card_id (:id source-card)
+ :value_field (mt/$ids $venues.name)}}]
+ :table_id (mt/id :venues)}
+ card-values)]]
+ (f {:source-card source-card
+ :card card
+ :field-filter-card field-filter-card
+ :param-keys {:static-list "_STATIC_CATEGORY_"
+ :static-list-label "_STATIC_CATEGORY_LABEL_"
+ :card "_CARD_"
+ :field-values "name_param_id"}}))))
+
+(defmacro with-card-param-values-fixtures
+ "Execute `body` with all needed setup to tests param values on card."
+ [[binding card-values] & body]
+ `(do-with-card-param-values-fixtures ~card-values (fn [~binding] ~@body)))
+
+(deftest paramters-using-old-style-field-values
+ (with-card-param-values-fixtures [{:keys [param-keys field-filter-card]}]
+ (testing "GET /api/card/:card-id/params/:param-key/values for field-filter based params"
+ (testing "without search query"
+ (let [response (mt/user-http-request :rasta :get 200
+ (param-values-url field-filter-card (:field-values param-keys)))]
+ (is (false? (:has_more_values response)))
+ (is (set/subset? #{["20th Century Cafe"] ["33 Taps"]}
+ (-> response :values set)))))
+ (testing "with search query"
+ (let [response (mt/user-http-request :rasta :get 200
+ (param-values-url field-filter-card
+ (:field-values param-keys)
+ "bar"))]
+ (is (set/subset? #{["Barney's Beanery"] ["bigmista's barbecue"]}
+ (-> response :values set)))
+ (is (not ((into #{} (mapcat identity) (:values response)) "The Virgil"))))))))
+
+(deftest parameters-with-source-is-card-test
+ (with-card-param-values-fixtures [{:keys [card param-keys]}]
+ (testing "GET /api/card/:card-id/params/:param-key/values"
+ (is (=? {:values ["Red Medicine"
+ "Stout Burgers & Beers"
+ "The Apple Pan"
+ "Wurstküche"
+ "Brite Spot Family Restaurant"]
+ :has_more_values false}
+ (mt/user-http-request :rasta :get 200 (param-values-url card (:card param-keys))))))
+
+ (testing "GET /api/card/:card-id/params/:param-key/search/:query"
+ (is (= {:values ["Red Medicine"]
+ :has_more_values false}
+ (mt/user-http-request :rasta :get 200 (param-values-url card (:card param-keys) "red")))))))
+
+(deftest parameters-with-source-is-static-list-test
+ (with-card-param-values-fixtures [{:keys [card param-keys]}]
+ (testing "we could get the values"
+ (is (= {:has_more_values false,
+ :values ["African" "American" "Asian"]}
+ (mt/user-http-request :rasta :get 200
+ (param-values-url card (:static-list param-keys)))))
+
+ (is (= {:has_more_values false,
+ :values [["African" "Af"] ["American" "Am"] ["Asian" "As"]]}
+ (mt/user-http-request :rasta :get 200
+ (param-values-url card (:static-list-label param-keys))))))
+
+ (testing "we could search the values"
+ (is (= {:has_more_values false,
+ :values ["African"]}
+ (mt/user-http-request :rasta :get 200
+ (param-values-url card (:static-list param-keys) "af"))))
+
+ (is (= {:has_more_values false,
+ :values [["African" "Af"]]}
+ (mt/user-http-request :rasta :get 200
+ (param-values-url card (:static-list-label param-keys) "af")))))
+
+ (testing "we could edit the values list"
+ (let [card (mt/user-http-request :rasta :put 200 (str "card/" (:id card))
+ {:parameters [{:name "Static Category",
+ :slug "static_category"
+ :id "_STATIC_CATEGORY_",
+ :type "category",
+ :values_source_type "static-list"
+ :values_source_config {"values" ["BBQ" "Bakery" "Bar"]}}]})]
+ (is (= [{:name "Static Category",
+ :slug "static_category"
+ :id "_STATIC_CATEGORY_",
+ :type "category",
+ :values_source_type "static-list"
+ :values_source_config {:values ["BBQ" "Bakery" "Bar"]}}]
+ (:parameters card)))))))
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index 0e855a0cc1d..1103992fe84 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -1694,7 +1694,11 @@
(do-with-chain-filter-fixtures nil f))
([dashboard-values f]
- (mt/with-temp* [Dashboard [dashboard (merge {:parameters [{:name "Category Name"
+ (mt/with-temp* [Card [{source-card-id :id}
+ (merge (mt/card-with-source-metadata-for-query (mt/mbql-query categories {:limit 5}))
+ {:database_id (mt/id)
+ :table_id (mt/id :venues)})]
+ Dashboard [dashboard (merge {:parameters [{:name "Category Name"
:slug "category_name"
:id "_CATEGORY_NAME_"
:type "category"}
@@ -1721,7 +1725,13 @@
:id "_STATIC_CATEGORY_LABEL_",
:type "category",
:values_source_type "static-list"
- :values_source_config {:values [["African" "Af"] ["American" "Am"] ["Asian" "As"]]}}]}
+ :values_source_config {:values [["African" "Af"] ["American" "Am"] ["Asian" "As"]]}}
+ {:id "_CARD_"
+ :type "category"
+ :name "CATEGORY"
+ :values_source_type "card"
+ :values_source_config {:card_id source-card-id
+ :value_field (mt/$ids $venues.name)}}]}
dashboard-values)]
Card [card {:database_id (mt/id)
:table_id (mt/id :venues)
@@ -1757,7 +1767,8 @@
:price "_PRICE_"
:id "_ID_"
:static-category "_STATIC_CATEGORY_"
- :static-category-label "_STATIC_CATEGORY_LABEL_"}}))))
+ :static-category-label "_STATIC_CATEGORY_LABEL_"
+ :card "_CARD_"}}))))
(defmacro with-chain-filter-fixtures [[binding dashboard-values] & body]
`(do-with-chain-filter-fixtures ~dashboard-values (fn [~binding] ~@body)))
@@ -2047,82 +2058,20 @@
:has_more_values false}
(mt/user-http-request :rasta :get 200 url)))))))))
-(defn- card-fields-from-table-metadata
- [card-id]
- (:fields (mt/user-http-request :rasta :get 200 (format "/table/card__%d/query_metadata" card-id))))
-
(deftest parameter-values-from-card-test
;; TODO add permissions tests
- (testing "getting values"
- (mt/with-temp*
- [Card [{card-id :id}
- (merge (mt/card-with-source-metadata-for-query (mt/mbql-query venues {:limit 5}))
- {:database_id (mt/id)
- :table_id (mt/id :venues)})]
- Dashboard [{dashboard-id :id}
- {:parameters [{:id "abc"
- :type "category"
- :name "CATEGORY"
- :values_source_type "card"
- :values_source_config {:card_id card-id
- :value_field (mt/$ids $venues.name)}}]}]]
-
- (testing "It uses the results of the card's query execution"
- (let-url [url (chain-filter-values-url dashboard-id "abc")]
- (is (= {:values ["Red Medicine"
- "Stout Burgers & Beers"
- "The Apple Pan"
- "Wurstküche"
- "Brite Spot Family Restaurant"]
- :has_more_values false}
- (mt/user-http-request :rasta :get 200 url)))))
+ (with-chain-filter-fixtures [{:keys [dashboard param-keys]}]
+ (testing "It uses the results of the card's query execution"
+ (let-url [url (chain-filter-values-url dashboard (:card param-keys))]
+ (is (= {:values ["African" "American" "Artisan" "Asian" "BBQ"]
+ :has_more_values false}
+ (mt/user-http-request :rasta :get 200 url)))))
- (testing "it only returns search matches"
- (let-url [url (chain-filter-search-url dashboard-id "abc" "red")]
- (is (= {:values ["Red Medicine"]
- :has_more_values false}
- (mt/user-http-request :rasta :get 200 url))))))
-
- (testing "field selection should compatible with field-id from /api/table/:card__id/query_metadata"
- ;; FE use the id returned by /api/table/:card__id/query_metadata
- ;; for the `values_source_config.value_field`, so we need to test to make sure
- ;; the id is a valid field that we could use to retrieve values.
- (mt/with-temp*
- ;; card with agggregation and binning columns
- [Card [{mbql-card-id :id}
- (merge (mt/card-with-source-metadata-for-query
- (mt/mbql-query venues {:limit 5
- :aggregation [:count]
- :breakout [[:field %latitude {:binning {:strategy :num-bins :num-bins 10}}]]}))
- {:name "MBQL question"
- :database_id (mt/id)
- :table_id (mt/id :venues)})]
- Card [{native-card-id :id}
- (merge (mt/card-with-source-metadata-for-query
- (mt/native-query {:query "select name from venues;"}))
- {:name "Native question"
- :database_id (mt/id)
- :table_id (mt/id :venues)})]]
-
- (let [mbql-card-fields (card-fields-from-table-metadata mbql-card-id)
- native-card-fields (card-fields-from-table-metadata native-card-id)
- fields->parameter (fn [fields card-id]
- (for [{:keys [id field_ref name]} fields]
- {:id (format "id_%s" name)
- :type "category"
- :name name
- :values_source_type "card"
- :values_source_config {:card_id card-id
- :value_field (if (number? id)
- field_ref
- id)}}))
- parameters (concat
- (fields->parameter mbql-card-fields mbql-card-id)
- (fields->parameter native-card-fields native-card-id))]
- (mt/with-temp Dashboard [{dash-id :id} {:parameters parameters}]
- (doseq [param parameters]
- (let-url [url (chain-filter-values-url dash-id (:id param))]
- (is (some? (mt/user-http-request :rasta :get 200 url)))))))))))
+ (testing "it only returns search matches"
+ (let-url [url (chain-filter-search-url dashboard (:card param-keys) "af")]
+ (is (= {:values ["African"]
+ :has_more_values false}
+ (mt/user-http-request :rasta :get 200 url)))))))
(deftest valid-filter-fields-test
(testing "GET /api/dashboard/params/valid-filter-fields"
diff --git a/test/metabase/api/embed_test.clj b/test/metabase/api/embed_test.clj
index 1cdf1078008..ab6aa2004bf 100644
--- a/test/metabase/api/embed_test.clj
+++ b/test/metabase/api/embed_test.clj
@@ -5,10 +5,12 @@
[buddy.sign.util :as buddy-util]
[clj-time.core :as time]
[clojure.data.csv :as csv]
+ [clojure.set :as set]
[clojure.string :as str]
[clojure.test :refer :all]
[crypto.random :as crypto-random]
[dk.ative.docjure.spreadsheet :as spreadsheet]
+ [metabase.api.card-test :as api.card-test]
[metabase.api.dashboard-test :as api.dashboard-test]
[metabase.api.embed :as api.embed]
[metabase.api.pivots :as api.pivots]
@@ -644,6 +646,56 @@
(db/update! Card (u/the-id card) :enable_embedding false)
(client/client :get 400 (field-values-url card (mt/id :venues :name)))))))
+(deftest card-param-values
+ (letfn [(search [card param-key prefix]
+ (client/client :get 200 (format "embed/card/%s/params/%s/search/%s"
+ (card-token card) param-key prefix)))
+ (dropdown [card param-key]
+ (client/client :get 200 (format "embed/card/%s/params/%s/values"
+ (card-token card) param-key)))]
+ (mt/with-temporary-setting-values [enable-embedding true]
+ (with-new-secret-key
+ (api.card-test/with-card-param-values-fixtures [{:keys [card field-filter-card param-keys]}]
+ (db/update! Card (:id field-filter-card)
+ {:enable_embedding true
+ :embedding_params (zipmap (map :slug (:parameters field-filter-card))
+ (repeat "enabled"))})
+ (db/update! Card (:id card)
+ {:enable_embedding true
+ :embedding_params (zipmap (map :slug (:parameters card))
+ (repeat "enabled"))})
+ (testing "field filter based param"
+ (let [response (dropdown field-filter-card (:field-values param-keys))]
+ (is (false? (:has_more_values response)))
+ (is (set/subset? #{["20th Century Cafe"] ["33 Taps"]}
+ (-> response :values set))))
+ (let [response (search field-filter-card (:field-values param-keys) "bar")]
+ (is (set/subset? #{["Barney's Beanery"] ["bigmista's barbecue"]}
+ (-> response :values set)))
+ (is (not ((into #{} (mapcat identity) (:values response)) "The Virgil")))))
+ (testing "static based param"
+ (let [response (dropdown card (:static-list param-keys))]
+ (is (= {:has_more_values false,
+ :values ["African" "American" "Asian"]}
+ response)))
+ (let [response (search card (:static-list param-keys) "af")]
+ (is (= {:has_more_values false,
+ :values ["African"]}
+ response))))
+ (testing "card based param"
+ (let [response (dropdown card (:card param-keys))]
+ (is (= {:values ["Red Medicine"
+ "Stout Burgers & Beers"
+ "The Apple Pan"
+ "Wurstküche"
+ "Brite Spot Family Restaurant"]
+ :has_more_values false}
+ response)))
+ (let [response (search card (:card param-keys) "red")]
+ (is (= {:has_more_values false,
+ :values ["Red Medicine"]}
+ response)))))))))
+
;;; ----------------------------- GET /api/embed/dashboard/:token/field/:field/values nil -----------------------------
(defn- do-with-embedding-enabled-and-temp-dashcard-referencing {:style/indent 2} [table-kw field-kw f]
diff --git a/test/metabase/api/public_test.clj b/test/metabase/api/public_test.clj
index 049108b040d..3665e3cbaac 100644
--- a/test/metabase/api/public_test.clj
+++ b/test/metabase/api/public_test.clj
@@ -6,6 +6,7 @@
[clojure.test :refer :all]
[dk.ative.docjure.spreadsheet :as spreadsheet]
[metabase.actions.test-util :as actions.test-util]
+ [metabase.api.card-test :as api.card-test]
[metabase.api.dashboard-test :as api.dashboard-test]
[metabase.api.pivots :as api.pivots]
[metabase.api.public :as api.public]
@@ -1009,65 +1010,137 @@
;;; --------------------------------------------- Param values endpoints ---------------------------------------------
-(defn- chain-filter-values-url [uuid param-key]
- (format "public/dashboard/%s/params/%s/values" uuid param-key))
+(defn- param-values-url
+ ([card-or-dashboard uuid param-key]
+ (param-values-url card-or-dashboard uuid param-key nil))
+ ([card-or-dashboard uuid param-key query]
+ (str "public/"
+ (name card-or-dashboard)
+ "/" uuid
+ "/params/" param-key
+ (if query
+ (str "/search/" query)
+ "/values"))))
+
+(deftest param-values-test
+ (mt/with-temporary-setting-values [enable-public-sharing true]
+ (testing "with dashboard"
+ (api.dashboard-test/with-chain-filter-fixtures [{:keys [dashboard param-keys]}]
+ (let [uuid (str (UUID/randomUUID))]
+ (is (= true
+ (db/update! Dashboard (u/the-id dashboard) :public_uuid uuid)))
+ (testing "GET /api/public/dashboard/:uuid/params/:param-key/values"
+ (testing "parameter with source is a static list"
+ (is (= {:values ["African" "American" "Asian"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :dashboard uuid (:static-category param-keys))))))
-(defn- chain-filter-search-url [uuid param-key query]
- (format "public/dashboard/%s/params/%s/search/%s" uuid param-key query))
+ (testing "parameter with source is card"
+ (is (= {:values ["African" "American" "Artisan" "Asian" "BBQ"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :dashboard uuid (:card param-keys))))))
-(deftest chain-filter-test
- (mt/with-temporary-setting-values [enable-public-sharing true]
- (api.dashboard-test/with-chain-filter-fixtures [{:keys [dashboard param-keys]}]
- (let [uuid (str (UUID/randomUUID))]
- (is (= true
- (db/update! Dashboard (u/the-id dashboard) :public_uuid uuid)))
- (testing "GET /api/public/dashboard/:uuid/params/:param-key/values"
- (is (= {:values [2 3 4 5 6]
- :has_more_values false}
- (->> (client/client :get 200 (chain-filter-values-url uuid (:category-id param-keys)))
- (chain-filter-test/take-n-values 5)))))
- (testing "GET /api/public/dashboard/:uuid/params/:param-key/search/:query"
- (is (= {:values ["Fast Food" "Food Truck" "Seafood"]
- :has_more_values false}
- (->> (client/client :get 200 (chain-filter-search-url uuid (:category-name param-keys) "food"))
- (chain-filter-test/take-n-values 3)))))))))
-
-(deftest chain-filter-ignore-current-user-permissions-test
- (testing "Should not fail if request is authenticated but current user does not have data permissions"
- (mt/with-temp-copy-of-db
- (perms/revoke-data-perms! (perms-group/all-users) (mt/db))
- (mt/with-temporary-setting-values [enable-public-sharing true]
- (api.dashboard-test/with-chain-filter-fixtures [{:keys [dashboard param-keys]}]
- (let [uuid (str (UUID/randomUUID))]
- (is (= true
- (db/update! Dashboard (u/the-id dashboard) :public_uuid uuid)))
- (testing "GET /api/public/dashboard/:uuid/params/:param-key/values"
+ (testing "parameter with source is chain filter"
(is (= {:values [2 3 4 5 6]
:has_more_values false}
- (->> (mt/user-http-request :rasta :get 200 (chain-filter-values-url uuid (:category-id param-keys)))
- (chain-filter-test/take-n-values 5)))))
- (testing "GET /api/public/dashboard/:uuid/params/:param-key/search/:prefix"
+ (->> (client/client :get 200 (param-values-url :dashboard uuid (:category-id param-keys)))
+ (chain-filter-test/take-n-values 5))))))
+
+ (testing "GET /api/public/dashboard/:uuid/params/:param-key/search/:query"
+ (testing "parameter with source is a static list"
+ (is (= {:values ["African"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :dashboard uuid (:static-category param-keys) "af")))))
+
+ (testing "parameter with source is card"
+ (is (= {:values ["African"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :dashboard uuid (:card param-keys) "af")))))
+
+ (testing "parameter with source is a chain filter"
(is (= {:values ["Fast Food" "Food Truck" "Seafood"]
:has_more_values false}
- (->> (mt/user-http-request :rasta :get 200 (chain-filter-search-url uuid (:category-name param-keys) "food"))
- (chain-filter-test/take-n-values 3)))))))))))
+ (->> (client/client :get 200 (param-values-url :dashboard uuid (:category-name param-keys) "food"))
+ (chain-filter-test/take-n-values 3)))))))))
+
+ (testing "with card"
+ (api.card-test/with-card-param-values-fixtures [{:keys [card param-keys]}]
+ (let [uuid (str (UUID/randomUUID))]
+ (is (= true
+ (db/update! Card (u/the-id card) :public_uuid uuid)))
+ (testing "GET /api/public/card/:uuid/params/:param-key/values"
+ (testing "parameter with source is a static list"
+ (is (= {:values ["African" "American" "Asian"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :card uuid (:static-list param-keys))))))
+
+ (testing "parameter with source is a card"
+ (is (= {:values ["Red Medicine"
+ "Stout Burgers & Beers"
+ "The Apple Pan"
+ "Wurstküche"
+ "Brite Spot Family Restaurant"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :card uuid (:card param-keys)))))))
-(deftest params-with-static-list-test
- (mt/with-temporary-setting-values [enable-public-sharing true]
- (api.dashboard-test/with-chain-filter-fixtures [{:keys [dashboard param-keys]}]
- (let [uuid (str (UUID/randomUUID))]
- (is (= true
- (db/update! Dashboard (u/the-id dashboard) :public_uuid uuid)))
- (testing "GET /api/public/dashboard/:uuid/params/:param-key/values"
- (is (= {:values ["African" "American" "Asian"]
- :has_more_values false}
- (->> (client/client :get 200 (chain-filter-values-url uuid (:static-category param-keys)))
- (chain-filter-test/take-n-values 5)))))
- (testing "GET /api/public/dashboard/:uuid/params/:param-key/search/:query"
- (is (= {:values [["African" "Af"]]
- :has_more_values false}
- (->> (client/client :get 200 (chain-filter-search-url uuid (:static-category-label param-keys) "af"))
- (chain-filter-test/take-n-values 3)))))))))
+ (testing "GET /api/public/card/:uuid/params/:param-key/search/:query"
+ (testing "parameter with source is a static list"
+ (is (= {:values ["African"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :card uuid (:static-list param-keys) "af")))))
+
+ (testing "parameter with source is a card"
+ (is (= {:values ["Red Medicine"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :card uuid (:card param-keys) "red")))))))))))
+
+(deftest param-values-ignore-current-user-permissions-test
+ (testing "Should not fail if request is authenticated but current user does not have data permissions"
+ (mt/with-temp-copy-of-db
+ (perms/revoke-data-perms! (perms-group/all-users) (mt/db))
+ (mt/with-temporary-setting-values [enable-public-sharing true]
+ (testing "with dashboard"
+ (api.dashboard-test/with-chain-filter-fixtures [{:keys [dashboard param-keys]}]
+ (let [uuid (str (UUID/randomUUID))]
+ (is (= true
+ (db/update! Dashboard (u/the-id dashboard) :public_uuid uuid)))
+ (testing "GET /api/public/dashboard/:uuid/params/:param-key/values"
+ (is (= {:values [2 3 4 5 6]
+ :has_more_values false}
+ (->> (mt/user-http-request :rasta :get 200 (param-values-url :dashboard uuid (:category-id param-keys)))
+ (chain-filter-test/take-n-values 5)))))
+ (testing "GET /api/public/dashboard/:uuid/params/:param-key/search/:prefix"
+ (is (= {:values ["Fast Food" "Food Truck" "Seafood"]
+ :has_more_values false}
+ (->> (mt/user-http-request :rasta :get 200 (param-values-url :dashboard uuid (:category-name param-keys) "food"))
+ (chain-filter-test/take-n-values 3))))))))
+
+ (testing "with card"
+ (api.card-test/with-card-param-values-fixtures [{:keys [card param-keys]}]
+ (let [uuid (str (UUID/randomUUID))]
+ (is (= true
+ (db/update! Card (u/the-id card) :public_uuid uuid)))
+ (testing "GET /api/public/card/:uuid/params/:param-key/values"
+ (is (= {:values ["African" "American" "Asian"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :card uuid (:static-list param-keys)))))
+
+ (is (= {:values ["Red Medicine"
+ "Stout Burgers & Beers"
+ "The Apple Pan"
+ "Wurstküche"
+ "Brite Spot Family Restaurant"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :card uuid (:card param-keys))))))
+
+ (testing "GET /api/public/card/:uuid/params/:param-key/search/:query"
+ (is (= {:values ["African"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :card uuid (:static-list param-keys) "af"))))
+
+ (is (= {:values ["Red Medicine"]
+ :has_more_values false}
+ (client/client :get 200 (param-values-url :card uuid (:card param-keys) "red"))))))))))))
;;; --------------------------------------------- Pivot tables ---------------------------------------------
diff --git a/test/metabase/models/card_test.clj b/test/metabase/models/card_test.clj
index 7dcd52cb98d..23c9ad070d9 100644
--- a/test/metabase/models/card_test.clj
+++ b/test/metabase/models/card_test.clj
@@ -2,7 +2,8 @@
(:require
[cheshire.core :as json]
[clojure.test :refer :all]
- [metabase.models :refer [Card Collection Dashboard DashboardCard]]
+ [metabase.models
+ :refer [Card Collection Dashboard DashboardCard NativeQuerySnippet]]
[metabase.models.card :as card]
[metabase.models.serialization.base :as serdes.base]
[metabase.models.serialization.hash :as serdes.hash]
@@ -357,6 +358,63 @@
(serdes.hash/raw-hash ["the card" (serdes.hash/identity-hash coll) now])
(serdes.hash/identity-hash card)))))))
+(deftest parameter-card-test
+ (let [default-params {:name "Category Name"
+ :slug "category_name"
+ :id "_CATEGORY_NAME_"
+ :type "category"}]
+ (testing "parameter with source is card create ParameterCard"
+ (tt/with-temp* [Card [{source-card-id-1 :id}]
+ Card [{source-card-id-2 :id}]
+ Card [{card-id :id}
+ {:parameters [(merge default-params
+ {:values_source_type "card"
+ :values_source_config {:card_id source-card-id-1}})]}]]
+ (is (=? [{:card_id source-card-id-1
+ :parameterized_object_type :card
+ :parameterized_object_id card-id
+ :parameter_id "_CATEGORY_NAME_"}]
+ (db/select 'ParameterCard :parameterized_object_type "card" :parameterized_object_id card-id)))
+
+ (testing "update values_source_config.card_id will update ParameterCard"
+ (db/update! Card card-id {:parameters [(merge default-params
+ {:values_source_type "card"
+ :values_source_config {:card_id source-card-id-2}})]})
+ (is (=? [{:card_id source-card-id-2
+ :parameterized_object_type :card
+ :parameterized_object_id card-id
+ :parameter_id "_CATEGORY_NAME_"}]
+ (db/select 'ParameterCard :parameterized_object_type "card" :parameterized_object_id card-id))))
+
+ (testing "delete the card will delete ParameterCard"
+ (db/delete! Card :id card-id)
+ (is (= []
+ (db/select 'ParameterCard :parameterized_object_type "card" :parameterized_object_id card-id))))))
+
+ (testing "Delete a card will delete any ParameterCard that linked to it"
+ (tt/with-temp* [Card [{source-card-id :id}]
+ Card [{card-id-1 :id}
+ {:parameters [(merge default-params
+ {:values_source_type "card"
+ :values_source_config {:card_id source-card-id}})]}]
+ Card [{card-id-2 :id}
+ {:parameters [(merge default-params
+ {:values_source_type "card"
+ :values_source_config {:card_id source-card-id}})]}]]
+ ;; makes sure we have ParameterCard to start with
+ (is (=? [{:card_id source-card-id
+ :parameterized_object_type :card
+ :parameterized_object_id card-id-1
+ :parameter_id "_CATEGORY_NAME_"}
+ {:card_id source-card-id
+ :parameterized_object_type :card
+ :parameterized_object_id card-id-2
+ :parameter_id "_CATEGORY_NAME_"}]
+ (db/select 'ParameterCard :card_id source-card-id)))
+ (db/delete! Card :id source-card-id)
+ (is (= []
+ (db/select 'ParameterCard :card_id source-card-id)))))))
+
(deftest serdes-descendants-test
(testing "regular cards don't depend on anything"
(mt/with-temp* [Card [card {:name "some card"}]]
@@ -367,6 +425,28 @@
Card [card2 {:name "derived card"
:dataset_query {:query {:source-table (str "card__" (:id card1))}}}]]
(is (empty? (serdes.base/serdes-descendants "Card" (:id card1))))
+ (is (= #{["Card" (:id card1)]}
+ (serdes.base/serdes-descendants "Card" (:id card2))))))
+
+ (testing "cards that has a native template tag"
+ (mt/with-temp* [NativeQuerySnippet [snippet {:name "category" :content "category = 'Gizmo'"}]
+ Card [card {:name "Business Card"
+ :dataset_query {:native
+ {:template-tags {:snippet {:name "snippet"
+ :type :snippet
+ :snippet-name "snippet"
+ :snippet-id (:id snippet)}}
+ :query "select * from products where {{snippet}}"}}}]]
+ (is (= #{["NativeQuerySnippet" (:id snippet)]}
+ (serdes.base/serdes-descendants "Card" (:id card))))))
+
+ (testing "cards which have parameter's source is another card"
+ (mt/with-temp* [Card [card1 {:name "base card"}]
+ Card [card2 {:name "derived card"
+ :parameters [{:id "valid-id"
+ :type "id"
+ :values_source_type "card"
+ :values_source_config {:card_id (:id card1)}}]}]]
(is (= #{["Card" (:id card1)]}
(serdes.base/serdes-descendants "Card" (:id card2)))))))
diff --git a/test/metabase/models/dashboard_test.clj b/test/metabase/models/dashboard_test.clj
index c73b5eeac8b..62f34844955 100644
--- a/test/metabase/models/dashboard_test.clj
+++ b/test/metabase/models/dashboard_test.clj
@@ -11,11 +11,13 @@
:refer [DashboardCard]]
[metabase.models.dashboard-card-series :refer [DashboardCardSeries]]
[metabase.models.database :refer [Database]]
+ [metabase.models.field :refer [Field]]
[metabase.models.interface :as mi]
[metabase.models.permissions :as perms]
[metabase.models.pulse :refer [Pulse]]
[metabase.models.pulse-card :refer [PulseCard]]
[metabase.models.revision :as revision]
+ [metabase.models.serialization.base :as serdes.base]
[metabase.models.serialization.hash :as serdes.hash]
[metabase.models.table :refer [Table]]
[metabase.models.user :as user]
@@ -408,3 +410,32 @@
(is (= "8cbf93b7"
(serdes.hash/raw-hash ["my dashboard" (serdes.hash/identity-hash c1) now])
(serdes.hash/identity-hash dash)))))))
+
+(deftest serdes-descendants-test
+ (testing "dashboard which have parameter's source is another card"
+ (mt/with-temp* [Field [field {:name "A field"}]
+ Card [card {:name "A card"}]
+ Dashboard [dashboard {:name "A dashboard"
+ :parameters [{:id "abc"
+ :type "category"
+ :values_source_type "card"
+ :values_source_config {:card_id (:id card)
+ :value_field [:field (:id field) nil]}}]}]]
+ (is (= #{["Card" (:id card)]}
+ (serdes.base/serdes-descendants "Dashboard" (:id dashboard))))))
+
+ (testing "dashboard in which its dashcards has parameter_mappings to a card"
+ (mt/with-temp* [Card [card1 {:name "Card attached to dashcard"}]
+ Card [card2 {:name "Card attached to parameters"}]
+ Dashboard [dashboard {:parameters [{:name "Category Name"
+ :slug "category_name"
+ :id "_CATEGORY_NAME_"
+ :type "category"}]}]
+ DashboardCard [_ {:card_id (:id card1)
+ :dashboard_id (:id dashboard)
+ :parameter_mappings [{:parameter_id "_CATEGORY_NAME_"
+ :card_id (:id card2)
+ :target [:dimension (mt/$ids $categories.name)]}]}]]
+ (is (= #{["Card" (:id card1)]
+ ["Card" (:id card2)]}
+ (serdes.base/serdes-descendants "Dashboard" (:id dashboard)))))))
--
GitLab