From 6b613e3076cca79e6efba1945f20ea04511dc940 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Wed, 21 Apr 2021 08:41:36 -0300 Subject: [PATCH] FEATURE: Review every post using the review queue. (#12734) * FEATURE: Review every post using the review queue. If the `review_every_post` setting is enabled, posts created and edited by regular uses are sent to the review queue so staff can review them. We'll skip PMs and posts created or edited by TL4 or staff users. Staff can choose to: - Approve the post (nothing happens) - Approve and restore the post (if deleted) - Approve and unhide the post (if hidden) - Reject and delete it - Reject and keep deleted (if deleted) - Reject and suspend the user - Reject and silence the user * Update config/locales/server.en.yml Co-authored-by: Robin Ward Co-authored-by: Robin Ward --- ...agged-post.js => reviewable-post-edits.js} | 0 .../components/reviewable-flagged-post.hbs | 8 +- .../components/reviewable-post-edits.hbs | 7 + .../templates/components/reviewable-post.hbs | 19 +++ app/models/reviewable.rb | 2 +- app/models/reviewable_post.rb | 111 +++++++++++++++ app/serializers/reviewable_post_serializer.rb | 22 +++ config/locales/client.en.yml | 2 + config/locales/server.en.yml | 12 ++ config/site_settings.yml | 3 + lib/post_creator.rb | 8 +- lib/post_revisor.rb | 4 + spec/components/post_creator_spec.rb | 26 ++++ spec/components/post_revisor_spec.rb | 27 ++++ spec/models/reviewable_post_spec.rb | 130 ++++++++++++++++++ 15 files changed, 372 insertions(+), 9 deletions(-) rename app/assets/javascripts/discourse/app/components/{reviewable-flagged-post.js => reviewable-post-edits.js} (100%) create mode 100644 app/assets/javascripts/discourse/app/templates/components/reviewable-post-edits.hbs create mode 100644 app/assets/javascripts/discourse/app/templates/components/reviewable-post.hbs create mode 100644 app/models/reviewable_post.rb create mode 100644 app/serializers/reviewable_post_serializer.rb create mode 100644 spec/models/reviewable_post_spec.rb diff --git a/app/assets/javascripts/discourse/app/components/reviewable-flagged-post.js b/app/assets/javascripts/discourse/app/components/reviewable-post-edits.js similarity index 100% rename from app/assets/javascripts/discourse/app/components/reviewable-flagged-post.js rename to app/assets/javascripts/discourse/app/components/reviewable-post-edits.js diff --git a/app/assets/javascripts/discourse/app/templates/components/reviewable-flagged-post.hbs b/app/assets/javascripts/discourse/app/templates/components/reviewable-flagged-post.hbs index f18feef0d5e..553f12d5880 100644 --- a/app/assets/javascripts/discourse/app/templates/components/reviewable-flagged-post.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/reviewable-flagged-post.hbs @@ -1,12 +1,6 @@
{{reviewable-topic-link reviewable=reviewable tagName=""}} - {{#if hasEdits}} - - {{d-icon "pencil-alt"}} - - {{/if}} + {{reviewable-post-edits reviewable=reviewable tagName=""}}
diff --git a/app/assets/javascripts/discourse/app/templates/components/reviewable-post-edits.hbs b/app/assets/javascripts/discourse/app/templates/components/reviewable-post-edits.hbs new file mode 100644 index 00000000000..e6f758133c3 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/components/reviewable-post-edits.hbs @@ -0,0 +1,7 @@ +{{#if hasEdits}} + + {{d-icon "pencil-alt"}} + +{{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/components/reviewable-post.hbs b/app/assets/javascripts/discourse/app/templates/components/reviewable-post.hbs new file mode 100644 index 00000000000..27310675a13 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/components/reviewable-post.hbs @@ -0,0 +1,19 @@ +
+ {{reviewable-topic-link reviewable=reviewable tagName=""}} + {{reviewable-post-edits reviewable=reviewable tagName=""}} +
+ +
+ {{reviewable-created-by user=reviewable.target_created_by tagName=""}} +
+ {{reviewable-post-header reviewable=reviewable createdBy=reviewable.target_created_by tagName=""}} +
+ {{#if reviewable.blank_post}} +

{{i18n "review.deleted_post"}}

+ {{else}} + {{html-safe reviewable.cooked}} + {{/if}} +
+ {{yield}} +
+
diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index 438fb51524b..cb964477f93 100644 --- a/app/models/reviewable.rb +++ b/app/models/reviewable.rb @@ -95,7 +95,7 @@ class Reviewable < ActiveRecord::Base end def self.types - %w[ReviewableFlaggedPost ReviewableQueuedPost ReviewableUser] + %w[ReviewableFlaggedPost ReviewableQueuedPost ReviewableUser ReviewablePost] end def self.custom_filters diff --git a/app/models/reviewable_post.rb b/app/models/reviewable_post.rb new file mode 100644 index 00000000000..8dbb06596b0 --- /dev/null +++ b/app/models/reviewable_post.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +class ReviewablePost < Reviewable + def self.action_aliases + { reject_and_silence: :reject_and_suspend } + end + + def self.queue_for_review_if_possible(post, created_or_edited_by) + return unless SiteSetting.review_every_post + return if post.post_type != Post.types[:regular] || post.topic.private_message? + return if Reviewable.where(target: post, status: Reviewable.statuses[:pending]).exists? + return if created_or_edited_by.bot? || created_or_edited_by.staff? || created_or_edited_by.has_trust_level?(TrustLevel[4]) + system_user = Discourse.system_user + + needs_review!( + target: post, + topic: post.topic, + created_by: system_user, + reviewable_by_moderator: true, + potential_spam: false + ).tap do |reviewable| + reviewable.add_score( + system_user, + ReviewableScore.types[:needs_approval], + force_review: true + ) + end + end + + def build_actions(actions, guardian, args) + return unless pending? + + if post.trashed? && guardian.can_recover_post?(post) + build_action(actions, :approve_and_restore, icon: 'check') + elsif post.hidden? + build_action(actions, :approve_and_unhide, icon: 'check') + else + build_action(actions, :approve, icon: 'check') + end + + reject = actions.add_bundle( + "#{id}-reject", icon: 'times', label: 'reviewables.actions.reject.bundle_title' + ) + + if post.trashed? + build_action(actions, :reject_and_keep_deleted, icon: 'trash-alt', bundle: reject) + elsif guardian.can_delete_post_or_topic?(post) + build_action(actions, :reject_and_delete, icon: 'trash-alt', bundle: reject) + end + + if guardian.can_suspend?(target_created_by) + build_action(actions, :reject_and_suspend, icon: 'ban', bundle: reject, client_action: 'suspend') + build_action(actions, :reject_and_silence, icon: 'microphone-slash', bundle: reject, client_action: 'silence') + end + end + + def perform_approve(performed_by, _args) + successful_transition :approved, recalculate_score: false + end + + def perform_reject_and_keep_deleted(performed_by, _args) + successful_transition :rejected, recalculate_score: false + end + + def perform_approve_and_restore(performed_by, _args) + PostDestroyer.new(performed_by, post).recover + + successful_transition :approved, recalculate_score: false + end + + def perform_approve_and_unhide(performed_by, _args) + post.unhide! + + successful_transition :approved, recalculate_score: false + end + + def perform_reject_and_delete(performed_by, _args) + PostDestroyer.new(performed_by, post, reviewable: self).destroy + + successful_transition :rejected, recalculate_score: false + end + + def perform_reject_and_suspend(performed_by, _args) + successful_transition :rejected, recalculate_score: false + end + + private + + def post + @post ||= (target || Post.with_deleted.find_by(id: target_id)) + end + + def build_action(actions, id, icon:, button_class: nil, bundle: nil, client_action: nil, confirm: false) + actions.add(id, bundle: bundle) do |action| + prefix = "reviewables.actions.#{id}" + action.icon = icon + action.button_class = button_class + action.label = "#{prefix}.title" + action.description = "#{prefix}.description" + action.client_action = client_action + action.confirm_message = "#{prefix}.confirm" if confirm + end + end + + def successful_transition(to_state, recalculate_score: true) + create_result(:success, to_state) do |result| + result.recalculate_score = recalculate_score + result.update_flag_stats = { status: to_state, user_ids: [created_by_id] } + end + end +end diff --git a/app/serializers/reviewable_post_serializer.rb b/app/serializers/reviewable_post_serializer.rb new file mode 100644 index 00000000000..9e38edf50d3 --- /dev/null +++ b/app/serializers/reviewable_post_serializer.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class ReviewablePostSerializer < ReviewableSerializer + target_attributes :cooked, :raw, :reply_count, :reply_to_post_number + attributes :blank_post, :post_updated_at, :post_version + + def post_version + object.target&.version + end + + def post_updated_at + object.target&.updated_at + end + + def blank_post + true + end + + def include_blank_post? + object.target.blank? + end +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index be41af4bfa6..683958ad32b 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -569,6 +569,8 @@ en: title: "Queued Post" reviewable_user: title: "User" + reviewable_post: + title: "Post" approval: title: "Post Needs Approval" description: "We've received your new post but it needs to be approved by a moderator before it will appear. Please be patient." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 32878142791..27bc4331327 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1585,6 +1585,7 @@ en: must_approve_users: "Staff must approve all new user accounts before they are allowed to access the site." invite_code: "User must type this code to be allowed account registration, ignored when empty (case-insensitive)" approve_suspect_users: "Add suspicious users to the review queue. Suspicious users have entered a bio/website but have no reading activity." + review_every_post: "All posts must be reviewed. WARNING! NOT RECOMMENDED FOR BUSY SITES." pending_users_reminder_delay: "Notify moderators if new users have been waiting for approval for longer than this many hours. Set to -1 to disable notifications." persistent_sessions: "Users will remain logged in when the web browser is closed" maximum_session_age: "User will remain logged in for n hours since last visit" @@ -5011,6 +5012,17 @@ en: description: "The user will be deleted, and we'll block their IP and email address." reject: title: "Reject" + bundle_title: "Reject..." + reject_and_suspend: + title: "Reject and Suspend user" + reject_and_silence: + title: "Reject and Silence user" + reject_and_delete: + title: "Reject and Delete the post" + reject_and_keep_deleted: + title: "Keep post deleted" + approve_and_restore: + title: "Approve and Restore post" delete_user: title: "Delete User" confirm: "Are you sure you want to delete that user? This will remove all of their posts and block their email and IP address." diff --git a/config/site_settings.yml b/config/site_settings.yml index ec707e762b5..b9703b5f106 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1009,6 +1009,9 @@ posting: show_published_pages_login_required: default: false skip_auto_delete_reply_likes: 5 + review_every_post: + default: false + email: email_time_window_mins: diff --git a/lib/post_creator.rb b/lib/post_creator.rb index e2d7de95336..bb28f3cdd24 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -239,7 +239,13 @@ class PostCreator auto_close end - handle_spam if !opts[:import_mode] && (@post || @spam) + if !opts[:import_mode] + handle_spam if (@spam || @post) + + if !@spam && @post && errors.blank? + ReviewablePost.queue_for_review_if_possible(@post, @user) + end + end @post end diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index 6d4e6e9ce6d..26c13b92191 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -235,6 +235,10 @@ class PostRevisor TopicLink.extract_from(@post) + if should_create_new_version? + ReviewablePost.queue_for_review_if_possible(@post, @editor) + end + successfully_saved_post_and_topic end diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index a5399551a94..0d07c5aea25 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -703,6 +703,13 @@ describe PostCreator do creator.create end + it 'does not create a reviewable post if the review_every_post setting is enabled' do + SiteSetting.review_every_post = true + GroupMessage.stubs(:create) + + expect { creator.create }.to change(ReviewablePost, :count).by(0) + end + end # more integration testing ... maximise our testing @@ -1710,4 +1717,23 @@ describe PostCreator do ) end end + + context 'queue for review' do + before { SiteSetting.review_every_post = true } + + it 'created a reviewable post after creating the post' do + title = "This is a valid title" + raw = "This is a really awesome post" + + post_creator = PostCreator.new(user, title: title, raw: raw) + + expect { post_creator.create }.to change(ReviewablePost, :count).by(1) + end + + it 'does not create a reviewable post if the post is not valid' do + post_creator = PostCreator.new(user, title: '', raw: '') + + expect { post_creator.create }.to change(ReviewablePost, :count).by(0) + end + end end diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index 118d7911977..5d1fe8bb4a4 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -1069,4 +1069,31 @@ describe PostRevisor do end end end + + context 'when the review_every_post setting is enabled' do + let(:post) { Fabricate(:post, post_args) } + let(:revisor) { PostRevisor.new(post) } + + before { SiteSetting.review_every_post = true } + + it 'queues the post when a regular user edits it' do + expect { + revisor.revise!(post.user, { raw: 'updated body' }, revised_at: post.updated_at + 10.minutes) + }.to change(ReviewablePost, :count).by(1) + end + + it 'does nothing when a staff member edits a post' do + admin = Fabricate(:admin) + + expect { revisor.revise!(admin, { raw: 'updated body' }) }.to change(ReviewablePost, :count).by(0) + end + + it 'skips ninja edits' do + SiteSetting.editing_grace_period = 1.minute + + expect { + revisor.revise!(post.user, { raw: 'updated body' }, revised_at: post.updated_at + 10.seconds) + }.to change(ReviewablePost, :count).by(0) + end + end end diff --git a/spec/models/reviewable_post_spec.rb b/spec/models/reviewable_post_spec.rb new file mode 100644 index 00000000000..26f0459e19f --- /dev/null +++ b/spec/models/reviewable_post_spec.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ReviewablePost do + fab!(:admin) { Fabricate(:admin) } + + describe '#build_actions' do + let(:post) { Fabricate.build(:post) } + let(:reviewable) { ReviewablePost.new(target: post) } + let(:guardian) { Guardian.new } + + it 'Does not return available actions when the reviewable is no longer pending' do + available_actions = (Reviewable.statuses.keys - [:pending]).reduce([]) do |actions, status| + reviewable.status = Reviewable.statuses[status] + + actions.concat reviewable_actions(guardian).to_a + end + + expect(available_actions).to be_empty + end + + it 'only shows the approve post action if users cannot delete the post' do + expect(reviewable_actions(guardian).has?(:approve)).to eq(true) + expect(reviewable_actions(guardian).has?(:reject_and_delete)).to eq(false) + end + + it 'includes the reject and delete action if the user is allowed' do + expect(reviewable_actions(Guardian.new(admin)).has?(:reject_and_delete)).to eq(true) + end + + it 'includes the approve post and unhide action if the post is hidden' do + post.hidden = true + + actions = reviewable_actions(guardian) + + expect(actions.has?(:approve_and_unhide)).to eq(true) + end + + it 'includes the reject post and keep deleted action is the post is deleted' do + post.deleted_at = 1.day.ago + + actions = reviewable_actions(guardian) + + expect(actions.has?(:approve_and_restore)).to eq(false) + expect(actions.has?(:reject_and_keep_deleted)).to eq(true) + end + + it 'includes an option to approve and restore the post if the user is allowed' do + + post.deleted_at = 1.day.ago + + actions = reviewable_actions(Guardian.new(admin)) + + expect(actions.has?(:approve_and_restore)).to eq(false) + end + + def reviewable_actions(guardian) + actions = Reviewable::Actions.new(reviewable, guardian, {}) + reviewable.build_actions(actions, guardian, {}) + + actions + end + end + + describe 'Performing actions' do + let(:post) { Fabricate(:post) } + let(:reviewable) { ReviewablePost.needs_review!(target: post, created_by: admin) } + + before { reviewable.created_new! } + + describe '#perform_approve' do + it 'transitions to the approved state' do + result = reviewable.perform admin, :approve + + expect(result.transition_to).to eq :approved + end + end + + describe '#perform_reject_and_suspend' do + it 'transitions to the rejected state' do + result = reviewable.perform admin, :reject_and_suspend + + expect(result.transition_to).to eq :rejected + end + end + + describe '#perform_reject_and_keep_deleted' do + it 'transitions to the rejected state and keep the post deleted' do + post.trash! + + result = reviewable.perform admin, :reject_and_keep_deleted + + expect(result.transition_to).to eq :rejected + expect(Post.where(id: post.id).exists?).to eq(false) + end + end + + describe '#perform_approve_and_restore' do + it 'transitions to the approved state and restores the post' do + post.trash! + + result = reviewable.reload.perform admin, :approve_and_restore + + expect(result.transition_to).to eq :approved + expect(Post.where(id: post.id).exists?).to eq(true) + end + end + + describe '#perform_approve_and_unhide' do + it 'transitions to the approved state and unhides the post' do + post.update!(hidden: true) + + result = reviewable.reload.perform admin, :approve_and_unhide + + expect(result.transition_to).to eq :approved + expect(post.reload.hidden).to eq(false) + end + end + + describe '#perform_reject_and_delete' do + it 'transitions to the rejected state and deletes the post' do + result = reviewable.perform admin, :reject_and_delete + + expect(result.transition_to).to eq :rejected + expect(Post.where(id: post.id).exists?).to eq(false) + end + end + end +end