From 5f0bc4557fb04928a8d38cba410d316a76f1bf8c Mon Sep 17 00:00:00 2001 From: Kelv Date: Mon, 31 Jul 2023 13:33:10 +0800 Subject: [PATCH] FEATURE: Count only approved flagged posts in user pages (#22799) FEATURE: Only approved flags for post counters * Why was this change necessary? The counters for flagged posts in the user's profile and user index from the admin view include flags that were rejected, ignored or pending review. This introduces unnecessary noise. Also the flagged posts counter in the user's profile includes custom flags which add further noise to this signal. * How does it address the problem? * Modifying User#flags_received_count to return posts with only approved standard flags * Refactoring User#number_of_flagged_posts to alias to User#flags_received_count * Updating the flagged post staff counter hyperlink to navigate to a filtered view of that user's approved flagged posts to maintain consistency with the counter * Adding system tests for the profile page to cover the flagged posts staff counter --- .../admin/addon/templates/user-index.hbs | 2 +- .../discourse/app/templates/user.hbs | 2 +- app/models/user.rb | 15 ++++--- spec/models/user_spec.rb | 37 +++++++++++++++--- spec/system/page_objects/pages/user.rb | 22 +++++++++++ spec/system/user_page/staff_info_spec.rb | 39 +++++++++++++++++++ 6 files changed, 102 insertions(+), 15 deletions(-) diff --git a/app/assets/javascripts/admin/addon/templates/user-index.hbs b/app/assets/javascripts/admin/addon/templates/user-index.hbs index b6e43794c14..a02eac646bd 100644 --- a/app/assets/javascripts/admin/addon/templates/user-index.hbs +++ b/app/assets/javascripts/admin/addon/templates/user-index.hbs @@ -734,7 +734,7 @@ @query={{hash username=this.model.username type="ReviewableFlaggedPost" - status="all" + status="approved" }} class="btn" > diff --git a/app/assets/javascripts/discourse/app/templates/user.hbs b/app/assets/javascripts/discourse/app/templates/user.hbs index ad8047619de..0e08cef4409 100644 --- a/app/assets/javascripts/discourse/app/templates/user.hbs +++ b/app/assets/javascripts/discourse/app/templates/user.hbs @@ -38,7 +38,7 @@ @route="review" @query={{hash username=this.model.username - status="all" + status="approved" type="ReviewableFlaggedPost" }} > diff --git a/app/models/user.rb b/app/models/user.rb index 17a05e4446f..7f37630f793 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1202,13 +1202,6 @@ class User < ActiveRecord::Base user_warnings.count end - def flags_received_count - posts - .includes(:post_actions) - .where("post_actions.post_action_type_id" => PostActionType.flag_types_without_custom.values) - .count - end - def private_topics_count topics_allowed.where(archetype: Archetype.private_message).count end @@ -1539,8 +1532,14 @@ class User < ActiveRecord::Base end def number_of_flagged_posts - ReviewableFlaggedPost.where(target_created_by: self.id).count + posts + .with_deleted + .includes(:post_actions) + .where("post_actions.post_action_type_id" => PostActionType.flag_types_without_custom.values) + .where("post_actions.agreed_at IS NOT NULL") + .count end + alias_method :flags_received_count, :number_of_flagged_posts def number_of_rejected_posts ReviewableQueuedPost.rejected.where(target_created_by_id: self.id).count diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 29d60c446a1..2c4f2fcbfe7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2002,16 +2002,43 @@ RSpec.describe User do end describe "#number_of_flagged_posts" do - it "counts flagged posts from the user" do - Fabricate(:reviewable_flagged_post, target_created_by: user) + fab!(:admin) { Fabricate(:admin) } - expect(user.number_of_flagged_posts).to eq(1) + it "counts only approved standard flagged posts from the user" do + %i[disagree ignore delete_and_ignore].each do |review_action| + PostActionCreator + .off_topic(admin, Fabricate(:post, user: user)) + .reviewable + .perform(admin, review_action) + end + %i[agree_and_keep delete_and_agree].each do |approval_action| + PostActionCreator + .off_topic(admin, Fabricate(:post, user: user)) + .reviewable + .perform(admin, approval_action) + end + + expect(user.number_of_flagged_posts).to eq 2 + end + + it "ignores custom flags from the user" do + PostActionCreator + .notify_moderators(admin, Fabricate(:post, user: user)) + .reviewable + .perform(admin, :agree_and_keep) + expect(user.number_of_flagged_posts).to be_zero end it "ignores flagged posts from another user" do - Fabricate(:reviewable_flagged_post, target_created_by: Fabricate(:user)) + other_user = Fabricate(:user) + %i[disagree ignore delete_and_ignore agree_and_keep].each do |review_action| + PostActionCreator + .off_topic(admin, Fabricate(:post, user: other_user)) + .reviewable + .perform(admin, review_action) + end - expect(user.number_of_flagged_posts).to eq(0) + expect(user.number_of_flagged_posts).to be_zero end end end diff --git a/spec/system/page_objects/pages/user.rb b/spec/system/page_objects/pages/user.rb index 171610d1699..457336e82f5 100644 --- a/spec/system/page_objects/pages/user.rb +++ b/spec/system/page_objects/pages/user.rb @@ -35,6 +35,28 @@ module PageObjects button.click if button["aria-expanded"] == "false" self end + + def has_reviewable_flagged_posts_path?(user) + params = { + status: "approved", + sort_order: "score", + type: "ReviewableFlaggedPost", + username: user.username, + } + page.has_current_path?("/review?#{params.to_query}") + end + + def staff_info_flagged_posts_counter + page.find(".staff-counters .flagged-posts") + end + + def has_staff_info_flagged_posts_count?(count:) + staff_info_flagged_posts_counter.text.to_i == count + end + + def has_no_staff_info_flagged_posts_counter? + page.has_no_css?(".staff-counters .flagged-posts") + end end end end diff --git a/spec/system/user_page/staff_info_spec.rb b/spec/system/user_page/staff_info_spec.rb index 1e29c9ef932..c9a642391ec 100644 --- a/spec/system/user_page/staff_info_spec.rb +++ b/spec/system/user_page/staff_info_spec.rb @@ -17,4 +17,43 @@ describe "Viewing user staff info as an admin", type: :system do expect(user_page).to have_warning_messages_path(user) end end + + context "for flagged posts" do + before do + %i[disagree ignore delete_and_ignore].each do |review_action| + PostActionCreator + .off_topic(admin, Fabricate(:post, user: user)) + .reviewable + .perform(admin, review_action) + end + end + + context "when there are no approved flagged posts" do + it "should not display a flagged-posts staff counter" do + user_page.visit(user) + expect(user_page).to have_no_staff_info_flagged_posts_counter + end + end + + context "when there are approved flagged posts" do + before do + 2.times do + PostActionCreator + .off_topic(admin, Fabricate(:post, user: user)) + .reviewable + .perform(admin, :agree_and_keep) + end + end + + it "should display a flagged-posts staff counter with the right count and link to user's flagged posts" do + user_page.visit(user) + + expect(user_page).to have_staff_info_flagged_posts_count(count: 2) + + user_page.staff_info_flagged_posts_counter.click + + expect(user_page).to have_reviewable_flagged_posts_path(user) + end + end + end end