Explorar o código

Fix crash when filtering for “dormant” relationships (#27306)

Claire hai 7 meses
pai
achega
47441e51f3

+ 2 - 2
app/models/account.rb

@@ -113,8 +113,8 @@ class Account < ApplicationRecord
   scope :searchable, -> { without_unapproved.without_suspended.where(moved_to_account_id: nil) }
   scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_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('(case when account_stats.last_status_at is null then 1 else 0 end) asc, account_stats.last_status_at desc, accounts.id desc')) }
-  scope :by_recent_sign_in, -> { order(Arel.sql('(case when users.current_sign_in_at is null then 1 else 0 end) asc, users.current_sign_in_at desc, accounts.id desc')) }
+  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: domain).or(where(arel_table[:domain].matches('%.' + domain))) }
   scope :not_excluded_by_account, ->(account) { where.not(id: account.excluded_from_timeline_account_ids) }

+ 1 - 1
app/models/relationship_filter.rb

@@ -112,7 +112,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 "Unknown activity: #{value}"
     end

+ 6 - 4
spec/fabricators/account_stat_fabricator.rb

@@ -1,6 +1,8 @@
+# frozen_string_literal: true
+
 Fabricator(:account_stat) do
-  account         nil
-  statuses_count  ""
-  following_count ""
-  followers_count ""
+  account { Fabricate.build(:account) }
+  statuses_count  '123'
+  following_count '456'
+  followers_count '789'
 end

+ 48 - 20
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
-      let(:subject) do
-        RelationshipFilter.new(account, 'order' => 'active').results
-      end
+    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
 
-      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 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
 
-      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' => 'active', 'activity' => 'dormant').results
+        end
 
-        expect(subject).to eq expected_result
+        it 'returns dormant followings ordered by last activity' do
+          expect(subject).to eq [account_of_7_months, silent_account]
+        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
+    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
+
+      context 'when filtering for dormant accounts' do
+        subject do
+          described_class.new(account, 'order' => 'recent', 'activity' => 'dormant').results
+        end
+
+        it 'returns dormant followings ordered by last activity' do
+          expect(subject).to eq [silent_account, account_of_7_months]
+        end
+      end
+    end
   end
 end