Use composable query in `User.active` scope (#29775)

app/lib/feed_manager.rb

@@ -18,7 +18,7 @@ class FeedManager
   # @yield [Account]
   # @return [void]
   def with_active_accounts(&block)
-    Account.joins(:user).where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago).find_each(&block)
+    Account.joins(:user).merge(User.signed_in_recently).find_each(&block)
   end
 
   # Redis key of a feed

app/lib/vacuum/feeds_vacuum.rb

@@ -21,7 +21,7 @@ class Vacuum::FeedsVacuum
   end
 
   def inactive_users
-    User.confirmed.inactive
+    User.confirmed.not_signed_in_recently
   end
 
   def inactive_users_lists

app/models/concerns/account/interactions.rb

@@ -255,13 +255,13 @@ module Account::Interactions
   def followers_for_local_distribution
     followers.local
              .joins(:user)
-             .where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago)
+             .merge(User.signed_in_recently)
   end
 
   def lists_for_local_distribution
     scope = lists.joins(account: :user)
     scope.where.not(list_accounts: { follow_id: nil }).or(scope.where(account_id: id))
-         .where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago)
+         .merge(User.signed_in_recently)
   end
 
   def remote_followers_hash(url)

app/models/user.rb

@@ -109,14 +109,16 @@ class User < ApplicationRecord
   validates :confirm_password, absence: true, on: :create
   validate :validate_role_elevation
 
+  scope :account_not_suspended, -> { joins(:account).merge(Account.without_suspended) }
   scope :recent, -> { order(id: :desc) }
   scope :pending, -> { where(approved: false) }
   scope :approved, -> { where(approved: true) }
   scope :confirmed, -> { where.not(confirmed_at: nil) }
   scope :enabled, -> { where(disabled: false) }
   scope :disabled, -> { where(disabled: true) }
-  scope :inactive, -> { where(arel_table[:current_sign_in_at].lt(ACTIVE_DURATION.ago)) }
+  scope :active, -> { confirmed.signed_in_recently.account_not_suspended }
-  scope :active, -> { confirmed.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where(accounts: { suspended_at: nil }) }
+  scope :signed_in_recently, -> { where(current_sign_in_at: ACTIVE_DURATION.ago..) }
+  scope :not_signed_in_recently, -> { where(current_sign_in_at: ...ACTIVE_DURATION.ago) }
   scope :matches_email, ->(value) { where(arel_table[:email].matches("#{value}%")) }
   scope :matches_ip, ->(value) { left_joins(:ips).where('user_ips.ip <<= ?', value).group('users.id') }
 

spec/models/user_spec.rb

@@ -107,12 +107,36 @@ RSpec.describe User do
       end
     end
 
-    describe 'inactive' do
+    describe 'signed_in_recently' do
-      it 'returns a relation of inactive users' do
+      it 'returns a relation of users who have signed in during the recent period' do
-        specified = Fabricate(:user, current_sign_in_at: 15.days.ago)
+        recent_sign_in_user = Fabricate(:user, current_sign_in_at: within_duration_window_days.ago)
-        Fabricate(:user, current_sign_in_at: 6.days.ago)
+        Fabricate(:user, current_sign_in_at: exceed_duration_window_days.ago)
 
-        expect(described_class.inactive).to contain_exactly(specified)
+        expect(described_class.signed_in_recently)
+          .to contain_exactly(recent_sign_in_user)
+      end
+    end
+
+    describe 'not_signed_in_recently' do
+      it 'returns a relation of users who have not signed in during the recent period' do
+        no_recent_sign_in_user = Fabricate(:user, current_sign_in_at: exceed_duration_window_days.ago)
+        Fabricate(:user, current_sign_in_at: within_duration_window_days.ago)
+
+        expect(described_class.not_signed_in_recently)
+          .to contain_exactly(no_recent_sign_in_user)
+      end
+    end
+
+    describe 'account_not_suspended' do
+      it 'returns with linked accounts that are not suspended' do
+        suspended_account = Fabricate(:account, suspended_at: 10.days.ago)
+        non_suspended_account = Fabricate(:account, suspended_at: nil)
+        suspended_user = Fabricate(:user, account: suspended_account)
+        non_suspended_user = Fabricate(:user, account: non_suspended_account)
+
+        expect(described_class.account_not_suspended)
+          .to include(non_suspended_user)
+          .and not_include(suspended_user)
       end
     end
 
@@ -137,6 +161,14 @@ RSpec.describe User do
         expect(described_class.matches_ip('2160:2160::/32')).to contain_exactly(user1)
       end
     end
+
+    def exceed_duration_window_days
+      described_class::ACTIVE_DURATION + 2.days
+    end
+
+    def within_duration_window_days
+      described_class::ACTIVE_DURATION - 2.days
+    end
   end
 
   describe 'blacklist' do