From e91d8feab3451ef2593effc2032c8434d10059f6 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 18 Oct 2023 11:38:17 +1000 Subject: [PATCH] Revert "FEATURE: Count only approved flagged posts in user pages (#22799)" (#23962) This reverts commit 5f0bc4557fb04928a8d38cba410d316a76f1bf8c. Through extensive internal discussion we have decided to revert this change, as it significantly impacted moderation flow for some Discourse site moderators, especially around "something else" flags. We need to re-approach how flags are counted holistically, so to that end this change is being reverted. --- .../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, 15 insertions(+), 102 deletions(-) diff --git a/app/assets/javascripts/admin/addon/templates/user-index.hbs b/app/assets/javascripts/admin/addon/templates/user-index.hbs index 2507aa12ee2..5faaf59ca33 100644 --- a/app/assets/javascripts/admin/addon/templates/user-index.hbs +++ b/app/assets/javascripts/admin/addon/templates/user-index.hbs @@ -724,7 +724,7 @@ @query={{hash username=this.model.username type="ReviewableFlaggedPost" - status="approved" + status="all" }} class="btn" > diff --git a/app/assets/javascripts/discourse/app/templates/user.hbs b/app/assets/javascripts/discourse/app/templates/user.hbs index 6b439b10a83..2bdf3c7cb5b 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="approved" + status="all" type="ReviewableFlaggedPost" }} > diff --git a/app/models/user.rb b/app/models/user.rb index 0963b485c8a..b7f35e27e31 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1194,6 +1194,13 @@ 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 @@ -1520,14 +1527,8 @@ class User < ActiveRecord::Base end def number_of_flagged_posts - 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 + ReviewableFlaggedPost.where(target_created_by: self.id).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 e7d72a01d54..3b606ef3663 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1994,43 +1994,16 @@ RSpec.describe User do end describe "#number_of_flagged_posts" do - fab!(:admin) { Fabricate(:admin) } + it "counts flagged posts from the user" do + Fabricate(:reviewable_flagged_post, target_created_by: user) - 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 + expect(user.number_of_flagged_posts).to eq(1) end it "ignores flagged posts from another user" do - 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 + Fabricate(:reviewable_flagged_post, target_created_by: Fabricate(:user)) - expect(user.number_of_flagged_posts).to be_zero + expect(user.number_of_flagged_posts).to eq(0) end end end diff --git a/spec/system/page_objects/pages/user.rb b/spec/system/page_objects/pages/user.rb index 8e5e8f0b5ee..5888015d0bc 100644 --- a/spec/system/page_objects/pages/user.rb +++ b/spec/system/page_objects/pages/user.rb @@ -52,28 +52,6 @@ module PageObjects 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 - private def primary_navigation_selector(name) diff --git a/spec/system/user_page/staff_info_spec.rb b/spec/system/user_page/staff_info_spec.rb index c9a642391ec..1e29c9ef932 100644 --- a/spec/system/user_page/staff_info_spec.rb +++ b/spec/system/user_page/staff_info_spec.rb @@ -17,43 +17,4 @@ 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