From 40f10855c621a2b059c5d91191a8b4a3b29adbf2 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Thu, 29 Nov 2018 22:44:18 +0530 Subject: [PATCH] FIX: defer flags (only) when handling a flag and deleting replies (#6702) --- .../admin/models/flagged-post.js.es6 | 2 +- .../javascripts/discourse/models/post.js.es6 | 4 +-- app/controllers/posts_controller.rb | 3 +- lib/post_destroyer.rb | 12 +++++-- spec/components/post_destroyer_spec.rb | 35 ++++++++++--------- spec/requests/posts_controller_spec.rb | 18 ++++++++++ 6 files changed, 52 insertions(+), 22 deletions(-) diff --git a/app/assets/javascripts/admin/models/flagged-post.js.es6 b/app/assets/javascripts/admin/models/flagged-post.js.es6 index d2a9c4510f9..fde2cdf01a4 100644 --- a/app/assets/javascripts/admin/models/flagged-post.js.es6 +++ b/app/assets/javascripts/admin/models/flagged-post.js.es6 @@ -136,7 +136,7 @@ export default Post.extend({ label: I18n.t("yes_value"), class: "btn-danger", callback() { - Post.deleteMany(replies.map(r => r.id)) + Post.deleteMany(replies.map(r => r.id), { deferFlags: true }) .then(action) .then(resolve) .catch(error => { diff --git a/app/assets/javascripts/discourse/models/post.js.es6 b/app/assets/javascripts/discourse/models/post.js.es6 index 3ea49fdc0bd..4de7c3b3476 100644 --- a/app/assets/javascripts/discourse/models/post.js.es6 +++ b/app/assets/javascripts/discourse/models/post.js.es6 @@ -369,10 +369,10 @@ Post.reopenClass({ }); }, - deleteMany(post_ids) { + deleteMany(post_ids, { deferFlags = false } = {}) { return ajax("/posts/destroy_many", { type: "DELETE", - data: { post_ids } + data: { post_ids, defer_flags: deferFlags } }); }, diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index f35ae00e690..e4e0ccf7611 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -336,6 +336,7 @@ class PostsController < ApplicationController def destroy_many params.require(:post_ids) + defer_flags = params[:defer_flags] || false posts = Post.where(id: post_ids_including_replies) raise Discourse::InvalidParameters.new(:post_ids) if posts.blank? @@ -344,7 +345,7 @@ class PostsController < ApplicationController posts.each { |p| guardian.ensure_can_delete!(p) } Post.transaction do - posts.each { |p| PostDestroyer.new(current_user, p).destroy } + posts.each { |p| PostDestroyer.new(current_user, p, defer_flags: defer_flags).destroy } end render body: nil diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 7336e981b64..efa292e91ef 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -147,7 +147,11 @@ class PostDestroyer update_user_counts TopicUser.update_post_action_cache(post_id: @post.id) DB.after_commit do - agree_with_flags + if @opts[:defer_flags].to_s == "true" + defer_flags + else + agree_with_flags + end end end @@ -225,7 +229,7 @@ class PostDestroyer if @post.has_active_flag? && @user.id > 0 && @user.staff? Jobs.enqueue( :send_system_message, - user_id: @post.user.id, + user_id: @post.user_id, message_type: :flags_agreed_and_post_deleted, message_options: { url: @post.url, @@ -241,6 +245,10 @@ class PostDestroyer PostAction.agree_flags!(@post, @user, delete_post: true) end + def defer_flags + PostAction.defer_flags!(@post, @user, delete_post: true) + end + def trash_user_actions UserAction.where(target_post_id: @post.id).each do |ua| row = { diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 7a91c3bef3f..e29543b7f4a 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -632,7 +632,7 @@ describe PostDestroyer do let!(:flag) { PostAction.act(moderator, second_post, PostActionType.types[:off_topic]) } before do - SiteSetting.queue_jobs = false + Jobs::SendSystemMessage.clear end it "should delete public post actions and agree with flags" do @@ -650,36 +650,39 @@ describe PostDestroyer do expect(second_post.bookmark_count).to eq(0) expect(second_post.off_topic_count).to eq(1) - notification = second_post.user.notifications.where(notification_type: Notification.types[:private_message]).last - expect(notification).to be_present - expect(notification.topic.title).to eq(I18n.t('system_messages.flags_agreed_and_post_deleted.subject_template')) + expect(Jobs::SendSystemMessage.jobs.size).to eq(1) end it "should not send the flags_agreed_and_post_deleted message if it was deleted by system" do - second_post.expects(:update_flagged_posts_count) + expect(PostAction.flagged_posts_count).to eq(1) PostDestroyer.new(Discourse.system_user, second_post).destroy - expect( - Topic.where(title: I18n.t('system_messages.flags_agreed_and_post_deleted.subject_template')).exists? - ).to eq(false) + expect(Jobs::SendSystemMessage.jobs.size).to eq(0) + expect(PostAction.flagged_posts_count).to eq(0) end it "should not send the flags_agreed_and_post_deleted message if it was deleted by author" do SiteSetting.delete_removed_posts_after = 0 - second_post.expects(:update_flagged_posts_count) + expect(PostAction.flagged_posts_count).to eq(1) PostDestroyer.new(second_post.user, second_post).destroy - expect( - Topic.where(title: I18n.t('system_messages.flags_agreed_and_post_deleted.subject_template')).exists? - ).to eq(false) + expect(Jobs::SendSystemMessage.jobs.size).to eq(0) + expect(PostAction.flagged_posts_count).to eq(0) end it "should not send the flags_agreed_and_post_deleted message if flags were deferred" do - second_post.expects(:update_flagged_posts_count) + expect(PostAction.flagged_posts_count).to eq(1) PostAction.defer_flags!(second_post, moderator) second_post.reload + expect(PostAction.flagged_posts_count).to eq(0) + PostDestroyer.new(moderator, second_post).destroy - expect( - Topic.where(title: I18n.t('system_messages.flags_agreed_and_post_deleted.subject_template')).exists? - ).to eq(false) + expect(Jobs::SendSystemMessage.jobs.size).to eq(0) + end + + it "should not send the flags_agreed_and_post_deleted message if defer_flags is true" do + expect(PostAction.flagged_posts_count).to eq(1) + PostDestroyer.new(moderator, second_post, defer_flags: true).destroy + expect(Jobs::SendSystemMessage.jobs.size).to eq(0) + expect(PostAction.flagged_posts_count).to eq(0) end it "should set the deleted_public_actions custom field" do diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index dac7ad784d5..e3e6490547e 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -243,6 +243,24 @@ describe PostsController do delete "/posts/destroy_many.json", params: { post_ids: [post1.id], reply_post_ids: [post1.id] } end end + + context "deleting flagged posts" do + let(:moderator) { Fabricate(:moderator) } + + before do + PostAction.act(moderator, post1, PostActionType.types[:off_topic]) + PostAction.act(moderator, post2, PostActionType.types[:off_topic]) + Jobs::SendSystemMessage.clear + end + + it "defers the posts" do + sign_in(moderator) + expect(PostAction.flagged_posts_count).to eq(2) + delete "/posts/destroy_many.json", params: { post_ids: [post1.id, post2.id], defer_flags: true } + expect(Jobs::SendSystemMessage.jobs.size).to eq(0) + expect(PostAction.flagged_posts_count).to eq(0) + end + end end end