Fix follow suggestions potentially including silenced or blocked accounts (#29306)
This commit is contained in:
		
							parent
							
								
									68600893d2
								
							
						
					
					
						commit
						ee8d0b9447
					
				| @ -2,31 +2,15 @@ | ||||
| 
 | ||||
| class AccountSuggestions::FriendsOfFriendsSource < AccountSuggestions::Source | ||||
|   def get(account, limit: DEFAULT_LIMIT) | ||||
|     Account.find_by_sql([<<~SQL.squish, { id: account.id, limit: limit }]).map { |row| [row.id, key] } | ||||
|       WITH first_degree AS ( | ||||
|           SELECT target_account_id | ||||
|           FROM follows | ||||
|           JOIN accounts AS target_accounts ON follows.target_account_id = target_accounts.id | ||||
|           WHERE account_id = :id | ||||
|             AND NOT target_accounts.hide_collections | ||||
|       ) | ||||
|       SELECT accounts.id, COUNT(*) AS frequency | ||||
|       FROM accounts | ||||
|       JOIN follows ON follows.target_account_id = accounts.id | ||||
|       JOIN account_stats ON account_stats.account_id = accounts.id | ||||
|       LEFT OUTER JOIN follow_recommendation_mutes ON follow_recommendation_mutes.target_account_id = accounts.id AND follow_recommendation_mutes.account_id = :id | ||||
|       WHERE follows.account_id IN (SELECT * FROM first_degree) | ||||
|         AND NOT EXISTS (SELECT 1 FROM follows f WHERE f.target_account_id = follows.target_account_id AND f.account_id = :id) | ||||
|         AND follows.target_account_id <> :id | ||||
|         AND accounts.discoverable | ||||
|         AND accounts.suspended_at IS NULL | ||||
|         AND accounts.silenced_at IS NULL | ||||
|         AND accounts.moved_to_account_id IS NULL | ||||
|         AND follow_recommendation_mutes.target_account_id IS NULL | ||||
|       GROUP BY accounts.id, account_stats.id | ||||
|       ORDER BY frequency DESC, account_stats.followers_count ASC | ||||
|       LIMIT :limit | ||||
|     SQL | ||||
|     first_degree = account.following.where.not(hide_collections: true).select(:id).reorder(nil) | ||||
|     base_account_scope(account) | ||||
|       .joins(:account_stat) | ||||
|       .where(id: Follow.where(account_id: first_degree).select(:target_account_id)) | ||||
|       .group('accounts.id, account_stats.id') | ||||
|       .reorder('frequency DESC, followers_count DESC') | ||||
|       .limit(limit) | ||||
|       .pluck(Arel.sql('accounts.id, COUNT(*) AS frequency')) | ||||
|       .map { |id, _frequency| [id, key] } | ||||
|   end | ||||
| 
 | ||||
|   private | ||||
|  | ||||
| @ -51,7 +51,8 @@ class AccountSuggestions::SimilarProfilesSource < AccountSuggestions::Source | ||||
|     recently_followed_account_ids = account.active_relationships.recent.limit(5).pluck(:target_account_id) | ||||
| 
 | ||||
|     if Chewy.enabled? && !recently_followed_account_ids.empty? | ||||
|       QueryBuilder.new(recently_followed_account_ids, account).build.limit(limit).hits.pluck('_id').map(&:to_i).zip([key].cycle) | ||||
|       ids_from_es = QueryBuilder.new(recently_followed_account_ids, account).build.limit(limit).hits.pluck('_id').map(&:to_i) | ||||
|       base_account_scope(account).where(id: ids_from_es).pluck(:id).zip([key].cycle) | ||||
|     else | ||||
|       [] | ||||
|     end | ||||
|  | ||||
| @ -12,6 +12,8 @@ class AccountSuggestions::Source | ||||
|   def base_account_scope(account) | ||||
|     Account | ||||
|       .searchable | ||||
|       .where(discoverable: true) | ||||
|       .without_silenced | ||||
|       .where.not(follows_sql, id: account.id) | ||||
|       .where.not(follow_requests_sql, id: account.id) | ||||
|       .not_excluded_by_account(account) | ||||
|  | ||||
| @ -0,0 +1,82 @@ | ||||
| # frozen_string_literal: true | ||||
| 
 | ||||
| require 'rails_helper' | ||||
| 
 | ||||
| RSpec.describe AccountSuggestions::FriendsOfFriendsSource do | ||||
|   describe '#get' do | ||||
|     subject { described_class.new } | ||||
| 
 | ||||
|     let!(:bob) { Fabricate(:account, discoverable: true, hide_collections: false) } | ||||
|     let!(:alice) { Fabricate(:account, discoverable: true, hide_collections: true) } | ||||
|     let!(:eve) { Fabricate(:account, discoverable: true, hide_collections: false) } | ||||
|     let!(:mallory) { Fabricate(:account, discoverable: false, hide_collections: false) } | ||||
|     let!(:eugen) { Fabricate(:account, discoverable: true, hide_collections: false) } | ||||
|     let!(:john) { Fabricate(:account, discoverable: true, hide_collections: false) } | ||||
|     let!(:jerk) { Fabricate(:account, discoverable: true, hide_collections: false) } | ||||
|     let!(:neil) { Fabricate(:account, discoverable: true, hide_collections: false) } | ||||
|     let!(:larry) { Fabricate(:account, discoverable: true, hide_collections: false) } | ||||
| 
 | ||||
|     context 'with follows and blocks' do | ||||
|       before do | ||||
|         bob.block!(jerk) | ||||
|         FollowRecommendationMute.create!(account: bob, target_account: neil) | ||||
| 
 | ||||
|         # bob follows eugen, alice and larry | ||||
|         [eugen, alice, larry].each { |account| bob.follow!(account) } | ||||
| 
 | ||||
|         # alice follows eve and mallory | ||||
|         [john, mallory].each { |account| alice.follow!(account) } | ||||
| 
 | ||||
|         # eugen follows eve, john, jerk, larry and neil | ||||
|         [eve, mallory, jerk, larry, neil].each { |account| eugen.follow!(account) } | ||||
|       end | ||||
| 
 | ||||
|       it 'returns eligible accounts', :aggregate_failures do | ||||
|         results = subject.get(bob) | ||||
| 
 | ||||
|         # eve is returned through eugen | ||||
|         expect(results).to include([eve.id, :friends_of_friends]) | ||||
| 
 | ||||
|         # john is not reachable because alice hides who she follows | ||||
|         expect(results).to_not include([john.id, :friends_of_friends]) | ||||
| 
 | ||||
|         # mallory is not discoverable | ||||
|         expect(results).to_not include([mallory.id, :friends_of_friends]) | ||||
| 
 | ||||
|         # larry is not included because he's followed already | ||||
|         expect(results).to_not include([larry.id, :friends_of_friends]) | ||||
| 
 | ||||
|         # jerk is blocked | ||||
|         expect(results).to_not include([jerk.id, :friends_of_friends]) | ||||
| 
 | ||||
|         # the suggestion for neil has already been rejected | ||||
|         expect(results).to_not include([neil.id, :friends_of_friends]) | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context 'with deterministic order' do | ||||
|       before do | ||||
|         # bob follows eve and mallory | ||||
|         [eve, mallory].each { |account| bob.follow!(account) } | ||||
| 
 | ||||
|         # eve follows eugen, john, and jerk | ||||
|         [jerk, eugen, john].each { |account| eve.follow!(account) } | ||||
| 
 | ||||
|         # mallory follows eugen, john, and neil | ||||
|         [neil, eugen, john].each { |account| mallory.follow!(account) } | ||||
| 
 | ||||
|         john.follow!(eugen) | ||||
|         john.follow!(neil) | ||||
|       end | ||||
| 
 | ||||
|       it 'returns eligible accounts in the expected order' do | ||||
|         expect(subject.get(bob)).to eq [ | ||||
|           [eugen.id, :friends_of_friends], # followed by 2 friends, 3 followers total | ||||
|           [john.id, :friends_of_friends], # followed by 2 friends, 2 followers total | ||||
|           [neil.id, :friends_of_friends], # followed by 1 friend, 2 followers total | ||||
|           [jerk.id, :friends_of_friends], # followed by 1 friend, 1 follower total | ||||
|         ] | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| end | ||||
| @ -11,14 +11,16 @@ RSpec.describe AccountSuggestions::Source do | ||||
|     end | ||||
| 
 | ||||
|     context 'with follows and follow requests' do | ||||
|       let!(:account_domain_blocked_account) { Fabricate(:account, domain: 'blocked.host') } | ||||
|       let!(:account) { Fabricate(:account) } | ||||
|       let!(:blocked_account) { Fabricate(:account) } | ||||
|       let!(:eligible_account) { Fabricate(:account) } | ||||
|       let!(:follow_recommendation_muted_account) { Fabricate(:account) } | ||||
|       let!(:follow_requested_account) { Fabricate(:account) } | ||||
|       let!(:following_account) { Fabricate(:account) } | ||||
|       let!(:moved_account) { Fabricate(:account, moved_to_account: Fabricate(:account)) } | ||||
|       let!(:account_domain_blocked_account) { Fabricate(:account, domain: 'blocked.host', discoverable: true) } | ||||
|       let!(:account) { Fabricate(:account, discoverable: true) } | ||||
|       let!(:blocked_account) { Fabricate(:account, discoverable: true) } | ||||
|       let!(:eligible_account) { Fabricate(:account, discoverable: true) } | ||||
|       let!(:follow_recommendation_muted_account) { Fabricate(:account, discoverable: true) } | ||||
|       let!(:follow_requested_account) { Fabricate(:account, discoverable: true) } | ||||
|       let!(:following_account) { Fabricate(:account, discoverable: true) } | ||||
|       let!(:moved_account) { Fabricate(:account, moved_to_account: Fabricate(:account), discoverable: true) } | ||||
|       let!(:silenced_account) { Fabricate(:account, silenced: true, discoverable: true) } | ||||
|       let!(:undiscoverable_account) { Fabricate(:account, discoverable: false) } | ||||
| 
 | ||||
|       before do | ||||
|         Fabricate :account_domain_block, account: account, domain: account_domain_blocked_account.domain | ||||
| @ -40,6 +42,8 @@ RSpec.describe AccountSuggestions::Source do | ||||
|           .and not_include(follow_requested_account) | ||||
|           .and not_include(following_account) | ||||
|           .and not_include(moved_account) | ||||
|           .and not_include(silenced_account) | ||||
|           .and not_include(undiscoverable_account) | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user