From 216cea1e277530d39eefddbfccb519d2eca72b8b Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 12 Mar 2024 04:38:32 -0400 Subject: [PATCH] Fix incorrect frequency value in `FriendsOfFriendsSource` data (#29550) --- .../friends_of_friends_source.rb | 12 +++++--- .../friends_of_friends_source_spec.rb | 30 +++++++++++++++---- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/app/models/account_suggestions/friends_of_friends_source.rb b/app/models/account_suggestions/friends_of_friends_source.rb index 0c95d21a3e9..b4f549bf314 100644 --- a/app/models/account_suggestions/friends_of_friends_source.rb +++ b/app/models/account_suggestions/friends_of_friends_source.rb @@ -2,15 +2,19 @@ class AccountSuggestions::FriendsOfFriendsSource < AccountSuggestions::Source def get(account, limit: DEFAULT_LIMIT) + source_query(account, limit: limit) + .map { |id, _frequency, _followers_count| [id, key] } + end + + def source_query(account, limit: DEFAULT_LIMIT) 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)) + .joins(:passive_relationships).where(passive_relationships: { account_id: first_degree }) .group('accounts.id, account_stats.id') - .reorder('frequency DESC, followers_count DESC') + .reorder(frequency: :desc, followers_count: :desc) .limit(limit) - .pluck(Arel.sql('accounts.id, COUNT(*) AS frequency')) - .map { |id, _frequency| [id, key] } + .pluck(Arel.sql('accounts.id, COUNT(*) AS frequency, followers_count')) end private diff --git a/spec/models/account_suggestions/friends_of_friends_source_spec.rb b/spec/models/account_suggestions/friends_of_friends_source_spec.rb index 56a974add54..d7915985f8c 100644 --- a/spec/models/account_suggestions/friends_of_friends_source_spec.rb +++ b/spec/models/account_suggestions/friends_of_friends_source_spec.rb @@ -11,9 +11,9 @@ RSpec.describe AccountSuggestions::FriendsOfFriendsSource do 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!(:neil) { 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 @@ -70,13 +70,31 @@ RSpec.describe AccountSuggestions::FriendsOfFriendsSource do 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 + expect(subject.get(bob)).to eq expected_results + end + + it 'contains correct underlying source data' do + expect(source_query_values) + .to contain_exactly( + [eugen.id, 2, 3], # Followed by 2 friends of bob (eve, mallory), 3 followers total (breaks tie) + [john.id, 2, 2], # Followed by 2 friends of bob (eve, mallory), 2 followers total + [neil.id, 1, 2], # Followed by 1 friends of bob (mallory), 2 followers total (breaks tie) + [jerk.id, 1, 1] # Followed by 1 friends of bob (eve), 1 followers total + ) + end + + def expected_results + [ + [eugen.id, :friends_of_friends], + [john.id, :friends_of_friends], + [neil.id, :friends_of_friends], + [jerk.id, :friends_of_friends], ] end + + def source_query_values + subject.source_query(bob).to_a + end end end end