diff --git a/src/metabase/api/embed.clj b/src/metabase/api/embed.clj index a2bbef47734360b12c16d9c1a91b050ab0578b0e..c21fa0a832b742116468291986dc0dcfffbd0d3a 100644 --- a/src/metabase/api/embed.clj +++ b/src/metabase/api/embed.clj @@ -117,23 +117,33 @@ :when (not (contains? params-to-remove (keyword (:slug param))))] param)) +(defn- classify-params-as-keep-or-remove + "Classifies the params in the `dashboard-or-card-params` seq and the param slugs in `embedding-params` map according to: + Parameters in `dashboard-or-card-params` whose slugs are NOT in the `embedding-params` map must be removed. + Parameter slugs in `embedding-params` with the value 'enabled' are kept, 'disabled' or 'locked' are not kept. + + The resulting classification is returned as a map with keys :keep and :remove whose values are sets of parameter slugs." + [dashboard-or-card-params embedding-params] + (let [param-slugs (map #(keyword (:slug %)) dashboard-or-card-params) + grouped-param-slugs {:remove (remove (fn [k] (contains? embedding-params k)) param-slugs)} + grouped-embedding-param-slugs (-> (group-by #(= (second %) "enabled") embedding-params) + (update-keys {true :keep false :remove}) + (update-vals #(into #{} (map first) %)))] + (merge-with (comp set concat) + {:keep #{} :remove #{}} + grouped-param-slugs + grouped-embedding-param-slugs))) + (defn- get-params-to-remove - "Gets the params in both the provided embedding-params and dashboard-or-card object that we should remove." - [dashboard-or-card embedding-params] - (set (concat (for [[param status] embedding-params - :when (not= status "enabled")] - param) - (for [{slug :slug} (:parameters dashboard-or-card) - :let [param (keyword slug)] - :when (not (contains? embedding-params param))] - param)))) + [dashboard-or-card-params embedding-params] + (:remove (classify-params-as-keep-or-remove dashboard-or-card-params embedding-params))) (mu/defn ^:private remove-locked-and-disabled-params "Remove the `:parameters` for `dashboard-or-card` that listed as `disabled` or `locked` in the `embedding-params` whitelist, or not present in the whitelist. This is done so the frontend doesn't display widgets for params the user can't set." [dashboard-or-card embedding-params :- ms/EmbeddingParams] - (let [params-to-remove (get-params-to-remove dashboard-or-card embedding-params)] + (let [params-to-remove (get-params-to-remove (:parameters dashboard-or-card) embedding-params)] (update dashboard-or-card :parameters remove-params-in-set params-to-remove))) (defn- remove-token-parameters @@ -276,20 +286,27 @@ (into {}))))) (defn- remove-locked-parameters [dashboard embedding-params] - (let [params-to-remove (get-params-to-remove dashboard embedding-params) - param-ids-to-remove (set (for [parameter (:parameters dashboard) - :when (contains? params-to-remove (keyword (:slug parameter)))] - (:id parameter))) - linked-field-ids (set (mapcat (params/get-linked-field-ids (:dashcards dashboard)) param-ids-to-remove)) - remove-parameters (fn [dashcard] - (update dashcard :parameter_mappings - (fn [param-mappings] - (remove (fn [{:keys [parameter_id]}] - (contains? param-ids-to-remove parameter_id)) param-mappings))))] + (let [params (:parameters dashboard) + {params-to-remove :remove + params-to-keep :keep} (classify-params-as-keep-or-remove params embedding-params) + param-ids-to-remove (set (keep (fn [{:keys [slug id]}] + (when (contains? params-to-remove (keyword slug)) id)) + params)) + param-ids-to-keep (set (keep (fn [{:keys [slug id]}] + (when (contains? params-to-keep (keyword slug)) id)) + params)) + field-ids-to-maybe-remove (set (mapcat (params/get-linked-field-ids (:dashcards dashboard)) param-ids-to-remove)) + field-ids-to-keep (set (mapcat (params/get-linked-field-ids (:dashcards dashboard)) param-ids-to-keep)) + field-ids-to-remove (set/difference field-ids-to-maybe-remove field-ids-to-keep) + remove-parameters (fn [dashcard] + (update dashcard :parameter_mappings + (fn [param-mappings] + (remove (fn [{:keys [parameter_id]}] + (contains? param-ids-to-remove parameter_id)) param-mappings))))] (-> dashboard (update :dashcards #(map remove-parameters %)) - (update :param_fields #(apply dissoc % linked-field-ids)) - (update :param_values #(apply dissoc % linked-field-ids))))) + (update :param_fields #(apply dissoc % field-ids-to-remove)) + (update :param_values #(apply dissoc % field-ids-to-remove))))) (defn dashboard-for-unsigned-token "Return the info needed for embedding about Dashboard specified in `token`. Additional `constraints` can be passed to diff --git a/test/metabase/api/embed_test.clj b/test/metabase/api/embed_test.clj index 0de52a954fcea3fa2d83d309451ff95b53f370a5..ea113ce45775db3c40fa9add27db0599eb74612c 100644 --- a/test/metabase/api/embed_test.clj +++ b/test/metabase/api/embed_test.clj @@ -631,6 +631,52 @@ :parameter_mappings count)))))))) +(deftest locked-params-removes-values-fields-when-not-used-in-enabled-params + (testing "check that locked params are not removed in parameter mappings, param_values, and param_fields when an enabled param uses them (#37914)" + (with-embedding-enabled-and-new-secret-key + (t2.with-temp/with-temp [Dashboard dashboard {:enable_embedding true + :embedding_params {:venue_name "locked" + :venue_name_2 "enabled"} + :name "Test Dashboard" + :parameters [{:name "venue_name" + :slug "venue_name" + :id "foo" + :type :string/= + :sectionId "string"} + {:name "venue_name_2" + :slug "venue_name_2" + :id "bar" + :type :string/= + :sectionId "string"}]} + Card {card-id :id} {:name "Dashboard Test Card"} + DashboardCard {_ :id} {:dashboard_id (:id dashboard) + :card_id card-id + :parameter_mappings [{:card_id card-id + :slug "venue_name" + :parameter_id "foo" + :target [:dimension + [:field (mt/id :venues :name) nil]]} + {:card_id card-id + :slug "venue_name_2" + :parameter_id "bar" + :target [:dimension + [:field (mt/id :venues :name) nil]]}]}] + (let [embedding-dashboard (client/client :get 200 (dashboard-url dashboard {:params {:foo "BCD Tofu House"}}))] + (is (some? + (-> embedding-dashboard + :param_values + (get (mt/id :venues :name))))) + (is (some? + (-> embedding-dashboard + :param_fields + (get (mt/id :venues :name))))) + (is (= 1 + (-> embedding-dashboard + :dashcards + first + :parameter_mappings + count)))))))) + (deftest linked-param-to-locked-removes-param-values-test (testing "Check that a linked parameter to a locked params we remove the param_values." (with-embedding-enabled-and-new-secret-key