diff --git a/app/jobs/scheduled/clean_up_inactive_users.rb b/app/jobs/scheduled/clean_up_inactive_users.rb index 801f05c4fb..99579dbab9 100644 --- a/app/jobs/scheduled/clean_up_inactive_users.rb +++ b/app/jobs/scheduled/clean_up_inactive_users.rb @@ -9,10 +9,11 @@ module Jobs destroyer = UserDestroyer.new(Discourse.system_user) User.joins("LEFT JOIN posts ON posts.user_id = users.id") - .where(last_posted_at: nil) - .where("posts.user_id IS NULL") - .where("users.last_seen_at < ?", SiteSetting.clean_up_inactive_users_after_days.days.ago) - .where(trust_level: 0) + .where(last_posted_at: nil, trust_level: TrustLevel.levels[:newuser]) + .where( + "posts.user_id IS NULL AND users.last_seen_at < ?", + SiteSetting.clean_up_inactive_users_after_days.days.ago + ) .find_each do |user| begin diff --git a/spec/jobs/clean_up_inactive_users_spec.rb b/spec/jobs/clean_up_inactive_users_spec.rb index 5d01233fe3..7790f050ae 100644 --- a/spec/jobs/clean_up_inactive_users_spec.rb +++ b/spec/jobs/clean_up_inactive_users_spec.rb @@ -1,30 +1,35 @@ require 'rails_helper' RSpec.describe Jobs::CleanUpInactiveUsers do - context 'when user is inactive' do - let(:user) { Fabricate(:user) } - let(:active_user) { Fabricate(:active_user) } + it "should clean up new users that have been inactive" do + SiteSetting.clean_up_inactive_users_after_days = 0 - it 'should clean up the user' do - user.update!(last_seen_at: 3.years.ago, trust_level: 0) - active_user + user = Fabricate(:user, + last_seen_at: 5.days.ago, + trust_level: TrustLevel.levels[:newuser] + ) - expect { described_class.new.execute({}) }.to change { User.count }.by(-1) - expect(User.find_by(id: user.id)).to eq(nil) - end - end + Fabricate(:active_user) - context 'when user is not inactive' do + Fabricate(:post, user: Fabricate(:user, + trust_level: TrustLevel.levels[:newuser], + last_seen_at: 5.days.ago + )).user - let!(:active_user_1) { Fabricate(:post, user: Fabricate(:user, trust_level: 0)).user } - let!(:active_user_2) { Fabricate(:user, trust_level: 0, last_seen_at: 2.days.ago) } - let!(:active_user_3) { Fabricate(:user, trust_level: 1) } + Fabricate(:user, + trust_level: TrustLevel.levels[:newuser], + last_seen_at: 2.days.ago + ) - it 'should not clean up active users' do - expect { described_class.new.execute({}) }.to_not change { User.count } - expect(User.find_by(id: active_user_1.id)).to_not eq(nil) - expect(User.find_by(id: active_user_2.id)).to_not eq(nil) - expect(User.find_by(id: active_user_3.id)).to_not eq(nil) - end + Fabricate(:user, trust_level: TrustLevel.levels[:basic]) + + expect { described_class.new.execute({}) }.to_not change { User.count } + + SiteSetting.clean_up_inactive_users_after_days = 4 + + expect { described_class.new.execute({}) } + .to change { User.count }.by(-1) + + expect(User.exists?(id: user.id)).to eq(false) end end