Do not show suspended followers in aggregated siblings for notification (#20174)

app/models/follow.rb

@@ -27,6 +27,15 @@ class Follow < ApplicationRecord
   scope :follower_podcast, ->(id) { where(follower_id: id, followable_type: "Podcast") }
   scope :follower_tag, ->(id) { where(follower_id: id, followable_type: "ActsAsTaggableOn::Tag") }
 
+  scope :non_suspended, lambda { |followable_type, followable_id|
+    joins("INNER JOIN users ON users.id = follows.follower_id")
+      .joins("LEFT JOIN users_roles ON users_roles.user_id = users.id")
+      .joins("LEFT JOIN roles ON roles.id = users_roles.role_id")
+      .where(followable_type: followable_type, followable_id: followable_id)
+      .where("follows.follower_type = 'User'")
+      .where("roles.name != 'suspended' OR roles.name IS NULL")
+  }
+
   counter_culture :follower, column_name: proc { |follow| COUNTER_CULTURE_COLUMN_NAME_BY_TYPE[follow.followable_type] },
                              column_names: COUNTER_CULTURE_COLUMNS_NAMES
   before_save :calculate_points

app/services/notifications/new_follower/send.rb

@@ -21,8 +21,8 @@ module Notifications
       delegate :user_data, to: Notifications
 
       def call
-        recent_follows = Follow.where(followable_type: followable_type, followable_id: followable_id)
-          .where("created_at > ?", 24.hours.ago).order(created_at: :desc)
+        recent_follows = Follow.non_suspended(followable_type, followable_id)
+          .where("follows.created_at > ?", 24.hours.ago).order(created_at: :desc)
 
         notification_params = { action: "Follow" }
         case followable_type

spec/models/follow_spec.rb

@@ -4,6 +4,11 @@ RSpec.describe Follow do
   let(:user) { create(:user) }
   let(:tag) { create(:tag) }
   let(:user_2) { create(:user) }
+  let(:suspended_user) { create(:user) }
+
+  before do
+    suspended_user.add_role(:suspended)
+  end
 
   describe "validations" do
     subject { user.follow(user_2) }
@@ -57,4 +62,34 @@ RSpec.describe Follow do
       end.to change(EmailMessage, :count).by(1)
     end
   end
+
+  describe "scopes" do
+    describe ".non_suspended" do
+      before do
+        user.follow(user_2)
+        user.follow(tag)
+        suspended_user.follow(user_2)
+      end
+
+      it "excludes suspended users from the result" do
+        result = described_class.non_suspended(user_2.class.name, user_2.id)
+        expect(result.map(&:follower)).to include(user)
+        expect(result.map(&:follower)).not_to include(suspended_user)
+      end
+
+      it "filters by followable type and id" do
+        result = described_class.non_suspended("ActsAsTaggableOn::Tag", tag.id)
+        expect(result.map(&:follower)).to include(user)
+        expect(result.map(&:follower)).not_to include(user_2)
+        expect(result.map(&:follower)).not_to include(suspended_user)
+      end
+
+      it "includes only Users in the result" do
+        result = described_class.non_suspended(user_2.class.name, user_2.id)
+        expect(result.map(&:follower).all?(User)).to be(true)
+      end
+
+      # Additional test cases for more edge cases
+    end
+  end
 end

spec/services/notifications/new_follower/send_spec.rb

@@ -115,6 +115,30 @@ RSpec.describe Notifications::NewFollower::Send, type: :service do
       expect(notification.json_data["user"]["class"]).to eq("name" => "User")
     end
 
+    it "does not include suspended users in aggregated_siblings" do
+      # Initial Setup: Let's assume user4 and user3 are following user2.
+      user4 = create(:user)
+      user5 = create(:user)
+      user6 = create(:user)
+      user4.follow(user2)
+      user5.follow(user2)
+      user6.follow(user2)
+
+      # Now, suspend user4
+      user5.add_role(:suspended)
+
+      # Trigger the described_class call for a new follow event, let's say from user3 to user2.
+      notification = described_class.call(follow_data(follow2)) # Assuming follow2 is from user3 to user2
+
+      # Aggregate names of siblings from the JSON data in the notification
+      aggregated_sibling_names = notification.json_data["aggregated_siblings"].pluck("name")
+
+      # Expectations: user4 should not be in the aggregated_siblings, but user3 should.
+      expect(aggregated_sibling_names).not_to include(user5.name)
+      expect(aggregated_sibling_names).to include(user4.name)
+      expect(aggregated_sibling_names).to include(user6.name)
+    end
+
     context "when notification exists" do
       before do
         create(:notification, user: user2, action: "Follow", notifiable: follow, notified_at: 1.year.ago)