From 340c390849c8a265dd11d0132e49b01042c58567 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 6 Oct 2023 12:58:16 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20crash=20when=20filtering=20for=20?= =?UTF-8?q?=E2=80=9Cdormant=E2=80=9D=20relationships=20(#27306)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/models/account.rb | 2 +- app/models/relationship_filter.rb | 2 +- spec/models/relationship_filter_spec.rb | 68 +++++++++++++++++-------- 3 files changed, 50 insertions(+), 22 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index bb70b3a7b8..a25ebc4aaf 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -127,7 +127,7 @@ class Account < ApplicationRecord scope :searchable, -> { without_unapproved.without_suspended.where(moved_to_account_id: nil) } scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).joins(:account_stat) } scope :followable_by, ->(account) { joins(arel_table.join(Follow.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(Follow.arel_table[:target_account_id]).and(Follow.arel_table[:account_id].eq(account.id))).join_sources).where(Follow.arel_table[:id].eq(nil)).joins(arel_table.join(FollowRequest.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(FollowRequest.arel_table[:target_account_id]).and(FollowRequest.arel_table[:account_id].eq(account.id))).join_sources).where(FollowRequest.arel_table[:id].eq(nil)) } - scope :by_recent_status, -> { order(Arel.sql('account_stats.last_status_at DESC NULLS LAST')) } + scope :by_recent_status, -> { includes(:account_stat).merge(AccountStat.order('last_status_at DESC NULLS LAST')).references(:account_stat) } scope :by_recent_sign_in, -> { order(Arel.sql('users.current_sign_in_at DESC NULLS LAST')) } scope :popular, -> { order('account_stats.followers_count desc') } scope :by_domain_and_subdomains, ->(domain) { where(domain: Instance.by_domain_and_subdomains(domain).select(:domain)) } diff --git a/app/models/relationship_filter.rb b/app/models/relationship_filter.rb index 955d7d188a..d686f9ed89 100644 --- a/app/models/relationship_filter.rb +++ b/app/models/relationship_filter.rb @@ -114,7 +114,7 @@ class RelationshipFilter def activity_scope(value) case value when 'dormant' - AccountStat.where(last_status_at: nil).or(AccountStat.where(AccountStat.arel_table[:last_status_at].lt(1.month.ago))) + Account.joins(:account_stat).where(account_stat: { last_status_at: [nil, ...1.month.ago] }) else raise Mastodon::InvalidParameterError, "Unknown activity: #{value}" end diff --git a/spec/models/relationship_filter_spec.rb b/spec/models/relationship_filter_spec.rb index ac31885774..fccd42aaad 100644 --- a/spec/models/relationship_filter_spec.rb +++ b/spec/models/relationship_filter_spec.rb @@ -6,32 +6,60 @@ describe RelationshipFilter do let(:account) { Fabricate(:account) } describe '#results' do - context 'when default params are used' do - subject do - described_class.new(account, 'order' => 'active').results + let(:account_of_7_months) { Fabricate(:account_stat, statuses_count: 1, last_status_at: 7.months.ago).account } + let(:account_of_1_day) { Fabricate(:account_stat, statuses_count: 1, last_status_at: 1.day.ago).account } + let(:account_of_3_days) { Fabricate(:account_stat, statuses_count: 1, last_status_at: 3.days.ago).account } + let(:silent_account) { Fabricate(:account_stat, statuses_count: 0, last_status_at: nil).account } + + before do + account.follow!(account_of_7_months) + account.follow!(account_of_1_day) + account.follow!(account_of_3_days) + account.follow!(silent_account) + end + + context 'when ordering by last activity' do + context 'when not filtering' do + subject do + described_class.new(account, 'order' => 'active').results + end + + it 'returns followings ordered by last activity' do + expect(subject).to eq [account_of_1_day, account_of_3_days, account_of_7_months, silent_account] + end end - before do - add_following_account_with(last_status_at: 7.days.ago) - add_following_account_with(last_status_at: 1.day.ago) - add_following_account_with(last_status_at: 3.days.ago) + context 'when filtering for dormant accounts' do + subject do + described_class.new(account, 'order' => 'active', 'activity' => 'dormant').results + end + + it 'returns dormant followings ordered by last activity' do + expect(subject).to eq [account_of_7_months, silent_account] + end + end + end + + context 'when ordering by account creation' do + context 'when not filtering' do + subject do + described_class.new(account, 'order' => 'recent').results + end + + it 'returns followings ordered by last account creation' do + expect(subject).to eq [silent_account, account_of_3_days, account_of_1_day, account_of_7_months] + end end - it 'returns followings ordered by last activity' do - expected_result = account.following.eager_load(:account_stat).reorder(nil).by_recent_status + context 'when filtering for dormant accounts' do + subject do + described_class.new(account, 'order' => 'recent', 'activity' => 'dormant').results + end - expect(subject).to eq expected_result + it 'returns dormant followings ordered by last activity' do + expect(subject).to eq [silent_account, account_of_7_months] + end end end end - - def add_following_account_with(last_status_at:) - following_account = Fabricate(:account) - Fabricate(:account_stat, account: following_account, - last_status_at: last_status_at, - statuses_count: 1, - following_count: 0, - followers_count: 0) - Fabricate(:follow, account: account, target_account: following_account).account - end end