UX: show if webhook is disabled (#7217)

+ show in staff logs when webhook is created/updated/destroyed

app/assets/javascripts/admin/components/admin-web-hook-status.js.es6

@@ -4,8 +4,8 @@ import { bufferedRender } from "discourse-common/lib/buffered-render";
 
 export default Ember.Component.extend(
   bufferedRender({
-    classes: ["text-muted", "text-danger", "text-successful"],
-    icons: ["circle-o", "times-circle", "circle"],
+    classes: ["text-muted", "text-danger", "text-successful", "text-muted"],
+    icons: ["circle-o", "times-circle", "circle", "circle"],
 
     @computed("deliveryStatuses", "model.last_delivery_status")
     status(deliveryStatuses, lastDeliveryStatus) {

app/controllers/admin/web_hooks_controller.rb

@@ -34,6 +34,7 @@ def create
     web_hook = WebHook.new(web_hook_params)
 
     if web_hook.save
+      StaffActionLogger.new(current_user).log_web_hook(web_hook, UserHistory.actions[:web_hook_create])
       render_serialized(web_hook, AdminWebHookSerializer, root: 'web_hook')
     else
       render_json_error web_hook.errors.full_messages
@@ -42,6 +43,7 @@ def create
 
   def update
     if @web_hook.update_attributes(web_hook_params)
+      StaffActionLogger.new(current_user).log_web_hook(@web_hook, UserHistory.actions[:web_hook_update], changes: @web_hook.saved_changes)
       render_serialized(@web_hook, AdminWebHookSerializer, root: 'web_hook')
     else
       render_json_error @web_hook.errors.full_messages
@@ -50,6 +52,7 @@ def update
 
   def destroy
     @web_hook.destroy!
+    StaffActionLogger.new(current_user).log_web_hook(@web_hook, UserHistory.actions[:web_hook_destroy])
     render json: success_json
   end
 

app/models/user_history.rb

@@ -85,7 +85,10 @@ def self.actions
       entity_export: 66,
       change_password: 67,
       topic_timestamps_changed: 68,
-      approve_user: 69
+      approve_user: 69,
+      web_hook_create: 70,
+      web_hook_update: 71,
+      web_hook_destroy: 72
     )
   end
 
@@ -149,7 +152,10 @@ def self.staff_actions
       :entity_export,
       :change_name,
       :topic_timestamps_changed,
-      :approve_user
+      :approve_user,
+      :web_hook_create,
+      :web_hook_update,
+      :web_hook_destroy
     ]
   end
 

app/models/web_hook.rb

@@ -28,7 +28,8 @@ def self.content_types
   def self.last_delivery_statuses
     @last_delivery_statuses ||= Enum.new(inactive: 1,
                                          failed: 2,
-                                         successful: 3)
+                                         successful: 3,
+                                         disabled: 4)
   end
 
   def self.default_event_types

app/serializers/admin_web_hook_serializer.rb

@@ -16,4 +16,8 @@ class AdminWebHookSerializer < ApplicationSerializer
   def web_hook_event_types
     ActiveModel::ArraySerializer.new(object.web_hook_event_types).as_json
   end
+
+  def last_delivery_status
+    object.active ? object.last_delivery_status : WebHook.last_delivery_statuses[:disabled]
+  end
 end

app/services/staff_action_logger.rb

@@ -583,6 +583,30 @@ def log_post_rejected(rejected_post, opts = {})
     ))
   end
 
+  def log_web_hook(web_hook, action, opts = {})
+    details = [
+      "webhook_id: #{web_hook.id}",
+      "payload_url: #{web_hook.payload_url}"
+    ]
+
+    if changes = opts[:changes]
+      changes.reject! { |k, v| k == "updated_at" }
+      old_values = []
+      new_values = []
+      changes.each do |k, v|
+        old_values << "#{k}: #{v[0]}"
+        new_values << "#{k}: #{v[1]}"
+      end
+    end
+
+    UserHistory.create!(params(opts).merge(
+      action: action,
+      context: details.join(", "),
+      previous_value: old_values&.join(", "),
+      new_value: new_values&.join(", ")
+    ))
+  end
+
   private
 
   def params(opts = nil)

config/locales/client.en.yml

@@ -3213,6 +3213,7 @@ en:
           inactive: "Inactive"
           failed: "Failed"
           successful: "Successful"
+          disabled: "Disabled"
         events:
           none: "There are no related events."
           redeliver: "Redeliver"
@@ -3702,6 +3703,9 @@ en:
             change_name: "change name"
             topic_timestamps_changed: "topic timestamps changed"
             approve_user: "approved user"
+            web_hook_create: "webhook create"
+            web_hook_update: "webhook update"
+            web_hook_destroy: "webhook destroy"
         screened_emails:
           title: "Screened Emails"
           description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed."

config/locales/server.en.yml

@@ -4349,3 +4349,6 @@ en:
     unknown: "unknown"
     user_merged: "%{username} was merged into this account"
     user_delete_self: "Deleted by self from %{url}"
+    update: "Updated"
+    create: "Created"
+    destroy: "Destroyed"

spec/requests/admin/web_hooks_controller_spec.rb

@@ -34,6 +34,7 @@
 
         json = ::JSON.parse(response.body)
         expect(json["web_hook"]["payload_url"]).to eq("https://meta.discourse.org/")
+        expect(UserHistory.where(acting_user_id: admin.id, action: UserHistory.actions[:web_hook_create]).count).to eq(1)
       end
 
       it 'returns error when field is not filled correctly' do
@@ -57,6 +58,30 @@
       end
     end
 
+    describe '#update' do
+      it "logs webhook update" do
+        put "/admin/api/web_hooks/#{web_hook.id}.json", params: {
+          web_hook: { active: false, payload_url: "https://test.com" }
+        }
+
+        expect(response.status).to eq(200)
+        expect(UserHistory.where(acting_user_id: admin.id,
+                                 action: UserHistory.actions[:web_hook_update],
+                                 new_value: "payload_url: https://test.com, active: false").exists?).to eq(true)
+      end
+    end
+
+    describe '#destroy' do
+      it "logs webhook destroy" do
+        delete "/admin/api/web_hooks/#{web_hook.id}.json", params: {
+          web_hook: { active: false, payload_url: "https://test.com" }
+        }
+
+        expect(response.status).to eq(200)
+        expect(UserHistory.where(acting_user_id: admin.id, action: UserHistory.actions[:web_hook_destroy]).exists?).to eq(true)
+      end
+    end
+
     describe '#ping' do
       it 'enqueues the ping event' do
         expect do