diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj
index be1e49794b1241bb5b18056f2104a5e201467952..559e23477d580b6d133c9727d45a813284285322 100644
--- a/src/metabase/api/card.clj
+++ b/src/metabase/api/card.clj
@@ -31,8 +31,7 @@
[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.params.custom-values :as custom-values]
[metabase.models.persisted-info :as persisted-info]
[metabase.models.pulse :as pulse]
[metabase.models.query :as query]
@@ -944,15 +943,7 @@ saved later when it is ready."
(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" (do
- (api/read-check Card (get-in param [:values_source_config :card_id]))
- (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}))))))
+ (custom-values/parameter->values param query (fn [] (mapping->field-values card param query))))))
(api/defendpoint GET "/:card-id/params/:param-key/values"
"Fetch possible values of the parameter whose ID is `:param-key`.
@@ -962,8 +953,7 @@ saved later when it is ready."
[card-id param-key]
{card-id ms/IntGreaterThanZero
param-key ms/NonBlankString}
- (let [card (api/read-check Card card-id)]
- (param-values card param-key)))
+ (param-values (api/read-check Card card-id) 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`.
@@ -976,7 +966,6 @@ saved later when it is ready."
{card-id ms/IntGreaterThanZero
param-key ms/NonBlankString
query ms/NonBlankString}
- (let [card (api/read-check Card card-id)]
- (param-values card param-key query)))
+ (param-values (api/read-check Card card-id) param-key query))
(api/define-routes)
diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index a321fdf02bac4dfa7f893501596ed4153e93a4d5..9235071d5512a470d0e1dae1413ac6e8dcf9753c 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -21,9 +21,8 @@
[metabase.models.field :refer [Field]]
[metabase.models.interface :as mi]
[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.params.custom-values :as custom-values]
[metabase.models.query :as query :refer [Query]]
[metabase.models.query.permissions :as query-perms]
[metabase.models.revision :as revision]
@@ -770,12 +769,7 @@
(throw (ex-info (tru "Dashboard does not have a parameter with the ID {0}" (pr-str param-key))
{:resolved-params (keys (:resolved-params dashboard))
:status-code 400})))
- (case (:values_source_type param)
- "static-list" (params.static-values/param->values param query)
- "card" (do
- (api/read-check Card (get-in param [:values_source_config :card_id]))
- (params.card-values/param->values param query))
- nil (chain-filter dashboard param-key constraint-param-key->value query)))))
+ (custom-values/parameter->values param query (fn [] (chain-filter dashboard param-key constraint-param-key->value query))))))
#_{:clj-kondo/ignore [:deprecated-var]}
(api/defendpoint-schema GET "/:id/params/:param-key/values"
diff --git a/src/metabase/api/dataset.clj b/src/metabase/api/dataset.clj
index 0a53da464e89bfdf292f0102dd7588977f562657..451076c9535e7e651f3152a0dbd8bca69daded1a 100644
--- a/src/metabase/api/dataset.clj
+++ b/src/metabase/api/dataset.clj
@@ -11,8 +11,7 @@
[metabase.mbql.schema :as mbql.s]
[metabase.models.card :refer [Card]]
[metabase.models.database :as database :refer [Database]]
- [metabase.models.params.card-values :as params.card-values]
- [metabase.models.params.static-values :as params.static-values]
+ [metabase.models.params.custom-values :as custom-values]
[metabase.models.persisted-info :as persisted-info]
[metabase.models.query :as query]
[metabase.models.table :refer [Table]]
@@ -179,6 +178,9 @@
(defn- parameter-field-values
[field-ids query]
+ (when-not (seq field-ids)
+ (throw (ex-info (tru "Missing field-ids for parameter")
+ {:status-code 400})))
(-> (reduce (fn [resp id]
(let [{values :values more? :has_more_values} (api.field/field-id->values id query)]
(-> resp
@@ -194,25 +196,16 @@
"Fetch parameter values. Parameter should be a full parameter, field-ids is an optional vector of field ids, only
consulted if `:values_source_type` is nil. Query is an optional string return matching field values not all."
[parameter field-ids query]
- (case (:values_source_type parameter)
- "static-list" (params.static-values/param->values parameter query)
- "card" (params.card-values/param->values parameter query)
- nil (if (seq field-ids)
- (parameter-field-values field-ids query)
- (throw (ex-info (tru "Missing field-ids for parameter")
- {:status-code 400
- :parameter parameter})))
- (throw (ex-info (tru "Invalid parameter source {0}" (:values_source_type parameter))
- {:status-code 400
- :parameter parameter}))))
+ (custom-values/parameter->values
+ parameter query
+ (fn [] (parameter-field-values field-ids query))))
(api/defendpoint POST "/parameter/values"
"Return parameter values for cards or dashboards that are being edited."
[:as {{:keys [parameter field_ids]} :body}]
{parameter ms/Parameter
field_ids [:maybe [:sequential ms/IntGreaterThanZero]]}
- (let [nil-query nil]
- (parameter-values parameter field_ids nil-query)))
+ (parameter-values parameter field_ids nil))
(api/defendpoint POST "/parameter/search/:query"
"Return parameter values for cards or dashboards that are being edited. Expects a query string at `?query=foo`."
diff --git a/src/metabase/models/params.clj b/src/metabase/models/params.clj
index 5404be959ece326e39adad5a64e3aa303c49a903..70078e9f19c24fcbcb244a0350e251a16d6f6f66 100644
--- a/src/metabase/models/params.clj
+++ b/src/metabase/models/params.clj
@@ -1,5 +1,15 @@
(ns metabase.models.params
- "Utility functions for dealing with parameters for Dashboards and Cards."
+ "Utility functions for dealing with parameters for Dashboards and Cards.
+
+ Parameter are objects that exists on Dashboard/Card. In FE terms, we call it \"Widget\".
+ The values of a parameter is provided so the Widget can show a list of options to the user.
+
+
+ There are 3 mains ways to provide values to a parameter:
+ - chain-filter: see [metabase.models.params.chain-filter]
+ - field-values: see [metabase.models.params.field-values]
+ - custom-values: see [metabase.models.params.custom-values]
+ "
(:require
[clojure.set :as set]
[medley.core :as m]
diff --git a/src/metabase/models/params/card_values.clj b/src/metabase/models/params/card_values.clj
deleted file mode 100644
index f39a17e0edd52217c9cdbcb10a9799357b84e526..0000000000000000000000000000000000000000
--- a/src/metabase/models/params/card_values.clj
+++ /dev/null
@@ -1,108 +0,0 @@
-(ns metabase.models.params.card-values
- "Code related to getting values for a parameter where its source values is a card."
- (:require
- [metabase.mbql.normalize :as mbql.normalize]
- [metabase.models :refer [Card]]
- [metabase.query-processor :as qp]
- [metabase.util :as u]
- [metabase.util.i18n :refer [tru]]
- [metabase.util.malli :as mu]
- [metabase.util.malli.schema :as ms]
- [toucan.db :as db]))
-
-(def ^:dynamic *max-rows*
- "Maximum number of rows returned when running a card.
- 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)
-
-(def field-options-for-identification
- "Set of FieldOptions that only mattered for identification purposes." ;; base-type is required for field that use name instead of id
- #{:source-field :join-alias :base-type})
-
-(defn- field-normalizer
- [field]
- (let [[type id-or-name options ] (mbql.normalize/normalize-tokens field)]
- [type id-or-name (select-keys options field-options-for-identification)]))
-
-(defn- field->field-info
- [field result-metadata]
- (let [[_ttype id-or-name options :as field] (field-normalizer field)]
- (or
- ;; try match field_ref first
- (first (filter (fn [field-info]
- (= field
- (-> field-info
- :field_ref
- field-normalizer)))
- result-metadata))
- ;; if not match name and base type for aggregation or field with string id
- (first (filter (fn [field-info]
- (and (= (:name field-info)
- id-or-name)
- (= (:base-type options)
- (:base-type field-info))))
- result-metadata)))))
-
-(defn- values-from-card-query
- [card value-field query]
- (let [value-base-type (:base_type (field->field-info value-field (:result_metadata card)))]
- {:database (:database_id card)
- :type :query
- :query (merge
- {:source-table (format "card__%d" (:id card))
- :breakout [value-field]
- :limit *max-rows*}
- {:filter [:and
- [(if-not (isa? value-base-type :type/Text)
- :not-null
- :not-empty)
- value-field]
- (when query
- (if-not (isa? value-base-type :type/Text)
- [:= value-field query]
- [:contains [:lower value-field] (u/lower-case-en query)]))]})
- :middleware {:disable-remaps? true}}))
-
-(mu/defn values-from-card
- "Get distinct values of a field from a card.
-
- (values-from-card 1 [:field \"name\" nil] \"red\")
- ;; will execute a mbql that looks like
- ;; {:source-table (format \"card__%d\" card-id)
- ;; :fields [value-field]
- ;; :breakout [value-field]
- ;; :filter [:contains [:lower value-field] \"red\"]
- ;; :limit *max-rows*}
- =>
- {:values [\"Red Medicine\"]
- :has_more_values false}
- "
- ([card value-field]
- (values-from-card card value-field nil))
-
- ([card :- (ms/InstanceOf Card)
- value-field :- ms/Field
- query :- [:any]]
- (let [mbql-query (values-from-card-query card value-field query)
- result (qp/process-query mbql-query)
- values (map first (get-in result [:data :rows]))]
- {:values values
- ;; if the row_count returned = the limit we specified, then it's probably has more than that
- :has_more_values (= (:row_count result)
- (get-in mbql-query [:query :limit]))})))
-
-(defn param->values
- "Given a param and query returns the values."
- [{config :values_source_config :as _param} query]
- (let [card-id (:card_id config)
- card (db/select-one Card :id card-id)]
- (when-not card
- (throw (ex-info (tru "Source card not found")
- {:card-id card-id
- :status-code 400})))
- (when (:archived card)
- (throw (ex-info (tru "Source card is archived")
- {:card-id card-id
- :status-code 400})))
- (values-from-card card (:value_field config) query)))
diff --git a/src/metabase/models/params/custom_values.clj b/src/metabase/models/params/custom_values.clj
new file mode 100644
index 0000000000000000000000000000000000000000..ccd3c6470bc3f9d5f69af66e22a0fca25c8fea9f
--- /dev/null
+++ b/src/metabase/models/params/custom_values.clj
@@ -0,0 +1,161 @@
+(ns metabase.models.params.custom-values
+ "Custom values for Parameters.
+
+ A parameter with custom values will need to define a source:
+ - static-list: the values is pre-defined and stored inside parameter's config
+ - card: the values is a column from a saved question
+ "
+ (:require
+ [clojure.string :as str]
+ [metabase.mbql.normalize :as mbql.normalize]
+ [metabase.models.card :refer [Card]]
+ [metabase.models.interface :as mi]
+ [metabase.query-processor :as qp]
+ [metabase.search.util :as search]
+ [metabase.util :as u]
+ [metabase.util.i18n :refer [tru]]
+ [metabase.util.malli :as mu]
+ [metabase.util.malli.schema :as ms]
+ [toucan.db :as db]))
+
+;;; ------------------------------------------------- source=static-list --------------------------------------------------
+(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-list-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}))
+
+;;; ---------------------------------------------------- source=card ------------------------------------------------------
+
+(def ^:dynamic *max-rows*
+ "Maximum number of rows returned when running a card.
+ 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)
+
+(def field-options-for-identification
+ "Set of FieldOptions that only mattered for identification purposes." ;; base-type is required for field that use name instead of id
+ #{:source-field :join-alias :base-type})
+
+(defn- field-normalizer
+ [field]
+ (let [[type id-or-name options ] (mbql.normalize/normalize-tokens field)]
+ [type id-or-name (select-keys options field-options-for-identification)]))
+
+(defn- field->field-info
+ [field result-metadata]
+ (let [[_ttype id-or-name options :as field] (field-normalizer field)]
+ (or
+ ;; try match field_ref first
+ (first (filter (fn [field-info]
+ (= field
+ (-> field-info
+ :field_ref
+ field-normalizer)))
+ result-metadata))
+ ;; if not match name and base type for aggregation or field with string id
+ (first (filter (fn [field-info]
+ (and (= (:name field-info)
+ id-or-name)
+ (= (:base-type options)
+ (:base_type field-info))))
+ result-metadata)))))
+
+(defn- values-from-card-query
+ [card value-field query]
+ (let [value-base-type (:base_type (field->field-info value-field (:result_metadata card)))]
+ {:database (:database_id card)
+ :type :query
+ :query (merge
+ {:source-table (format "card__%d" (:id card))
+ :breakout [value-field]
+ :limit *max-rows*}
+ {:filter [:and
+ [(if (isa? value-base-type :type/Text)
+ :not-empty
+ :not-null)
+ value-field]
+ (when query
+ (if-not (isa? value-base-type :type/Text)
+ [:= value-field query]
+ [:contains [:lower value-field] (u/lower-case-en query)]))]})
+ :middleware {:disable-remaps? true}}))
+
+(mu/defn values-from-card
+ "Get distinct values of a field from a card.
+
+ (values-from-card 1 [:field \"name\" nil] \"red\")
+ ;; will execute a mbql that looks like
+ ;; {:source-table (format \"card__%d\" card-id)
+ ;; :fields [value-field]
+ ;; :breakout [value-field]
+ ;; :filter [:contains [:lower value-field] \"red\"]
+ ;; :limit *max-rows*}
+ =>
+ {:values [\"Red Medicine\"]
+ :has_more_values false}
+ "
+ ([card value-field]
+ (values-from-card card value-field nil))
+
+ ([card :- (ms/InstanceOf Card)
+ value-field :- ms/Field
+ query :- [:any]]
+ (let [mbql-query (values-from-card-query card value-field query)
+ result (qp/process-query mbql-query)
+ values (map first (get-in result [:data :rows]))]
+ {:values values
+ ;; if the row_count returned = the limit we specified, then it's probably has more than that
+ :has_more_values (= (:row_count result)
+ (get-in mbql-query [:query :limit]))})))
+
+(defn card-values
+ "Given a param and query returns the values."
+ [{config :values_source_config :as _param} query]
+ (let [card-id (:card_id config)
+ card (db/select-one Card :id card-id)]
+ (values-from-card card (:value_field config) query)))
+
+(defn- can-get-card-values?
+ [card value-field]
+ (boolean
+ (and (not (:archived card))
+ (some? (field->field-info value-field (:result_metadata card))))))
+
+;;; --------------------------------------------- Putting it together ----------------------------------------------
+
+(defn parameter->values
+ "Given a parameter with a custom-values source, return the values.
+
+ `default-case-fn` is a 0-arity function that returns values list when:
+ - :values_source_type = card but the card is archived or the card no longer contains the value-field.
+ - :values_source_type = nil."
+ [parameter query default-case-fn]
+ (case (:values_source_type parameter)
+ "static-list" (static-list-values parameter query)
+ "card" (let [card (db/select-one Card :id (get-in parameter [:values_source_config :card_id]))]
+ (when-not (mi/can-read? card)
+ (throw (ex-info "You don't have permissions to do that." {:status-code 403})))
+ (if (can-get-card-values? card (get-in parameter [:values_source_config :value_field]))
+ (card-values parameter query)
+ (default-case-fn)))
+ nil (default-case-fn)
+ (throw (ex-info (tru "Invalid parameter source {0}" (:values_source_type parameter))
+ {:status-code 400
+ :parameter parameter}))))
diff --git a/src/metabase/models/params/static_values.clj b/src/metabase/models/params/static_values.clj
deleted file mode 100644
index a172868ae2fcd1c6052e094f8d7e3d5fea751848..0000000000000000000000000000000000000000
--- a/src/metabase/models/params/static_values.clj
+++ /dev/null
@@ -1,28 +0,0 @@
-(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/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index 38252d9a10c68635e1438cbcecaa10348398538d..1867c31cfee2134fb105f0e85dff5297f7276bea 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -2396,6 +2396,32 @@
:has_more_values false}
(mt/user-http-request :rasta :get 200 (param-values-url card (:card param-keys) "red")))))))
+ (testing "fallback to field-values"
+ (with-redefs [api.card/mapping->field-values (constantly "field-values")]
+ (testing "if value-field not found in source card"
+ (mt/with-temp* [Card [{source-card-id :id}]
+ Card [card
+ {:parameters [{:id "abc"
+ :type "category"
+ :name "CATEGORY"
+ :values_source_type "card"
+ :values_source_config {:card_id source-card-id
+ :value_field (mt/$ids $venues.name)}}]}]]
+ (let [url (param-values-url card "abc")]
+ (is (= "field-values" (mt/user-http-request :rasta :get 200 url))))))
+
+ (testing "if card is archived"
+ (mt/with-temp* [Card [{source-card-id :id} {:archived true}]
+ Card [card
+ {:parameters [{:id "abc"
+ :type "category"
+ :name "CATEGORY"
+ :values_source_type "card"
+ :values_source_config {:card_id source-card-id
+ :value_field (mt/$ids $venues.name)}}]}]]
+ (let [url (param-values-url card "abc")]
+ (is (= "field-values" (mt/user-http-request :rasta :get 200 url))))))))
+
(testing "users must have permissions to read the collection that source card is in"
(mt/with-non-admin-groups-no-root-collection-perms
(mt/with-temp*
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index 46bdcdba278ca90f555c05d8f08b5ef9c05da938..7925f441114c2639f2f0c28485f65e87b6225c22 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -2149,6 +2149,32 @@
:has_more_values false}
(mt/user-http-request :rasta :get 200 url)))))))
+ (testing "fallback to chain-filter"
+ (with-redefs [api.dashboard/chain-filter (constantly "chain-filter")]
+ (testing "if value-field not found in source card"
+ (mt/with-temp* [Card [{card-id :id}]
+ Dashboard [dashboard
+ {:parameters [{:id "abc"
+ :type "category"
+ :name "CATEGORY"
+ :values_source_type "card"
+ :values_source_config {:card_id card-id
+ :value_field (mt/$ids $venues.name)}}]}]]
+ (let-url [url (chain-filter-values-url dashboard "abc")]
+ (is (= "chain-filter" (mt/user-http-request :rasta :get 200 url))))))
+
+ (testing "if card is archived"
+ (mt/with-temp* [Card [{card-id :id} {:archived true}]
+ Dashboard [dashboard
+ {:parameters [{:id "abc"
+ :type "category"
+ :name "CATEGORY"
+ :values_source_type "card"
+ :values_source_config {:card_id card-id
+ :value_field (mt/$ids $venues.name)}}]}]]
+ (let-url [url (chain-filter-values-url dashboard "abc")]
+ (is (= "chain-filter" (mt/user-http-request :rasta :get 200 url))))))))
+
(testing "users must have permissions to read the collection that source card is in"
(mt/with-non-admin-groups-no-root-collection-perms
(mt/with-temp*
@@ -2159,14 +2185,14 @@
:table_id (mt/id :venues)
:dataset_query (mt/mbql-query venues {:limit 5})}]
Collection [coll2 {:name "Dashboard collections"}]
- Dashboard [{dashboard-id :id}
- {:collection_id (:id coll2)
- :parameters [{:id "abc"
- :type "category"
- :name "CATEGORY"
- :values_source_type "card"
- :values_source_config {:card_id source-card-id
- :value_field (mt/$ids $venues.name)}}]}]]
+ Dashboard [{dashboard-id :id}
+ {:collection_id (:id coll2)
+ :parameters [{:id "abc"
+ :type "category"
+ :name "CATEGORY"
+ :values_source_type "card"
+ :values_source_config {:card_id source-card-id
+ :value_field (mt/$ids $venues.name)}}]}]]
(testing "Fail because user doesn't have read permissions to coll1"
(is (=? "You don't have permissions to do that."
(mt/user-http-request :rasta :get 403 (chain-filter-values-url dashboard-id "abc"))))
diff --git a/test/metabase/api/dataset_test.clj b/test/metabase/api/dataset_test.clj
index 71b652120761f2871f167ca91c2b244576d995a1..3061d00b7fe6a22eff3eb201c7375a12fa3fcaa3 100644
--- a/test/metabase/api/dataset_test.clj
+++ b/test/metabase/api/dataset_test.clj
@@ -9,6 +9,7 @@
[clojure.string :as str]
[clojure.test :refer :all]
[medley.core :as m]
+ [metabase.api.dataset :as api.dataset]
[metabase.api.pivots :as api.pivots]
[metabase.driver :as driver]
[metabase.http-client :as client]
@@ -444,6 +445,7 @@
{:parameter parameter})
:values set)]
(is (= #{"Gizmo" "Widget" "Gadget"} values)))))))
+
(testing "nil value (current behavior of field values)"
(let [parameter {:values_query_type "list",
:values_source_type nil,
@@ -480,4 +482,28 @@
:field_ids [(mt/id :people :source)
(mt/id :people :source)]})
:values)]
- (is (= [["Twitter"] ["Organic"] ["Affiliate"] ["Google"] ["Facebook"]] values))))))))
+ (is (= [["Twitter"] ["Organic"] ["Affiliate"] ["Google"] ["Facebook"]] values))))))
+
+ (testing "fallback to field-values"
+ (with-redefs [api.dataset/parameter-field-values (constantly "field-values")]
+ (testing "if value-field not found in source card"
+ (mt/with-temp Card [{source-card-id :id}]
+ (is (= "field-values"
+ (mt/user-http-request :rasta :post 200 "dataset/parameter/values"
+ {:parameter {:values_source_type "card"
+ :values_source_config {:card_id source-card-id
+ :value_field (mt/$ids $people.source)}
+ :type :string/=,
+ :name "Text"
+ :id "abc"}})))))
+
+ (testing "if value-field not found in source card"
+ (mt/with-temp Card [{source-card-id :id} {:archived true}]
+ (is (= "field-values"
+ (mt/user-http-request :rasta :post 200 "dataset/parameter/values"
+ {:parameter {:values_source_type "card"
+ :values_source_config {:card_id source-card-id
+ :value_field (mt/$ids $people.source)}
+ :type :string/=,
+ :name "Text"
+ :id "abc"}})))))))))
diff --git a/test/metabase/models/params/card_values_test.clj b/test/metabase/models/params/custom_values_test.clj
similarity index 68%
rename from test/metabase/models/params/card_values_test.clj
rename to test/metabase/models/params/custom_values_test.clj
index ff0c1954236eca6721f979dbfe8d9e9ee0781d3e..833888676e8314330c3e4069b147be410db8ec02 100644
--- a/test/metabase/models/params/card_values_test.clj
+++ b/test/metabase/models/params/custom_values_test.clj
@@ -1,15 +1,17 @@
-(ns metabase.models.params.card-values-test
+(ns metabase.models.params.custom-values-test
(:require
[clojure.test :refer :all]
- [metabase.models :refer [Card]]
- [metabase.models.params.card-values :as params.card-values]
+ [metabase.models :refer [Card Collection]]
+ [metabase.models.params.custom-values :as custom-values]
[metabase.test :as mt]
[toucan.db :as db]))
+;;; --------------------------------------------- source=card ----------------------------------------------
+
(deftest with-mbql-card-test
(doseq [dataset? [true false]]
(testing (format "source card is a %s" (if dataset? "model" "question"))
- (binding [params.card-values/*max-rows* 3]
+ (binding [custom-values/*max-rows* 3]
(testing "with simple mbql"
(mt/with-temp* [Card [{card-id :id}
(merge (mt/card-with-source-metadata-for-query (mt/mbql-query venues))
@@ -19,14 +21,14 @@
(testing "get values"
(is (=? {:has_more_values true,
:values ["20th Century Cafe" "25°" "33 Taps"]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
(mt/$ids $venues.name)))))
(testing "case in-sensitve search test"
(is (=? {:has_more_values false
:values ["Liguria Bakery" "Noe Valley Bakery"]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
(mt/$ids $venues.name)
"bakery"))))))
@@ -44,21 +46,21 @@
(testing "get values from breakout columns"
(is (=? {:has_more_values true,
:values ["American" "Artisan" "Asian"]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
(mt/$ids $categories.name)))))
(testing "get values from aggregation column"
(is (=? {:has_more_values true,
:values [1 2 3]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
[:field "sum" {:base-type :type/Float}]))))
(testing "can search on aggregation column"
(is (=? {:has_more_values false,
:values [2]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
[:field "sum" {:base-type :type/Float}]
2))))
@@ -66,7 +68,7 @@
(testing "doing case in-sensitve search on breakout columns"
(is (=? {:has_more_values false
:values ["Bakery"]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
[:field (mt/id :categories :name) {:source-field (mt/id :venues :category_id)}]
"bakery"))))))
@@ -84,7 +86,7 @@
(testing "get values returns the value, not remapped values"
(is (=? {:has_more_values true,
:values [2 3 4]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
(mt/$ids $venues.category_id)))))
@@ -92,7 +94,7 @@
(testing "search with the value, not remapped values"
(is (=? {:has_more_values false,
:values [2]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
(mt/$ids $venues.category_id)
2)))))))))))
@@ -100,7 +102,7 @@
(deftest with-native-card-test
(doseq [dataset? [true false]]
(testing (format "source card is a %s with native question" (if dataset? "model" "question"))
- (binding [params.card-values/*max-rows* 3]
+ (binding [custom-values/*max-rows* 3]
(mt/with-temp* [Card [{card-id :id}
(merge (mt/card-with-source-metadata-for-query
(mt/native-query {:query "select * from venues where lower(name) like '%red%'"}))
@@ -110,7 +112,7 @@
(testing "get values from breakout columns"
(is (=? {:has_more_values false,
:values ["Fred 62" "Red Medicine"]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
[:field "NAME" {:base-type :type/Text}]))))
@@ -118,7 +120,7 @@
(testing "doing case in-sensitve search on breakout columns"
(is (=? {:has_more_values false
:values ["Red Medicine"]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
[:field "NAME" {:base-type :type/Text}]
"medicine")))))))))
@@ -133,7 +135,7 @@
(testing "get values from breakout columns"
(is (=? {:has_more_values false,
:values ["Affiliate" "Facebook" "Google" "Organic" "Twitter"]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
[:field "SOURCE" {:base-type :type/Text}]))))
@@ -141,7 +143,7 @@
(testing "doing case in-sensitve search on breakout columns"
(is (=? {:has_more_values false
:values ["Facebook" "Google"]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
[:field "SOURCE" {:base-type :type/Text}]
"oo"))))))
@@ -153,7 +155,7 @@
(testing "get values from breakout columns"
(is (=? {:has_more_values false,
:values ["Affiliate" "Facebook" "Google" "Organic" "Twitter"]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
(mt/$ids $people.source)))))
@@ -161,36 +163,59 @@
(testing "doing case in-sensitve search on breakout columns"
(is (=? {:has_more_values false
:values ["Facebook" "Google"]}
- (params.card-values/values-from-card
+ (custom-values/values-from-card
(db/select-one Card :id card-id)
(mt/$ids $people.source)
"oo")))))))))
(deftest errors-test
- (testing "error if source card does not exist"
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"Source card not found"
- (params.card-values/param->values
- {:name "Card as source"
- :slug "card"
- :id "_CARD_"
- :type "category"
- :values_source_type "card"
- :values_source_config {:card_id (inc (db/count Card))
- :value_field (mt/$ids $venues.name)}}
- nil))))
- (testing "error if source card is archived"
- (mt/with-temp Card [card {:archived true}]
- (is (thrown-with-msg?
- clojure.lang.ExceptionInfo
- #"Source card is archived"
- (params.card-values/param->values
- {:name "Card as source"
- :slug "card"
- :id "_CARD_"
- :type "category"
- :values_source_type "card"
- :values_source_config {:card_id (:id card)
- :value_field (mt/$ids $venues.name)}}
- nil))))))
+ (testing "error if doesn't have permissions"
+ (mt/with-current-user (mt/user->id :rasta)
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (mt/with-temp*
+ [Collection [coll]
+ Card [card {:collection_id (:id coll)}]]
+ (is (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"You don't have permissions to do that."
+ (custom-values/parameter->values
+ {:name "Card as source"
+ :slug "card"
+ :id "_CARD_"
+ :type "category"
+ :values_source_type "card"
+ :values_source_config {:card_id (:id card)
+ :value_field (mt/$ids $venues.name)}}
+ nil
+ (fn [] (throw (ex-info "Shouldn't call this function" {}))))))))))
+
+ ;; bind to an admin to bypass the permissions check
+ (mt/with-current-user (mt/user->id :crowberto)
+ (testing "call to default-case-fn if "
+ (testing "souce card is archived"
+ (mt/with-temp Card [card {:archived true}]
+ (is (= :archived
+ (custom-values/parameter->values
+ {:name "Card as source"
+ :slug "card"
+ :id "_CARD_"
+ :type "category"
+ :values_source_type "card"
+ :values_source_config {:card_id (:id card)
+ :value_field (mt/$ids $venues.name)}}
+ nil
+ (constantly :archived))))))
+
+ (testing "value-field not found in card's result_metadata"
+ (mt/with-temp Card [card {}]
+ (is (= :field-not-found
+ (custom-values/parameter->values
+ {:name "Card as source"
+ :slug "card"
+ :id "_CARD_"
+ :type "category"
+ :values_source_type "card"
+ :values_source_config {:card_id (:id card)
+ :value_field [:field 0 nil]}}
+ nil
+ (constantly :field-not-found)))))))))
diff --git a/test/metabase/models/permissions_test.clj b/test/metabase/models/permissions_test.clj
index b3d7fbd26bf09d7d64f032b523f46e18240b44d2..cce5ddaef0aed29cdb587b76421a1010899e8abd 100644
--- a/test/metabase/models/permissions_test.clj
+++ b/test/metabase/models/permissions_test.clj
@@ -753,7 +753,7 @@
(testing "Partial permission graphs with no changes to the existing graph do not error when run repeatedly (#25221)"
(mt/with-temp PermissionsGroup [group]
;; Bind *current-user* so that permission revisions are written, which was the source of the original error
- (mt/with-current-user 1
+ (mt/with-current-user (mt/user->id :rasta)
(is (nil? (perms/update-data-perms-graph! {:groups {(u/the-id group) {(mt/id) {:data {:native :none :schemas :none}}}}
:revision (:revision (perms/data-perms-graph))})))
(is (nil? (perms/update-data-perms-graph! {:groups {(u/the-id group) {(mt/id) {:data {:native :none :schemas :none}}}}
@@ -925,8 +925,8 @@
w w w w w w w;
w w;
w w
- w w w w;
- ) ) ) ) ) )
+ w w w w))))));
+
(deftest data-permissions-v2-migration-move-test
(testing "move admin"