FIX: Generate webhook payloads before destroy events (#6325)

FIX: Generate webhook payloads before destroy events (#6325)

diff --git a/app/models/category.rb b/app/models/category.rb
index 959fe8e..d494d42 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -8,6 +8,7 @@ class Category < ActiveRecord::Base
   include HasCustomFields
   include CategoryHashtag
   include AnonCacheInvalidator
+  include HasDestroyedWebHook
 
   REQUIRE_TOPIC_APPROVAL = 'require_topic_approval'
   REQUIRE_REPLY_APPROVAL = 'require_reply_approval'
diff --git a/app/models/concerns/has_destroyed_web_hook.rb b/app/models/concerns/has_destroyed_web_hook.rb
new file mode 100644
index 0000000..98d165d
--- /dev/null
+++ b/app/models/concerns/has_destroyed_web_hook.rb
@@ -0,0 +1,17 @@
+module HasDestroyedWebHook
+  extend ActiveSupport::Concern
+
+  included do
+    around_destroy :enqueue_destroyed_web_hook
+  end
+
+  def enqueue_destroyed_web_hook
+    type = self.class.name.underscore.to_sym
+    payload = WebHook.generate_payload(type, self)
+    yield
+    WebHook.enqueue_hooks(type, "#{type}_destroyed".to_sym,
+      id: id,
+      payload: payload
+    )
+  end
+end
diff --git a/app/models/group.rb b/app/models/group.rb
index a07ddad..5c8d137 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -5,6 +5,7 @@ require_dependency 'enum'
 class Group < ActiveRecord::Base
   include HasCustomFields
   include AnonCacheInvalidator
+  include HasDestroyedWebHook
 
   cattr_accessor :preloaded_custom_field_names
   self.preloaded_custom_field_names = Set.new
diff --git a/app/models/tag.rb b/app/models/tag.rb
index 705d676..11002e2 100644
--- a/app/models/tag.rb
+++ b/app/models/tag.rb
@@ -1,5 +1,6 @@
 class Tag < ActiveRecord::Base
   include Searchable
+  include HasDestroyedWebHook
 
   validates :name, presence: true, uniqueness: true
 
diff --git a/app/models/user.rb b/app/models/user.rb
index f6583ad..80f560f 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -19,6 +19,7 @@ class User < ActiveRecord::Base
   include Roleable
   include HasCustomFields
   include SecondFactorManager
+  include HasDestroyedWebHook
 
   has_many :posts
   has_many :notifications, dependent: :destroy
diff --git a/app/models/web_hook.rb b/app/models/web_hook.rb
index 8bea527..518b076 100644
--- a/app/models/web_hook.rb
+++ b/app/models/web_hook.rb
@@ -41,24 +41,21 @@ class WebHook < ActiveRecord::Base
       .distinct
   end
 
-  def self.enqueue_hooks(type, opts = {})
+  def self.enqueue_hooks(type, event, opts = {})
     active_web_hooks(type).each do |web_hook|
       Jobs.enqueue(:emit_web_hook_event, opts.merge(
-        web_hook_id: web_hook.id, event_type: type.to_s
+        web_hook_id: web_hook.id, event_name: event.to_s, event_type: type.to_s
       ))
     end
   end
 
   def self.enqueue_object_hooks(type, object, event, serializer = nil)
     if active_web_hooks(type).exists?
-      serializer ||= "WebHook#{type.capitalize}Serializer".constantize
-
-      WebHook.enqueue_hooks(type,
-        event_name: event.to_s,
-        payload: serializer.new(object,
-          scope: self.guardian,
-          root: false
-        ).to_json
+      payload = WebHook.generate_payload(type, object, serializer)
+
+      WebHook.enqueue_hooks(type, event,
+        id: object.id,
+        payload: payload
       )
     end
   end
@@ -66,31 +63,38 @@ class WebHook < ActiveRecord::Base
   def self.enqueue_topic_hooks(event, topic)
     if active_web_hooks('topic').exists?
       topic_view = TopicView.new(topic.id, Discourse.system_user)
+      payload = WebHook.generate_payload(:topic, topic_view, WebHookTopicViewSerializer)
 
-      WebHook.enqueue_hooks(:topic,
+      WebHook.enqueue_hooks(:topic, event,
+        id: topic.id,
         category_id: topic&.category_id,
-        event_name: event.to_s,
-        payload: WebHookTopicViewSerializer.new(topic_view,
-          scope: self.guardian,
-          root: false
-        ).to_json
+        payload: payload
       )
     end
   end
 
   def self.enqueue_post_hooks(event, post)
     if active_web_hooks('post').exists?
-      WebHook.enqueue_hooks(:post,
+      payload = WebHook.generate_payload(:post, post)
+
+      WebHook.enqueue_hooks(:post, event,
+        id: post.id,
         category_id: post&.topic&.category_id,
-        event_name: event.to_s,
-        payload: WebHookPostSerializer.new(post,
-          scope: self.guardian,
-          root: false
-        ).to_json
+        payload: payload
       )
     end
   end
 
+  def self.generate_payload(type, object, serializer = nil)
+    serializer ||= TagSerializer if type == :tag
+    serializer ||= "WebHook#{type.capitalize}Serializer".constantize
+
+    serializer.new(object,
+      scope: self.guardian,
+      root: false
+    ).to_json
+  end
+
   private
 
   def self.guardian
diff --git a/config/initializers/012-web_hook_events.rb b/config/initializers/012-web_hook_events.rb
index 0834b87..a99b958 100644
--- a/config/initializers/012-web_hook_events.rb
+++ b/config/initializers/012-web_hook_events.rb
@@ -1,5 +1,4 @@
 %i(
-  topic_destroyed
   topic_recovered
 ).each do |event|
   DiscourseEvent.on(event) do |topic, _|
@@ -17,7 +16,6 @@ end
 
 %i(
   post_created
-  post_destroyed
   post_recovered
 ).each do |event|
   DiscourseEvent.on(event) do |post, _, _|
@@ -41,7 +39,6 @@ end
   user_logged_in
   user_approved
   user_updated
-  user_destroyed
 ).each do |event|
   DiscourseEvent.on(event) do |user|
     WebHook.enqueue_object_hooks(:user, user, event)
@@ -51,7 +48,6 @@ end
 %i(
   group_created
   group_updated
-  group_destroyed
 ).each do |event|
   DiscourseEvent.on(event) do |group|
     WebHook.enqueue_object_hooks(:group, group, event)
@@ -61,7 +57,6 @@ end
 %i(
   category_created
   category_updated
-  category_destroyed
 ).each do |event|
   DiscourseEvent.on(event) do |category|
     WebHook.enqueue_object_hooks(:category, category, event)
@@ -71,7 +66,6 @@ end
 %i(
   tag_created
   tag_updated
-  tag_destroyed
 ).each do |event|
   DiscourseEvent.on(event) do |tag|
     WebHook.enqueue_object_hooks(:tag, tag, event, TagSerializer)
diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb
index bd932b4..87ded04 100644
--- a/lib/post_destroyer.rb
+++ b/lib/post_destroyer.rb
@@ -45,6 +45,14 @@ class PostDestroyer
   end
 
   def destroy
+    payload = WebHook.generate_payload(:post, @post)
+    topic = @post.topic
+
+    if @post.is_first_post? && topic
+      topic_view = TopicView.new(topic.id, Discourse.system_user)
+      topic_payload = WebHook.generate_payload(:topic, topic_view, WebHookTopicViewSerializer)
+    end
+
     delete_removed_posts_after = @opts[:delete_removed_posts_after] || SiteSetting.delete_removed_posts_after
 
     if @user.staff? || delete_removed_posts_after < 1
@@ -53,9 +61,19 @@ class PostDestroyer
       mark_for_deletion(delete_removed_posts_after)
     end
     DiscourseEvent.trigger(:post_destroyed, @post, @opts, @user)
+    WebHook.enqueue_hooks(:post, :post_destroyed,
+      id: @post.id,
+      category_id: @post&.topic&.category_id,
+      payload: payload
+    )
 
     if @post.is_first_post? && @post.topic
       DiscourseEvent.trigger(:topic_destroyed, @post.topic, @user)
+      WebHook.enqueue_hooks(:topic, :topic_destroyed,
+        id: topic.id,
+        category_id: topic&.category_id,
+        payload: topic_payload
+      )
     end
   end
 
diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb
index 96603a4..5702bf6 100644
--- a/spec/components/post_destroyer_spec.rb
+++ b/spec/components/post_destroyer_spec.rb
@@ -424,6 +424,16 @@ describe PostDestroyer do
       }.to_not change { author.topic_count }
       expect(author.post_count).to eq(0) # also unchanged
     end
+
+    it 'triggers the extensibility events' do
+      events = DiscourseEvent.track_events { PostDestroyer.new(admin, first_post).destroy }.last(2)
+
+      expect(events[0][:event_name]).to eq(:post_destroyed)
+      expect(events[0][:params].first).to eq(first_post)
+

[... diff too long, it was truncated ...]

GitHub sha: 8430ea92

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/when-does-it-log-check-personal-message/162285/5