From e91e00b395f585d2aaee271de0c8247c092d0a0b Mon Sep 17 00:00:00 2001
From: Ngoc Khuat <qn.khuat@gmail.com>
Date: Thu, 11 May 2023 11:50:30 +0700
Subject: [PATCH] Fix custom mapping doesn't work on dashboard (#30217)
* revert remove hydrate param values in #22836
* fix tests in ee ns
* fix param field tests
* Add e2e test
* Add e2e tests
* Add e2e tests
* fix failed test and use t2 instead of db
* fix wrong t2 call spec
* fix failing tests
* Merge branch 'master' into ngoc-revert-remove-hydrate-param-values
* Merge branch 'master' into ngoc-revert-remove-hydrate-param-values
* apply Tamas's suggestions and unskip e2e tests
* delete the e2e tests that AlexP previously added in this PR but brought it into a seperated PR"
"
* create Field values if not found
* add a tests for when field values is not existed, we create them
* Merge branch 'master' into ngoc-revert-remove-hydrate-param-values
* Fix the e2e test suite for remapping
* Fix the e2e test suite for remapping
* Fix the e2e test suite for remapping
* Fix the e2e test suite for remapping
* Fix the e2e test suite for remapping
* revert the changes to create full values on demand
* remove unused requires
* Merge branch 'master' into ngoc-revert-remove-hydrate-param-values
* fix tests
* Merge branch 'master' into ngoc-revert-remove-hydrate-param-values
* Merge branch 'master' into ngoc-revert-remove-hydrate-param-values
Co-Authored-By: Alexander Polyankin <alexander.polyankin@metabase.com>
---
.../29347-remapped-field-values.cy.spec.js | 53 ++++++++++++--
.../sandbox/models/params/field_values.clj | 21 ++++++
.../sandbox/api/dashboard_test.clj | 41 +++++++++++
src/metabase/api/dashboard.clj | 2 +-
src/metabase/api/public.clj | 38 +++++-----
src/metabase/models/params.clj | 49 ++++++++++++-
src/metabase/models/params/field_values.clj | 26 +++++++
test/metabase/api/dashboard_test.clj | 31 ++++++++
test/metabase/api/embed_test.clj | 3 +-
test/metabase/api/public_test.clj | 70 ++++++++++++++++++-
10 files changed, 307 insertions(+), 27 deletions(-)
diff --git a/e2e/test/scenarios/dashboard-filters/reproductions/29347-remapped-field-values.cy.spec.js b/e2e/test/scenarios/dashboard-filters/reproductions/29347-remapped-field-values.cy.spec.js
index 58ea760d6b9..4b535b8c3da 100644
--- a/e2e/test/scenarios/dashboard-filters/reproductions/29347-remapped-field-values.cy.spec.js
+++ b/e2e/test/scenarios/dashboard-filters/reproductions/29347-remapped-field-values.cy.spec.js
@@ -27,7 +27,7 @@ const questionDetails = {
},
};
-const dashboardDetails = {
+const editableDashboardDetails = {
parameters: [filterDetails],
enable_embedding: true,
embedding_params: {
@@ -35,12 +35,19 @@ const dashboardDetails = {
},
};
-describe.skip("issues 29347, 29346", () => {
+const lockedDashboardDetails = {
+ parameters: [filterDetails],
+ enable_embedding: true,
+ embedding_params: {
+ [filterDetails.slug]: "locked",
+ },
+};
+
+describe("issues 29347, 29346", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
addFieldRemapping(ORDERS.QUANTITY);
- createDashboard();
});
describe("regular dashboards", () => {
@@ -50,6 +57,7 @@ describe.skip("issues 29347, 29346", () => {
});
it("should be able to filter on remapped values (metabase#29347, metabase#29346)", () => {
+ createDashboard();
cy.get("@dashboardId").then(dashboardId => visitDashboard(dashboardId));
cy.wait("@dashboard");
cy.wait("@cardQuery");
@@ -61,6 +69,7 @@ describe.skip("issues 29347, 29346", () => {
});
it("should be able to filter on remapped values in the url (metabase#29347, metabase#29346)", () => {
+ createDashboard();
cy.get("@dashboardId").then(dashboardId => {
visitDashboard(dashboardId, {
params: { [filterDetails.slug]: filterValue },
@@ -80,6 +89,7 @@ describe.skip("issues 29347, 29346", () => {
});
it("should be able to filter on remapped values (metabase#29347, metabase#29346)", () => {
+ createDashboard();
cy.get("@dashboardId").then(dashboardId =>
visitEmbeddedPage({
resource: { dashboard: dashboardId },
@@ -96,15 +106,37 @@ describe.skip("issues 29347, 29346", () => {
});
it("should be able to filter on remapped values in the token (metabase#29347, metabase#29346)", () => {
+ createDashboard({ dashboardDetails: lockedDashboardDetails });
cy.get("@dashboardId").then(dashboardId => {
visitEmbeddedPage({
resource: { dashboard: dashboardId },
- params: { [filterDetails.slug]: filterValue },
+ params: {
+ [filterDetails.slug]: filterValue,
+ },
});
});
cy.wait("@dashboard");
cy.wait("@cardQuery");
+ verifyRemappedCardValues(filterValue);
+ });
+
+ it("should be able to filter on remapped values in the url (metabase#29347, metabase#29346)", () => {
+ createDashboard();
+ cy.get("@dashboardId").then(dashboardId => {
+ visitEmbeddedPage(
+ {
+ resource: { dashboard: dashboardId },
+ params: {},
+ },
+ {
+ setFilters: `${filterDetails.slug}=${filterValue}`,
+ },
+ );
+ });
+ cy.wait("@dashboard");
+ cy.wait("@cardQuery");
+
verifyRemappedValues(filterValue);
});
});
@@ -116,6 +148,7 @@ describe.skip("issues 29347, 29346", () => {
});
it("should be able to filter on remapped values (metabase#29347, metabase#29346)", () => {
+ createDashboard();
cy.get("@dashboardId").then(dashboardId =>
visitPublicDashboard(dashboardId),
);
@@ -129,6 +162,7 @@ describe.skip("issues 29347, 29346", () => {
});
it("should be able to filter on remapped values in the url (metabase#29347, metabase#29346)", () => {
+ createDashboard();
cy.get("@dashboardId").then(dashboardId => {
visitPublicDashboard(dashboardId, {
params: { [filterDetails.slug]: filterValue },
@@ -165,7 +199,9 @@ const addFieldRemapping = fieldId => {
);
};
-const createDashboard = () => {
+const createDashboard = ({
+ dashboardDetails = editableDashboardDetails,
+} = {}) => {
cy.createQuestionAndDashboard({ questionDetails, dashboardDetails }).then(
({ body: { id, card_id, dashboard_id } }) => {
cy.request("PUT", `/api/dashboard/${dashboard_id}/cards`, {
@@ -205,10 +241,17 @@ const filterOnRemappedValues = fieldValue => {
};
const verifyRemappedValues = fieldValue => {
+ verifyRemappedFilterValues(filterValue);
+ verifyRemappedCardValues(fieldValue);
+};
+
+const verifyRemappedFilterValues = fieldValue => {
filterWidget().within(() => {
cy.findByText(getRemappedValue(fieldValue)).should("be.visible");
});
+};
+const verifyRemappedCardValues = fieldValue => {
getDashboardCard().within(() => {
cy.findAllByText(getRemappedValue(fieldValue)).should("have.length", 2);
});
diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/models/params/field_values.clj b/enterprise/backend/src/metabase_enterprise/sandbox/models/params/field_values.clj
index c04d8734fa8..c31a340e45e 100644
--- a/enterprise/backend/src/metabase_enterprise/sandbox/models/params/field_values.clj
+++ b/enterprise/backend/src/metabase_enterprise/sandbox/models/params/field_values.clj
@@ -10,6 +10,7 @@
[metabase.models.field-values :as field-values]
[metabase.models.params.field-values :as params.field-values]
[metabase.public-settings.premium-features :refer [defenterprise]]
+ [metabase.util :as u]
[toucan.hydrate :refer [hydrate]]
[toucan2.core :as t2]))
@@ -75,6 +76,26 @@
(mbql.u/match-one v [:dimension [:field field-id _]] field-id))]
{k (get login-attributes k)})))])))
+(defenterprise field-id->field-values-for-current-user
+ "Fetch *existing* FieldValues for a sequence of `field-ids` for the current User. Values are returned as a map of
+ {field-id FieldValues-instance}
+ Returns `nil` if `field-ids` is empty or no matching FieldValues exist."
+ :feature :sandboxes
+ [field-ids]
+ (let [fields (when (seq field-ids)
+ (hydrate (t2/select Field :id [:in (set field-ids)]) :table))
+ {unsandboxed-fields false
+ sandboxed-fields true} (group-by (comp boolean field-is-sandboxed?) fields)]
+ (merge
+ ;; use the normal OSS batched implementation for any Fields that aren't subject to sandboxing.
+ (when (seq unsandboxed-fields)
+ (params.field-values/default-field-id->field-values-for-current-user
+ (map u/the-id unsandboxed-fields)))
+ ;; for sandboxed fields, fetch the sandboxed values individually.
+ (into {} (for [{field-id :id, :as field} sandboxed-fields]
+ [field-id (select-keys (params.field-values/get-or-create-advanced-field-values! :sandbox field)
+ [:values :human_readable_values :field_id])])))))
+
(defenterprise get-or-create-field-values-for-current-user!*
"Fetch cached FieldValues for a `field`, creating them if needed if the Field should have FieldValues. These
should be filtered as appropriate for the current User (currently this only applies to the EE impl)."
diff --git a/enterprise/backend/test/metabase_enterprise/sandbox/api/dashboard_test.clj b/enterprise/backend/test/metabase_enterprise/sandbox/api/dashboard_test.clj
index db835c4e582..0d455683a1d 100644
--- a/enterprise/backend/test/metabase_enterprise/sandbox/api/dashboard_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/sandbox/api/dashboard_test.clj
@@ -2,6 +2,7 @@
"Tests for special behavior of `/api/metabase/dashboard` endpoints in the Metabase Enterprise Edition."
(:require
[clojure.test :refer :all]
+ [metabase-enterprise.sandbox.test-util :as mt.tu]
[metabase-enterprise.test :as met]
[metabase.api.dashboard-test :as api.dashboard-test]
[metabase.models :refer [Card Dashboard DashboardCard FieldValues]]
@@ -14,6 +15,46 @@
[schema.core :as s]
[toucan2.core :as t2]))
+(deftest params-values-test
+ (testing "Return sandboxed `param_values` for Fields to which the current User only has sandboxed access."
+ (met/with-gtaps {:gtaps {:venues
+ {:remappings {:cat [:variable [:field-id (mt/id :venues :category_id)]]}
+ :query (mt.tu/restricted-column-query (mt/id))}}
+ :attributes {:cat 50}}
+ (perms/grant-permissions! &group (perms/table-read-path (mt/id :categories)))
+ (mt/with-temp* [Dashboard [{dashboard-id :id} {:name "Test Dashboard"}]
+ Card [{card-id :id} {:name "Dashboard Test Card"}]
+ DashboardCard [{_ :id} {:dashboard_id dashboard-id
+ :card_id card-id
+ :parameter_mappings [{:card_id card-id
+ :parameter_id "foo"
+ :target [:dimension
+ [:field (mt/id :venues :name) nil]]}
+ ;; should be returned normally since user has non-sandbox perms
+ {:card_id card-id
+ :parameter_id "bar"
+ :target [:dimension
+ [:field (mt/id :categories :name) nil]]}
+ ;; shouldn't be returned since user has no perms
+ {:card_id card-id
+ :parameter_id "bax"
+ :target [:dimension
+ [:field (mt/id :users :name) nil]]}]}]]
+ (is (= {(mt/id :venues :name) {:values ["Garaje"
+ "Gordo Taqueria"
+ "La Tortilla"]
+ :human_readable_values []
+ :field_id (mt/id :venues :name)}
+
+ (mt/id :categories :name) {:values ["African"
+ "American"
+ "Artisan"]
+ :human_readable_values []
+ :field_id (mt/id :categories :name)}}
+ (let [response (:param_values (mt/user-http-request :rasta :get 200 (str "dashboard/" dashboard-id)))]
+ (into {} (for [[field-id m] response]
+ [field-id (update m :values (partial take 3))])))))))))
+
(deftest chain-filter-sandboxed-field-values-test
(testing "When chain filter endpoints would normally return cached FieldValues (#13832), make sure sandboxing is respected"
(met/with-gtaps {:gtaps {:categories {:query (mt/mbql-query categories {:filter [:< $id 3]})}}}
diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index 74b0022a054..be84366f61f 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -218,7 +218,7 @@
;; have a hydration key and an id. moderation_reviews currently aren't batch hydrated but i'm worried they
;; cannot be in this situation
(hydrate [:ordered_cards [:card [:moderation_reviews :moderator_details]] :series :dashcard/action :dashcard/linkcard-info]
- :collection_authority_level :can_write :param_fields)
+ :collection_authority_level :can_write :param_fields :param_values)
api/read-check
api/check-not-archived
hide-unreadable-cards
diff --git a/src/metabase/api/public.clj b/src/metabase/api/public.clj
index 474aa1bbc89..3ea0f9920d6 100644
--- a/src/metabase/api/public.clj
+++ b/src/metabase/api/public.clj
@@ -83,13 +83,14 @@
(defn public-card
"Return a public Card matching key-value `conditions`, removing all columns that should not be visible to the general
- public. Throws a 404 if the Card doesn't exist."
+ public. Throws a 404 if the Card doesn't exist."
[& conditions]
- (-> (api/check-404 (apply t2/select-one [Card :id :dataset_query :description :display :name :parameters :visualization_settings]
- :archived false, conditions))
- remove-card-non-public-columns
- combine-parameters-and-template-tags
- (hydrate :param_fields)))
+ (binding [params/*ignore-current-user-perms-and-return-all-field-values* true]
+ (-> (api/check-404 (apply t2/select-one [Card :id :dataset_query :description :display :name :parameters :visualization_settings]
+ :archived false, conditions))
+ remove-card-non-public-columns
+ combine-parameters-and-template-tags
+ (hydrate :param_values :param_fields))))
(defn- card-with-uuid [uuid] (public-card :public_uuid uuid))
@@ -202,20 +203,21 @@
(defn public-dashboard
"Return a public Dashboard matching key-value `conditions`, removing all columns that should not be visible to the
- general public. Throws a 404 if the Dashboard doesn't exist."
+ general public. Throws a 404 if the Dashboard doesn't exist."
[& conditions]
{:pre [(even? (count conditions))]}
- (-> (api/check-404 (apply t2/select-one [Dashboard :name :description :id :parameters :auto_apply_filters], :archived false, conditions))
- (hydrate [:ordered_cards :card :series :dashcard/action] :param_fields)
- api.dashboard/add-query-average-durations
- (update :ordered_cards (fn [dashcards]
- (for [dashcard dashcards]
- (-> (select-keys dashcard [:id :card :card_id :dashboard_id :series :col :row :size_x
- :size_y :parameter_mappings :visualization_settings :action])
- (update :card remove-card-non-public-columns)
- (update :series (fn [series]
- (for [series series]
- (remove-card-non-public-columns series))))))))))
+ (binding [params/*ignore-current-user-perms-and-return-all-field-values* true]
+ (-> (api/check-404 (apply t2/select-one [Dashboard :name :description :id :parameters :auto_apply_filters], :archived false, conditions))
+ (hydrate [:ordered_cards :card :series :dashcard/action] :param_values :param_fields)
+ api.dashboard/add-query-average-durations
+ (update :ordered_cards (fn [dashcards]
+ (for [dashcard dashcards]
+ (-> (select-keys dashcard [:id :card :card_id :dashboard_id :series :col :row :size_x
+ :size_y :parameter_mappings :visualization_settings :action])
+ (update :card remove-card-non-public-columns)
+ (update :series (fn [series]
+ (for [series series]
+ (remove-card-non-public-columns series)))))))))))
(defn- dashboard-with-uuid [uuid] (public-dashboard :public_uuid uuid))
diff --git a/src/metabase/models/params.clj b/src/metabase/models/params.clj
index fe1ca37734c..a30e3190061 100644
--- a/src/metabase/models/params.clj
+++ b/src/metabase/models/params.clj
@@ -18,6 +18,7 @@
[metabase.mbql.schema :as mbql.s]
[metabase.mbql.util :as mbql.u]
[metabase.models.interface :as mi]
+ [metabase.models.params.field-values :as params.field-values]
[metabase.util :as u]
[metabase.util.i18n :refer [tru]]
[metabase.util.log :as log]
@@ -67,6 +68,28 @@
:else
field-id-or-form))
+(def ^:dynamic *ignore-current-user-perms-and-return-all-field-values*
+ "Whether to ignore permissions for the current User and return *all* FieldValues for the Fields being parameterized by
+ Cards and Dashboards. This determines how `:param_values` gets hydrated for Card and Dashboard. Normally, this is
+ `false`, but the public and embed versions of the API endpoints can bind this to `true` to bypass normal perms
+ checks (since there is no current User) and get *all* values."
+ false)
+
+(defn- field-ids->param-field-values-ignoring-current-user
+ [param-field-ids]
+ (t2/select-fn->fn :field_id identity ['FieldValues :values :human_readable_values :field_id]
+ :type :full
+ :field_id [:in param-field-ids]))
+
+(defn- field-ids->param-field-values
+ "Given a collection of `param-field-ids` return a map of FieldValues for the Fields they reference.
+ This map is returned by various endpoints as `:param_values`, if `param-field-ids` is empty, return `nil`"
+ [param-field-ids]
+ (when (seq param-field-ids)
+ ((if *ignore-current-user-perms-and-return-all-field-values*
+ field-ids->param-field-values-ignoring-current-user
+ params.field-values/field-id->field-values-for-current-user) param-field-ids)))
+
(defn- template-tag->field-form
"Fetch the `:field` clause from `dashcard` referenced by `template-tag`.
@@ -158,7 +181,20 @@
(hydrate :has_field_values :name_field [:dimensions :human_readable_field])
remove-dimensions-nonpublic-columns))))
-(defmulti ^:private param-fields
+
+(defmulti ^:private ^{:hydrate :param_values} param-values
+ "Add a `:param_values` map (Field ID -> FieldValues) containing FieldValues for the Fields referenced by the
+ parameters of a Card or a Dashboard. Implementations are in respective sections below."
+ t2/model)
+
+#_{:clj-kondo/ignore [:unused-private-var]}
+(mi/define-simple-hydration-method ^:private hydrate-param-values
+ :param_values
+ "Hydration method for `:param_fields`."
+ [instance]
+ (param-values instance))
+
+(defmulti ^:private ^{:hydrate :param_fields} param-fields
"Add a `:param_fields` map (Field ID -> Field) for all of the Fields referenced by the parameters of a Card or
Dashboard. Implementations are below in respective sections."
t2/model)
@@ -205,6 +241,15 @@
id))
(cards->card-param-field-ids (map :card dashcards))))
+(defn- dashboard->param-field-values
+ "Return a map of Field ID to FieldValues (if any) for any Fields referenced by Cards in `dashboard`,
+ or `nil` if none are referenced or none of them have FieldValues."
+ [dashboard]
+ (field-ids->param-field-values (dashcards->param-field-ids (:ordered_cards dashboard))))
+
+(defmethod param-values :metabase.models.dashboard/Dashboard [dashboard]
+ (dashboard->param-field-values dashboard))
+
(defmethod param-fields :metabase.models.dashboard/Dashboard [dashboard]
(-> (hydrate dashboard [:ordered_cards :card])
:ordered_cards
@@ -231,6 +276,8 @@
(set (mbql.u/match (seq (card->template-tag-field-clauses card))
[:field (id :guard integer?) _]
id)))
+(defmethod param-values :model/Card [card]
+ (-> card card->template-tag-field-ids field-ids->param-field-values))
(defmethod param-fields :model/Card [card]
(-> card card->template-tag-field-ids param-field-ids->fields))
diff --git a/src/metabase/models/params/field_values.clj b/src/metabase/models/params/field_values.clj
index 5fbdc58462b..5f78d25c1d1 100644
--- a/src/metabase/models/params/field_values.clj
+++ b/src/metabase/models/params/field_values.clj
@@ -2,6 +2,8 @@
"Code related to fetching FieldValues for Fields to populate parameter widgets. Always used by the field
values (`GET /api/field/:id/values`) endpoint; used by the chain filter endpoints under certain circumstances."
(:require
+ [medley.core :as m]
+ [metabase.models.field :as field :refer [Field]]
[metabase.models.field-values :as field-values :refer [FieldValues]]
[metabase.models.interface :as mi]
[metabase.plugins.classloader :as classloader]
@@ -39,6 +41,30 @@
(select-keys [:values :field_id :has_more_values]))
{:values [], :field_id (u/the-id field), :has_more_values false}))
+(defn default-field-id->field-values-for-current-user
+ "OSS implementation; used as a fallback for the EE implementation for any fields that aren't subject to sandboxing."
+ [field-ids]
+ (when (seq field-ids)
+ (not-empty
+ (let [field-values (t2/select [FieldValues :values :human_readable_values :field_id]
+ :type :full
+ :field_id [:in (set field-ids)])
+ readable-fields (when (seq field-values)
+ (field/readable-fields-only (t2/select [Field :id :table_id]
+ :id [:in (set (map :field_id field-values))])))
+ readable-field-ids (set (map :id readable-fields))]
+ (->> field-values
+ (filter #(contains? readable-field-ids (:field_id %)))
+ (m/index-by :field_id))))))
+
+(defenterprise field-id->field-values-for-current-user
+ "Fetch *existing* FieldValues for a sequence of `field-ids` for the current User. Values are returned as a map of
+ {field-id FieldValues-instance}
+ Returns `nil` if `field-ids` is empty of no matching FieldValues exist."
+ metabase-enterprise.sandbox.models.params.field-values
+ [field-ids]
+ (default-field-id->field-values-for-current-user field-ids))
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Advanced FieldValues |
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index 228e7ea5d12..22e2759551d 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -303,6 +303,7 @@
:collection_authority_level nil
:can_write false
:param_fields nil
+ :param_values nil
:last-edit-info {:timestamp true :id true :first_name "Test" :last_name "User" :email "test@example.com"}
:ordered_cards [{:size_x 4
:size_y 4
@@ -383,6 +384,7 @@
:collection_id true
:collection_authority_level nil
:can_write false
+ :param_values nil
:param_fields {field-id {:id field-id
:table_id table-id
:display_name display-name
@@ -446,6 +448,35 @@
(is (= "You don't have permissions to do that."
(mt/user-http-request :rasta :get 403 (format "dashboard/%d" dashboard-id)))))))))
+(deftest param-values-test
+ (testing "Don't return `param_values` for Fields for which the current User has no data perms."
+ (mt/with-temp-copy-of-db
+ (perms/revoke-data-perms! (perms-group/all-users) (mt/id))
+ (perms/grant-permissions! (perms-group/all-users) (perms/table-read-path (mt/id :venues)))
+ (mt/with-temp* [Dashboard [{dashboard-id :id} {:name "Test Dashboard"}]
+ Card [{card-id :id} {:name "Dashboard Test Card"}]
+ DashboardCard [{_ :id} {:dashboard_id dashboard-id
+ :card_id card-id
+ :parameter_mappings [{:card_id card-id
+ :parameter_id "foo"
+ :target [:dimension
+ [:field (mt/id :venues :name) nil]]}
+ {:card_id card-id
+ :parameter_id "bar"
+ :target [:dimension
+ [:field (mt/id :categories :name) nil]]}]}]]
+
+ (is (= {(mt/id :venues :name) {:values ["20th Century Cafe"
+ "25°"
+ "33 Taps"]
+ :field_id (mt/id :venues :name)
+ :human_readable_values []}}
+ (let [response (:param_values (mt/user-http-request :rasta :get 200 (str "dashboard/" dashboard-id)))]
+ (into {} (for [[field-id m] response]
+ [field-id (update m :values (partial take 3))])))))))))
+
+
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | PUT /api/dashboard/:id |
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/test/metabase/api/embed_test.clj b/test/metabase/api/embed_test.clj
index d827e77bf23..ea64aa8db7b 100644
--- a/test/metabase/api/embed_test.clj
+++ b/test/metabase/api/embed_test.clj
@@ -146,10 +146,11 @@
:visualization_settings {}
:dataset_query {:type "query"}
:parameters []
+ :param_values nil
:param_fields nil})
(def successful-dashboard-info
- {:auto_apply_filters true, :description nil, :parameters [], :ordered_cards [], :param_fields nil})
+ {:auto_apply_filters true, :description nil, :parameters [], :ordered_cards [], :param_values nil, :param_fields nil})
(def ^:private yesterday (time/minus (time/now) (time/days 1)))
diff --git a/test/metabase/api/public_test.clj b/test/metabase/api/public_test.clj
index dacafcb9258..d254355d4b4 100644
--- a/test/metabase/api/public_test.clj
+++ b/test/metabase/api/public_test.clj
@@ -136,7 +136,7 @@
(with-temp-public-card [{uuid :public_uuid, card-id :id}]
(testing "Happy path -- should be able to fetch the Card"
- (is (= #{:dataset_query :description :display :id :name :visualization_settings :parameters :param_fields}
+ (is (= #{:dataset_query :description :display :id :name :visualization_settings :parameters :param_values :param_fields}
(set (keys (client/client :get 200 (str "public/card/" uuid)))))))
(testing "Check that we cannot fetch a public Card if public sharing is disabled"
@@ -149,6 +149,30 @@
(is (= "Not found."
(client/client :get 404 (str "public/card/" uuid))))))))))
+(deftest make-sure-param-values-get-returned-as-expected
+ (let [category-name-id (mt/id :categories :name)]
+ (mt/with-temp Card [card {:dataset_query
+ {:database (mt/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" category-name-id nil]
+ :widget-type "category"
+ :required true}}}}}]
+ (is (= {(mt/id :categories :name) {:values (t2/select-one-fn (comp count :values)
+ 'FieldValues :field_id category-name-id)
+ :human_readable_values []
+ :field_id category-name-id}}
+ (-> (:param_values (#'api.public/public-card :id (u/the-id card)))
+ (update-in [category-name-id :values] count)
+ (update category-name-id #(into {} %))))))))
+
;;; ------------------------- GET /api/public/card/:uuid/query (and JSON/CSV/XSLX versions) --------------------------
(deftest check-that-we--cannot--execute-a-publiccard-if-the-setting-is-disabled
@@ -618,6 +642,50 @@
(is (= [1 2 3 4]
(t2/select-one-fn :values FieldValues :field_id (mt/id :venues :price)))))
+(defn- price-param-values []
+ {(mt/id :venues :price) {:values [1 2 3 4]
+ :human_readable_values []
+ :field_id (mt/id :venues :price)}})
+
+(defn- add-price-param-to-dashboard! [dashboard]
+ (t2/update! Dashboard (u/the-id dashboard) {:parameters [{:name "Price", :type "category", :slug "price", :id "_PRICE_"}]}))
+
+(defn- add-dimension-param-mapping-to-dashcard! [dashcard card dimension]
+ (t2/update! DashboardCard (u/the-id dashcard) {:parameter_mappings [{:card_id (u/the-id card)
+ :target ["dimension" dimension]}]}))
+
+(defn- GET-param-values [dashboard]
+ (mt/with-temporary-setting-values [enable-public-sharing true]
+ (:param_values (client/client :get 200 (str "public/dashboard/" (:public_uuid dashboard))))))
+
+(deftest check-that-param-info-comes-back-for-sql-cards
+ (with-temp-public-dashboard-and-card [dash card dashcard]
+ (t2/update! Card (u/the-id card)
+ {:dataset_query {:database (mt/id)
+ :type :native
+ :native {:template-tags {:price {:name "price"
+ :display-name "Price"
+ :type "dimension"
+ :dimension ["field" (mt/id :venues :price) nil]}}}}})
+ (add-price-param-to-dashboard! dash)
+ (add-dimension-param-mapping-to-dashcard! dashcard card ["template-tag" "price"])
+ (is (= (price-param-values)
+ (GET-param-values dash)))))
+
+(deftest check-that-param-info-comes-back-for-mbql-cards--field-id-
+ (with-temp-public-dashboard-and-card [dash card dashcard]
+ (add-price-param-to-dashboard! dash)
+ (add-dimension-param-mapping-to-dashcard! dashcard card ["field" (mt/id :venues :price) nil])
+ (is (= (price-param-values)
+ (GET-param-values dash)))))
+
+(deftest check-that-param-info-comes-back-for-mbql-cards--fk---
+ (with-temp-public-dashboard-and-card [dash card dashcard]
+ (add-price-param-to-dashboard! dash)
+ (add-dimension-param-mapping-to-dashcard! dashcard card [:field (mt/id :venues :price) {:source-field (mt/id :checkins :venue_id)}])
+ (is (= (price-param-values)
+ (GET-param-values dash)))))
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | New FieldValues search endpoints |
;;; +----------------------------------------------------------------------------------------------------------------+
--
GitLab