From 07ffb2e3bedf4fe93cf8a47719d01a0288a2a18a Mon Sep 17 00:00:00 2001
From: Anna Buianova <lightallloy@gmail.com>
Date: Thu, 8 Feb 2024 16:57:15 +0300
Subject: [PATCH] Removed spammers from followers list (#20603)

* Remove spam and suspended users from followers list (api and dashboards)

* Fixed the api followers endpoint

* Fixed issue with follower_ids query
---
 app/controllers/concerns/api/followers_controller.rb |  2 +-
 app/controllers/dashboards_controller.rb             |  2 +-
 app/models/follow.rb                                 |  1 +
 app/models/user.rb                                   |  4 ++++
 app/services/notifications/new_follower/send.rb      |  3 +--
 app/views/api/v1/users/me.json.jbuilder              |  2 +-
 app/views/dashboards/_actions.html.erb               |  2 +-
 app/views/dashboards/_actions_mobile.html.erb        |  2 +-
 .../notify_mailer/new_follower_email.html.erb        |  2 +-
 spec/requests/api/v1/users_spec.rb                   |  8 ++++++++
 spec/requests/dashboard_spec.rb                      | 12 +++++++++++-
 11 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/app/controllers/concerns/api/followers_controller.rb b/app/controllers/concerns/api/followers_controller.rb
index bbd7b9e16..28394a946 100644
--- a/app/controllers/concerns/api/followers_controller.rb
+++ b/app/controllers/concerns/api/followers_controller.rb
@@ -10,7 +10,7 @@ module Api
     private_constant :USERS_ATTRIBUTES_FOR_SERIALIZATION
 
     def users
-      @follows = Follow.followable_user(@user.id)
+      @follows = Follow.non_suspended("User", @user.id)
         .includes(:follower)
         .select(USERS_ATTRIBUTES_FOR_SERIALIZATION)
         .order(order_criteria)
diff --git a/app/controllers/dashboards_controller.rb b/app/controllers/dashboards_controller.rb
index a392080df..347054fe5 100644
--- a/app/controllers/dashboards_controller.rb
+++ b/app/controllers/dashboards_controller.rb
@@ -76,7 +76,7 @@ class DashboardsController < ApplicationController
 
   def followers
     fetch_and_authorize_user
-    @follows = Follow.followable_user(@user.id)
+    @follows = Follow.non_suspended("User", @user.id)
       .includes(:follower).order(created_at: :desc).limit(follows_limit)
     @collections_count = collections_count(@user)
   end
diff --git a/app/models/follow.rb b/app/models/follow.rb
index fc7281716..670b7de83 100644
--- a/app/models/follow.rb
+++ b/app/models/follow.rb
@@ -35,6 +35,7 @@ class Follow < ApplicationRecord
       .where(followable_type: followable_type, followable_id: followable_id)
       .where("follows.follower_type = 'User'")
       .where("roles.name NOT IN (?) OR roles.name IS NULL", %w[suspended spam])
+      .distinct
   }
 
   counter_culture :follower, column_name: proc { |follow| COUNTER_CULTURE_COLUMN_NAME_BY_TYPE[follow.followable_type] },
diff --git a/app/models/user.rb b/app/models/user.rb
index 85f7fd4f1..eca0764f5 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -262,6 +262,10 @@ class User < ApplicationRecord
     find_by(id: Settings::General.mascot_user_id)
   end
 
+  def good_standing_followers_count
+    Follow.non_suspended("User", id).count
+  end
+
   def tag_line
     profile.summary
   end
diff --git a/app/services/notifications/new_follower/send.rb b/app/services/notifications/new_follower/send.rb
index 095289228..4d109d99c 100644
--- a/app/services/notifications/new_follower/send.rb
+++ b/app/services/notifications/new_follower/send.rb
@@ -31,8 +31,7 @@ module Notifications
         when "Organization"
           notification_params[:organization_id] = followable_id
         end
-
-        followers = User.where(id: recent_follows.select(:follower_id))
+        followers = User.where(id: recent_follows.map(&:follower_id))
         aggregated_siblings = followers.map { |follower| user_data(follower) }
         if aggregated_siblings.empty?
           notification = Notification.find_by(notification_params)&.destroy
diff --git a/app/views/api/v1/users/me.json.jbuilder b/app/views/api/v1/users/me.json.jbuilder
index 760073970..f754f1241 100644
--- a/app/views/api/v1/users/me.json.jbuilder
+++ b/app/views/api/v1/users/me.json.jbuilder
@@ -1,2 +1,2 @@
 json.partial! "api/v1/shared/user_show_extended", user: @user
-json.followers_count @user.followers_count
+json.followers_count @user.good_standing_followers_count
diff --git a/app/views/dashboards/_actions.html.erb b/app/views/dashboards/_actions.html.erb
index f25d3f2a2..3d7cff51b 100644
--- a/app/views/dashboards/_actions.html.erb
+++ b/app/views/dashboards/_actions.html.erb
@@ -24,7 +24,7 @@
         href="/dashboard/user_followers"
         <%= params[:action] == "followers" ? ' aria-current="page"'.html_safe : "" %>>
         <%= t("views.dashboard.actions.followers") %>
