From 084416f1ce5562aeaea4a03fdc711bedde60e3b1 Mon Sep 17 00:00:00 2001 From: Cal Herries <39073188+calherries@users.noreply.github.com> Date: Fri, 17 Feb 2023 10:42:58 +0100 Subject: [PATCH] Fix 28368: null parameters for implicit actions in dashboards (#28369) * Fix null parameters for implicit actions * Fix test for dashboard and add action_id GET /api/dashboard/:id/cards endpoint schema * Fix endpoint * Update src/metabase/models/action.clj Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com> --------- Co-authored-by: Ngoc Khuat <qn.khuat@gmail.com> --- src/metabase/api/dashboard.clj | 7 ++++--- src/metabase/models/action.clj | 4 ++-- test/metabase/api/dashboard_test.clj | 13 +++++++++---- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj index f5320f80b1d..30223bbb737 100644 --- a/src/metabase/api/dashboard.clj +++ b/src/metabase/api/dashboard.clj @@ -480,14 +480,15 @@ ;; TODO - param should be `card_id`, not `cardId` (fix here + on frontend at the same time) #_{:clj-kondo/ignore [:deprecated-var]} (api/defendpoint-schema POST "/:id/cards" - "Add a `Card` to a Dashboard." - [id :as {{:keys [cardId parameter_mappings row col size_x size_y], :as dashboard-card} :body}] + "Add a `Card` or `Action` to a Dashboard." + [id :as {{:keys [cardId parameter_mappings row col size_x size_y action_id], :as dashboard-card} :body}] {cardId (s/maybe su/IntGreaterThanZero) parameter_mappings (s/maybe [dashboard-card/ParamMapping]) row su/IntGreaterThanOrEqualToZero col su/IntGreaterThanOrEqualToZero size_x su/IntGreaterThanZero - size_y su/IntGreaterThanZero} + size_y su/IntGreaterThanZero + action_id (s/maybe su/IntGreaterThanZero)} (api/check-not-archived (api/write-check Dashboard id)) (when cardId (api/check-not-archived (api/read-check Card cardId))) diff --git a/src/metabase/models/action.clj b/src/metabase/models/action.clj index 37d8f2a1bf4..44ceb5e5033 100644 --- a/src/metabase/models/action.clj +++ b/src/metabase/models/action.clj @@ -247,10 +247,10 @@ (mi/define-batched-hydration-method dashcard-action :dashcard/action - "Hydrates action from DashboardCard." + "Hydrates action from DashboardCards." [dashcards] (let [actions-by-id (when-let [action-ids (seq (keep :action_id dashcards))] - (m/index-by :id (select-actions (map :card dashcards) :id [:in action-ids])))] + (m/index-by :id (select-actions nil :id [:in action-ids])))] (for [dashcard dashcards] (m/assoc-some dashcard :action (get actions-by-id (:action_id dashcard)))))) diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj index bf03d4f8a11..edc421b9a08 100644 --- a/test/metabase/api/dashboard_test.clj +++ b/test/metabase/api/dashboard_test.clj @@ -2490,14 +2490,19 @@ (mt/test-drivers (mt/normal-drivers-with-feature :actions) (mt/with-actions-test-data-and-actions-enabled (doseq [action-type [:http :implicit :query]] - (mt/with-actions [{:keys [action-id model-id]} {:type action-type :visualization_settings {:hello true}}] + (mt/with-actions [{:keys [action-id]} {:type action-type :visualization_settings {:hello true}}] (testing (str "Creating dashcard with action: " action-type) (mt/with-temp* [Dashboard [{dashboard-id :id}]] (is (partial= {:visualization_settings {:label "Update"} - :action_id action-id - :card_id model-id} + :action_id action-id + :card_id nil} (mt/user-http-request :crowberto :post 200 (format "dashboard/%s/cards" dashboard-id) - {:size_x 1 :size_y 1 :row 1 :col 1 :cardId model-id :action_id action-id + {:size_x 1 + :size_y 1 + :row 1 + :col 1 + :cardId nil + :action_id action-id :visualization_settings {:label "Update"}}))) (is (partial= {:ordered_cards [{:action {:visualization_settings {:hello true} :type (name action-type) -- GitLab