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
This commit is contained in:
Anna Buianova 2024-02-08 16:57:15 +03:00 committed by GitHub
parent 2d9c87d9d3
commit 07ffb2e3be
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 31 additions and 9 deletions

View file

@ -10,7 +10,7 @@ module Api
private_constant :USERS_ATTRIBUTES_FOR_SERIALIZATION private_constant :USERS_ATTRIBUTES_FOR_SERIALIZATION
def users def users
@follows = Follow.followable_user(@user.id) @follows = Follow.non_suspended("User", @user.id)
.includes(:follower) .includes(:follower)
.select(USERS_ATTRIBUTES_FOR_SERIALIZATION) .select(USERS_ATTRIBUTES_FOR_SERIALIZATION)
.order(order_criteria) .order(order_criteria)

View file

@ -76,7 +76,7 @@ class DashboardsController < ApplicationController
def followers def followers
fetch_and_authorize_user 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) .includes(:follower).order(created_at: :desc).limit(follows_limit)
@collections_count = collections_count(@user) @collections_count = collections_count(@user)
end end

View file

@ -35,6 +35,7 @@ class Follow < ApplicationRecord
.where(followable_type: followable_type, followable_id: followable_id) .where(followable_type: followable_type, followable_id: followable_id)
.where("follows.follower_type = 'User'") .where("follows.follower_type = 'User'")
.where("roles.name NOT IN (?) OR roles.name IS NULL", %w[suspended spam]) .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] }, counter_culture :follower, column_name: proc { |follow| COUNTER_CULTURE_COLUMN_NAME_BY_TYPE[follow.followable_type] },

View file

@ -262,6 +262,10 @@ class User < ApplicationRecord
find_by(id: Settings::General.mascot_user_id) find_by(id: Settings::General.mascot_user_id)
end end
def good_standing_followers_count
Follow.non_suspended("User", id).count
end
def tag_line def tag_line
profile.summary profile.summary
end end

View file

@ -31,8 +31,7 @@ module Notifications
when "Organization" when "Organization"
notification_params[:organization_id] = followable_id notification_params[:organization_id] = followable_id
end end
followers = User.where(id: recent_follows.map(&:follower_id))
followers = User.where(id: recent_follows.select(:follower_id))
aggregated_siblings = followers.map { |follower| user_data(follower) } aggregated_siblings = followers.map { |follower| user_data(follower) }
if aggregated_siblings.empty? if aggregated_siblings.empty?
notification = Notification.find_by(notification_params)&.destroy notification = Notification.find_by(notification_params)&.destroy

View file

@ -1,2 +1,2 @@
json.partial! "api/v1/shared/user_show_extended", user: @user json.partial! "api/v1/shared/user_show_extended", user: @user
json.followers_count @user.followers_count json.followers_count @user.good_standing_followers_count

View file

@ -24,7 +24,7 @@
href="/dashboard/user_followers" href="/dashboard/user_followers"
<%= params[:action] == "followers" ? ' aria-current="page"'.html_safe : "" %>> <%= params[:action] == "followers" ? ' aria-current="page"'.html_safe : "" %>>
<%= t("views.dashboard.actions.followers") %> <%= t("views.dashboard.actions.followers") %>
<span class="c-indicator"><%= @user.followers_count %></span> <span class="c-indicator"><%= @user.good_standing_followers_count %></span>
</a> </a>
</li> </li>
<% end %> <% end %>

View file

@ -1,7 +1,7 @@
<div class="block m:hidden pt-3"> <div class="block m:hidden pt-3">
<select id="mobile_nav_dashboard" class="crayons-select"> <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" <%= "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_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_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> <option value="/dashboard/following_organizations" <%= "selected" if params[:action] == "following_organizations" %>><%= t("views.dashboard.actions.mobile.following_orgs", num: @user.following_organizations_count) %></option>

View file

@ -6,7 +6,7 @@
<h2 style="font-size:32px;"> <h2 style="font-size:32px;">
<a href="<%= user_url(@follower) %>"><%= @follower.name %></a> just followed you 🤗</h2> <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> <a href="<%= app_url("/dashboard/user_followers") %>">right here</a>.</h3>
<p> <p>
When someone follows you on When someone follows you on

View file

@ -155,6 +155,14 @@ RSpec.describe "Api::V1::Users" do
response_user = response.parsed_body response_user = response.parsed_body
expect(response_user["followers_count"]).to eq(1) expect(response_user["followers_count"]).to eq(1)
end 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
end end

View file

@ -336,11 +336,21 @@ RSpec.describe "Dashboards" do
end end
context "when logged in" do 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 second_user.follow user
spam_user.follow user
suspended_user.follow user
sign_in user sign_in user
end
it "only includes good standing users as followers (not spam or suspended)", :aggregated_failures do
get "/dashboard/user_followers" get "/dashboard/user_followers"
expect(response.body).to include CGI.escapeHTML(second_user.name) 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 end
end end