Add generic with-action for action tests (#24418)

Combined `with-query-action` and `with-http-action` into `with-action`
that also allows options to be provided to override the created
action.

Chose to make `options-map` required, unlike with-temp, so kondo can use
`let` to lint the macro rather than having to create a new hook.

.clj-kondo/config.edn

@@ -354,8 +354,7 @@
   metabase-enterprise.serialization.test-util/with-random-dump-dir                clojure.core/let
   metabase.actions.test-util/with-card-emitter                                    clojure.core/let
   metabase.actions.test-util/with-dashboard-emitter                               clojure.core/let
-  metabase.actions.test-util/with-http-action                                     clojure.core/fn
-  metabase.actions.test-util/with-query-action                                    clojure.core/fn
+  metabase.actions.test-util/with-action                                          clojure.core/let
   metabase.api.common/let-404                                                     clojure.core/let
   metabase.db.data-migrations/defmigration                                        clojure.core/def
   metabase.db.liquibase/with-liquibase                                            clojure.core/let

test/metabase/actions/test_util.clj

@@ -92,8 +92,7 @@
   `(do-with-actions-test-data (fn [] ~@body)))
 
 (deftest with-actions-test-data-test
-  ;; TODO -- use the feature `:actions` once #22691 is merged in
-  (mt/test-drivers #{:h2 :postgres}
+  (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
     (dotimes [i 2]
       (testing (format "Iteration %d" i)
         (with-actions-test-data
@@ -110,60 +109,58 @@
               (is (= [[74]]
                      (row-count))))))))))
 
-(defn do-with-query-action
-  "Impl for [[with-query-action]]."
-  [f]
-  (mt/with-temp* [Card [{card-id :id} {:database_id   (mt/id)
-                                       :dataset_query {:database (mt/id)
-                                                       :type     :native
-                                                       :native   {:query         (str "UPDATE categories\n"
-                                                                                      "SET name = 'Bird Shop'\n"
-                                                                                      "WHERE id = {{id}}")
-                                                                  :template-tags {"id" {:name         "id"
-                                                                                        :display-name "ID"
-                                                                                        :type         :number
-                                                                                        :required     true}}}}
-                                       :name          "Query Example"
-                                       :parameters    [{:id "id" :type "number"}]
-                                       :is_write      true}]]
-    (let [action-id (db/select-one-field :action_id QueryAction :card_id card-id)]
-      (f {:query-action-card-id card-id
-          :action-id            action-id}))))
-
-(defmacro with-query-action
-  "Execute `body` with a newly created QueryAction. `bindings` is a map with keys `:action-id` and
-  `:query-action-card-id`.
-
-    (with-query-action [{:keys [action-id query-action-card-id], :as context}]
-      (do-something))"
-  {:style/indent 1}
-  [[bindings] & body]
-  `(do-with-query-action (fn [~bindings] ~@body)))
-
-(defn do-with-http-action
-  "Impl for [[with-http-action]]."
-  [f]
+(defn do-with-action
+  "Impl for [[with-action]]."
+  [options-map f]
   (initialize/initialize-if-needed! :web-server)
-  (mt/with-model-cleanup [Action]
-    (let [action-id (action/insert! {:type :http
-                                     :name "Echo Example"
-                                     :template {:url (client/build-url "testing/echo[[?fail={{fail}}]]" {})
-                                                :method "POST"
-                                                :body "{\"the_parameter\": {{id}}}"
-                                                :headers "{\"x-test\": \"{{id}}\"}"
-                                                :parameters [{:id "id" :type "number"}
-                                                             {:id "fail" :type "text"}]}
-                                     :response_handle ".body"})]
-      (f {:action-id action-id}))))
-
-(defmacro with-http-action
-  "Execute `body` with a newly created QueryAction. `bindings` is a map with key `:action-id`.
-
-    (with-http-action [{:keys [action-id], :as context}]
+  (case (:type options-map)
+    :query
+    (mt/with-temp* [Card [{card-id :id} (merge
+                                          {:database_id   (mt/id)
+                                           :dataset_query {:database (mt/id)
+                                                           :type     :native
+                                                           :native   {:query         (str "UPDATE categories\n"
+                                                                                          "SET name = 'Bird Shop'\n"
+                                                                                          "WHERE id = {{id}}")
+                                                                      :template-tags {"id" {:name         "id"
+                                                                                            :display-name "ID"
+                                                                                            :type         :number
+                                                                                            :required     true}}}}
+                                           :name          "Query Example"
+                                           :parameters    [{:id "id" :type "number"}]
+                                           :is_write      true}
+                                          (dissoc options-map :type))]]
+      (let [action-id (db/select-one-field :action_id QueryAction :card_id card-id)]
+        (f {:query-action-card-id card-id
+            :action-id            action-id})))
+
+    :http
+    (mt/with-model-cleanup [Action]
+      (let [action-id (action/insert! (merge
+                                        {:type :http
+                                         :name "Echo Example"
+                                         :template {:url (client/build-url "testing/echo[[?fail={{fail}}]]" {})
+                                                    :method "POST"
+                                                    :body "{\"the_parameter\": {{id}}}"
+                                                    :headers "{\"x-test\": \"{{id}}\"}"
+                                                    :parameters [{:id "id" :type "number"}
+                                                                 {:id "fail" :type "text"}]}
+                                         :response_handle ".body"}
+                                        options-map))]
+        (f {:action-id action-id})))))
+
+(defmacro with-action
+  "Execute `body` with a newly created Action.
+   `binding-form` is a returned map with key `:action-id`, and `:query-action-card-id` for QueryActions.
+   `options-map` contains overrides for the action. Defaults to a sane QueryAction.
+
+    (with-action [{:keys [action-id], :as context} {:type :http :name \"Temp HTTP Action\"}]
       (do-something))"
   {:style/indent 1}
-  [[bindings] & body]
-  `(do-with-http-action (fn [~bindings] ~@body)))
+  [[binding-form options-map] & body]
+  `(do-with-action
+     (merge {:type :query} ~options-map)
+     (fn [~binding-form] ~@body)))
 
 (defn do-with-emitter
   "Impl for [[with-emitter]]."

test/metabase/api/action_test.clj

@@ -30,7 +30,7 @@
 (deftest list-actions-test
   (testing "GET /api/action"
     (actions.test-util/with-actions-enabled
-      (actions.test-util/with-query-action [{:keys [action-id]}]
+      (actions.test-util/with-action [{:keys [action-id]} {}]
         (let [response (mt/user-http-request :crowberto :get 200 "action")]
           (is (schema= [{:id       su/IntGreaterThanZero
                          s/Keyword s/Any}]
@@ -47,7 +47,7 @@
   (testing "GET /api/action/:id"
     (testing "Should return Card dataset_query deserialized (#23201)"
       (actions.test-util/with-actions-enabled
-        (actions.test-util/with-query-action [{:keys [action-id]}]
+        (actions.test-util/with-action [{:keys [action-id]} {}]
           (let [action (mt/user-http-request :crowberto :get 200 (format "action/%d" action-id))]
             (testing "Should return Card dataset_query deserialized (#23201)"
               (is (schema= ExpectedGetCardActionAPIResponse

test/metabase/api/emitter_test.clj

@@ -12,7 +12,7 @@
   (testing "POST /api/emitter"
     (testing "Creating an emitter with the POST endpoint should return the newly created Emitter"
       (actions.test-util/with-actions-test-data-and-actions-enabled
-        (actions.test-util/with-query-action [{:keys [action-id query-action-card-id]}]
+        (actions.test-util/with-action [{:keys [action-id query-action-card-id]} {}]
           (let [expected-response {:id                 su/IntGreaterThanZero
                                    :parameter_mappings (s/eq nil)
                                    :action_id          (s/eq action-id)
@@ -36,7 +36,7 @@
   (testing "PUT /api/emitter/:id"
     (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
       (actions.test-util/with-actions-test-data-and-actions-enabled
-        (actions.test-util/with-query-action [action]
+        (actions.test-util/with-action [action {}]
           (actions.test-util/with-card-emitter [{:keys [emitter-id]} action]
             (testing "Should be able to update an emitter"
               (mt/user-http-request :crowberto :put 204 (format "emitter/%d" emitter-id)
@@ -51,8 +51,8 @@
   (testing "DELETE /api/emitter/:id"
     (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
       (actions.test-util/with-actions-test-data-and-actions-enabled
-        (actions.test-util/with-query-action [action]
-          (actions.test-util/with-card-emitter [{:keys [emitter-id]} action]
+        (actions.test-util/with-action [context {}]
+          (actions.test-util/with-card-emitter [{:keys [emitter-id]} context]
             (testing "Should be able to delete an emitter"
               (is (nil? (mt/user-http-request :crowberto :delete 204 (format "emitter/%d" emitter-id)))))
             (testing "Should 404 if bad emitter-id"
@@ -63,8 +63,8 @@
 (deftest execute-query-action-test
   (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
     (actions.test-util/with-actions-test-data-and-actions-enabled
-      (actions.test-util/with-query-action [action]
-        (actions.test-util/with-card-emitter [{:keys [emitter-id]} action]
+      (actions.test-util/with-action [context {}]
+        (actions.test-util/with-card-emitter [{:keys [emitter-id]} context]
           (let [emitter-path  (format "emitter/%d/execute" emitter-id)]
             (testing "Should be able to execute an emitter"
               (is (= {:rows-affected 1}
@@ -96,7 +96,7 @@
 (deftest execute-http-action-test
   (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
     (actions.test-util/with-actions-test-data-and-actions-enabled
-      (actions.test-util/with-http-action [context]
+      (actions.test-util/with-action [context {:type :http}]
         (actions.test-util/with-card-emitter [{:keys [emitter-id]} context]
           (let [emitter-path (format "emitter/%d/execute" emitter-id)]
             (testing "Should be able to execute an emitter"

test/metabase/models/action_test.clj

@@ -8,7 +8,7 @@
 (deftest hydrate-query-action-test
   (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
     (actions.test-util/with-actions-test-data-and-actions-enabled
-      (actions.test-util/with-query-action [{:keys [query-action-card-id action-id] :as context}]
+      (actions.test-util/with-action [{:keys [query-action-card-id action-id] :as context} {}]
         (actions.test-util/with-card-emitter [{:keys [emitter-id]} context]
           (let [emitter (Emitter emitter-id)
                 hydrated-emitter (hydrate emitter :action)]
@@ -22,7 +22,7 @@
 (deftest hydrate-http-action-test
   (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
     (actions.test-util/with-actions-test-data-and-actions-enabled
-      (actions.test-util/with-http-action [{:keys [action-id] :as context}]
+      (actions.test-util/with-action [{:keys [action-id] :as context} {:type :http}]
         (actions.test-util/with-card-emitter [{:keys [emitter-id]} context]
           (let [emitter (Emitter emitter-id)
                 hydrated-emitter (hydrate emitter :action)]

test/metabase/models/emitter_test.clj

@@ -8,7 +8,7 @@
 (deftest test-hydration
   (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
     (actions.test-util/with-actions-test-data-and-actions-enabled
-      (actions.test-util/with-query-action [context]
+      (actions.test-util/with-action [context {}]
         (actions.test-util/with-card-emitter [{:keys [emitter-id emitter-parent-id]} context]
             (let [card (Card emitter-parent-id)
                   hydrated-card (hydrate card :emitters)]
@@ -19,7 +19,7 @@
 (deftest dashboard-emitter-hydration-test
   (mt/test-drivers (mt/normal-drivers-with-feature :actions/custom)
     (actions.test-util/with-actions-test-data-and-actions-enabled
-      (actions.test-util/with-query-action [context]
+      (actions.test-util/with-action [context {}]
         (actions.test-util/with-dashboard-emitter [{:keys [emitter-id emitter-parent-id]} context]
           (let [dashboard (Dashboard emitter-parent-id)
                 hydrated-card (hydrate dashboard [:emitters :action])]