From 292017dd25aa4bfe445239f6c3863b8ae2041b4d Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Mon, 24 May 2021 09:10:22 +1000
Subject: [PATCH] FIX: Do not call :post_edited webhook twice when editing OP
 (#13112)

When editing the first post for the topic we do two AJAX requests
to two separate controllers in this order:

PUT /t/topic-name
PUT /posts/2489523

This causes two post revisor calls, which end up triggering the
:post_edited DiscourseEvent twice. This is then picked up and sent
as a WebHook event twice. However we do not need to send a :post_edited
webhook event if the first post is being edited and topic_changed is
true from the :post_edited DiscourseEvent, because a second event will
shortly come through for just the post.

See https://meta.discourse.org/t/post-webhook-fires-two-times-on-post-edited-for-first-post-in-a-topic/162408

Continued on from https://github.com/discourse/discourse/pull/10590
---
 config/initializers/012-web_hook_events.rb |  9 +++++-
 lib/post_revisor.rb                        |  2 +-
 spec/models/web_hook_spec.rb               |  3 +-
 spec/requests/topics_controller_spec.rb    | 34 +++++++++++++++++++++-
 4 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/config/initializers/012-web_hook_events.rb b/config/initializers/012-web_hook_events.rb
index 942260c85ce..809967bbd11 100644
--- a/config/initializers/012-web_hook_events.rb
+++ b/config/initializers/012-web_hook_events.rb
@@ -27,10 +27,17 @@ end
 
 DiscourseEvent.on(:post_edited) do |post, topic_changed|
   unless post.topic&.trashed?
-    WebHook.enqueue_post_hooks(:post_edited, post)
 
+    # if we are editing the OP and the topic is changed, do not send
+    # the post_edited event -- this event is sent separately because
+    # when we update the OP in the UI we send two API calls in this order:
+    #
+    # PUT /t/topic-name
+    # PUT /post/243552
     if post.is_first_post? && topic_changed
       WebHook.enqueue_topic_hooks(:topic_edited, post.topic)
+    else
+      WebHook.enqueue_post_hooks(:post_edited, post)
     end
   end
 end
diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb
index 571cf94a575..cbd5f71a5e5 100644
--- a/lib/post_revisor.rb
+++ b/lib/post_revisor.rb
@@ -170,7 +170,7 @@ class PostRevisor
 
     @validate_topic = true
     @validate_topic = @opts[:validate_topic] if @opts.has_key?(:validate_topic)
-    @validate_topic = !@opts[:validate_topic] if @opts.has_key?(:skip_validations)
+    @validate_topic = !@opts[:skip_validations] if @opts.has_key?(:skip_validations)
 
     @skip_revision = false
     @skip_revision = @opts[:skip_revision] if @opts.has_key?(:skip_revision)
diff --git a/spec/models/web_hook_spec.rb b/spec/models/web_hook_spec.rb
index 570ba6a95c6..73ef8c913ae 100644
--- a/spec/models/web_hook_spec.rb
+++ b/spec/models/web_hook_spec.rb
@@ -197,7 +197,8 @@ describe WebHook do
       expect do
         PostRevisor.new(post, post.topic).revise!(
           post.user,
-          category_id: category.id,
+          { category_id: category.id },
+          { skip_validations: true },
         )
       end.to change { Jobs::EmitWebHookEvent.jobs.length }.by(1)
 
diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb
index bd4e31e2938..684a8608493 100644
--- a/spec/requests/topics_controller_spec.rb
+++ b/spec/requests/topics_controller_spec.rb
@@ -1268,12 +1268,44 @@ RSpec.describe TopicsController do
           topic.reload
           expect(topic.title).to eq('This is a new title for the topic')
 
+          # emits a topic_edited event but not a post_edited web hook event
+          expect(Jobs::EmitWebHookEvent.jobs.length).to eq(1)
+          job_args = Jobs::EmitWebHookEvent.jobs[0]["args"].first
+
+          expect(job_args["event_name"]).to eq("topic_edited")
+          payload = JSON.parse(job_args["payload"])
+          expect(payload["title"]).to eq('This is a new title for the topic')
+        end
+
+        it 'allows a change of then updating the OP' do
+          topic.update(user: user)
+          topic.first_post.update(user: user)
+
+          put "/t/#{topic.slug}/#{topic.id}.json", params: {
+            title: 'This is a new title for the topic'
+          }
+
+          topic.reload
+          expect(topic.title).to eq('This is a new title for the topic')
+
+          update_params = {
+            post: { raw: 'edited body', edit_reason: 'typo' },
+          }
+          put "/posts/#{topic.first_post.id}.json", params: update_params
+
+          # emits a topic_edited event and a post_edited web hook event
           expect(Jobs::EmitWebHookEvent.jobs.length).to eq(2)
           job_args = Jobs::EmitWebHookEvent.jobs[0]["args"].first
 
+          expect(job_args["event_name"]).to eq("topic_edited")
+          payload = JSON.parse(job_args["payload"])
+          expect(payload["title"]).to eq('This is a new title for the topic')
+
+          job_args = Jobs::EmitWebHookEvent.jobs[1]["args"].first
+
           expect(job_args["event_name"]).to eq("post_edited")
           payload = JSON.parse(job_args["payload"])
-          expect(payload["topic_title"]).to eq('This is a new title for the topic')
+          expect(payload["raw"]).to eq("edited body")
         end
 
         it "returns errors with invalid titles" do