-        <span class="c-indicator"><%= @user.followers_count %></span>
+        <span class="c-indicator"><%= @user.good_standing_followers_count %></span>
       </a>
     </li>
     <% end %>
diff --git a/app/views/dashboards/_actions_mobile.html.erb b/app/views/dashboards/_actions_mobile.html.erb
index 273445452..0284c138c 100644
--- a/app/views/dashboards/_actions_mobile.html.erb
+++ b/app/views/dashboards/_actions_mobile.html.erb
@@ -1,7 +1,7 @@
 <div class="block m:hidden pt-3">
   <select id="mobile_nav_dashboard" class="crayons-select">
     <option value="/dashboard" <%= "selected" if params[:action] == "show" && (params[:which] == "organization" || params[:which].blank?) %>><%= t("views.dashboard.actions.mobile.posts", num: @user.articles_count) %></option>
-    <option value="/dashboard/user_followers" <%= "selected" if params[:action] == "followers" %>><%= t("views.dashboard.actions.mobile.followers", num: @user.followers_count) %></option>
+    <option value="/dashboard/user_followers" <%= "selected" if params[:action] == "followers" %>><%= t("views.dashboard.actions.mobile.followers", num: @user.good_standing_followers_count) %></option>
     <option value="/dashboard/following_tags" <%= "selected" if params[:action] == "following_tags" %>><%= t("views.dashboard.actions.mobile.following_tags", num: @user.cached_followed_tag_names.size) %></option>
     <option value="/dashboard/following_users" <%= "selected" if params[:action] == "following_users" %>><%= t("views.dashboard.actions.mobile.following_users", num: @user.following_users_count) %></option>
     <option value="/dashboard/following_organizations" <%= "selected" if params[:action] == "following_organizations" %>><%= t("views.dashboard.actions.mobile.following_orgs", num: @user.following_organizations_count) %></option>
diff --git a/app/views/mailers/notify_mailer/new_follower_email.html.erb b/app/views/mailers/notify_mailer/new_follower_email.html.erb
index 1aedf8de5..8318a6a08 100644
--- a/app/views/mailers/notify_mailer/new_follower_email.html.erb
+++ b/app/views/mailers/notify_mailer/new_follower_email.html.erb
@@ -6,7 +6,7 @@
 
 <h2 style="font-size:32px;">
   <a href="<%= user_url(@follower) %>"><%= @follower.name %></a> just followed you 🤗</h2>
-<h3 style="font-weight:600">You now have <%= pluralize(@user.followers_count, "total follower") %>. See all of your recent followers
+<h3 style="font-weight:600">You now have <%= pluralize(@user.good_standing_followers_count, "total follower") %>. See all of your recent followers
   <a href="<%= app_url("/dashboard/user_followers") %>">right here</a>.</h3>
 <p>
   When someone follows you on
diff --git a/spec/requests/api/v1/users_spec.rb b/spec/requests/api/v1/users_spec.rb
index 8b21f42af..12ff1d28c 100644
--- a/spec/requests/api/v1/users_spec.rb
+++ b/spec/requests/api/v1/users_spec.rb
@@ -155,6 +155,14 @@ RSpec.describe "Api::V1::Users" do
         response_user = response.parsed_body
         expect(response_user["followers_count"]).to eq(1)
       end
+
+      it "doesn't include spammers in followers_count" do
+        follower = create(:user, :spam)
+        create(:follow, followable: user, follower: follower)
+        get me_api_users_path, headers: auth_headers
+        response_user = response.parsed_body
+        expect(response_user["followers_count"]).to eq(0)
+      end
     end
   end
 
diff --git a/spec/requests/dashboard_spec.rb b/spec/requests/dashboard_spec.rb
index 9c58a46cf..a3aeacec4 100644
--- a/spec/requests/dashboard_spec.rb
+++ b/spec/requests/dashboard_spec.rb
@@ -336,11 +336,21 @@ RSpec.describe "Dashboards" do
     end
 
     context "when logged in" do
-      it "renders the current user's followers" do
+      let(:spam_user) { create(:user, :spam) }
+      let(:suspended_user) { create(:user, :suspended) }
+
+      before do
         second_user.follow user
+        spam_user.follow user
+        suspended_user.follow user
         sign_in user
+      end
+
+      it "only includes good standing users as followers (not spam or suspended)", :aggregated_failures do
         get "/dashboard/user_followers"
         expect(response.body).to include CGI.escapeHTML(second_user.name)
+        expect(response.body).not_to include CGI.escapeHTML(spam_user.name)
+        expect(response.body).not_to include CGI.escapeHTML(suspended_user.name)
       end
     end
   end