From 425982841d66734f9f07f393334711454b42def2 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 19 Nov 2024 03:51:34 -0500 Subject: [PATCH] Use group/count approach in annual report classes (#32914) --- .../commonly_interacted_with_accounts.rb | 2 +- app/lib/annual_report/most_reblogged_accounts.rb | 2 +- app/lib/annual_report/most_used_apps.rb | 2 +- app/lib/annual_report/top_hashtags.rb | 8 +++++++- .../commonly_interacted_with_accounts_spec.rb | 13 +++++++++++-- .../annual_report/most_reblogged_accounts_spec.rb | 12 ++++++++++-- spec/lib/annual_report/most_used_apps_spec.rb | 11 ++++++++--- spec/lib/annual_report/top_hashtags_spec.rb | 15 +++++++++++++-- 8 files changed, 52 insertions(+), 13 deletions(-) diff --git a/app/lib/annual_report/commonly_interacted_with_accounts.rb b/app/lib/annual_report/commonly_interacted_with_accounts.rb index 30ab671d8a..2316789f2a 100644 --- a/app/lib/annual_report/commonly_interacted_with_accounts.rb +++ b/app/lib/annual_report/commonly_interacted_with_accounts.rb @@ -17,6 +17,6 @@ class AnnualReport::CommonlyInteractedWithAccounts < AnnualReport::Source private def commonly_interacted_with_accounts - report_statuses.where.not(in_reply_to_account_id: @account.id).group(:in_reply_to_account_id).having('count(*) > 1').order(total: :desc).limit(SET_SIZE).pluck(Arel.sql('in_reply_to_account_id, count(*) AS total')) + report_statuses.where.not(in_reply_to_account_id: @account.id).group(:in_reply_to_account_id).having('count(*) > 1').order(count_all: :desc).limit(SET_SIZE).count end end diff --git a/app/lib/annual_report/most_reblogged_accounts.rb b/app/lib/annual_report/most_reblogged_accounts.rb index cfc4022ca7..5c750b19bf 100644 --- a/app/lib/annual_report/most_reblogged_accounts.rb +++ b/app/lib/annual_report/most_reblogged_accounts.rb @@ -17,6 +17,6 @@ class AnnualReport::MostRebloggedAccounts < AnnualReport::Source private def most_reblogged_accounts - report_statuses.where.not(reblog_of_id: nil).joins(reblog: :account).group('accounts.id').having('count(*) > 1').order(total: :desc).limit(SET_SIZE).pluck(Arel.sql('accounts.id, count(*) as total')) + report_statuses.where.not(reblog_of_id: nil).joins(reblog: :account).group('accounts.id').having('count(*) > 1').order(count_all: :desc).limit(SET_SIZE).count end end diff --git a/app/lib/annual_report/most_used_apps.rb b/app/lib/annual_report/most_used_apps.rb index fb1ca1d167..e10ba52503 100644 --- a/app/lib/annual_report/most_used_apps.rb +++ b/app/lib/annual_report/most_used_apps.rb @@ -17,6 +17,6 @@ class AnnualReport::MostUsedApps < AnnualReport::Source private def most_used_apps - report_statuses.joins(:application).group('oauth_applications.name').order(total: :desc).limit(SET_SIZE).pluck(Arel.sql('oauth_applications.name, count(*) as total')) + report_statuses.joins(:application).group('oauth_applications.name').order(count_all: :desc).limit(SET_SIZE).count end end diff --git a/app/lib/annual_report/top_hashtags.rb b/app/lib/annual_report/top_hashtags.rb index 32bd10d698..ae000a8beb 100644 --- a/app/lib/annual_report/top_hashtags.rb +++ b/app/lib/annual_report/top_hashtags.rb @@ -17,6 +17,12 @@ class AnnualReport::TopHashtags < AnnualReport::Source private def top_hashtags - Tag.joins(:statuses).where(statuses: { id: report_statuses.select(:id) }).group(:id).having('count(*) > 1').order(total: :desc).limit(SET_SIZE).pluck(Arel.sql('COALESCE(tags.display_name, tags.name), count(*) AS total')) + Tag.joins(:statuses).where(statuses: { id: report_statuses.select(:id) }).group(coalesced_tag_names).having('count(*) > 1').order(count_all: :desc).limit(SET_SIZE).count + end + + def coalesced_tag_names + Arel.sql(<<~SQL.squish) + COALESCE(tags.display_name, tags.name) + SQL end end diff --git a/spec/lib/annual_report/commonly_interacted_with_accounts_spec.rb b/spec/lib/annual_report/commonly_interacted_with_accounts_spec.rb index 0e31827912..12bf3810db 100644 --- a/spec/lib/annual_report/commonly_interacted_with_accounts_spec.rb +++ b/spec/lib/annual_report/commonly_interacted_with_accounts_spec.rb @@ -21,18 +21,27 @@ RSpec.describe AnnualReport::CommonlyInteractedWithAccounts do let(:account) { Fabricate :account } let(:other_account) { Fabricate :account } + let(:most_other_account) { Fabricate :account } before do _other = Fabricate :status + Fabricate :status, account: account, reply: true, in_reply_to_id: Fabricate(:status, account: other_account).id Fabricate :status, account: account, reply: true, in_reply_to_id: Fabricate(:status, account: other_account).id + + Fabricate :status, account: account, reply: true, in_reply_to_id: Fabricate(:status, account: most_other_account).id + Fabricate :status, account: account, reply: true, in_reply_to_id: Fabricate(:status, account: most_other_account).id + Fabricate :status, account: account, reply: true, in_reply_to_id: Fabricate(:status, account: most_other_account).id end it 'builds a report for an account' do expect(subject.generate) .to include( - commonly_interacted_with_accounts: contain_exactly( - include(account_id: other_account.id.to_s, count: 2) + commonly_interacted_with_accounts: eq( + [ + { account_id: most_other_account.id.to_s, count: 3 }, + { account_id: other_account.id.to_s, count: 2 }, + ] ) ) end diff --git a/spec/lib/annual_report/most_reblogged_accounts_spec.rb b/spec/lib/annual_report/most_reblogged_accounts_spec.rb index 2f04934e47..956549c325 100644 --- a/spec/lib/annual_report/most_reblogged_accounts_spec.rb +++ b/spec/lib/annual_report/most_reblogged_accounts_spec.rb @@ -21,18 +21,26 @@ RSpec.describe AnnualReport::MostRebloggedAccounts do let(:account) { Fabricate :account } let(:other_account) { Fabricate :account } + let(:most_other_account) { Fabricate :account } before do _other = Fabricate :status Fabricate :status, account: account, reblog: Fabricate(:status, account: other_account) Fabricate :status, account: account, reblog: Fabricate(:status, account: other_account) + + Fabricate :status, account: account, reblog: Fabricate(:status, account: most_other_account) + Fabricate :status, account: account, reblog: Fabricate(:status, account: most_other_account) + Fabricate :status, account: account, reblog: Fabricate(:status, account: most_other_account) end it 'builds a report for an account' do expect(subject.generate) .to include( - most_reblogged_accounts: contain_exactly( - include(account_id: other_account.id.to_s, count: 2) + most_reblogged_accounts: eq( + [ + { account_id: most_other_account.id.to_s, count: 3 }, + { account_id: other_account.id.to_s, count: 2 }, + ] ) ) end diff --git a/spec/lib/annual_report/most_used_apps_spec.rb b/spec/lib/annual_report/most_used_apps_spec.rb index d2fcecc4d8..ab7022ef20 100644 --- a/spec/lib/annual_report/most_used_apps_spec.rb +++ b/spec/lib/annual_report/most_used_apps_spec.rb @@ -20,18 +20,23 @@ RSpec.describe AnnualReport::MostUsedApps do context 'with an active account' do let(:account) { Fabricate :account } - let(:application) { Fabricate :application } + let(:application) { Fabricate :application, name: 'App' } + let(:most_application) { Fabricate :application, name: 'Most App' } before do _other = Fabricate :status Fabricate.times 2, :status, account: account, application: application + Fabricate.times 3, :status, account: account, application: most_application end it 'builds a report for an account' do expect(subject.generate) .to include( - most_used_apps: contain_exactly( - include(name: application.name, count: 2) + most_used_apps: eq( + [ + { name: most_application.name, count: 3 }, + { name: application.name, count: 2 }, + ] ) ) end diff --git a/spec/lib/annual_report/top_hashtags_spec.rb b/spec/lib/annual_report/top_hashtags_spec.rb index 58a9152184..b9cc9392ed 100644 --- a/spec/lib/annual_report/top_hashtags_spec.rb +++ b/spec/lib/annual_report/top_hashtags_spec.rb @@ -21,20 +21,31 @@ RSpec.describe AnnualReport::TopHashtags do let(:account) { Fabricate :account } let(:tag) { Fabricate :tag } + let(:most_tag) { Fabricate :tag } before do _other = Fabricate :status + first = Fabricate :status, account: account first.tags << tag + first.tags << most_tag + last = Fabricate :status, account: account last.tags << tag + last.tags << most_tag + + middle = Fabricate :status, account: account + middle.tags << most_tag end it 'builds a report for an account' do expect(subject.generate) .to include( - top_hashtags: contain_exactly( - include(name: tag.name, count: 2) + top_hashtags: eq( + [ + { name: most_tag.name, count: 3 }, + { name: tag.name, count: 2 }, + ] ) ) end