FEATURE: A low priority filter for the review queue. (#12822)

This filter hides reviewables with a score lower than the "reviewable_low_priority_threshold" setting. We only use reviewables that already met this threshold to calculate the Medium and High priority filters.

app/assets/javascripts/discourse/app/controllers/review-index.js

@@ -49,7 +49,7 @@ export default Controller.extend({
 
   @discourseComputed
   priorities() {
-    return ["low", "medium", "high"].map((priority) => {
+    return ["any", "low", "medium", "high"].map((priority) => {
       return {
         id: priority,
         name: I18n.t(`review.filters.priority.${priority}`),

app/jobs/scheduled/reviewable_priorities.rb

@@ -16,13 +16,16 @@ class Jobs::ReviewablePriorities < ::Jobs::Scheduled
   def execute(args)
     return unless Reviewable.where('score > 0').count >= self.class.min_reviewables
 
-    res = DB.query_single(<<~SQL, target_count: self.class.target_count)
+    min_priority_threshold = SiteSetting.reviewable_low_priority_threshold
+
+    res = DB.query_single(<<~SQL, target_count: self.class.target_count, min_priority: min_priority_threshold)
       SELECT COALESCE(PERCENTILE_DISC(0.5) WITHIN GROUP (ORDER BY score), 0.0) AS medium,
         COALESCE(PERCENTILE_DISC(0.85) WITHIN GROUP (ORDER BY score), 0.0) AS high
       FROM (
         SELECT r.score
         FROM reviewables AS r
         INNER JOIN reviewable_scores AS rs ON rs.reviewable_id = r.id
+        WHERE r.score >= :min_priority
         GROUP BY r.id
         HAVING COUNT(*) >= :target_count
       ) AS x
@@ -32,6 +35,10 @@ class Jobs::ReviewablePriorities < ::Jobs::Scheduled
 
     medium, high = res
 
-    Reviewable.set_priorities(medium: medium, high: high)
+    Reviewable.set_priorities(
+      low: min_priority_threshold,
+      medium: medium,
+      high: high
+    )
   end
 end

app/models/reviewable.rb

@@ -444,8 +444,6 @@ class Reviewable < ActiveRecord::Base
     to_date: nil,
     additional_filters: {}
   )
-    min_score = Reviewable.min_score_for_priority(priority)
-
     order = case sort_order
             when 'score_asc'
               'reviewables.score ASC, reviewables.created_at DESC'
@@ -488,6 +486,8 @@ class Reviewable < ActiveRecord::Base
       )
     end
 
+    min_score = Reviewable.min_score_for_priority(priority)
+
     if min_score > 0 && status == :pending
       result = result.where("reviewables.score >= ? OR reviewables.force_review", min_score)
     elsif min_score > 0

config/initializers/014-track-setting-changes.rb

@@ -48,4 +48,9 @@ DiscourseEvent.on(:site_setting_changed) do |name, old_value, new_value|
   if SiteSetting::WATCHED_SETTINGS.include?(name)
     SiteSetting.reset_cached_settings!
   end
+
+  # Make sure medium and high priority thresholds were calculated.
+  if name == :reviewable_low_priority_threshold && Reviewable.min_score_for_priority(:medium) > 0
+    Reviewable.set_priorities(low: new_value)
+  end
 end

config/locales/client.en.yml

@@ -516,7 +516,8 @@ en:
 
         priority:
           title: "Minimum Priority"
-          low: "(any)"
+          any: "(any)"
+          low: "Low"
           medium: "Medium"
           high: "High"
 

config/locales/server.en.yml

@@ -1956,6 +1956,7 @@ en:
     reviewable_claiming: "Does reviewable content need to be claimed before it can be acted upon?"
     reviewable_default_topics: "Show reviewable content grouped by topic by default"
     reviewable_default_visibility: "Don't show reviewable items unless they meet this priority"
+    reviewable_low_priority_threshold: "The priority filter hides reviewable that doesn't meet this score unless the '(any)' filter is used."
     high_trust_flaggers_auto_hide_posts: "New user posts are automatically hidden after being flagged as spam by a TL3+ user"
     cooldown_hours_until_reflag: "How much time users will have to wait until they are able to reflag a post"
 

config/site_settings.yml

@@ -1710,6 +1710,10 @@ spam:
     type: enum
     default: low
     enum: "ReviewablePrioritySetting"
+  reviewable_low_priority_threshold:
+    default: 1
+    min: 1
+
 
 rate_limits:
   unique_posts_mins: 5

spec/initializers/track_setting_changes_spec.rb

@@ -22,4 +22,27 @@ describe 'Setting changes' do
       expect(non_approved_user.reload.approved?).to eq(true)
     end
   end
+
+  describe '#reviewable_low_priority_threshold' do
+    let(:new_threshold) { 5 }
+
+    it 'sets the low priority value' do
+      medium_threshold = 10
+      Reviewable.set_priorities(medium: medium_threshold)
+
+      expect(Reviewable.min_score_for_priority(:low)).not_to eq(new_threshold)
+
+      SiteSetting.reviewable_low_priority_threshold = new_threshold
+
+      expect(Reviewable.min_score_for_priority(:low)).to eq(new_threshold)
+    end
+
+    it "does nothing if the other thresholds were not calculated" do
+      Reviewable.set_priorities(medium: 0.0)
+
+      SiteSetting.reviewable_low_priority_threshold = new_threshold
+
+      expect(Reviewable.min_score_for_priority(:low)).not_to eq(new_threshold)
+    end
+  end
 end

spec/jobs/reviewable_priorities_spec.rb

@@ -6,42 +6,65 @@ describe Jobs::ReviewablePriorities do
 
   it "needs returns 0s with no existing reviewables" do
     Jobs::ReviewablePriorities.new.execute({})
-    expect(Reviewable.min_score_for_priority(:low)).to eq(0.0)
+
-    expect(Reviewable.min_score_for_priority(:medium)).to eq(0.0)
+    expect_min_score(:low, 0.0)
-    expect(Reviewable.min_score_for_priority(:high)).to eq(0.0)
+    expect_min_score(:medium, 0.0)
+    expect_min_score(:high, 0.0)
     expect(Reviewable.score_required_to_hide_post).to eq(8.33)
   end
 
-  fab!(:u0) { Fabricate(:user) }
+  fab!(:user_0) { Fabricate(:user) }
-  fab!(:u1) { Fabricate(:user) }
+  fab!(:user_1) { Fabricate(:user) }
 
   def create_reviewables(count)
-    (1..count).each do |i|
+    (1..count).each { |i| create_with_score(i) }
-      r = Fabricate(:reviewable_flagged_post)
+  end
-      r.add_score(u0, PostActionType.types[:off_topic])
+
-      r.add_score(u1, PostActionType.types[:off_topic])
+  def create_with_score(score)
-      r.update!(score: i)
+    Fabricate(:reviewable_flagged_post).tap do |reviewable|
+      reviewable.add_score(user_0, PostActionType.types[:off_topic])
+      reviewable.add_score(user_1, PostActionType.types[:off_topic])
+      reviewable.update!(score: score)
     end
   end
 
   it "needs a minimum amount of reviewables before it calculates anything" do
     create_reviewables(5)
+
     Jobs::ReviewablePriorities.new.execute({})
-    expect(Reviewable.min_score_for_priority(:low)).to eq(0.0)
+
-    expect(Reviewable.min_score_for_priority(:medium)).to eq(0.0)
+    expect_min_score(:low, 0.0)
-    expect(Reviewable.min_score_for_priority(:high)).to eq(0.0)
+    expect_min_score(:medium, 0.0)
+    expect_min_score(:high, 0.0)
     expect(Reviewable.score_required_to_hide_post).to eq(8.33)
   end
 
   it "will set priorities based on the maximum score" do
     create_reviewables(Jobs::ReviewablePriorities.min_reviewables)
+
     Jobs::ReviewablePriorities.new.execute({})
 
-    expect(Reviewable.min_score_for_priority(:low)).to eq(0.0)
+    expect_min_score(:low, SiteSetting.reviewable_low_priority_threshold)
-    expect(Reviewable.min_score_for_priority(:medium)).to eq(8.0)
+    expect_min_score(:medium, 8.0)
-    expect(Reviewable.min_score_for_priority('medium')).to eq(8.0)
+    expect_min_score('medium', 8.0)
-    expect(Reviewable.min_score_for_priority(:high)).to eq(13.0)
+    expect_min_score(:high, 13.0)
     expect(Reviewable.score_required_to_hide_post).to eq(8.66)
   end
 
+  it 'ignore negative scores when calculating priorities' do
+    create_reviewables(Jobs::ReviewablePriorities.min_reviewables)
+    negative_score = -9
+    10.times { create_with_score(negative_score) }
+
+    Jobs::ReviewablePriorities.new.execute({})
+
+    expect_min_score(:low, SiteSetting.reviewable_low_priority_threshold)
+    expect_min_score(:medium, 8.0)
+    expect_min_score(:high, 13.0)
+    expect(Reviewable.score_required_to_hide_post).to eq(8.66)
+  end
+
+  def expect_min_score(priority, score)
+    expect(Reviewable.min_score_for_priority(priority)).to eq(score)
+  end
 end