[FEATURE] Disallow ignoring self, admins or moderators users (#7202)

app/controllers/users_controller.rb

@@ -997,10 +997,9 @@ class UsersController < ApplicationController
 
   def ignore
     raise Discourse::NotFound unless SiteSetting.ignore_user_enabled
+    guardian.ensure_can_ignore_user!(params[:ignored_user_id])
 
-    ::IgnoredUser.find_or_create_by!(
+    IgnoredUser.find_or_create_by!(user: current_user, ignored_user_id: params[:ignored_user_id])
-      user: current_user,
-      ignored_user_id: params[:ignored_user_id])
     render json: success_json
   end
 

app/serializers/basic_post_serializer.rb

@@ -44,8 +44,11 @@ class BasicPostSerializer < ApplicationSerializer
   end
 
   def ignored
-    object.is_first_post? && IgnoredUser.where(user_id: scope.current_user&.id,
+    object.is_first_post? &&
-                                               ignored_user_id: object.user_id).exists?
+      scope.current_user&.id != object.user_id &&
+      !object.user&.staff? &&
+      IgnoredUser.where(user_id: scope.current_user&.id,
+                        ignored_user_id: object.user_id).exists?
   end
 
   def include_name?

app/serializers/user_serializer.rb

@@ -282,7 +282,7 @@ class UserSerializer < BasicUserSerializer
   end
 
   def can_ignore_user
-    SiteSetting.ignore_user_enabled
+    SiteSetting.ignore_user_enabled? && !object.staff? && scope.current_user != object
   end
 
   # Needed because 'send_private_message_to_user' will always return false

app/services/user_updater.rb

@@ -149,7 +149,8 @@ class UserUpdater
 
   def update_muted_users(usernames)
     usernames ||= ""
-    desired_ids = User.where(username: usernames.split(",")).pluck(:id)
+    desired_usernames = usernames.split(",").reject { |username| user.username == username }
+    desired_ids = User.where(username: desired_usernames).pluck(:id)
     if desired_ids.empty?
       MutedUser.where(user_id: user.id).destroy_all
     else
@@ -168,7 +169,8 @@ class UserUpdater
 
   def update_ignored_users(usernames)
     usernames ||= ""
-    desired_ids = User.where(username: usernames.split(",")).pluck(:id)
+    desired_usernames = usernames.split(",").reject { |username| user.username == username }
+    desired_ids = User.where(username: desired_usernames).where(admin: false, moderator: false).pluck(:id)
     if desired_ids.empty?
       IgnoredUser.where(user_id: user.id).destroy_all
     else

lib/guardian.rb

@@ -390,6 +390,10 @@ class Guardian
     UserExport.where(user_id: @user.id, created_at: (Time.zone.now.beginning_of_day..Time.zone.now.end_of_day)).count == 0
   end
 
+  def can_ignore_user?(user_id)
+    @user.id != user_id && User.where(id: user_id, admin: false, moderator: false).exists?
+  end
+
   def allow_themes?(theme_ids, include_preview: false)
     return true if theme_ids.blank?
 

lib/topic_view.rb

@@ -603,7 +603,18 @@ class TopicView
     @filtered_posts = unfiltered_posts
 
     if SiteSetting.ignore_user_enabled
-      ignored_user_ids = IgnoredUser.where(user_id: @user&.id).pluck(:ignored_user_id)
+
+      sql = <<~SQL
+          SELECT ignored_user_id
+          FROM ignored_users as ig
+          JOIN users as u ON u.id = ig.ignored_user_id
+          WHERE ig.user_id = :current_user_id
+            AND ig.ignored_user_id <> :current_user_id
+            AND NOT u.admin
+            AND NOT u.moderator
+      SQL
+
+      ignored_user_ids = DB.query_single(sql, current_user_id: @user&.id)
 
       if ignored_user_ids.present?
         @filtered_posts = @filtered_posts.where.not("user_id IN (?) AND id <> ?", ignored_user_ids, first_post_id)

spec/components/guardian_spec.rb

@@ -2640,6 +2640,32 @@ describe Guardian do
     end
   end
 
+  describe '#can_ignore_user?' do
+    let(:guardian) { Guardian.new(user) }
+
+    context "when ignored user is the same as guardian user" do
+      it 'does not allow ignoring user' do
+        expect(guardian.can_ignore_user?(user.id)).to eq(false)
+      end
+    end
+
+    context "when ignored user is a staff user" do
+      let!(:admin) { Fabricate(:user, admin: true) }
+
+      it 'does not allow ignoring user' do
+        expect(guardian.can_ignore_user?(admin.id)).to eq(false)
+      end
+    end
+
+    context "when ignored user is a normal user" do
+      let!(:another_user) { Fabricate(:user) }
+
+      it 'allows ignoring user' do
+        expect(guardian.can_ignore_user?(another_user.id)).to eq(true)
+      end
+    end
+  end
+
   describe "#allow_themes?" do
     let(:theme) { Fabricate(:theme) }
     let(:theme2) { Fabricate(:theme) }

spec/components/topic_view_spec.rb

@@ -44,8 +44,7 @@ describe TopicView do
           SiteSetting.ignore_user_enabled = false
 
           tv = TopicView.new(topic.id, evil_trout)
-          expect(tv.filtered_post_ids.size).to eq(3)
+          expect(tv.filtered_post_ids).to eq([post.id, post2.id, post3.id])
-          expect(tv.filtered_post_ids).to match_array([post.id, post2.id, post3.id])
         end
       end
 
@@ -57,8 +56,7 @@ describe TopicView do
 
         it "filters out ignored user posts" do
           tv = TopicView.new(topic.id, evil_trout)
-          expect(tv.filtered_post_ids.size).to eq(2)
+          expect(tv.filtered_post_ids).to eq([post.id, post2.id])
-          expect(tv.filtered_post_ids).to match_array([post.id, post2.id])
         end
 
         describe "when an ignored user made the original post" do
@@ -66,8 +64,7 @@ describe TopicView do
 
           it "filters out ignored user posts only" do
             tv = TopicView.new(topic.id, evil_trout)
-            expect(tv.filtered_post_ids.size).to eq(2)
+            expect(tv.filtered_post_ids).to eq([post.id, post2.id])
-            expect(tv.filtered_post_ids).to match_array([post.id, post2.id])
           end
         end
 
@@ -77,8 +74,7 @@ describe TopicView do
 
           it "filters out ignored user posts only" do
             tv = TopicView.new(topic.id, evil_trout)
-            expect(tv.filtered_post_ids.size).to eq(3)
+            expect(tv.filtered_post_ids).to eq([post.id, post2.id, post4.id])
-            expect(tv.filtered_post_ids).to match_array([post.id, post2.id, post4.id])
           end
         end
 
@@ -88,8 +84,18 @@ describe TopicView do
 
           it "filters out ignored user posts only" do
             tv = TopicView.new(topic.id, nil)
-            expect(tv.filtered_post_ids.size).to eq(4)
+            expect(tv.filtered_post_ids).to eq([post.id, post2.id, post3.id, post4.id])
-            expect(tv.filtered_post_ids).to match_array([post.id, post2.id, post3.id, post4.id])
+          end
+        end
+
+        describe "when a staff user is ignored" do
+          let!(:admin) { Fabricate(:user, admin: true) }
+          let!(:admin_ignored_user) { Fabricate(:ignored_user, user: evil_trout, ignored_user: admin) }
+          let!(:post4) { Fabricate(:post, topic: topic, user: admin) }
+
+          it "filters out ignored user excluding the staff user" do
+            tv = TopicView.new(topic.id, evil_trout)
+            expect(tv.filtered_post_ids).to eq([post.id, post2.id, post4.id])
           end
         end
       end

spec/jobs/ignored_users_summary_spec.rb

@@ -48,8 +48,8 @@ describe Jobs::IgnoredUsersSummary do
             subtype: TopicSubtype.system_message
           })
           expect(posts.count).to eq(2)
