FIX: Don't try to delete staged, unused admins and mods

app/jobs/scheduled/clean_up_unused_staged_users.rb

@@ -10,7 +10,7 @@ module Jobs
 
       User.joins("LEFT JOIN posts ON posts.user_id = users.id")
         .where("posts.user_id IS NULL")
-        .where(staged: true)
+        .where(staged: true, admin: false, moderator: false)
         .where("users.created_at < ?", 1.year.ago)
         .find_each do |user|
 

spec/jobs/clean_up_unused_staged_users_spec.rb

@@ -3,37 +3,52 @@
 require 'rails_helper'
 
 RSpec.describe Jobs::CleanUpUnusedStagedUsers do
-  fab!(:user) { Fabricate(:user) }
   fab!(:staged_user) { Fabricate(:user, staged: true) }
 
-  context 'when staged user is unused' do
+  shared_examples "does not delete" do
-    it 'should clean up the staged user' do
+    it "doesn't delete the staged user" do
-      user
+      expect { described_class.new.execute({}) }.to_not change { User.count }
-      staged_user.update!(created_at: 2.years.ago)
+      expect(User.exists?(staged_user.id)).to eq(true)
-
-      expect { described_class.new.execute({}) }.to change { User.count }.by(-1)
-      expect(User.find_by(id: staged_user.id)).to eq(nil)
     end
+  end
 
-    describe 'when staged user is not old enough' do
+  context "when staged user is unused" do
-      it 'should not clean up the staged user' do
+    context "when staged user is old enough" do
-        user
+      before { staged_user.update!(created_at: 2.years.ago) }
-        staged_user.update!(created_at: 5.months.ago)
 
-        expect { described_class.new.execute({}) }.to_not change { User.count }
+      context "regular staged user" do
-        expect(User.find_by(id: staged_user.id)).to eq(staged_user)
+        it "deletes the staged user" do
+          expect { described_class.new.execute({}) }.to change { User.count }.by(-1)
+          expect(User.exists?(staged_user.id)).to eq(false)
+        end
+      end
+
+      context "staged admin" do
+        before { staged_user.update!(admin: true) }
+        include_examples "does not delete"
+      end
+
+      context "staged moderator" do
+        before { staged_user.update!(moderator: true) }
+        include_examples "does not delete"
       end
     end
-  end
 
-  context 'when staged user is not unused' do
+    context 'when staged user is not old enough' do
-    it 'should not clean up the staged user' do
+      before { staged_user.update!(created_at: 5.months.ago) }
-      user
+      include_examples "does not delete"
-      Fabricate(:post, user: staged_user)
-      user.update!(created_at: 2.years.ago)
-
-      expect { described_class.new.execute({}) }.to_not change { User.count }
-      expect(User.find_by(id: staged_user.id)).to eq(staged_user)
     end
   end
+
+  context "when staged user has posts" do
+    before { Fabricate(:post, user: staged_user) }
+    include_examples "does not delete"
+  end
+
+  it "doesn't delete regular, unused user" do
+    user = Fabricate(:user, created_at: 2.years.ago)
+
+    expect { described_class.new.execute({}) }.to_not change { User.count }
+    expect(User.exists?(user.id)).to eq(true)
+  end
 end