Add Database-local Setting for enabling Actions for a specific Database (#22571)

src/metabase/actions.clj

 (ns metabase.actions
   "Code related to the new writeback Actions."
   (:require [metabase.models.setting :as setting]
-            [metabase.util.i18n :as i18n]))
+            [metabase.models.table :refer [Table]]
+            [metabase.util.i18n :as i18n]
+            [toucan.db :as db]))
 
 (setting/defsetting experimental-enable-actions
   (i18n/deferred-tru "Whether to enable using the new experimental Actions features globally. (Actions must also be enabled for each Database.)")
   :default false
   :type :boolean
   :visibility :public)
+
+(setting/defsetting database-enable-actions
+  (i18n/deferred-tru "Whether to enable using the new experimental Actions for a specific Database.")
+  :default false
+  :type :boolean
+  :visibility :public
+  :database-local :only)
+
+;; TODO -- should these be ASYNC!!!!
+(defmulti table-action!
+  "Multimethod for doing an action on a Table as a whole, e.g. inserting a new row."
+  {:arglists '([action {:keys [table-id], :as arg-map}])}
+  (fn [action _arg-map]
+    (keyword action)))
+
+(defmethod table-action! :default
+  [action _arg-map]
+  (throw (ex-info (i18n/tru "Unknown Table action {0}." (pr-str (some-> action name)))
+                  {:status-code 404})))
+
+(defmethod table-action! :insert
+  [_action {:keys [table-id values]}]
+  {:pre [(map? values)]}
+  ;; placeholder until we really implement it.
+  {:insert-into (db/select-one-field :name Table :id table-id)
+   :values      values})
+
+(defmulti row-action!
+  "Multimethod for doing an action against a specific row as a whole, e.g. updating or deleting that row."
+  {:arglists '([action {:keys [table-id pk], :as arg-map}])}
+  (fn [action _arg-map]
+    (keyword action)))
+
+(defmethod row-action! :default
+  [action _arg-map]
+  (throw (ex-info (i18n/tru "Unknown row action {0}." (pr-str (some-> action name)))
+                  {:status-code 404})))
+
+(defn- pk-where-clause [pk]
+  {:pre [(map? pk)]}
+  (let [clauses (for [[k v] pk]
+                  [:= (keyword k) v])]
+    (if (= (count clauses) 1)
+      (first clauses)
+      (into [:and] clauses))))
+
+(defmethod row-action! :delete
+  [_action {:keys [table-id pk]}]
+  ;; placeholder until we really implement it.
+  {:delete-from (db/select-one-field :name Table :id table-id)
+   :where       (pk-where-clause pk)})
+
+(defmethod row-action! :update
+  [_action {:keys [table-id pk values]}]
+  ;; placeholder until we really implement it.
+  {:update (db/select-one-field :name Table :id table-id)
+   :set    values
+   :where  (pk-where-clause pk)})

src/metabase/api/actions.clj

 (ns metabase.api.actions
   "`/api/actions/` endpoints."
-  (:require [compojure.core :refer [GET]]
-            [metabase.actions :as actions]
-            [metabase.api.common :as api]
-            [metabase.util.i18n :as i18n]))
+  (:require
+   [compojure.core :as compojure :refer [POST]]
+   [metabase.actions :as actions]
+   [metabase.api.common :as api]
+   [metabase.models.database :refer [Database]]
+   [metabase.models.setting :as setting]
+   [metabase.models.table :refer [Table]]
+   [metabase.util.i18n :as i18n]
+   [metabase.util.schema :as su]
+   [toucan.db :as db]))
 
-(api/defendpoint GET "/dummy"
-  "Dummy API endpoint to test feature flagging with the [[metabase.actions/experimental-enable-actions]] feature flag.
-  We can remove this and test other endpoints once we have other endpoints."
-  []
-  {:dummy true})
+(defn- do-action-for-table [table-id thunk]
+  {:pre [(integer? table-id)]}
+  (let [database-id (api/check-404 (db/select-one-field :db_id Table :id table-id))
+        db-settings (db/select-one-field :settings Database :id database-id)]
+    (binding [setting/*database-local-values* db-settings]
+      ;; make sure Actions are enabled for this Database
+      (when-not (actions/database-enable-actions)
+        (throw (ex-info (i18n/tru "Actions are not enabled for Database {0}." database-id)
+                        {:status-code 400})))
+      ;; TODO -- need to check permissions once the perms code is in place.
+      (thunk))))
+
+(api/defendpoint POST "/table/:action"
+  "Generic API endpoint for doing an action against a specific Table."
+  [action :as {{:keys [table-id], :as body} :body}]
+  {table-id su/IntGreaterThanZero}
+  (do-action-for-table
+   table-id
+   (fn []
+     (actions/table-action! (keyword action) (assoc body :table-id table-id)))))
+
+(api/defendpoint POST "/row/:action"
+  "Generic API endpoint for doing an action against a specific row."
+  [action :as {{:keys [table-id pk], :as body} :body}]
+  {table-id su/IntGreaterThanZero
+   pk       su/Map}
+  (do-action-for-table
+   table-id
+   (fn []
+     (actions/row-action! (keyword action) (assoc body :table-id table-id)))))
 
 (defn- +check-actions-enabled
   "Ring middleware that checks that the [[metabase.actions/experimental-enable-actions]] feature flag is enabled, and

src/metabase/models/setting.clj

@@ -907,6 +907,11 @@
   The ability of this Setting to be /Database-local/. Valid values are `:only`, `:allowed`, and `:never`. Default:
   `:never`. See docstring for [[metabase.models.setting]] for more information.
 
+  ###### `:user-local`
+
+  Whether this Setting is /User-local/. Valid values are `:only`, `:allowed`, and `:never`. Default: `:never`. See
+  docstring for [[metabase.models.setting]] for more info.
+
   ###### `:deprecated`
 
   If this setting is deprecated, this should contain a string of the Metabase version in which the setting was

test/metabase/api/actions_test.clj

 (ns metabase.api.actions-test
-  (:require [clojure.test :refer :all]
-            [metabase.api.actions :as api.actions]
-            [metabase.test :as mt]))
+  (:require
+   [clojure.string :as str]
+   [clojure.test :refer :all]
+   [metabase.api.actions :as api.actions]
+   [metabase.models.database :refer [Database]]
+   [metabase.test :as mt]))
 
 (comment api.actions/keep-me)
 
-;; TODO -- once we add a new endpoint rework these tests to test those and remove the dummy endpoint.
-(deftest global-feature-flag-test
-  (testing "Enable or disable endpoints based on the `experimental-enable-actions` feature flag"
-    (testing "Should return a 400 if feature flag is disabled"
-      (mt/with-temporary-setting-values [experimental-enable-actions false]
-        (is (= "Actions are not enabled."
-               (mt/user-http-request :crowberto :get 400 "actions/dummy")))))
-    (testing "Should work if feature flag is enabled"
+(defn mock-requests []
+  [{:action       "actions/table/insert"
+    :request-body {:table-id (mt/id :venues)
+                   :values   {:name "Toucannery"}}
+    :expected     {:insert-into "VENUES"
+                   :values      {:name "Toucannery"}}}
+   {:action       "actions/row/update"
+    :request-body {:table-id (mt/id :venues)
+                   :pk       {:id 1, :name "Red Medicine"}
+                   :values   {:name "Toucannery"}}
+    :expected     {:update "VENUES"
+                   :set    {:name "Toucannery"}
+                   :where  ["and"
+                            ["=" "id" 1]
+                            ["=" "name" "Red Medicine"]]}}
+   {:action       "actions/row/delete"
+    :request-body {:table-id (mt/id :venues)
+                   :pk       {:id 1}}
+    :expected     {:delete-from "VENUES"
+                   :where       ["=" "id" 1]}}])
+
+(defn- row-action? [action]
+  (str/starts-with? action "actions/row"))
+
+(deftest happy-path-test
+  (testing "Make sure it's possible to use known actions end-to-end if preconditions are satisfied"
+    (mt/with-temp-copy-of-db
       (mt/with-temporary-setting-values [experimental-enable-actions true]
-        (is (= {:dummy true}
-               (mt/user-http-request :crowberto :get 200 "actions/dummy")))))))
+        (mt/with-temp-vals-in-db Database (mt/id) {:settings {:database-enable-actions true}}
+          (doseq [{:keys [action request-body expected]} (mock-requests)]
+            (testing action
+              (is (= expected
+                     (mt/user-http-request :crowberto :post 200 action request-body))))))))))
+
+(deftest feature-flags-test
+  (testing "Disable endpoints unless both global and Database feature flags are enabled"
+    (doseq [{:keys [action request-body]} (mock-requests)
+            enable-global-feature-flag?   [true false]
+            enable-database-feature-flag? [true false]]
+      (testing action
+        (mt/with-temporary-setting-values [experimental-enable-actions enable-global-feature-flag?]
+          (mt/with-temp-vals-in-db Database (mt/id) {:settings {:database-enable-actions enable-database-feature-flag?}}
+            (cond
+              (not enable-global-feature-flag?)
+              (testing "Should return a 400 if global feature flag is disabled"
+                (is (= "Actions are not enabled."
+                       (mt/user-http-request :crowberto :post 400 action request-body))))
+
+              (not enable-database-feature-flag?)
+              (testing "Should return a 400 if Database feature flag is disabled."
+                (is (re= #"^Actions are not enabled for Database [\d,]+\.$"
+                         (mt/user-http-request :crowberto :post 400 action request-body)))))))))))
+
+(deftest validation-test
+  (mt/with-temporary-setting-values [experimental-enable-actions true]
+    (mt/with-temp-vals-in-db Database (mt/id) {:settings {:database-enable-actions true}}
+      (doseq [{:keys [action]} (mock-requests)]
+        (testing action
+          (testing "Require `:table-id`"
+            (is (= {:errors {:table-id "value must be an integer greater than zero."}}
+                   (mt/user-http-request :crowberto :post 400 action))))
+          (when (row-action? action)
+            (testing "Require `:pk` for row actions"
+              (is (= {:errors {:pk "value must be a map."}}
+                     (mt/user-http-request :crowberto :post 400 action {:table-id (mt/id :venues)})))
+              (testing "`:pk` must be a map"
+                (is (= {:errors {:pk "value must be a map."}}
+                       (mt/user-http-request :crowberto :post 400 action {:table-id (mt/id :venues), :pk 1})))))))))))
+
+(deftest four-oh-four-test
+  (mt/with-temporary-setting-values [experimental-enable-actions true]
+    (mt/with-temp-vals-in-db Database (mt/id) {:settings {:database-enable-actions true}}
+      (doseq [{:keys [action]} (mock-requests)]
+        (testing action
+          (testing "404 for unknown Table"
+            (is (= "Not found."
+                   (mt/user-http-request :crowberto :post 404 action {:table-id Integer/MAX_VALUE, :pk {:id 1}}))))))
+      (testing "404 for unknown Table action"
+        (is (= "Unknown Table action \"fake\"."
+               (mt/user-http-request :crowberto :post 404 "actions/table/fake" {:table-id (mt/id :venues), :pk {:id 1}}))))
+      (testing "404 for unknown row action"
+        (is (= "Unknown row action \"fake\"."
+               (mt/user-http-request :crowberto :post 404 "actions/row/fake" {:table-id (mt/id :venues), :pk {:id 1}})))))))