From 52fdc1468d3148c1d4cd79e31fcd15ead3b33944 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Fri, 30 Aug 2019 10:27:52 -0300 Subject: [PATCH] Feature/Fix: Flagged posts user notifications (#8041) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * FIX: User should get notified when a post is deleted * FEATURE: Notify posters when restoring flagged posts * Fix typo Co-Authored-By: Régis Hanol * Improve tests --- app/models/reviewable_flagged_post.rb | 32 +++++++++---- config/locales/server.en.yml | 16 +++++++ lib/post_destroyer.rb | 52 +++++++++++---------- spec/models/reviewable_flagged_post_spec.rb | 31 ++++++++++++ 4 files changed, 98 insertions(+), 33 deletions(-) diff --git a/app/models/reviewable_flagged_post.rb b/app/models/reviewable_flagged_post.rb index cc67cacf5dd..1c6e2279aa6 100644 --- a/app/models/reviewable_flagged_post.rb +++ b/app/models/reviewable_flagged_post.rb @@ -196,6 +196,7 @@ class ReviewableFlaggedPost < Reviewable # Undo hide/silence if applicable if post&.hidden? + notify_poster(performed_by) post.unhide! UserSilencer.unsilence(post.user) if UserSilencer.was_silenced_for?(post) end @@ -208,30 +209,26 @@ class ReviewableFlaggedPost < Reviewable def perform_delete_and_ignore(performed_by, args) result = perform_ignore(performed_by, args) - PostDestroyer.new(performed_by, post).destroy + destroyer(performed_by, post).destroy result end def perform_delete_and_ignore_replies(performed_by, args) result = perform_ignore(performed_by, args) - - reply_ids = post.reply_ids(Guardian.new(performed_by), only_replies_to_single_post: false) - replies = Post.where(id: reply_ids.map { |r| r[:id] }) - PostDestroyer.new(performed_by, post).destroy - replies.each { |reply| PostDestroyer.new(performed_by, reply).destroy } + PostDestroyer.delete_with_replies(performed_by, post, self) result end def perform_delete_and_agree(performed_by, args) result = agree(performed_by, args) - PostDestroyer.new(performed_by, post).destroy + destroyer(performed_by, post).destroy result end def perform_delete_and_agree_replies(performed_by, args) result = agree(performed_by, args) - PostDestroyer.delete_with_replies(performed_by, post) + PostDestroyer.delete_with_replies(performed_by, post, self) result end @@ -281,6 +278,25 @@ protected end end +private + + def destroyer(performed_by, post) + PostDestroyer.new(performed_by, post, reviewable: self) + end + + def notify_poster(performed_by) + return unless performed_by.human? && performed_by.staff? + + Jobs.enqueue( + :send_system_message, + user_id: post.user_id, + message_type: :flags_disagreed, + message_options: { + flagged_post_raw_content: post.raw, + url: post.url + } + ) + end end # == Schema Information diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 190d51557ba..7fe0ae55193 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2524,7 +2524,23 @@ en: Multiple community members flagged this post before it was hidden. **Because this post has been hidden more than once, your post will now remain hidden until it is handled by a staff member.** For additional guidance, please refer to our [community guidelines](%{base_url}/guidelines). + + flags_disagreed: + title: "Flagged post restored by staff" + subject_template: "Flagged post restored by staff" + text_body_template: | + Hello, + This is an automated message from %{site_name} to let you know that [your post](%{base_url}%{url}) was restored. + + This post was flagged by the community and a staff member opted to restore it. + + [details="Click to expand restored post"] + ``` markdown + %{flagged_post_raw_content} + ``` + [/details] + flags_agreed_and_post_deleted: title: "Flagged post removed by staff" subject_template: "Flagged post removed by staff" diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index e0c5ff3991c..881ca857111 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -39,10 +39,10 @@ class PostDestroyer end end - def self.delete_with_replies(performed_by, post) + def self.delete_with_replies(performed_by, post, reviewable = nil) reply_ids = post.reply_ids(Guardian.new(performed_by), only_replies_to_single_post: false) replies = Post.where(id: reply_ids.map { |r| r[:id] }) - PostDestroyer.new(performed_by, post).destroy + PostDestroyer.new(performed_by, post, reviewable: reviewable).destroy replies.each { |reply| PostDestroyer.new(performed_by, reply).destroy } end @@ -156,12 +156,10 @@ class PostDestroyer TopicUser.update_post_action_cache(post_id: @post.id) DB.after_commit do - if reviewable = @post.reviewable_flag - if @opts[:defer_flags] - ignore(reviewable) - else - agree(reviewable) - end + if @opts[:reviewable] + notify_deletion(@opts[:reviewable]) + elsif reviewable = @post.reviewable_flag + @opts[:defer_flags] ? ignore(reviewable) : agree(reviewable) end end end @@ -262,23 +260,7 @@ class PostDestroyer end def agree(reviewable) - if @user.human? && @user.staff? && rs = reviewable.reviewable_scores.order('created_at DESC').first - Jobs.enqueue( - :send_system_message, - user_id: @post.user_id, - message_type: :flags_agreed_and_post_deleted, - message_options: { - flagged_post_raw_content: @post.raw, - url: @post.url, - flag_reason: I18n.t( - "flag_reasons.#{PostActionType.types[rs.reviewable_score_type]}", - locale: SiteSetting.default_locale, - base_path: Discourse.base_path - ) - } - ) - end - + notify_deletion(reviewable) result = reviewable.perform(@user, :agree_and_keep, post_was_deleted: true) reviewable.transition_to(result.transition_to, @user) end @@ -288,6 +270,26 @@ class PostDestroyer reviewable.transition_to(:ignored, @user) end + def notify_deletion(reviewable) + allowed_user = @user.human? && @user.staff? + return unless allowed_user && rs = reviewable.reviewable_scores.order('created_at DESC').first + + Jobs.enqueue( + :send_system_message, + user_id: @post.user_id, + message_type: :flags_agreed_and_post_deleted, + message_options: { + flagged_post_raw_content: @post.raw, + url: @post.url, + flag_reason: I18n.t( + "flag_reasons.#{PostActionType.types[rs.reviewable_score_type]}", + locale: SiteSetting.default_locale, + base_path: Discourse.base_path + ) + } + ) + end + def trash_user_actions UserAction.where(target_post_id: @post.id).each do |ua| row = { diff --git a/spec/models/reviewable_flagged_post_spec.rb b/spec/models/reviewable_flagged_post_spec.rb index 52413d16f56..1d1a7720bee 100644 --- a/spec/models/reviewable_flagged_post_spec.rb +++ b/spec/models/reviewable_flagged_post_spec.rb @@ -265,4 +265,35 @@ RSpec.describe ReviewableFlaggedPost, type: :model do end + describe "#perform_delete_and_agree" do + it "notifies the user about the flagged post deletion" do + reviewable = Fabricate(:reviewable_flagged_post) + reviewable.add_score( + moderator, PostActionType.types[:spam], + created_at: reviewable.created_at + ) + + reviewable.perform(moderator, :delete_and_agree) + + assert_pm_creation_enqueued(reviewable.post.user_id, "flags_agreed_and_post_deleted") + end + end + + describe "#perform_disagree" do + it "notifies the user about the flagged post being restored" do + reviewable = Fabricate(:reviewable_flagged_post) + reviewable.post.update(hidden: true, hidden_at: Time.zone.now, hidden_reason_id: PostActionType.types[:spam]) + + reviewable.perform(moderator, :disagree) + + assert_pm_creation_enqueued(reviewable.post.user_id, "flags_disagreed") + end + end + + def assert_pm_creation_enqueued(user_id, pm_type) + expect(Jobs::SendSystemMessage.jobs.length).to eq(1) + job = Jobs::SendSystemMessage.jobs[0] + expect(job["args"][0]["user_id"]).to eq(user_id) + expect(job["args"][0]["message_type"]).to eq(pm_type) + end end