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
This commit is contained in:
Martin Brennan 2021-05-24 09:10:22 +10:00 committed by GitHub
parent a5dd24c445
commit 292017dd25
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 44 additions and 4 deletions

View File

@ -27,10 +27,17 @@ end
DiscourseEvent.on(:post_edited) do |post, topic_changed| DiscourseEvent.on(:post_edited) do |post, topic_changed|
unless post.topic&.trashed? 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 if post.is_first_post? && topic_changed
WebHook.enqueue_topic_hooks(:topic_edited, post.topic) WebHook.enqueue_topic_hooks(:topic_edited, post.topic)
else
WebHook.enqueue_post_hooks(:post_edited, post)
end end
end end
end end

View File

@ -170,7 +170,7 @@ class PostRevisor
@validate_topic = true @validate_topic = true
@validate_topic = @opts[:validate_topic] if @opts.has_key?(:validate_topic) @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 = false
@skip_revision = @opts[:skip_revision] if @opts.has_key?(:skip_revision) @skip_revision = @opts[:skip_revision] if @opts.has_key?(:skip_revision)

View File

@ -197,7 +197,8 @@ describe WebHook do
expect do expect do
PostRevisor.new(post, post.topic).revise!( PostRevisor.new(post, post.topic).revise!(
post.user, post.user,
category_id: category.id, { category_id: category.id },
{ skip_validations: true },
) )
end.to change { Jobs::EmitWebHookEvent.jobs.length }.by(1) end.to change { Jobs::EmitWebHookEvent.jobs.length }.by(1)

View File

@ -1268,12 +1268,44 @@ RSpec.describe TopicsController do
topic.reload topic.reload
expect(topic.title).to eq('This is a new title for the topic') 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) expect(Jobs::EmitWebHookEvent.jobs.length).to eq(2)
job_args = Jobs::EmitWebHookEvent.jobs[0]["args"].first 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") expect(job_args["event_name"]).to eq("post_edited")
payload = JSON.parse(job_args["payload"]) 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 end
it "returns errors with invalid titles" do it "returns errors with invalid titles" do