From 851ef14096f0b6f3c6726151099483907f2c0805 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Wed, 28 Nov 2018 10:21:11 +0530 Subject: [PATCH] Revert "FIX: do not agree flags by default when deleting posts" This reverts commit cb6fc8057b7f66e3f0b19b0ec62f23a6823e1d2f. --- app/controllers/admin/flags_controller.rb | 7 ++-- lib/post_destroyer.rb | 13 ++----- spec/components/post_destroyer_spec.rb | 43 +++++++++++------------ 3 files changed, 27 insertions(+), 36 deletions(-) diff --git a/app/controllers/admin/flags_controller.rb b/app/controllers/admin/flags_controller.rb index 5d1be837266..a94eae4352f 100644 --- a/app/controllers/admin/flags_controller.rb +++ b/app/controllers/admin/flags_controller.rb @@ -86,7 +86,8 @@ class Admin::FlagsController < Admin::AdminController restore_post = params[:action_on_post] == "restore" if delete_post - destroy_post(post, agree_flags: true) + # PostDestroy calls PostAction.agree_flags! + destroy_post(post) elsif restore_post PostAction.agree_flags!(post, current_user, delete_post) PostDestroyer.new(current_user, post).recover @@ -137,12 +138,12 @@ class Admin::FlagsController < Admin::AdminController private - def destroy_post(post, agree_flags: false) + def destroy_post(post) if post.is_first_post? topic = Topic.find_by(id: post.topic_id) guardian.ensure_can_delete!(topic) if topic.present? end - PostDestroyer.new(current_user, post, agree_flags: agree_flags).destroy + PostDestroyer.new(current_user, post).destroy end end diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 4398d06ec89..7336e981b64 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -147,11 +147,7 @@ class PostDestroyer update_user_counts TopicUser.update_post_action_cache(post_id: @post.id) DB.after_commit do - if @opts[:agree_flags] - agree_with_flags - else - defer_flags - end + agree_with_flags end end @@ -229,7 +225,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,11 +237,8 @@ class PostDestroyer } ) end - PostAction.agree_flags!(@post, @user, delete_post: true) - end - def defer_flags - PostAction.defer_flags!(@post, @user, delete_post: true) + PostAction.agree_flags!(@post, @user, delete_post: true) end def trash_user_actions diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 83fbc3b239b..7a91c3bef3f 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -632,13 +632,13 @@ describe PostDestroyer do let!(:flag) { PostAction.act(moderator, second_post, PostActionType.types[:off_topic]) } before do - Jobs::SendSystemMessage.clear + SiteSetting.queue_jobs = false end it "should delete public post actions and agree with flags" do second_post.expects(:update_flagged_posts_count) - PostDestroyer.new(moderator, second_post, agree_flags: true).destroy + PostDestroyer.new(moderator, second_post).destroy expect(PostAction.find_by(id: bookmark.id)).to eq(nil) @@ -650,39 +650,36 @@ describe PostDestroyer do expect(second_post.bookmark_count).to eq(0) expect(second_post.off_topic_count).to eq(1) - expect(Jobs::SendSystemMessage.jobs.size).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')) end it "should not send the flags_agreed_and_post_deleted message if it was deleted by system" do - expect(PostAction.flagged_posts_count).to eq(1) - PostDestroyer.new(Discourse.system_user, second_post, agree_flags: true).destroy - expect(Jobs::SendSystemMessage.jobs.size).to eq(0) - expect(PostAction.flagged_posts_count).to eq(0) + second_post.expects(:update_flagged_posts_count) + 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) 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 - expect(PostAction.flagged_posts_count).to eq(1) - PostDestroyer.new(second_post.user, second_post, agree_flags: true).destroy - expect(Jobs::SendSystemMessage.jobs.size).to eq(0) - expect(PostAction.flagged_posts_count).to eq(0) + second_post.expects(:update_flagged_posts_count) + 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) end it "should not send the flags_agreed_and_post_deleted message if flags were deferred" do - expect(PostAction.flagged_posts_count).to eq(1) + second_post.expects(:update_flagged_posts_count) PostAction.defer_flags!(second_post, moderator) second_post.reload - expect(PostAction.flagged_posts_count).to eq(0) - - PostDestroyer.new(moderator, second_post, agree_flags: true).destroy - expect(Jobs::SendSystemMessage.jobs.size).to eq(0) - end - - it "should not send the flags_agreed_and_post_deleted message if agree_flags is false" do - expect(PostAction.flagged_posts_count).to eq(1) - PostDestroyer.new(moderator, second_post, agree_flags: false).destroy - expect(Jobs::SendSystemMessage.jobs.size).to eq(0) - 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) end it "should set the deleted_public_actions custom field" do