Merge pull request #2882 from penpot/niwinz-webhooks-fix

🐛 Fix incorrect state management on webhooks crud

backend/src/app/rpc/commands/webhooks.clj

@@ -8,6 +8,7 @@
   (:require
    [app.common.exceptions :as ex]
    [app.common.spec :as us]
+   [app.common.uri :as u]
    [app.common.uuid :as uuid]
    [app.db :as db]
    [app.http.client :as http]
@@ -22,10 +23,15 @@
    [cuerdas.core :as str]
    [promesa.core :as p]))
 
+(defn decode-row
+  [{:keys [uri] :as row}]
+  (cond-> row
+    (string? uri) (assoc :uri (u/uri uri))))
+
 ;; --- Mutation: Create Webhook
 
 (s/def ::team-id ::us/uuid)
-(s/def ::uri ::us/not-empty-string)
+(s/def ::uri ::us/uri)
 (s/def ::is-active ::us/boolean)
 (s/def ::mtype
   #{"application/json"
@@ -59,7 +65,7 @@
 
     (if (not= (:uri whook) (:uri params))
       (->> (http/req! cfg {:method :head
-                           :uri (:uri params)
+                           :uri (str (:uri params))
                            :timeout (dt/duration "3s")})
            (p/hmap (fn [response exception]
                      (if exception
@@ -79,22 +85,24 @@
 
 (defn- insert-webhook!
   [{:keys [::db/pool]} {:keys [team-id uri mtype is-active] :as params}]
-  (db/insert! pool :webhook
+  (-> (db/insert! pool :webhook
                   {:id (uuid/next)
                    :team-id team-id
-               :uri uri
+                   :uri (str uri)
                    :is-active is-active
-               :mtype mtype}))
+                   :mtype mtype})
+      (decode-row)))
 
 (defn- update-webhook!
   [{:keys [::db/pool] :as cfg} {:keys [id] :as wook} {:keys [uri mtype is-active] :as params}]
-  (db/update! pool :webhook
-              {:uri uri
+  (-> (db/update! pool :webhook
+                  {:uri (str uri)
                    :is-active is-active
                    :mtype mtype
                    :error-code nil
                    :error-count 0}
-              {:id id}))
+                  {:id id})
+      (decode-row)))
 
 (sv/defmethod ::create-webhook
   {::doc/added "1.17"}
@@ -110,7 +118,7 @@
 (sv/defmethod ::update-webhook
   {::doc/added "1.17"}
   [{:keys [::db/pool ::wrk/executor] :as cfg} {:keys [::rpc/profile-id id] :as params}]
-  (let [whook (db/get pool :webhook {:id id})]
+  (let [whook (-> (db/get pool :webhook {:id id}) (decode-row))]
     (check-edition-permissions! pool profile-id (:team-id whook))
     (->> (validate-webhook! cfg whook params)
          (p/fmap executor (fn [_] (update-webhook! cfg whook params))))))
@@ -123,7 +131,7 @@
   {::doc/added "1.17"}
   [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id]}]
   (db/with-atomic [conn pool]
-    (let [whook (db/get conn :webhook {:id id})]
+    (let [whook (-> (db/get conn :webhook {:id id}) decode-row)]
       (check-edition-permissions! conn profile-id (:team-id whook))
       (db/delete! conn :webhook {:id id})
       nil)))
@@ -143,4 +151,5 @@
   [{:keys [pool] :as cfg} {:keys [::rpc/profile-id team-id]}]
   (with-open [conn (db/open pool)]
     (check-read-permissions! conn profile-id team-id)
-    (db/exec! conn [sql:get-webhooks team-id])))
+    (->> (db/exec! conn [sql:get-webhooks team-id])
+         (mapv decode-row))))

backend/test/backend_tests/rpc_webhooks_test.clj

@@ -6,11 +6,12 @@
 
 (ns backend-tests.rpc-webhooks-test
   (:require
+   [app.common.uri :as u]
    [app.common.uuid :as uuid]
    [app.db :as db]
    [app.http :as http]
-   [app.storage :as sto]
    [app.rpc :as-alias rpc]
+   [app.storage :as sto]
    [backend-tests.helpers :as th]
    [clojure.test :as t]
    [mockery.core :refer [with-mocks]]))
@@ -31,7 +32,7 @@
         (let [params {::th/type :create-webhook
                       ::rpc/profile-id (:id prof)
                       :team-id team-id
-                      :uri "http://example.com"
+                      :uri (u/uri "http://example.com")
                       :mtype "application/json"}
               out    (th/command! params)]
 

frontend/src/app/main/ui/dashboard/team.cljs

@@ -637,7 +637,9 @@
   {::mf/register modal/components
    ::mf/register-as :webhook}
   [{:keys [webhook] :as props}]
-  (let [initial (mf/use-memo (fn [] (or webhook {:is-active false :mtype "application/json"})))
+  ;; FIXME: this is a workaround because input fields do not support rendering hooks
+  (let [initial (mf/use-memo (fn [] (or (some-> webhook (update :uri str))
+                                        {:is-active false :mtype "application/json"})))
         form    (fm/use-form :spec ::webhook-form
                              :initial initial)
         on-success
@@ -735,8 +737,6 @@
           ]
          [:div.explain (tr "dashboard.webhooks.active.explain")]]]
 
-
-
        [:div.modal-footer
         [:div.action-buttons
          [:input.cancel-button
@@ -827,7 +827,7 @@
         {:success? (nil? error-code)
          :text last-delivery-text}]]]
      [:div.table-field.uri
-      [:div (:uri webhook)]]
+      [:div (dm/str (:uri webhook))]]
      [:div.table-field.active
       [:div (if (:is-active webhook)
               (tr "labels.active")