FEATURE: Display the reason for many reviewable items

Queued Posts and Users will now display a reason why they are in the
review queue.
This commit is contained in:
Robin Ward 2019-04-11 11:11:35 -04:00
parent 7143572e0c
commit 331a809738
13 changed files with 105 additions and 13 deletions

View File

@ -19,7 +19,7 @@
</td>
<td class="reviewable-score-spacer">
{{d-icon "angle-double-right"}}
{{d-icon "angle-double-right"}}
</td>
<td class='reviewed-by'>
@ -38,12 +38,19 @@
</td>
<td>
{{#if rs.reviewed_by}}
{{format-date rs.reviewed_at format="tiny"}}
{{format-date rs.reviewed_at format="tiny"}}
{{/if}}
</td>
</tr>
{{#if rs.reason}}
<tr>
<td colspan='7'>
<div class='reviewable-score-reason'>{{{rs.reason}}}</div>
</td>
</tr>
{{/if}}
{{#if rs.reviewable_conversation}}
<tr>
<td colspan='7'>

View File

@ -296,6 +296,12 @@
}
}
.reviewable-score-reason {
margin: 0.5em 0;
max-width: $topic-body-width;
margin-bottom: 0.5em;
}
.reviewable-conversation {
margin: 0.5em 0;

View File

@ -2,6 +2,12 @@ class Jobs::CreateUserReviewable < Jobs::Base
def execute(args)
raise Discourse::InvalidParameters unless args[:user_id].present?
reason = nil
reason ||= :must_approve_users if SiteSetting.must_approve_users?
reason ||= :invite_only if SiteSetting.invite_only?
return unless reason
if user = User.find_by(id: args[:user_id])
return if user.approved?
@ -18,6 +24,7 @@ class Jobs::CreateUserReviewable < Jobs::Base
reviewable.add_score(
Discourse.system_user,
ReviewableScore.types[:needs_approval],
reason: reason,
force_review: true
)
end

View File

@ -115,6 +115,7 @@ class Reviewable < ActiveRecord::Base
def add_score(
user,
reviewable_score_type,
reason: nil,
created_at: nil,
take_action: false,
meta_topic_id: nil,
@ -130,7 +131,7 @@ class Reviewable < ActiveRecord::Base
sub_total = SiteSetting.min_score_default_visibility
end
rs = reviewable_scores.create!(
rs = reviewable_scores.new(
user: user,
status: ReviewableScore.statuses[:pending],
reviewable_score_type: reviewable_score_type,
@ -139,6 +140,8 @@ class Reviewable < ActiveRecord::Base
take_action_bonus: take_action_bonus,
created_at: created_at || Time.zone.now
)
rs.reason = reason.to_s if reason
rs.save!
update(score: self.score + rs.score, latest_score: rs.created_at)
topic.update(reviewable_score: topic.reviewable_score + rs.score) if topic

View File

@ -2,7 +2,7 @@ require_dependency 'reviewable_score_type_serializer'
class ReviewableScoreSerializer < ApplicationSerializer
attributes :id, :score, :agree_stats, :status, :created_at, :reviewed_at
attributes :id, :score, :agree_stats, :status, :reason, :created_at, :reviewed_at
has_one :user, serializer: BasicUserSerializer, root: 'users'
has_one :score_type, serializer: ReviewableScoreTypeSerializer
has_one :reviewable_conversation, serializer: ReviewableConversationSerializer
@ -16,4 +16,28 @@ class ReviewableScoreSerializer < ApplicationSerializer
}
end
def reason
return unless object.reason
if text = I18n.t("reviewables.reasons.#{object.reason}", default: nil)
# Create a convenient link to any site settings if the user is staff
settings_url = "#{Discourse.base_uri}/admin/site_settings/category/all_results?filter="
text.gsub!(/`[a-z_]+`/) do |m|
if scope.is_staff?
setting = m[1..-2]
"<a href=\"#{settings_url}#{setting}\">#{setting.gsub('_', ' ')}</a>"
else
m.gsub('_', ' ')
end
end
end
text
end
def include_reason?
reason.present?
end
end

View File

@ -429,6 +429,7 @@ en:
conversation:
view_full: "view full conversation"
scores:
score: "Score"
date: "Date"
type: "Type"
status: "Status"

View File

@ -4385,6 +4385,18 @@ en:
reviewables:
missing_version: "You must supply a version parameter"
conflict: "There was an update conflict preventing you from doing that."
reasons:
post_count: "The poster has not met the `approve_post_count` requirements."
trust_level: "The poster has not met the `approve_unless_trust_level` requirement."
new_topics_unless_trust_level: "The poster has not met the `approve_new_topics_unless_trust_level` requirement."
fast_typer: "The poster typed in their post suspiciously fast. See: `min_first_post_typing_time`."
auto_silence_regexp: "The post matched the `auto_silence_first_post_regex` setting."
watched_word: "The post included a Watched Word."
staged: "The poster was staged and `approve_unless_staged` was set."
category: "The category of the post requires approval."
must_approve_users: "The user must be approved because `must_approve_users` is enabled."
invite_only: "The user must be approved because `invite_only` is enabled."
actions:
agree:
title: "Agree..."

View File

@ -0,0 +1,5 @@
class AddReasonToReviewableScores < ActiveRecord::Migration[5.2]
def change
add_column :reviewable_scores, :reason, :string
end
end

View File

@ -198,12 +198,9 @@ class NewPostManager
def enqueue(reason = nil)
result = NewPostResult.new(:enqueued)
payload = { raw: @args[:raw], tags: @args[:tags] }
payload[:reason] = reason.to_s if reason
reviewable = ReviewableQueuedPost.new(
created_by: @user,
payload: payload,
payload: { raw: @args[:raw], tags: @args[:tags] },
topic_id: @args[:topic_id],
reviewable_by_moderator: true
)
@ -225,6 +222,7 @@ class NewPostManager
reviewable.add_score(
Discourse.system_user,
ReviewableScore.types[:needs_approval],
reason: reason,
force_review: true
)
else

View File

@ -6,14 +6,40 @@ describe Jobs::CreateUserReviewable do
let(:user) { Fabricate(:user) }
it "creates the reviewable" do
SiteSetting.must_approve_users = true
described_class.new.execute(user_id: user.id)
reviewable = Reviewable.find_by(target: user)
expect(reviewable).to be_present
expect(reviewable.pending?).to eq(true)
expect(reviewable.reviewable_scores).to be_present
expect(reviewable.payload['username']).to eq(user.username)
expect(reviewable.payload['name']).to eq(user.name)
expect(reviewable.payload['email']).to eq(user.email)
end
describe "reasons" do
it "does nothing if there's no reason" do
described_class.new.execute(user_id: user.id)
expect(Reviewable.find_by(target: user)).to be_blank
end
it "adds must_approve_users if enabled" do
SiteSetting.must_approve_users = true
described_class.new.execute(user_id: user.id)
reviewable = Reviewable.find_by(target: user)
score = reviewable.reviewable_scores.first
expect(score).to be_present
expect(score.reason).to eq('must_approve_users')
end
it "adds invite_only if enabled" do
SiteSetting.invite_only = true
described_class.new.execute(user_id: user.id)
reviewable = Reviewable.find_by(target: user)
score = reviewable.reviewable_scores.first
expect(score).to be_present
expect(score.reason).to eq('invite_only')
end
end
end

View File

@ -38,6 +38,7 @@ RSpec.describe ReviewableUser, type: :model do
context "when a user is deleted" do
it "should reject the reviewable" do
SiteSetting.must_approve_users = true
Jobs::CreateUserReviewable.new.execute(user_id: user.id)
reviewable = Reviewable.find_by(target: user)
expect(reviewable.pending?).to eq(true)

View File

@ -801,7 +801,7 @@ describe PostsController do
expect(user).to be_silenced
rp = ReviewableQueuedPost.find_by(created_by: user)
expect(rp.payload['reason']).to eq('fast_typer')
expect(rp.reviewable_scores.first.reason).to eq('fast_typer')
mod = Fabricate(:moderator)
rp.perform(mod, :approve)
@ -851,7 +851,8 @@ describe PostsController do
expect(parsed["action"]).to eq("enqueued")
reviewable = ReviewableQueuedPost.find_by(created_by: user)
expect(reviewable.payload['reason']).to eq('auto_silence_regex')
score = reviewable.reviewable_scores.first
expect(score.reason).to eq('auto_silence_regex')
user.reload
expect(user).to be_silenced

View File

@ -6,6 +6,7 @@ describe ReviewableUserSerializer do
let(:admin) { Fabricate(:admin) }
it "includes the user fields for review" do
SiteSetting.must_approve_users = true
Jobs::CreateUserReviewable.new.execute(user_id: user.id)
reviewable = Reviewable.find_by(target: user)