REFACTOR: Move redundant ignored user check into guardian (#7219)

* REFACTOR: Move redundant ignored user check into guardian

app/serializers/basic_post_serializer.rb

@@ -44,9 +44,9 @@ class BasicPostSerializer < ApplicationSerializer
   end
 
   def ignored
+    return false unless SiteSetting.ignore_user_enabled?
     object.is_first_post? &&
       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

app/serializers/user_serializer.rb

@@ -282,10 +282,7 @@ class UserSerializer < BasicUserSerializer
   end
 
   def can_ignore_user
-    SiteSetting.ignore_user_enabled? &&
-      !object.staff? &&
-      scope.current_user != object &&
-      (scope.current_user.staff? || scope.current_user.trust_level >= TrustLevel.levels[:member])
+    scope.can_ignore_user?(object.id)
   end
 
   # Needed because 'send_private_message_to_user' will always return false

app/services/user_updater.rb

@@ -168,7 +168,7 @@ class UserUpdater
   end
 
   def update_ignored_users(usernames)
-    return if user.trust_level < TrustLevel.levels[:member]
+    return unless guardian.can_ignore_users?
 
     usernames ||= ""
     desired_usernames = usernames.split(",").reject { |username| user.username == username }

lib/guardian.rb

@@ -391,9 +391,12 @@ class Guardian
   end
 
   def can_ignore_user?(user_id)
-    @user.id != user_id &&
-      (@user.staff? || @user.trust_level >= TrustLevel.levels[:member]) &&
-      User.where(id: user_id, admin: false, moderator: false).exists?
+    can_ignore_users? && @user.id != user_id && User.where(id: user_id, admin: false, moderator: false).exists?
+  end
+
+  def can_ignore_users?
+    return false if anonymous?
+    SiteSetting.ignore_user_enabled? && (@user.staff? || @user.trust_level >= TrustLevel.levels[:member])
   end
 
   def allow_themes?(theme_ids, include_preview: false)

spec/components/guardian_spec.rb

@@ -2641,6 +2641,10 @@ describe Guardian do
   end
 
   describe '#can_ignore_user?' do
+    before do
+      SiteSetting.ignore_user_enabled = true
+    end
+
     let(:guardian) { Guardian.new(trust_level_2) }
 
     context "when ignored user is the same as guardian user" do

spec/services/user_updater_spec.rb

@@ -19,9 +19,9 @@ describe UserUpdater do
       updater = UserUpdater.new(u3, u3)
       updater.update_muted_users("")
 
-      expect(MutedUser.where(user_id: u2.id).count).to eq 2
-      expect(MutedUser.where(user_id: u1.id).count).to eq 2
-      expect(MutedUser.where(user_id: u3.id).count).to eq 0
+      expect(MutedUser.where(user_id: u2.id).pluck(:muted_user_id)).to match_array([u3.id, u1.id])
+      expect(MutedUser.where(user_id: u1.id).pluck(:muted_user_id)).to match_array([u2.id, u3.id])
+      expect(MutedUser.where(user_id: u3.id).count).to eq(0)
     end
 
     it 'excludes acting user' do
@@ -30,11 +30,15 @@ describe UserUpdater do
       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
+      expect(MutedUser.where(muted_user_id: u2.id).pluck(:muted_user_id)).to match_array([u2.id])
     end
   end
 
   describe '#update_ignored_users' do
+    before do
+      SiteSetting.ignore_user_enabled = true
+    end
+
     it 'updates ignored users' do
       u1 = Fabricate(:user, trust_level: 2)
       u2 = Fabricate(:user, trust_level: 2)
@@ -49,9 +53,9 @@ describe UserUpdater do
       updater = UserUpdater.new(u3, u3)
       updater.update_ignored_users("")
 
-      expect(IgnoredUser.where(user_id: u2.id).count).to eq 2
-      expect(IgnoredUser.where(user_id: u1.id).count).to eq 2
-      expect(IgnoredUser.where(user_id: u3.id).count).to eq 0
+      expect(IgnoredUser.where(user_id: u2.id).pluck(:ignored_user_id)).to match_array([u3.id, u1.id])
+      expect(IgnoredUser.where(user_id: u1.id).pluck(:ignored_user_id)).to match_array([u2.id, u3.id])
+      expect(IgnoredUser.where(user_id: u3.id).count).to eq(0)
     end
 
     it 'excludes acting user' do
@@ -60,7 +64,7 @@ describe UserUpdater do
       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
+      expect(IgnoredUser.where(user_id: u1.id).pluck(:ignored_user_id)).to match_array([u2.id])
     end
 
     context 'when acting user\'s trust level is below tl2' do
@@ -70,7 +74,18 @@ describe UserUpdater do
         updater = UserUpdater.new(u1, u1)
         updater.update_ignored_users("#{u2.username}")
 
-        expect(IgnoredUser.where(ignored_user_id: u2.id).count).to eq 0
+        expect(IgnoredUser.where(ignored_user_id: u2.id).count).to eq(0)
+      end
+    end
+
+    context 'when acting user is admin' do
+      it 'excludes acting user' do
+        u1 = Fabricate(:admin)
+        u2 = Fabricate(:user)
+        updater = UserUpdater.new(u1, u1)
+        updater.update_ignored_users("#{u1.username},#{u2.username}")
+
+        expect(IgnoredUser.where(user_id: u1.id).pluck(:ignored_user_id)).to match_array([u2.id])
       end
     end
   end