diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml
index 1c99056f602d6848532f578595d588a8abf39440..d38a1e834a491613645d4d19de6e6adda727c879 100644
--- a/resources/migrations/000_migrations.yaml
+++ b/resources/migrations/000_migrations.yaml
@@ -4020,3 +4020,14 @@ databaseChangeLog:
remarks: 'True if a XLS of the data should be included for this pulse card'
constraints:
nullable: false
+ - changeSet:
+ id: 75
+ author: camsaul
+ comment: 'Added 0.28.2'
+ changes:
+ - addColumn:
+ tableName: report_card
+ columns:
+ name: read_permissions
+ type: text
+ remarks: 'Permissions required to view this Card and run its query.'
diff --git a/src/metabase/api/database.clj b/src/metabase/api/database.clj
index 97d03cd54e5d3f6ece2cfd277bc0fa542851ba26..0c5ac65cc61e3ef0e1570896897b75d17bd6b022 100644
--- a/src/metabase/api/database.clj
+++ b/src/metabase/api/database.clj
@@ -55,7 +55,9 @@
native (e.g. SQL) queries. Will be one of `:write`, `:read`, or `:none`."
[dbs]
(for [db dbs]
- (let [user-has-perms? (fn [path-fn] (perms/set-has-full-permissions? @api/*current-user-permissions-set* (path-fn (u/get-id db))))]
+ (let [user-has-perms? (fn [path-fn]
+ (perms/set-has-full-permissions? @api/*current-user-permissions-set*
+ (path-fn (u/get-id db))))]
(assoc db :native_permissions (cond
(user-has-perms? perms/native-readwrite-path) :write
(user-has-perms? perms/native-read-path) :read
@@ -103,7 +105,8 @@
(defn- source-query-cards
"Fetch the Cards that can be used as source queries (e.g. presented as virtual tables)."
[]
- (as-> (db/select [Card :name :description :database_id :dataset_query :id :collection_id :result_metadata]
+ (as-> (db/select [Card :name :description :database_id :dataset_query :id :collection_id :result_metadata
+ :read_permissions]
:result_metadata [:not= nil] :archived false
{:order-by [[:%lower.name :asc]]}) <>
(filter card-database-supports-nested-queries? <>)
@@ -142,7 +145,9 @@
include-cards? add-virtual-tables-for-saved-cards)))
(api/defendpoint GET "/"
- "Fetch all `Databases`."
+ "Fetch all `Databases`. `include_tables` means we should hydrate the Tables belonging to each DB. `include_cards` here
+ means we should also include virtual Table entries for saved Questions, e.g. so we can easily use them as source
+ Tables in queries. Default for both is `false`."
[include_tables include_cards]
{include_tables (s/maybe su/BooleanString)
include_cards (s/maybe su/BooleanString)}
diff --git a/src/metabase/db/migrations.clj b/src/metabase/db/migrations.clj
index 1dc0844abf562b697f2e6629db3c648fe1089528..e6077ffb556f32d0af8d23675b8dff26df0f29ca 100644
--- a/src/metabase/db/migrations.clj
+++ b/src/metabase/db/migrations.clj
@@ -361,3 +361,16 @@
;; either way, delete the old value from the DB since we'll never be using it again.
;; use `simple-delete!` because `Setting` doesn't have an `:id` column :(
(db/simple-delete! Setting {:key "enable-advanced-humanization"})))
+
+;; for every Card in the DB, pre-calculate the read permissions required to read the Card/run its query and save them
+;; under the new `read_permissions` column. Calculating read permissions is too expensive to do on the fly for Cards,
+;; since it requires parsing their queries and expanding things like FKs or Segment/Metric macros. Simply calling
+;; `update!` on each Card will cause it to be saved with updated `read_permissions` as a side effect of Card's
+;; `pre-update` implementation.
+;;
+;; Caching these permissions will prevent 1000+ DB call API calls. See https://github.com/metabase/metabase/issues/6889
+(defmigration ^{:author "camsaul", :added "0.28.2"} populate-card-read-permissions
+ (run!
+ (fn [card]
+ (db/update! Card (u/get-id card) {}))
+ (db/select-reducible Card :archived false, :read_permissions nil)))
diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj
index dbe47eec12ab2da9d1c567282e568e824c1ce8c9..389969c91d256a8dc398daf40cec71a1e051b8d4 100644
--- a/src/metabase/models/card.clj
+++ b/src/metabase/models/card.clj
@@ -114,28 +114,48 @@
(u/pprint-to-str (u/filtered-stacktrace e)))
#{"/db/0/"}))) ; DB 0 will never exist
-;; it takes a lot of DB calls and function calls to expand/resolve a query, and since they're pure functions we can
-;; save ourselves some a lot of DB calls by caching the results. Cache the permissions reqquired to run a given query
-;; dictionary for up to 6 hours
+;; Calculating Card read permissions is rather expensive, since we must parse and expand the Card's query in order to
+;; find the Tables it references. Since we read Cards relatively often, these permissions are cached in the
+;; `:read_permissions` column of Card. There should not be situtations where these permissions are not cached; but if
+;; for some strange reason they are we will go ahead and recalculate them.
+
;; TODO - what if the query uses a source query, and that query changes? Not sure if that will cause an issue or not.
-;; May need to revisit this
-(defn- query-perms-set* [{query-type :type, database :database, :as query} read-or-write]
+(defn query-perms-set
+ "Calculate the set of permissions required to `:read`/run or `:write` (update) a Card with `query`. In normal cases
+ for read permissions you should look at a Card's `:read_permissions`, which is precalculated. If you specifically
+ need to calculate permissions for a query directly, and ignore anything precomputed, use this function. Otherwise
+ you should rely on one of the optimized ones below."
+ [{query-type :type, database :database, :as query} read-or-write]
(cond
- (= query {}) #{}
+ (empty? query) #{}
(= (keyword query-type) :native) #{(native-permissions-path read-or-write database)}
(= (keyword query-type) :query) (mbql-permissions-path-set read-or-write query)
:else (throw (Exception. (str "Invalid query type: " query-type)))))
-(def ^{:arglists '([query read-or-write])} query-perms-set
- "Return a set of required permissions for *running* QUERY (if READ-OR-WRITE is `:read`) or *saving* it (if
- READ-OR-WRITE is `:write`)."
- (memoize/ttl query-perms-set* :ttl/threshold (* 6 60 60 1000))) ; memoize for 6 hours
-(defn- perms-objects-set
- "Return a set of required permissions object paths for CARD.
- Optionally specify whether you want `:read` or `:write` permissions; default is `:read`.
- (`:write` permissions only affects native queries)."
- [{query :dataset_query, collection-id :collection_id, public-uuid :public_uuid, in-public-dash? :in_public_dashboard}
+(defn- card-perms-set-for-query
+ "Return the permissions required to `read-or-write` `card` based on its query, disregarding the collection the Card is
+ in, whether it is publicly shared, etc. This will return precalculated `:read_permissions` if they are present."
+ [{read-perms :read_permissions, id :id, query :dataset_query} read-or-write]
+ (cond
+ ;; for WRITE permissions always recalculate since these will be determined relatively infrequently (hopefully)
+ ;; e.g. when updating a Card
+ (= :write read-or-write) (query-perms-set query :write)
+ ;; if the Card has populated `:read_permissions` and we're looking up read pems return those rather than calculating
+ ;; on-the-fly. Cast to `set` to be extra-double-sure it's a set like we'd expect when it gets deserialized from JSON
+ read-perms (set read-perms)
+ ;; otherwise if :read_permissions was NOT populated. This should not normally happen since the data migration
+ ;; should have pre-populated values for all the Cards. If it does it might mean something like we fetched the Card
+ ;; without its `read_permissions` column. Since that would be "doing something wrong" warn about it.
+ :else (do (log/warn "Card" id "is missing its read_permissions. Calculating them now...")
+ (query-perms-set query :read))))
+
+(defn- card-perms-set-for-current-user
+ "Calculate the permissions required to `read-or-write` `card` *for the current user*. This takes into account whether
+ the Card is publicly available, or in a collection the current user can view; it also attempts to use precalcuated
+ `read_permissions` when possible. This is the function that should be used for general permissions checking for a
+ Card."
+ [{collection-id :collection_id, public-uuid :public_uuid, in-public-dash? :in_public_dashboard, :as card}
read-or-write]
(cond
;; you don't need any permissions to READ a public card, which is PUBLIC by definition :D
@@ -148,7 +168,7 @@
(collection/perms-objects-set collection-id read-or-write)
:else
- (query-perms-set query read-or-write)))
+ (card-perms-set-for-query card read-or-write)))
;;; -------------------------------------------------- Dependencies --------------------------------------------------
@@ -201,15 +221,16 @@
:query_type (keyword query-type)})
card))
-(defn- pre-insert [{:keys [dataset_query], :as card}]
+(defn- pre-insert [{query :dataset_query, :as card}]
;; TODO - make sure if `collection_id` is specified that we have write permissions for that collection
- (u/prog1 card
+ ;; Save the new Card with read permissions since calculating them dynamically is so expensive.
+ (u/prog1 (assoc card :read_permissions (query-perms-set query :read))
;; for native queries we need to make sure the user saving the card has native query permissions for the DB
;; because users can always see native Cards and we don't want someone getting around their lack of permissions
;; that way
(when (and *current-user-id*
- (= (keyword (:type dataset_query)) :native))
- (let [database (db/select-one ['Database :id :name], :id (:database dataset_query))]
+ (= (keyword (:type query)) :native))
+ (let [database (db/select-one ['Database :id :name], :id (:database query))]
(qp-perms/throw-if-cannot-run-new-native-query-referencing-db database)))))
(defn- post-insert [card]
@@ -220,8 +241,9 @@
(log/info "Card references Fields in params:" field-ids)
(field-values/update-field-values-for-on-demand-dbs! field-ids))))
-(defn- pre-update [{archived? :archived, :as card}]
- (u/prog1 card
+(defn- pre-update [{archived? :archived, query :dataset_query, :as card}]
+ ;; save the updated Card with updated read permissions.
+ (u/prog1 (assoc card :read_permissions (query-perms-set query :read))
;; if the Card is archived, then remove it from any Dashboards
(when archived?
(db/delete! 'DashboardCard :card_id (u/get-id card)))
@@ -260,7 +282,8 @@
:embedding_params :json
:query_type :keyword
:result_metadata :json
- :visualization_settings :json})
+ :visualization_settings :json
+ :read_permissions :json-set})
:properties (constantly {:timestamped? true})
:pre-update (comp populate-query-fields pre-update)
:pre-insert (comp populate-query-fields pre-insert)
@@ -272,7 +295,7 @@
(merge i/IObjectPermissionsDefaults
{:can-read? (partial i/current-user-has-full-permissions? :read)
:can-write? (partial i/current-user-has-full-permissions? :write)
- :perms-objects-set perms-objects-set})
+ :perms-objects-set card-perms-set-for-current-user})
revision/IRevisioned
(assoc revision/IRevisionedDefaults
diff --git a/src/metabase/models/common.clj b/src/metabase/models/common.clj
index b40d81aa5b05fc6dbcc9b579a8e99993545bf185..cc35601ae04f0108acd0e13df3b66e956a90bfb5 100644
--- a/src/metabase/models/common.clj
+++ b/src/metabase/models/common.clj
@@ -1,6 +1,6 @@
(ns metabase.models.common)
-(def ^:const timezones
+(def timezones
"The different timezones supported by Metabase.
Presented as options for the `report-timezone` Setting in the admin panel."
["Africa/Algiers"
diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj
index 796d154c6600c81bfb7592eee23d3acd2e77f7eb..b938bed4f8dd4bf53234119f41ee8e665f1fc510 100644
--- a/src/metabase/models/interface.clj
+++ b/src/metabase/models/interface.clj
@@ -32,6 +32,12 @@
:in json-in
:out json-out)
+;; json-set is just like json but calls `set` on it when coming out of the DB. Intended for storing things like a
+;; permissions set
+(models/add-type! :json-set
+ :in json-in
+ :out #(when % (set (json-out %))))
+
(models/add-type! :clob
:in identity
:out u/jdbc-clob->str)
diff --git a/src/metabase/task/sync_databases.clj b/src/metabase/task/sync_databases.clj
index 12450ed14b47a1a4c4e946bfa575726b02f939bb..3c1394e53f02721e5bb1f310c21442b22a948815 100644
--- a/src/metabase/task/sync_databases.clj
+++ b/src/metabase/task/sync_databases.clj
@@ -20,9 +20,9 @@
(:import metabase.models.database.DatabaseInstance
[org.quartz CronTrigger DisallowConcurrentExecution JobDetail JobKey TriggerKey]))
-;;; +------------------------------------------------------------------------------------------------------------------------+
-;;; | JOB LOGIC |
-;;; +------------------------------------------------------------------------------------------------------------------------+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | JOB LOGIC |
+;;; +----------------------------------------------------------------------------------------------------------------+
(s/defn ^:private job-context->database :- DatabaseInstance
"Get the Database referred to in JOB-CONTEXT. Guaranteed to return a valid Database."
@@ -43,9 +43,9 @@
(when (:is_full_sync database)
(field-values/update-field-values! (job-context->database job-context)))))
-;;; +------------------------------------------------------------------------------------------------------------------------+
-;;; | TASK INFO AND GETTER FUNCTIONS |
-;;; +------------------------------------------------------------------------------------------------------------------------+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | TASK INFO AND GETTER FUNCTIONS |
+;;; +----------------------------------------------------------------------------------------------------------------+
(def ^:private TaskInfo
"One-off schema for information about the various sync tasks we run for a DB."
@@ -64,7 +64,8 @@
:job-class UpdateFieldValues})
-;; These getter functions are not strictly neccesary but are provided primarily so we can get some extra validation by using them
+;; These getter functions are not strictly neccesary but are provided primarily so we can get some extra validation by
+;; using them
(s/defn ^:private job-key :- JobKey
"Return an appropriate string key for the job described by TASK-INFO for DATABASE-OR-ID."
@@ -97,15 +98,16 @@
(format "%s for all databases" (name (:key task-info))))
-;;; +------------------------------------------------------------------------------------------------------------------------+
-;;; | DELETING TASKS FOR A DB |
-;;; +------------------------------------------------------------------------------------------------------------------------+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | DELETING TASKS FOR A DB |
+;;; +----------------------------------------------------------------------------------------------------------------+
(s/defn ^:private delete-task!
"Cancel a single sync task for DATABASE-OR-ID and TASK-INFO."
[database :- DatabaseInstance, task-info :- TaskInfo]
(let [trigger-key (trigger-key database task-info)]
- (log/debug (u/format-color 'red "Unscheduling task for Database %d: trigger: %s" (u/get-id database) (.getName trigger-key)))
+ (log/debug (u/format-color 'red "Unscheduling task for Database %d: trigger: %s"
+ (u/get-id database) (.getName trigger-key)))
(task/delete-trigger! trigger-key)))
(s/defn unschedule-tasks-for-db!
@@ -114,9 +116,10 @@
(delete-task! database sync-analyze-task-info)
(delete-task! database field-values-task-info))
-;;; +------------------------------------------------------------------------------------------------------------------------+
-;;; | (RE)SCHEDULING TASKS FOR A DB |
-;;; +------------------------------------------------------------------------------------------------------------------------+
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | (RE)SCHEDULING TASKS FOR A DB |
+;;; +----------------------------------------------------------------------------------------------------------------+
(s/defn ^:private job :- JobDetail
"Build a durable Quartz Job for TASK-INFO. Durable in Quartz allows the job to exist even if there are no triggers
@@ -158,17 +161,19 @@
;; unschedule any tasks that might already be scheduled
(unschedule-tasks-for-db! database)
- (log/debug (u/format-color 'green "Scheduling sync/analyze and field-values task for database %d: trigger: %s and trigger: %s"
- (u/get-id database) (.getName (.getKey sync-trigger))
- (u/get-id database) (.getName (.getKey fv-trigger))))
+ (log/debug
+ (u/format-color 'green "Scheduling sync/analyze and field-values task for database %d: trigger: %s and trigger: %s"
+ (u/get-id database) (.getName (.getKey sync-trigger))
+ (u/get-id database) (.getName (.getKey fv-trigger))))
;; now (re)schedule all the tasks
(task/add-trigger! sync-trigger)
(task/add-trigger! fv-trigger)))
-;;; +------------------------------------------------------------------------------------------------------------------------+
-;;; | TASK INITIALIZATION |
-;;; +------------------------------------------------------------------------------------------------------------------------+
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | TASK INITIALIZATION |
+;;; +----------------------------------------------------------------------------------------------------------------+
(defn- job-init
"Separated from `task-init` primarily as it's useful in testing. Adds the sync and field-values job that all of the
@@ -179,7 +184,7 @@
(defn task-init
"Automatically called during startup; start the jobs for syncing/analyzing and updating FieldValues for all
- Databases."
+ Databases."
[]
(job-init)
(doseq [database (db/select Database)]
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index 39eda790784de774a2da37c068947d2230e66c46..055f44e5ea9a6a5c1cb31fe42c8c12eab14f1b6b 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -213,9 +213,10 @@
:database_id database-id ; these should be inferred automatically
:table_id table-id
:labels []
- :can_write true,
- :dashboard_count 0,
+ :can_write true
+ :dashboard_count 0
:collection nil
+ :read_permissions [(format "/db/%d/schema//table/%d/" database-id table-id)]
:creator (match-$ (fetch-user :rasta)
{:common_name "Rasta Toucan"
:is_superuser false
@@ -295,6 +296,7 @@
:id $})
:updated_at $
:dataset_query $
+ :read_permissions [(format "/db/%d/schema//table/%d/" database-id table-id)]
:id $
:display "table"
:visualization_settings {}
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index c79305af0615355491ef2e3e4fc2fe52e610e545..788c6bd7e03ab9fa42aaad86ebe2b79465df447f 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -133,6 +133,7 @@
:display "table"
:query_type nil
:dataset_query {}
+ :read_permissions []
:visualization_settings {}
:query_average_duration nil
:in_public_dashboard false
diff --git a/test/metabase/api/embed_test.clj b/test/metabase/api/embed_test.clj
index 6818ada0e8b198d27844d0fcf6ad07c8bf549715..74cfc365e5a9362608538b0fc1441e29e163fb7e 100644
--- a/test/metabase/api/embed_test.clj
+++ b/test/metabase/api/embed_test.clj
@@ -139,10 +139,12 @@
[{:id nil, :type "date/single", :target ["variable" ["template-tag" "d"]], :name "d", :slug "d", :default nil}]
(with-embedding-enabled-and-new-secret-key
(with-temp-card [card {:enable_embedding true
- :dataset_query {:native {:template_tags {:a {:type "date", :name "a", :display_name "a"}
- :b {:type "date", :name "b", :display_name "b"}
- :c {:type "date", :name "c", :display_name "c"}
- :d {:type "date", :name "d", :display_name "d"}}}}
+ :dataset_query {:database (data/id)
+ :type :native
+ :native {:template_tags {:a {:type "date", :name "a", :display_name "a"}
+ :b {:type "date", :name "b", :display_name "b"}
+ :c {:type "date", :name "c", :display_name "c"}
+ :d {:type "date", :name "d", :display_name "d"}}}}
:embedding_params {:a "locked", :b "disabled", :c "enabled", :d "enabled"}}]
(:parameters (http/client :get 200 (card-url card {:params {:c 100}}))))))
diff --git a/test/metabase/api/preview_embed_test.clj b/test/metabase/api/preview_embed_test.clj
index e43f6f91636318691512faa2dd146f09d9ba98d7..322278c8b19d45837cd336a6af6c26a0565643ee 100644
--- a/test/metabase/api/preview_embed_test.clj
+++ b/test/metabase/api/preview_embed_test.clj
@@ -48,10 +48,12 @@
[{:id nil, :type "date/single", :target ["variable" ["template-tag" "d"]], :name "d", :slug "d", :default nil}]
(embed-test/with-embedding-enabled-and-new-secret-key
(embed-test/with-temp-card [card {:dataset_query
- {:native {:template_tags {:a {:type "date", :name "a", :display_name "a"}
- :b {:type "date", :name "b", :display_name "b"}
- :c {:type "date", :name "c", :display_name "c"}
- :d {:type "date", :name "d", :display_name "d"}}}}}]
+ {:database (data/id)
+ :type :native
+ :native {:template_tags {:a {:type "date", :name "a", :display_name "a"}
+ :b {:type "date", :name "b", :display_name "b"}
+ :c {:type "date", :name "c", :display_name "c"}
+ :d {:type "date", :name "d", :display_name "d"}}}}}]
(-> ((test-users/user->client :crowberto) :get 200 (card-url card {:_embedding_params {:a "locked"
:b "disabled"
:c "enabled"
diff --git a/test/metabase/api/public_test.clj b/test/metabase/api/public_test.clj
index 1356f0ceda35488926818f189546569b3b426793..088808c9ef04d16a3bc822739e49e90d918209c0 100644
--- a/test/metabase/api/public_test.clj
+++ b/test/metabase/api/public_test.clj
@@ -53,7 +53,9 @@
(defn- add-card-to-dashboard! [card dashboard]
(db/insert! DashboardCard :dashboard_id (u/get-id dashboard), :card_id (u/get-id card)))
-(defmacro with-temp-public-dashboard-and-card {:style/indent 1} [[dashboard-binding card-binding & [dashcard-binding]] & body]
+(defmacro ^:private with-temp-public-dashboard-and-card
+ {:style/indent 1}
+ [[dashboard-binding card-binding & [dashcard-binding]] & body]
`(with-temp-public-dashboard [dash#]
(with-temp-public-card [card#]
(let [~dashboard-binding dash#
@@ -98,18 +100,20 @@
{(data/id :categories :name) {:values 75
:human_readable_values {}
:field_id (data/id :categories :name)}}
- (tt/with-temp Card [card {:dataset_query {:type :native
- :native {:query (str "SELECT COUNT(*) "
- "FROM venues "
- "LEFT JOIN categories ON venues.category_id = categories.id "
- "WHERE {{category}}")
- :collection "CATEGORIES"
- :template_tags {:category {:name "category"
- :display_name "Category"
- :type "dimension"
- :dimension ["field-id" (data/id :categories :name)]
- :widget_type "category"
- :required true}}}}}]
+ (tt/with-temp Card [card {:dataset_query
+ {:database (data/id)
+ :type :native
+ :native {:query (str "SELECT COUNT(*) "
+ "FROM venues "
+ "LEFT JOIN categories ON venues.category_id = categories.id "
+ "WHERE {{category}}")
+ :collection "CATEGORIES"
+ :template_tags {:category {:name "category"
+ :display_name "Category"
+ :type "dimension"
+ :dimension ["field-id" (data/id :categories :name)]
+ :widget_type "category"
+ :required true}}}}}]
(-> (:param_values (#'public-api/public-card :id (u/get-id card)))
(update-in [(data/id :categories :name) :values] count))))
@@ -313,7 +317,9 @@
(tu/with-temporary-setting-values [enable-public-sharing true]
(with-temp-public-dashboard-and-card [dash card]
(with-temp-public-card [card-2]
- (tt/with-temp DashboardCardSeries [_ {:dashboardcard_id (db/select-one-id DashboardCard :card_id (u/get-id card), :dashboard_id (u/get-id dash))
+ (tt/with-temp DashboardCardSeries [_ {:dashboardcard_id (db/select-one-id DashboardCard
+ :card_id (u/get-id card)
+ :dashboard_id (u/get-id dash))
:card_id (u/get-id card-2)}]
(qp-test/rows (http/client :get 200 (dashcard-url-path dash card-2))))))))
@@ -334,7 +340,8 @@
(db/update! Dashboard (u/get-id dashboard) :parameters [{:name "Price", :type "category", :slug "price"}]))
(defn- add-dimension-param-mapping-to-dashcard! [dashcard card dimension]
- (db/update! DashboardCard (u/get-id dashcard) :parameter_mappings [{:card_id (u/get-id card), :target ["dimension" dimension]}]))
+ (db/update! DashboardCard (u/get-id dashcard) :parameter_mappings [{:card_id (u/get-id card)
+ :target ["dimension" dimension]}]))
(defn- GET-param-values [dashboard]
(tu/with-temporary-setting-values [enable-public-sharing true]
@@ -344,7 +351,13 @@
(expect
(price-param-values)
(with-temp-public-dashboard-and-card [dash card dashcard]
- (db/update! Card (u/get-id card) :dataset_query {:native {:template_tags {:price {:name "price", :display_name "Price", :type "dimension", :dimension ["field-id" (data/id :venues :price)]}}}})
+ (db/update! Card (u/get-id card)
+ :dataset_query {:database (data/id)
+ :type :native
+ :native {:template_tags {:price {:name "price"
+ :display_name "Price"
+ :type "dimension"
+ :dimension ["field-id" (data/id :venues :price)]}}}})
(add-price-param-to-dashboard! dash)
(add-dimension-param-mapping-to-dashcard! dashcard card ["template-tag" "price"])
(GET-param-values dash)))
diff --git a/test/metabase/events/revision_test.clj b/test/metabase/events/revision_test.clj
index 85ce227362a4d8e699d39a4e354630c32a251dfa..27a7177a1ce304747d6b2197afaffd989fb3625c 100644
--- a/test/metabase/events/revision_test.clj
+++ b/test/metabase/events/revision_test.clj
@@ -33,6 +33,7 @@
:creator_id (:creator_id card)
:database_id (id)
:dataset_query (:dataset_query card)
+ :read_permissions (vec (:read_permissions card))
:description nil
:display "table"
:enable_embedding false