Skip to content
Snippets Groups Projects
Unverified Commit ff2c972d authored by Ngoc Khuat's avatar Ngoc Khuat Committed by GitHub
Browse files

Parameter with source is card fallback when card/field not found (#28055)

parent 06f52bd4
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 347 additions and 233 deletions
src/metabase/api/card.clj
+ 4
− 15
View file @ ff2c972d
......@@ -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)
src/metabase/api/dashboard.clj
+ 2
− 8
View file @ ff2c972d
......@@ -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"
......
src/metabase/api/dataset.clj
+ 8
− 15
View file @ ff2c972d
......@@ -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`."
......
src/metabase/models/params.clj
+ 11
− 1
View file @ ff2c972d
(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]
......
src/metabase/models/params/card_values.clj deleted 100644 → 0
+ 0
− 108
View file @ 06f52bd4
(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)))
src/metabase/models/params/custom_values.clj 0 → 100644
+ 161
− 0
View file @ ff2c972d
(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}))))
src/metabase/models/params/static_values.clj deleted 100644 → 0
+ 0
− 28
View file @ 06f52bd4
(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}))
test/metabase/api/card_test.clj
+ 26
− 0
View file @ ff2c972d
......@@ -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*
......
test/metabase/api/dashboard_test.clj
+ 34
− 8
View file @ ff2c972d
......@@ -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"))))
......
test/metabase/api/dataset_test.clj
+ 27
− 1
View file @ ff2c972d
......@@ -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"}})))))))))
test/metabase/models/params/card_values_test.clj → test/metabase/models/params/custom_values_test.clj
+ 71
− 46
View file @ ff2c972d
(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)))))))))
test/metabase/models/permissions_test.clj
+ 3
− 3
View file @ ff2c972d
......@@ -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"
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment