From 9343a605553213d35c0363117356467ef910f1b7 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 8 Apr 2024 09:53:49 -0400 Subject: [PATCH] Use composable query in `User.active` scope (#29775) --- app/lib/feed_manager.rb | 2 +- app/lib/vacuum/feeds_vacuum.rb | 2 +- app/models/concerns/account_interactions.rb | 4 +- app/models/user.rb | 6 ++- spec/models/user_spec.rb | 42 ++++++++++++++++++--- spec/rails_helper.rb | 1 + 6 files changed, 46 insertions(+), 11 deletions(-) diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 32236f20d9..366c0c8236 100644 --- a/app/lib/feed_manager.rb +++ b/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 diff --git a/app/lib/vacuum/feeds_vacuum.rb b/app/lib/vacuum/feeds_vacuum.rb index fb0b8a8472..4292157601 100644 --- a/app/lib/vacuum/feeds_vacuum.rb +++ b/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 diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 74ba1695fa..add2444138 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -261,13 +261,13 @@ module AccountInteractions 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) diff --git a/app/models/user.rb b/app/models/user.rb index 0c8d481c4c..95da92c683 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -110,14 +110,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.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where(accounts: { suspended_at: nil }) } + scope :active, -> { confirmed.signed_in_recently.account_not_suspended } + 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') } scope :emailable, -> { confirmed.enabled.joins(:account).merge(Account.searchable) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f06150f02c..6c557f8c9e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -81,12 +81,36 @@ RSpec.describe User do end end - describe 'inactive' do - it 'returns a relation of inactive users' do - specified = Fabricate(:user, current_sign_in_at: 15.days.ago) - Fabricate(:user, current_sign_in_at: 6.days.ago) + describe 'signed_in_recently' do + it 'returns a relation of users who have signed in during the recent period' do + recent_sign_in_user = Fabricate(:user, current_sign_in_at: within_duration_window_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 @@ -111,6 +135,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 diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 17067c58f5..a36d919b98 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -196,6 +196,7 @@ RSpec::Sidekiq.configure do |config| end RSpec::Matchers.define_negated_matcher :not_change, :change +RSpec::Matchers.define_negated_matcher :not_include, :include def request_fixture(name) Rails.root.join('spec', 'fixtures', 'requests', name).read