diff --git a/app/assets/javascripts/discourse/app/controllers/review-index.js b/app/assets/javascripts/discourse/app/controllers/review-index.js index dc28bf6558e20d..73f2c655429ece 100644 --- a/app/assets/javascripts/discourse/app/controllers/review-index.js +++ b/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}`), diff --git a/app/jobs/scheduled/reviewable_priorities.rb b/app/jobs/scheduled/reviewable_priorities.rb index 7e0f5bbd7e9109..cb74ead9cf600d 100644 --- a/app/jobs/scheduled/reviewable_priorities.rb +++ b/app/jobs/scheduled/reviewable_priorities.rb @@ -16,13 +16,16 @@ def self.target_count 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 @@ def execute(args) medium, high = res - Reviewable.set_priorities(medium: medium, high: high) + Reviewable.set_priorities( + low: min_priority_threshold, + medium: medium, + high: high + ) end end diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index cb964477f93273..9b729bdf8d469f 100644 --- a/app/models/reviewable.rb +++ b/app/models/reviewable.rb @@ -444,8 +444,6 @@ def self.list_for( 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 @@ def self.list_for( ) 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 diff --git a/config/initializers/014-track-setting-changes.rb b/config/initializers/014-track-setting-changes.rb index fed99a618c8300..1340d1defe9555 100644 --- a/config/initializers/014-track-setting-changes.rb +++ b/config/initializers/014-track-setting-changes.rb @@ -48,4 +48,9 @@ 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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 0cf26e86fb5efd..40213e184232f4 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -516,7 +516,8 @@ en: priority: title: "Minimum Priority" - low: "(any)" + any: "(any)" + low: "Low" medium: "Medium" high: "High" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index a05451336babf8..66589a878ea556 100644 --- a/config/locales/server.en.yml +++ b/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" diff --git a/config/site_settings.yml b/config/site_settings.yml index 5464916ada844e..beacd3a502268a 100644 --- a/config/site_settings.yml +++ b/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 diff --git a/spec/initializers/track_setting_changes_spec.rb b/spec/initializers/track_setting_changes_spec.rb index 82b7963ec4d655..b6e86c161a0890 100644 --- a/spec/initializers/track_setting_changes_spec.rb +++ b/spec/initializers/track_setting_changes_spec.rb @@ -22,4 +22,27 @@ 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 diff --git a/spec/jobs/reviewable_priorities_spec.rb b/spec/jobs/reviewable_priorities_spec.rb index 22276374fde21f..9a5c96d7332064 100644 --- a/spec/jobs/reviewable_priorities_spec.rb +++ b/spec/jobs/reviewable_priorities_spec.rb @@ -6,42 +6,65 @@ 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(Reviewable.min_score_for_priority(:high)).to eq(0.0) + + expect_min_score(:low, 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!(:u1) { Fabricate(:user) } + fab!(:user_0) { Fabricate(:user) } + fab!(:user_1) { Fabricate(:user) } def create_reviewables(count) - (1..count).each do |i| - r = Fabricate(:reviewable_flagged_post) - r.add_score(u0, PostActionType.types[:off_topic]) - r.add_score(u1, PostActionType.types[:off_topic]) - r.update!(score: i) + (1..count).each { |i| create_with_score(i) } + end + + def create_with_score(score) + 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(Reviewable.min_score_for_priority(:high)).to eq(0.0) + + expect_min_score(:low, 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_min_score(:low, SiteSetting.reviewable_low_priority_threshold) + expect_min_score(:medium, 8.0) + expect_min_score('medium', 8.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(Reviewable.min_score_for_priority(:low)).to eq(0.0) - expect(Reviewable.min_score_for_priority(:medium)).to eq(8.0) - expect(Reviewable.min_score_for_priority('medium')).to eq(8.0) - expect(Reviewable.min_score_for_priority(:high)).to eq(13.0) + 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