diff --git a/src/metabase/models/params.clj b/src/metabase/models/params.clj
index a23c504852c1b4e2a77d7d4f8024070db3c8ee01..f109cac99b6e24bedf34ac59e302170f4bfcd997 100644
--- a/src/metabase/models/params.clj
+++ b/src/metabase/models/params.clj
@@ -21,14 +21,14 @@
"Receive a Paremeterized Object and check if its parameters is valid."
[{:keys [parameters]}]
(when (s/check (s/maybe [su/Parameter]) parameters)
- (throw (ex-info (tru ":parameters must be a sequence of maps with String :id key")
+ (throw (ex-info (tru ":parameters must be a sequence of maps with :id and :type keys")
{:parameters parameters}))))
(defn assert-valid-parameter-mappings
"Receive a Paremeterized Object and check if its parameters is valid."
[{:keys [parameter_mappings]}]
(when (s/check (s/maybe [su/ParameterMapping]) parameter_mappings)
- (throw (ex-info (tru ":parameter_mappings must be a sequence of maps with String :parameter_id key")
+ (throw (ex-info (tru ":parameter_mappings must be a sequence of maps with :parameter_id and :type keys")
{:parameter_mappings parameter_mappings}))))
(s/defn unwrap-field-clause :- mbql.s/field
diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj
index 5b73789dbefb47e964698e8b454d65042450a353..ee8e8ed2419390572834d76f190cb9b0dc8d9b7a 100644
--- a/src/metabase/models/user.clj
+++ b/src/metabase/models/user.clj
@@ -348,4 +348,4 @@
;; do things like automatically set the `is_superuser` flag for a User
(doseq [group-id to-add]
(db/insert! PermissionsGroupMembership {:user_id user-id, :group_id group-id}))))
- true))
+ true))
diff --git a/src/metabase/util/schema.clj b/src/metabase/util/schema.clj
index 1a3dee65ac93ee903c4c99b45090729763e5fca5..afa7b0f4bf39b10f4a560d40506d70d58bd04ed9 100644
--- a/src/metabase/util/schema.clj
+++ b/src/metabase/util/schema.clj
@@ -329,16 +329,26 @@
(deferred-tru "value must be a valid JSON string.")))
(def Parameter
- "Schema for a valid Parameter."
- (with-api-error-message {:id NonBlankString
- s/Keyword s/Any}
- (deferred-tru "parameter must be a map with String :id key")))
+ "Schema for a valid Parameter.
+ We're not using [metabase.mbql.schema/Parameter] here because this Parameter is meant to be used for
+ Parameters we store on dashboard/card, and it has some difference with Parameter in MBQL."
+ (with-api-error-message {:id NonBlankString
+ :type NonBlankString
+ ;; Allow blank name and slug #15279
+ (s/optional-key :name) s/Str
+ (s/optional-key :slug) s/Str
+ (s/optional-key :default) s/Any
+ (s/optional-key :sectionId) NonBlankString
+ s/Keyword s/Any}
+ (deferred-tru "parameter must be a map with :id and :type keys")))
(def ParameterMapping
"Schema for a valid Parameter Mapping"
- (with-api-error-message {:parameter_id NonBlankString
- s/Keyword s/Any}
- (deferred-tru "parameter mapping must be a String :parameter_id key")))
+ (with-api-error-message {:parameter_id NonBlankString
+ :target s/Any
+ (s/optional-key :card_id) IntGreaterThanZero
+ s/Keyword s/Any}
+ (deferred-tru "parameter_mapping must be a map with :parameter_id and :target keys")))
(def EmbeddingParams
"Schema for a valid map of embedding params."
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index 07c373c05345c2fe8ed20995b863f054e98d1aef..8657fc1fbe9c8728e67f0b322140d138ee70d409 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -314,7 +314,7 @@
(mbql-count-query (mt/id) (mt/id :venues)))
:collection_id (u/the-id collection)
:parameters [{:id "abc123", :name "test", :type "date"}]
- :parameter_mappings [{:parameter_id "abc123", :card_id "10",
+ :parameter_mappings [{:parameter_id "abc123", :card_id 10,
:target [:dimension [:template-tags "category"]]}])]
(is (= (merge
card-defaults
@@ -323,7 +323,7 @@
:collection true
:creator_id (mt/user->id :rasta)
:parameters [{:id "abc123", :name "test", :type "date"}]
- :parameter_mappings [{:parameter_id "abc123", :card_id "10",
+ :parameter_mappings [{:parameter_id "abc123", :card_id 10,
:target ["dimension" ["template-tags" "category"]]}]
:dataset_query true
:query_type "query"
@@ -364,7 +364,7 @@
(mt/user-http-request :crowberto :post 400 "card" {:visualization_settings "ABC"})))
(is (= {:errors {:parameters (str "value may be nil, or if non-nil, value must be an array. "
- "Each parameter must be a map with String :id key")}}
+ "Each parameter must be a map with :id and :type keys")}}
(mt/user-http-request :crowberto :post 400 "card" {:visualization_settings {:global {:title nil}}
:parameters "abc"})))))
@@ -804,16 +804,16 @@
(testing "PUT /api/card/:id"
(mt/with-temp Card [card]
(testing "successfully update with valid parameter_mappings"
- (is (partial= {:parameter_mappings [{:parameter_id "abc123", :card_id "10",
+ (is (partial= {:parameter_mappings [{:parameter_id "abc123", :card_id 10,
:target ["dimension" ["template-tags" "category"]]}]}
(mt/user-http-request :rasta :put 202 (str "card/" (u/the-id card))
- {:parameter_mappings [{:parameter_id "abc123", :card_id "10",
+ {:parameter_mappings [{:parameter_id "abc123", :card_id 10,
:target ["dimension" ["template-tags" "category"]]}]})))))
- (mt/with-temp Card [card {:parameter_mappings [{:parameter_id "abc123", :card_id "10",
+ (mt/with-temp Card [card {:parameter_mappings [{:parameter_id "abc123", :card_id 10,
:target ["dimension" ["template-tags" "category"]]}]}]
(testing "nil parameters will no-op"
- (is (partial= {:parameter_mappings [{:parameter_id "abc123", :card_id "10",
+ (is (partial= {:parameter_mappings [{:parameter_id "abc123", :card_id 10,
:target ["dimension" ["template-tags" "category"]]}]}
(mt/user-http-request :rasta :put 202 (str "card/" (u/the-id card))
{:parameters nil}))))
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index c3d24acc593d16edfb94540bc599afab229a4d3a..d7c1ab7b0ef5563b6595e2a4fad991dadaff5c44 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -114,7 +114,7 @@
(mt/user-http-request :rasta :post 400 "dashboard" {})))
(is (= {:errors {:parameters (str "value may be nil, or if non-nil, value must be an array. "
- "Each parameter must be a map with String :id key")}}
+ "Each parameter must be a map with :id and :type keys")}}
(mt/user-http-request :crowberto :post 400 "dashboard" {:name "Test"
:parameters "abc"})))))
diff --git a/test/metabase/api/embed_test.clj b/test/metabase/api/embed_test.clj
index a20143580c1511bdab318daa9761e74677c39d7e..702cbdcd22aa26aa9729ead1e2f7426cb5dfee4f 100644
--- a/test/metabase/api/embed_test.clj
+++ b/test/metabase/api/embed_test.clj
@@ -1126,7 +1126,7 @@
:parameters [{:name "Name"
:slug "name"
:id "_name_"
- :type :string/=
+ :type "string/="
:sectionId "string"}]}]
DashboardCard [{dashcard-id :id, :as dashcard} {:card_id card-id
diff --git a/test/metabase/api/preview_embed_test.clj b/test/metabase/api/preview_embed_test.clj
index 473e48fbd75a15966822a154428d53eeb32a03d6..b15b8805aaf0e33b4fea661759ce0a7f61e9941e 100644
--- a/test/metabase/api/preview_embed_test.clj
+++ b/test/metabase/api/preview_embed_test.clj
@@ -307,10 +307,12 @@
(embed-test/with-temp-dashcard [dashcard {:dash {:enable_embedding true
:parameters [{:id "_SECOND_DATE_SEEN_"
:slug "2nd_date_seen"
- :name "Second Date Seen"}
+ :name "Second Date Seen"
+ :type "date"}
{:id "_NUM_BIRDS_"
:slug "num_birds"
- :name "Number of Birds"}]}}]
+ :name "Number of Birds"
+ :type "number"}]}}]
(is (schema= {:status (s/eq "completed")
s/Keyword s/Any}
(mt/user-http-request :crowberto :get 202 (str (dashcard-url dashcard
@@ -470,13 +472,14 @@
:parameters [{:name "Name"
:slug "name"
:id "_name_"
- :type :string/=
+ :type "string/="
:sectionId "string"}]}]
DashboardCard [{dashcard-id :id, :as dashcard} {:card_id card-id
:dashboard_id dashboard-id
:parameter_mappings [{:parameter_id "_name_"
:card_id card-id
+ :type "string/="
:target [:dimension [:template-tag "NAME"]]}]}]]
(let [url (dashcard-url dashcard {:_embedding_params {:name "enabled"}})]
(is (= [[1]]
diff --git a/test/metabase/api/pulse_test.clj b/test/metabase/api/pulse_test.clj
index 43f15331f5645e3daac98b0364e698cb8aad6824..8a97e676095bd0e4b956bd2348a2fae678cd4e8f 100644
--- a/test/metabase/api/pulse_test.clj
+++ b/test/metabase/api/pulse_test.clj
@@ -892,7 +892,7 @@
Dashboard [{dashboard-id :id} {:parameters [{:name "X"
:slug "x"
:id "__X__"
- :type :category
+ :type "category"
:default 3}]}]
DashboardCard [_ {:card_id card-id
:dashboard_id dashboard-id
diff --git a/test/metabase/dashboard_subscription_test.clj b/test/metabase/dashboard_subscription_test.clj
index 291a82f50286989caffc4889da73352bc9c1d6cd..59d33ce1e01eabba264f9e67ebc9fd979b45f5bf 100644
--- a/test/metabase/dashboard_subscription_test.clj
+++ b/test/metabase/dashboard_subscription_test.clj
@@ -375,12 +375,12 @@
:parameters [{:name "Category"
:slug "category"
:id "_MBQL_CATEGORY_"
- :type :category
+ :type "category"
:default ["Doohickey"]}
{:name "SQL Category"
:slug "sql_category"
:id "_SQL_CATEGORY_"
- :type :category
+ :type "category"
:default ["Gizmo"]}]}]
(testing "MBQL query"
(mt/with-temp* [Card [{mbql-card-id :id} {:name "Orders"
diff --git a/test/metabase/models/card_test.clj b/test/metabase/models/card_test.clj
index f5a61e90351b815f481acf2cff329196c5211a62..40d980f128fd27dcd9003e7657a32375ceeb7c7b 100644
--- a/test/metabase/models/card_test.clj
+++ b/test/metabase/models/card_test.clj
@@ -300,56 +300,55 @@
(testing "creating"
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #":parameters must be a sequence of maps with String :id key"
+ #":parameters must be a sequence of maps with :id and :type keys"
(mt/with-temp Card [_ {:parameters {:a :b}}])))
- (mt/with-temp Card [card {:parameters [{:id "valid-id"}]}]
+ (mt/with-temp Card [card {:parameters [{:id "valid-id"
+ :type "id"}]}]
(is (some? card))))
(testing "updating"
- (mt/with-temp Card [{:keys [id]} {:parameters []}]
+ (mt/with-temp Card [{:keys [id] :as card} {:parameters []}]
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #":parameters must be a sequence of maps with String :id key"
+ #":parameters must be a sequence of maps with :id and :type keys"
(db/update! Card id :parameters [{:id 100}])))
- (is (some? (db/update! Card id :parameters [{:id "new-valid-id"}])))))))
+ (is (some? (db/update! Card id :parameters [{:id "new-valid-id"
+ :type "id"}])))))))
(deftest normalize-parameters-test
(testing ":parameters should get normalized when coming out of the DB"
(doseq [[target expected] {[:dimension [:field-id 1000]] [:dimension [:field 1000 nil]]
[:field-id 1000] [:field 1000 nil]}]
(testing (format "target = %s" (pr-str target))
- (mt/with-temp Card [{card-id :id} {:parameters [{:name "Category Name"
- :slug "category_name"
- :id "_CATEGORY_NAME_"
- :type "category"
- :target target}]}]
- (is (= [{:name "Category Name"
- :slug "category_name"
- :id "_CATEGORY_NAME_"
- :type :category
+ (mt/with-temp Card [{card-id :id} {:parameter_mappings [{:parameter_id "_CATEGORY_NAME_"
+ :target target}]}]
+
+ (is (= [{:parameter_id "_CATEGORY_NAME_"
:target expected}]
- (db/select-one-field :parameters Card :id card-id))))))))
+ (db/select-one-field :parameter_mappings Card :id card-id))))))))
(deftest validate-parameter-mappings-test
(testing "Should validate Card :parameter_mappings when"
(testing "creating"
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #":parameter_mappings must be a sequence of maps with String :parameter_id key"
+ #":parameter_mappings must be a sequence of maps with :parameter_id and :type keys"
(mt/with-temp Card [_ {:parameter_mappings {:a :b}}])))
- (mt/with-temp Card [card {:parameter_mappings [{:parameter_id "valid-id"}]}]
+ (mt/with-temp Card [card {:parameter_mappings [{:parameter_id "valid-id"
+ :target [:field-id 1000]}]}]
(is (some? card))))
(testing "updating"
(mt/with-temp Card [{:keys [id]} {:parameter_mappings []}]
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #":parameter_mappings must be a sequence of maps with String :parameter_id key"
+ #":parameter_mappings must be a sequence of maps with :parameter_id and :type keys"
(db/update! Card id :parameter_mappings [{:parameter_id 100}])))
- (is (some? (db/update! Card id :parameter_mappings [{:parameter_id "new-valid-id"}])))))))
+ (is (some? (db/update! Card id :parameter_mappings [{:parameter_id "new-valid-id"
+ :target [:field-id 1000]}])))))))
(deftest normalize-parameter-mappings-test
(testing ":parameter_mappings should get normalized when coming out of the DB"
diff --git a/test/metabase/models/dashboard_test.clj b/test/metabase/models/dashboard_test.clj
index 4a473f1894f1829113a48e413a0b367d1fab15ad..4da09a3ad1ba7a9ced2cb70e7ddc84639e14d5b5 100644
--- a/test/metabase/models/dashboard_test.clj
+++ b/test/metabase/models/dashboard_test.clj
@@ -283,13 +283,13 @@
(testing "creating"
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #":parameters must be a sequence of maps with String :id key"
+ #":parameters must be a sequence of maps with :id and :type keys"
(mt/with-temp Dashboard [_ {:parameters {:a :b}}]))))
(testing "updating"
(mt/with-temp Dashboard [{:keys [id]} {:parameters []}]
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
- #":parameters must be a sequence of maps with String :id key"
+ #":parameters must be a sequence of maps with :id and :type keys"
(db/update! Dashboard id :parameters [{:id 100}])))))))
(deftest normalize-parameters-test
diff --git a/test/metabase/query_processor/card_test.clj b/test/metabase/query_processor/card_test.clj
index 0df68c1a2d6817f42fa81e07b9f5b0ae1b37937b..0de72127d9f8613191d5d3e02ec04b3947ca650c 100644
--- a/test/metabase/query_processor/card_test.clj
+++ b/test/metabase/query_processor/card_test.clj
@@ -171,7 +171,7 @@
(deftest validate-card-parameters-test
(mt/with-temp Card [{card-id :id} {:dataset_query (mt/mbql-query checkins {:aggregation [[:count]]})
:parameters [{:id "_DATE_"
- :type :date/single
+ :type "date/single"
:name "Date"
:slug "DATE"}]}]
(testing "API request should fail if request parameter does not contain ID"
@@ -181,7 +181,7 @@
s/Keyword s/Any}
(mt/user-http-request :rasta :post (format "card/%d/query" card-id)
{:parameters [{:name "date"
- :type :date/single
+ :type "date/single"
:value "2016-01-01"}]}))))
(testing "API request should fail if request parameter ID does not exist on the card"
@@ -192,12 +192,12 @@
(mt/user-http-request :rasta :post (format "card/%d/query" card-id)
{:parameters [{:id "_FAKE_"
:name "date"
- :type :date/single
+ :type "date/single"
:value "2016-01-01"}]}))))
(testing "Happy path -- API request should succeed if request parameter correlates to a card parameter by ID"
(is (= [1000]
(mt/first-row (mt/user-http-request :rasta :post (format "card/%d/query" card-id)
{:parameters [{:id "_DATE_"
- :type :date/single
+ :type "date/single"
:value "2016-01-01"}]})))))))
diff --git a/test/metabase/query_processor/dashboard_test.clj b/test/metabase/query_processor/dashboard_test.clj
index c1efbe9b432bf45b30a0bd6a3d71b42bc041e7ca..85d1ea23fabbf70b3277df10e40c2500715fdc64 100644
--- a/test/metabase/query_processor/dashboard_test.clj
+++ b/test/metabase/query_processor/dashboard_test.clj
@@ -68,7 +68,7 @@
Dashboard [{dashboard-id :id} {:parameters [{:name "category"
:slug "category"
:id "abc123"
- :type :string/=
+ :type "string/="
:default ["Widget"]}]}]
DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id
:card_id card-id
@@ -99,13 +99,13 @@
{:id "f0774ef5-a14a-e181-f557-2d4bb1fc94ae"
:name "filter"
:display-name "Filter"
- :type :text
+ :type "text"
:required true
:default "Foo"}}}}}]
Dashboard [{dashboard-id :id} {:parameters [{:name "Text"
:slug "text"
:id "5791ff38"
- :type :string/=
+ :type "string/="
:default "Bar"}]}]
DashboardCard [{dashcard-id :id} {:dashboard_id dashboard-id
:card_id card-id
@@ -133,11 +133,11 @@
Dashboard [{dashboard-id :id} {:parameters [{:name "Category (DashCard 1)"
:slug "category_1"
:id "CATEGORY_1"
- :type :string/=}
+ :type "string/="}
{:name "Category (DashCard 2)"
:slug "category_2"
:id "CATEGORY_2"
- :type :string/=}]}]
+ :type "string/="}]}]
DashboardCard [{dashcard-1-id :id} {:card_id card-id
:dashboard_id dashboard-id
:parameter_mappings [{:parameter_id "CATEGORY_1"
@@ -187,7 +187,7 @@
Dashboard [{dashboard-id :id} {:parameters [{:name "Text"
:slug "text"
:id "_text_"
- :type :string/=
+ :type "string/="
:sectionId "string"
:default ["Doohickey"]}]}]
DashboardCard [{dashcard-id :id} {:parameter_mappings [{:parameter_id "_text_"
@@ -211,7 +211,7 @@
:parameters [{:name "Category"
:slug "category"
:id "_CATEGORY_"
- :type :category
+ :type "category"
:default ["Doohickey"]}]}]
DashboardCard [{dashcard-id :id} {:parameter_mappings [{:parameter_id "_CATEGORY_"
:card_id card-id
@@ -232,5 +232,5 @@
:parameters [{:name "Category"
:slug "category"
:id "_CATEGORY_"
- :type :category
+ :type "category"
:default ["Gizmo"]}])))))))))