From ac60a843290bab828d08ec52326fc959ecfeb363 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 7 May 2018 15:14:18 -0400 Subject: [PATCH] FEATURE: New site setting `min_flags_staff_visibility` When set higher than 1, flags won't show up for staff in the admin section unless the minimum threshold of flags on a post is reached. --- app/models/post_action.rb | 13 +++++-- config/locales/server.en.yml | 1 + config/site_settings.yml | 1 + lib/flag_query.rb | 27 +++++++++++---- spec/components/flag_query_spec.rb | 55 ++++++++++++++++++++++++++++++ spec/models/post_action_spec.rb | 11 ++++++ 6 files changed, 99 insertions(+), 9 deletions(-) diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 8e705527cdf..b60fe0f5dfa 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -52,13 +52,22 @@ class PostAction < ActiveRecord::Base end def self.update_flagged_posts_count - posts_flagged_count = PostAction.active + flagged_relation = PostAction.active .flags .joins(post: :topic) .where('posts.deleted_at' => nil) .where('topics.deleted_at' => nil) .where('posts.user_id > 0') - .count('DISTINCT posts.id') + .group("posts.id") + + if SiteSetting.min_flags_staff_visibility > 1 + flagged_relation = flagged_relation + .having("count(*) >= ?", SiteSetting.min_flags_staff_visibility) + end + + posts_flagged_count = flagged_relation + .pluck("posts.id") + .count $redis.set('posts_flagged_count', posts_flagged_count) user_ids = User.staff.pluck(:id) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 41b7fdcab21..db052f97cfa 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1473,6 +1473,7 @@ en: auto_silence_fast_typers_max_trust_level: "Maximum trust level to auto silence fast typers" auto_silence_first_post_regex: "Case insensitive regex that if passed will cause first post by user to be silenced and sent to approval queue. Example: raging|a[bc]a , will cause all posts containing raging or aba or aca to be silenced on first. Only applies to first post." flags_default_topics: "Show flagged topics by default in the admin section" + min_flags_staff_visibility: "The minimum amount of flags on a post must have before staff can see it in the admin section" reply_by_email_enabled: "Enable replying to topics via email." reply_by_email_address: "Template for reply by email incoming email address, for example: %{reply_key}@reply.example.com or replies+%{reply_key}@example.com" diff --git a/config/site_settings.yml b/config/site_settings.yml index a6bfc78d92a..eeb504b4801 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1096,6 +1096,7 @@ spam: flags_default_topics: default: false client: true + min_flags_staff_visibility: 1 rate_limits: unique_posts_mins: 5 diff --git a/lib/flag_query.rb b/lib/flag_query.rb index 268e4abef16..11db2aa887d 100644 --- a/lib/flag_query.rb +++ b/lib/flag_query.rb @@ -21,12 +21,16 @@ module FlagQuery total_rows = actions.count - post_ids = actions.limit(per_page) + post_ids_relation = actions.limit(per_page) .offset(offset) .group(:post_id) .order('MIN(post_actions.created_at) DESC') - .pluck(:post_id) - .uniq + + if opts[:filter] != "old" && SiteSetting.min_flags_staff_visibility > 1 + post_ids_relation = post_ids_relation.having("count(*) >= ?", SiteSetting.min_flags_staff_visibility) + end + + post_ids = post_ids_relation.pluck(:post_id).uniq posts = SqlBuilder.new(" SELECT p.id, @@ -182,18 +186,25 @@ module FlagQuery ft_by_id = {} users_by_id = {} topics_by_id = {} + counts_by_post = {} results.each do |pa| if pa.post.present? && pa.post.topic.present? - ft = ft_by_id[pa.post.topic.id] ||= OpenStruct.new( + topic_id = pa.post.topic.id + + ft = ft_by_id[topic_id] ||= OpenStruct.new( topic: pa.post.topic, flag_counts: {}, user_ids: [], - last_flag_at: pa.created_at + last_flag_at: pa.created_at, + meets_minimum: false ) - topics_by_id[pa.post.topic.id] = pa.post.topic + counts_by_post[pa.post.id] ||= 0 + sum = counts_by_post[pa.post.id] += 1 + ft.meets_minimum = true if sum >= SiteSetting.min_flags_staff_visibility + topics_by_id[topic_id] = pa.post.topic ft.flag_counts[pa.post_action_type_id] ||= 0 ft.flag_counts[pa.post_action_type_id] += 1 @@ -204,9 +215,11 @@ module FlagQuery end end + flagged_topics = ft_by_id.values.select { |ft| ft.meets_minimum } + Topic.preload_custom_fields(topics_by_id.values, TopicList.preloaded_custom_fields) - { flagged_topics: ft_by_id.values, users: users_by_id.values } + { flagged_topics: flagged_topics, users: users_by_id.values } end private diff --git a/spec/components/flag_query_spec.rb b/spec/components/flag_query_spec.rb index 592045e6411..428a5aa319f 100644 --- a/spec/components/flag_query_spec.rb +++ b/spec/components/flag_query_spec.rb @@ -5,6 +5,41 @@ describe FlagQuery do let(:codinghorror) { Fabricate(:coding_horror) } + describe "flagged_topics" do + it "respects `min_flags_staff_visibility`" do + admin = Fabricate(:admin) + moderator = Fabricate(:moderator) + + post = create_post + + PostAction.act(moderator, post, PostActionType.types[:spam]) + + SiteSetting.min_flags_staff_visibility = 1 + + result = FlagQuery.flagged_topics + expect(result[:flagged_topics]).to be_present + ft = result[:flagged_topics].first + expect(ft.topic).to eq(post.topic) + expect(ft.flag_counts).to eq(PostActionType.types[:spam] => 1) + + SiteSetting.min_flags_staff_visibility = 2 + + result = FlagQuery.flagged_topics + expect(result[:flagged_topics]).to be_blank + + PostAction.act(admin, post, PostActionType.types[:inappropriate]) + result = FlagQuery.flagged_topics + expect(result[:flagged_topics]).to be_present + ft = result[:flagged_topics].first + expect(ft.topic).to eq(post.topic) + expect(ft.flag_counts).to eq( + PostActionType.types[:spam] => 1, + PostActionType.types[:inappropriate] => 1 + ) + end + + end + describe "flagged_posts_report" do it "does not return flags on system posts" do admin = Fabricate(:admin) @@ -75,7 +110,27 @@ describe FlagQuery do posts, users = FlagQuery.flagged_posts_report(moderator) expect(posts.count).to eq(1) + end + it "respects `min_flags_staff_visibility`" do + admin = Fabricate(:admin) + moderator = Fabricate(:moderator) + + post = create_post + + PostAction.act(moderator, post, PostActionType.types[:spam]) + + SiteSetting.min_flags_staff_visibility = 2 + posts, topics, users = FlagQuery.flagged_posts_report(admin) + expect(posts).to be_blank + expect(topics).to be_blank + expect(users).to be_blank + + PostAction.act(admin, post, PostActionType.types[:inappropriate]) + posts, topics, users = FlagQuery.flagged_posts_report(admin) + expect(posts).to be_present + expect(topics).to be_present + expect(users).to be_present end end diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index da30be3cc94..7a10f346485 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -141,6 +141,17 @@ describe PostAction do expect(PostAction.flagged_posts_count).to eq(0) end + it "respects min_flags_staff_visibility" do + SiteSetting.min_flags_staff_visibility = 2 + expect(PostAction.flagged_posts_count).to eq(0) + + PostAction.act(codinghorror, post, PostActionType.types[:off_topic]) + expect(PostAction.flagged_posts_count).to eq(0) + + PostAction.act(eviltrout, post, PostActionType.types[:off_topic]) + expect(PostAction.flagged_posts_count).to eq(1) + end + it "should reset counts when a topic is deleted" do PostAction.act(codinghorror, post, PostActionType.types[:off_topic]) post.topic.trash!