DEV: Refactor tests for `Jobs::CleanUpInactiveUsers`.

* Remove use of 0 in favor of `TrustLevel.levels[:newuser]`.
* Consolidate two tests into a single one.
* Test that disabling the feature works.
* Avoid loading full ActiveRecord object in test when we only need to
know the existence of the record.

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(last_posted_at: nil, trust_level: TrustLevel.levels[:newuser])
-        .where("posts.user_id IS NULL")
+        .where(
-        .where("users.last_seen_at < ?", SiteSetting.clean_up_inactive_users_after_days.days.ago)
+          "posts.user_id IS NULL AND users.last_seen_at < ?",
-        .where(trust_level: 0)
+          SiteSetting.clean_up_inactive_users_after_days.days.ago
+        )
         .find_each do |user|
 
         begin

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
+  it "should clean up new users that have been inactive" do
-    let(:user) { Fabricate(:user) }
+    SiteSetting.clean_up_inactive_users_after_days = 0
-    let(:active_user) { Fabricate(:active_user) }
 
-    it 'should clean up the user' do
+    user = Fabricate(:user,
-      user.update!(last_seen_at: 3.years.ago, trust_level: 0)
+      last_seen_at: 5.days.ago,
-      active_user
+      trust_level: TrustLevel.levels[:newuser]
+    )
 
-      expect { described_class.new.execute({}) }.to change { User.count }.by(-1)
+    Fabricate(:active_user)
-      expect(User.find_by(id: user.id)).to eq(nil)
-    end
-  end
 
-  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 }
+    Fabricate(:user,
-    let!(:active_user_2) { Fabricate(:user, trust_level: 0, last_seen_at: 2.days.ago) }
+      trust_level: TrustLevel.levels[:newuser],
-    let!(:active_user_3) { Fabricate(:user, trust_level: 1) }
+      last_seen_at: 2.days.ago
+    )
+
+    Fabricate(:user, trust_level: TrustLevel.levels[:basic])
 
-    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)
+    SiteSetting.clean_up_inactive_users_after_days = 4
-      expect(User.find_by(id: active_user_3.id)).to_not eq(nil)
+
-    end
+    expect { described_class.new.execute({}) }
+      .to change { User.count }.by(-1)
+
+    expect(User.exists?(id: user.id)).to eq(false)
   end
 end