FIX: Store Reviewable's force_review as a boolean. (#11219)

* FIX: Store Reviewable's force_review as a boolean.

Using the `force_review` flag raises the score to hit the minimum visibility threshold. This strategy turned out to be ineffective on sites with a high number of flags, where these values could rapidly fluctuate.

This change adds a `force_review` column on the reviewables table and modifies the `Reviewable#list_for` method to show these items when passing the `status: :pending` option, even if the score is not high enough. ReviewableQueuedPosts and ReviewableUsers are always created using this option.
This commit is contained in:
Roman Rizzi 2020-11-13 08:19:01 -03:00 committed by GitHub
parent bd0b558a89
commit f2bef7ea8f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 15 additions and 21 deletions

View File

@ -197,11 +197,6 @@ class Reviewable < ActiveRecord::Base
user_accuracy_bonus = ReviewableScore.user_accuracy_bonus(user) user_accuracy_bonus = ReviewableScore.user_accuracy_bonus(user)
sub_total = ReviewableScore.calculate_score(user, type_bonus, take_action_bonus) sub_total = ReviewableScore.calculate_score(user, type_bonus, take_action_bonus)
# We can force a reviewable to hit the threshold, for example with queued posts
if force_review && sub_total < Reviewable.min_score_for_priority
sub_total = Reviewable.min_score_for_priority
end
rs = reviewable_scores.new( rs = reviewable_scores.new(
user: user, user: user,
status: ReviewableScore.statuses[:pending], status: ReviewableScore.statuses[:pending],
@ -215,7 +210,7 @@ class Reviewable < ActiveRecord::Base
rs.reason = reason.to_s if reason rs.reason = reason.to_s if reason
rs.save! rs.save!
update(score: self.score + rs.score, latest_score: rs.created_at) update(score: self.score + rs.score, latest_score: rs.created_at, force_review: force_review)
topic.update(reviewable_score: topic.reviewable_score + rs.score) if topic topic.update(reviewable_score: topic.reviewable_score + rs.score) if topic
rs rs
@ -475,11 +470,8 @@ class Reviewable < ActiveRecord::Base
result = result.where("reviewables.created_at >= ?", from_date) if from_date result = result.where("reviewables.created_at >= ?", from_date) if from_date
result = result.where("reviewables.created_at <= ?", to_date) if to_date result = result.where("reviewables.created_at <= ?", to_date) if to_date
if min_score > 0 && status == :pending && type.nil? if min_score > 0 && status == :pending
result = result.where( result = result.where("reviewables.score >= ? OR reviewables.force_review", min_score)
"reviewables.score >= ? OR reviewables.type IN (?)",
min_score, [ReviewableQueuedPost.name, ReviewableUser.name]
)
elsif min_score > 0 elsif min_score > 0
result = result.where("reviewables.score >= ?", min_score) result = result.where("reviewables.score >= ?", min_score)
end end

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddReviewablesForceReview < ActiveRecord::Migration[6.0]
def change
add_column :reviewables, :force_review, :boolean, default: false, null: false
end
end

View File

@ -363,8 +363,6 @@ describe NewPostManager do
end end
it "calls custom enqueuing handlers" do it "calls custom enqueuing handlers" do
Reviewable.set_priorities(high: 20.5)
SiteSetting.reviewable_default_visibility = 'high'
SiteSetting.tagging_enabled = true SiteSetting.tagging_enabled = true
SiteSetting.min_trust_to_create_tag = 0 SiteSetting.min_trust_to_create_tag = 0
SiteSetting.min_trust_level_to_tag_topics = 0 SiteSetting.min_trust_level_to_tag_topics = 0
@ -384,7 +382,7 @@ describe NewPostManager do
expect(reviewable).to be_present expect(reviewable).to be_present
expect(reviewable.payload['title']).to eq('this is the title of the queued post') expect(reviewable.payload['title']).to eq('this is the title of the queued post')
expect(reviewable.reviewable_scores).to be_present expect(reviewable.reviewable_scores).to be_present
expect(reviewable.score).to eq(20.5) expect(reviewable.force_review).to eq(true)
expect(reviewable.reviewable_by_moderator?).to eq(true) expect(reviewable.reviewable_by_moderator?).to eq(true)
expect(reviewable.category).to be_present expect(reviewable.category).to be_present
expect(reviewable.payload['tags']).to eq(['hello', 'world']) expect(reviewable.payload['tags']).to eq(['hello', 'world'])

View File

@ -120,15 +120,12 @@ describe PostActionCreator do
expect(post.hidden?).to eq(false) expect(post.hidden?).to eq(false)
end end
it 'forces the review to surpass the minimum priority threshold' do it 'sets the force_review field' do
Reviewable.set_priorities(high: 40.0)
SiteSetting.reviewable_default_visibility = 'high'
result = PostActionCreator.create(user, post, :spam) result = PostActionCreator.create(user, post, :spam)
reviewable = result.reviewable reviewable = result.reviewable
reviewable_score = reviewable.reviewable_scores.find_by(user: user)
expect(reviewable_score.score).to eq(Reviewable.min_score_for_priority) expect(reviewable.force_review).to eq(true)
end end
end end

View File

@ -180,8 +180,8 @@ RSpec.describe Reviewable, type: :model do
before do before do
SiteSetting.reviewable_default_visibility = :high SiteSetting.reviewable_default_visibility = :high
Reviewable.set_priorities(high: 10) Reviewable.set_priorities(high: 10)
@queued_post = Fabricate(:reviewable_queued_post, score: 0, target: post) @queued_post = Fabricate(:reviewable_queued_post, score: 0, target: post, force_review: true)
@queued_user = Fabricate(:reviewable_user, score: 0) @queued_user = Fabricate(:reviewable_user, score: 0, force_review: true)
end end
it 'includes queued posts when searching for pending reviewables' do it 'includes queued posts when searching for pending reviewables' do