-          expect(posts[0].raw).to include(matt.username)
+          expect(posts.find { |post| post.raw.include?(matt.username) }).to be_present
-          expect(posts[1].raw).to include(john.username)
+          expect(posts.find { |post| post.raw.include?(john.username) }).to be_present
         end
       end
     end

spec/services/user_updater_spec.rb

@@ -23,6 +23,15 @@ describe UserUpdater do
       expect(MutedUser.where(user_id: u1.id).count).to eq 2
       expect(MutedUser.where(user_id: u3.id).count).to eq 0
     end
+
+    it 'excludes acting user' do
+      u1 = Fabricate(:user)
+      u2 = Fabricate(:user)
+      updater = UserUpdater.new(u1, u1)
+      updater.update_muted_users("#{u1.username},#{u2.username}")
+
+      expect(MutedUser.where(muted_user_id: u2.id).count).to eq 1
+    end
   end
 
   describe '#update_ignored_users' do
@@ -44,6 +53,15 @@ describe UserUpdater do
       expect(IgnoredUser.where(user_id: u1.id).count).to eq 2
       expect(IgnoredUser.where(user_id: u3.id).count).to eq 0
     end
+
+    it 'excludes acting user' do
+      u1 = Fabricate(:user)
+      u2 = Fabricate(:user)
+      updater = UserUpdater.new(u1, u1)
+      updater.update_ignored_users("#{u1.username},#{u2.username}")
+
+      expect(IgnoredUser.where(ignored_user_id: u2.id).count).to eq 1
+    end
   end
 
   describe '#update' do