FIX: invalidating inactive admin emails should mark them as not active

FIX: invalidating inactive admin emails should mark them as not active
diff --git a/app/jobs/scheduled/invalidate_inactive_admins.rb b/app/jobs/scheduled/invalidate_inactive_admins.rb
index c31cea4..208a627 100644
--- a/app/jobs/scheduled/invalidate_inactive_admins.rb
+++ b/app/jobs/scheduled/invalidate_inactive_admins.rb
@@ -11,6 +11,7 @@ module Jobs
         .where('last_seen_at < ?', SiteSetting.invalidate_inactive_admin_email_after_days.days.ago)
         .each do |user|
 
+        user.deactivate
         user.email_tokens.update_all(confirmed: false, expired: true)
 
         Discourse.authenticators.each do |authenticator|
diff --git a/spec/jobs/invalidate_inactive_admins_spec.rb b/spec/jobs/invalidate_inactive_admins_spec.rb
index 1e8bbae..62cacc3 100644
--- a/spec/jobs/invalidate_inactive_admins_spec.rb
+++ b/spec/jobs/invalidate_inactive_admins_spec.rb
@@ -11,7 +11,7 @@ describe Jobs::InvalidateInactiveAdmins do
   it "does nothing when all admins have been seen recently" do
     SiteSetting.invalidate_inactive_admin_email_after_days = 365
     subject
-    expect(active_admin.active).to eq(true)
+    expect(active_admin.reload.active).to eq(true)
     expect(active_admin.email_tokens.where(confirmed: true).exists?).to eq(true)
   end
 
@@ -24,14 +24,14 @@ describe Jobs::InvalidateInactiveAdmins do
         SiteSetting.invalidate_inactive_admin_email_after_days = 365
       end
 
-      it 'marks email tokens as unconfirmed and keeps user as active' do
+      it 'marks email tokens as unconfirmed' do
         subject
-        expect(not_seen_admin.email_tokens.where(confirmed: true).exists?).to eq(false)
+        expect(not_seen_admin.reload.email_tokens.where(confirmed: true).exists?).to eq(false)
       end
 
-      it 'keeps the user active' do
+      it 'makes the user as not active' do
         subject
-        expect(not_seen_admin.active).to eq(true)
+        expect(not_seen_admin.reload.active).to eq(false)
       end
 
       context 'with social logins' do
@@ -59,9 +59,9 @@ describe Jobs::InvalidateInactiveAdmins do
 
       it 'does nothing' do
         subject
-        expect(active_admin.active).to eq(true)
+        expect(active_admin.reload.active).to eq(true)
         expect(active_admin.email_tokens.where(confirmed: true).exists?).to eq(true)
-        expect(not_seen_admin.active).to eq(true)
+        expect(not_seen_admin.reload.active).to eq(true)
         expect(not_seen_admin.email_tokens.where(confirmed: true).exists?).to eq(true)
       end
     end

GitHub

2 Likes

Should the following actions happen in a transaction?

2 Likes

DEV: add transaction and active check to invalidate job