mirror of
https://github.com/discourse/discourse.git
synced 2024-11-22 15:25:35 +08:00
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 <robin.ward@gmail.com> Co-authored-by: Robin Ward <robin.ward@gmail.com>
This commit is contained in:
parent
45ccadeeeb
commit
6b613e3076
|
@ -1,12 +1,6 @@
|
||||||
<div class="flagged-post-header">
|
<div class="flagged-post-header">
|
||||||
{{reviewable-topic-link reviewable=reviewable tagName=""}}
|
{{reviewable-topic-link reviewable=reviewable tagName=""}}
|
||||||
{{#if hasEdits}}
|
{{reviewable-post-edits reviewable=reviewable tagName=""}}
|
||||||
<a href {{action "showEditHistory"}}
|
|
||||||
class="has-edits {{historyClass}}"
|
|
||||||
title={{i18n "post.last_edited_on" dateTime=editedDate}}>
|
|
||||||
{{d-icon "pencil-alt"}}
|
|
||||||
</a>
|
|
||||||
{{/if}}
|
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div class="post-contents-wrapper">
|
<div class="post-contents-wrapper">
|
||||||
|
|
|
@ -0,0 +1,7 @@
|
||||||
|
{{#if hasEdits}}
|
||||||
|
<a href {{action "showEditHistory"}}
|
||||||
|
class="has-edits {{historyClass}}"
|
||||||
|
title={{i18n "post.last_edited_on" dateTime=editedDate}}>
|
||||||
|
{{d-icon "pencil-alt"}}
|
||||||
|
</a>
|
||||||
|
{{/if}}
|
|
@ -0,0 +1,19 @@
|
||||||
|
<div class="flagged-post-header">
|
||||||
|
{{reviewable-topic-link reviewable=reviewable tagName=""}}
|
||||||
|
{{reviewable-post-edits reviewable=reviewable tagName=""}}
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<div class="post-contents-wrapper">
|
||||||
|
{{reviewable-created-by user=reviewable.target_created_by tagName=""}}
|
||||||
|
<div class="post-contents">
|
||||||
|
{{reviewable-post-header reviewable=reviewable createdBy=reviewable.target_created_by tagName=""}}
|
||||||
|
<div class="post-body">
|
||||||
|
{{#if reviewable.blank_post}}
|
||||||
|
<p>{{i18n "review.deleted_post"}}</p>
|
||||||
|
{{else}}
|
||||||
|
{{html-safe reviewable.cooked}}
|
||||||
|
{{/if}}
|
||||||
|
</div>
|
||||||
|
{{yield}}
|
||||||
|
</div>
|
||||||
|
</div>
|
|
@ -95,7 +95,7 @@ class Reviewable < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.types
|
def self.types
|
||||||
%w[ReviewableFlaggedPost ReviewableQueuedPost ReviewableUser]
|
%w[ReviewableFlaggedPost ReviewableQueuedPost ReviewableUser ReviewablePost]
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.custom_filters
|
def self.custom_filters
|
||||||
|
|
111
app/models/reviewable_post.rb
Normal file
111
app/models/reviewable_post.rb
Normal file
|
@ -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
|
22
app/serializers/reviewable_post_serializer.rb
Normal file
22
app/serializers/reviewable_post_serializer.rb
Normal file
|
@ -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
|
|
@ -569,6 +569,8 @@ en:
|
||||||
title: "Queued Post"
|
title: "Queued Post"
|
||||||
reviewable_user:
|
reviewable_user:
|
||||||
title: "User"
|
title: "User"
|
||||||
|
reviewable_post:
|
||||||
|
title: "Post"
|
||||||
approval:
|
approval:
|
||||||
title: "Post Needs 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."
|
description: "We've received your new post but it needs to be approved by a moderator before it will appear. Please be patient."
|
||||||
|
|
|
@ -1585,6 +1585,7 @@ en:
|
||||||
must_approve_users: "Staff must approve all new user accounts before they are allowed to access the site."
|
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)"
|
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."
|
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."
|
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"
|
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"
|
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."
|
description: "The user will be deleted, and we'll block their IP and email address."
|
||||||
reject:
|
reject:
|
||||||
title: "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:
|
delete_user:
|
||||||
title: "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."
|
confirm: "Are you sure you want to delete that user? This will remove all of their posts and block their email and IP address."
|
||||||
|
|
|
@ -1009,6 +1009,9 @@ posting:
|
||||||
show_published_pages_login_required:
|
show_published_pages_login_required:
|
||||||
default: false
|
default: false
|
||||||
skip_auto_delete_reply_likes: 5
|
skip_auto_delete_reply_likes: 5
|
||||||
|
review_every_post:
|
||||||
|
default: false
|
||||||
|
|
||||||
|
|
||||||
email:
|
email:
|
||||||
email_time_window_mins:
|
email_time_window_mins:
|
||||||
|
|
|
@ -239,7 +239,13 @@ class PostCreator
|
||||||
auto_close
|
auto_close
|
||||||
end
|
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
|
@post
|
||||||
end
|
end
|
||||||
|
|
|
@ -235,6 +235,10 @@ class PostRevisor
|
||||||
|
|
||||||
TopicLink.extract_from(@post)
|
TopicLink.extract_from(@post)
|
||||||
|
|
||||||
|
if should_create_new_version?
|
||||||
|
ReviewablePost.queue_for_review_if_possible(@post, @editor)
|
||||||
|
end
|
||||||
|
|
||||||
successfully_saved_post_and_topic
|
successfully_saved_post_and_topic
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -703,6 +703,13 @@ describe PostCreator do
|
||||||
creator.create
|
creator.create
|
||||||
end
|
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
|
end
|
||||||
|
|
||||||
# more integration testing ... maximise our testing
|
# more integration testing ... maximise our testing
|
||||||
|
@ -1710,4 +1717,23 @@ describe PostCreator do
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
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
|
end
|
||||||
|
|
|
@ -1069,4 +1069,31 @@ describe PostRevisor do
|
||||||
end
|
end
|
||||||
end
|
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
|
end
|
||||||
|
|
130
spec/models/reviewable_post_spec.rb
Normal file
130
spec/models/reviewable_post_spec.rb
Normal file
|
@ -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
|
Loading…
Reference in New Issue
Block a user