Dashboard Subscriptions *should* use `:default` param values. Dashboard QP...

Dashboard Subscriptions *should* use `:default` param values. Dashboard QP should ignore them. (#20569)

src/metabase/pulse.clj

@@ -29,6 +29,18 @@
 
 ;;; ------------------------------------------------- PULSE SENDING --------------------------------------------------
 
+(defn- merge-default-values
+  "For the specific case of Dashboard Subscriptions we should use `:default` parameter values as the actual `:value` for
+  the parameter if none is specified. Normally the FE client will take `:default` and pass it in as `:value` if it
+  wants to use it (see #20503 for more details) but this obviously isn't an option for Dashboard Subscriptions... so
+  go thru `parameters` and change `:default` to `:value` unless a `:value` is explicitly specified."
+  [parameters]
+  (for [{default-value :default, :as parameter} parameters]
+    (merge
+     (when default-value
+       {:value default-value})
+     (dissoc parameter :default))))
+
 (defn- execute-dashboard-subscription-card
   [owner-id dashboard dashcard card-or-id parameters]
   (try
@@ -41,7 +53,7 @@
                      :dashcard-id   (u/the-id dashcard)
                      :context       :pulse ; TODO - we should support for `:dashboard-subscription` and use that to differentiate the two
                      :export-format :api
-                     :parameters    parameters
+                     :parameters    (merge-default-values parameters)
                      :middleware    {:process-viz-settings? true
                                      :js-int-to-string?     false}
                      :run           (fn [query info]

src/metabase/query_processor/dashboard.clj

@@ -112,6 +112,9 @@
    request-params :- (s/maybe [su/Map])]
   (log/tracef "Resolving Dashboard %d Card %d query request parameters" dashboard-id card-id)
   (let [request-params            (normalize/normalize-fragment [:parameters] request-params)
+        ;; ignore default values in request params as well. (#20516)
+        request-params            (for [param request-params]
+                                    (dissoc param :default))
         dashboard                 (api/check-404 (db/select-one Dashboard :id dashboard-id))
         dashboard-param-id->param (into {}
                                         ;; remove the `:default` values from Dashboard params. We don't ACTUALLY want to

test/metabase/dashboard_subscription_test.clj

@@ -367,3 +367,60 @@
       :email
       (fn [_ _]
         (is (= {} (mt/summarize-multipart-email))))}}))
+
+(deftest use-default-values-test
+  (testing "Dashboard Subscriptions SHOULD use default values for Dashboard parameters when running (#20516)"
+    (mt/dataset sample-dataset
+      (mt/with-temp Dashboard [{dashboard-id :id, :as dashboard} {:name       "20516 Dashboard"
+                                                                  :parameters [{:name    "Category"
+                                                                                :slug    "category"
+                                                                                :id      "_MBQL_CATEGORY_"
+                                                                                :type    :category
+                                                                                :default ["Doohickey"]}
+                                                                               {:name    "SQL Category"
+                                                                                :slug    "sql_category"
+                                                                                :id      "_SQL_CATEGORY_"
+                                                                                :type    :category
+                                                                                :default ["Gizmo"]}]}]
+        (testing "MBQL query"
+          (mt/with-temp* [Card [{mbql-card-id :id} {:name          "Orders"
+                                                    :dataset_query (mt/mbql-query products
+                                                                     {:fields   [$id $title $category]
+                                                                      :order-by [[:asc $id]]
+                                                                      :limit    2})}]
+                          DashboardCard [_ {:parameter_mappings [{:parameter_id "_MBQL_CATEGORY_"
+                                                                  :card_id      mbql-card-id
+                                                                  :target       [:dimension [:field (mt/id :products :category) nil]]}]
+                                            :card_id            mbql-card-id
+                                            :dashboard_id       dashboard-id}]]
+            (let [[mbql-results] (map :result (@#'pulse/execute-dashboard {:creator_id (mt/user->id :rasta)} dashboard))]
+              (is (= [[2 "Small Marble Shoes"        "Doohickey"]
+                      [3 "Synergistic Granite Chair" "Doohickey"]]
+                     (mt/rows mbql-results))))))
+        (testing "SQL Query"
+          (mt/with-temp* [Card [{sql-card-id :id} {:name          "Products (SQL)"
+                                                   :dataset_query (mt/native-query
+                                                                    {:query
+                                                                     (str "SELECT id, title, category\n"
+                                                                          "FROM products\n"
+                                                                          "WHERE {{category}}\n"
+                                                                          "ORDER BY id ASC\n"
+                                                                          "LIMIT 2")
+
+                                                                     :template-tags
+                                                                     {"category"
+                                                                      {:id           "_SQL_CATEGORY_TEMPLATE_TAG_"
+                                                                       :name         "category"
+                                                                       :display-name "Category"
+                                                                       :type         :dimension
+                                                                       :dimension    [:field (mt/id :products :category) nil]
+                                                                       :widget-type  :category}}})}]
+                          DashboardCard [_ {:parameter_mappings [{:parameter_id "_SQL_CATEGORY_"
+                                                                  :card_id      sql-card-id
+                                                                  :target       [:dimension [:template-tag "category"]]}]
+                                            :card_id            sql-card-id
+                                            :dashboard_id       dashboard-id}]]
+            (let [[results] (map :result (@#'pulse/execute-dashboard {:creator_id (mt/user->id :rasta)} dashboard))]
+              (is (= [[1  "Rustic Paper Wallet"   "Gizmo"]
+                      [10 "Mediocre Wooden Table" "Gizmo"]]
+                     (mt/rows results))))))))))

test/metabase/query_processor/dashboard_test.clj

@@ -198,3 +198,39 @@
                                                           :dashboard_id           dashboard-id}]]
           (is (= [[200]]
                  (mt/rows (run-query-for-dashcard dashboard-id card-id dashcard-id)))))))))
+
+(deftest ignore-default-values-in-request-parameters-test
+  (testing "Parameters passed in from the request with only default values (but no actual values) should get ignored (#20516)"
+    (mt/dataset sample-dataset
+      (mt/with-temp* [Card [{card-id :id} {:name          "Orders"
+                                           :dataset_query (mt/mbql-query products
+                                                            {:fields   [$id $title $category]
+                                                             :order-by [[:asc $id]]
+                                                             :limit    2})}]
+                      Dashboard [{dashboard-id :id} {:name       "20516 Dashboard"
+                                                     :parameters [{:name    "Category"
+                                                                   :slug    "category"
+                                                                   :id      "_CATEGORY_"
+                                                                   :type    :category
+                                                                   :default ["Doohickey"]}]}]
+                      DashboardCard [{dashcard-id :id} {:parameter_mappings [{:parameter_id "_CATEGORY_"
+                                                                              :card_id      card-id
+                                                                              :target       [:dimension [:field (mt/id :products :category) nil]]}]
+                                                        :card_id            card-id
+                                                        :dashboard_id       dashboard-id}]]
+        (testing "No parameters -- ignore Dashboard default (#20493, #20503)"
+          ;; [[metabase.query-processor.middleware.large-int-id]] middleware is converting the IDs to strings I guess
+          (is (= [["1" "Rustic Paper Wallet" "Gizmo"]
+                  ["2" "Small Marble Shoes" "Doohickey"]]
+                 (mt/rows
+                  (run-query-for-dashcard dashboard-id card-id dashcard-id)))))
+        (testing "Request parameters with :default -- ignore these as well (#20516)"
+          (is (= [["1" "Rustic Paper Wallet" "Gizmo"]
+                  ["2" "Small Marble Shoes" "Doohickey"]]
+                 (mt/rows
+                  (run-query-for-dashcard dashboard-id card-id dashcard-id
+                                          :parameters [{:name    "Category"
+                                                        :slug    "category"
+                                                        :id      "_CATEGORY_"
+                                                        :type    :category
+                                                        :default ["Gizmo"]}])))))))))