Merge branch 'codyw-fix-dormantuser-cron' into 'master'

Fix Admin Mode bug in DeactivateDormantUsersWorker

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/136907



Merged-by: Alper Akgun <aakgun@gitlab.com>
Approved-by: Josianne Hyson <jhyson@gitlab.com>
Approved-by: Smriti Garg <sgarg@gitlab.com>
Reviewed-by: Josianne Hyson <jhyson@gitlab.com>
Reviewed-by: Huzaifa Iftikhar <hiftikhar@gitlab.com>
Co-authored-by: Cody West <cwest@gitlab.com>
Co-authored-by: Josianne Hyson <jhyson@gitlab.com>

app/workers/users/deactivate_dormant_users_worker.rb

@@ -18,8 +18,10 @@ def perform
       admin_bot = Users::Internal.admin_bot
       return unless admin_bot
 
-      deactivate_users(User.dormant, admin_bot)
-      deactivate_users(User.with_no_activity, admin_bot)
+      Gitlab::Auth::CurrentUserMode.bypass_session!(admin_bot.id) do
+        deactivate_users(User.dormant, admin_bot)
+        deactivate_users(User.with_no_activity, admin_bot)
+      end
     end
 
     private

spec/workers/users/deactivate_dormant_users_worker_spec.rb

@@ -10,34 +10,27 @@
     let_it_be(:inactive) { create(:user, last_activity_on: nil, created_at: User::MINIMUM_DAYS_CREATED.days.ago.to_date) }
     let_it_be(:inactive_recently_created) { create(:user, last_activity_on: nil, created_at: (User::MINIMUM_DAYS_CREATED - 1).days.ago.to_date) }
 
-    let(:admin_bot) { create(:user, :admin_bot) }
-    let(:deactivation_service) { instance_spy(Users::DeactivateService) }
-
-    before do
-      allow(Users::DeactivateService).to receive(:new).and_return(deactivation_service)
-    end
-
     subject(:worker) { described_class.new }
 
     it 'does not run for SaaS', :saas do
-      # Now makes a call to current settings to determine period of dormancy
-
       worker.perform
 
-      expect(deactivation_service).not_to have_received(:execute)
-    end
-
-    context 'when automatic deactivation of dormant users is enabled' do
-      before do
-        stub_application_setting(deactivate_dormant_users: true)
+      expect_any_instance_of(::Users::DeactivateService) do |deactivation_service|
+        expect(deactivation_service).not_to have_received(:execute)
       end
+    end
 
-      it 'deactivates dormant users' do
-        worker.perform
-
-        expect(deactivation_service).to have_received(:execute).twice
+    shared_examples 'deactivates dormant users' do
+      specify do
+        expect { worker.perform }
+          .to change { dormant.reload.state }
+          .to('deactivated')
+          .and change { inactive.reload.state }
+          .to('deactivated')
       end
+    end
 
+    shared_examples 'deactivates certain user types' do
       where(:user_type, :expected_state) do
         :human | 'deactivated'
         :support_bot | 'active'
@@ -52,33 +45,69 @@
       end
 
       with_them do
-        it 'deactivates certain user types' do
+        specify do
           user = create(:user, user_type: user_type, state: :active, last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date)
 
           worker.perform
 
-          if expected_state == 'deactivated'
-            expect(deactivation_service).to have_received(:execute).with(user)
-          else
-            expect(deactivation_service).not_to have_received(:execute).with(user)
+          expect_any_instance_of(::Users::DeactivateService) do |deactivation_service|
+            if expected_state == 'deactivated'
+              expect(deactivation_service).to receive(:execute).with(user).and_call_original
+            else
+              expect(deactivation_service).not_to have_received(:execute).with(user)
+            end
           end
+
+          expect(user.reload.state).to eq expected_state
         end
       end
+    end
 
-      it 'does not deactivate non-active users' do
+    shared_examples 'does not deactivate non-active users' do
+      specify do
         human_user = create(:user, user_type: :human, state: :blocked, last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date)
         service_user = create(:user, user_type: :service_user, state: :blocked, last_activity_on: Gitlab::CurrentSettings.deactivate_dormant_users_period.days.ago.to_date)
 
         worker.perform
 
-        expect(deactivation_service).not_to have_received(:execute).with(human_user)
-        expect(deactivation_service).not_to have_received(:execute).with(service_user)
+        expect_any_instance_of(::Users::DeactivateService) do |deactivation_service|
+          expect(deactivation_service).not_to have_received(:execute).with(human_user)
+          expect(deactivation_service).not_to have_received(:execute).with(service_user)
+        end
       end
+    end
 
-      it 'does not deactivate recently created users' do
+    shared_examples 'does not deactivate recently created users' do
+      specify do
         worker.perform
 
-        expect(deactivation_service).not_to have_received(:execute).with(inactive_recently_created)
+        expect_any_instance_of(::Users::DeactivateService) do |deactivation_service|
+          expect(deactivation_service).not_to have_received(:execute).with(inactive_recently_created)
+        end
+      end
+    end
+
+    context 'when automatic deactivation of dormant users is enabled' do
+      before do
+        stub_application_setting(deactivate_dormant_users: true)
+      end
+
+      context 'when admin mode is not enabled', :do_not_mock_admin_mode_setting do
+        include_examples 'deactivates dormant users'
+        include_examples 'deactivates certain user types'
+        include_examples 'does not deactivate non-active users'
+        include_examples 'does not deactivate recently created users'
+      end
+
+      context 'when admin mode is enabled', :request_store do
+        before do
+          stub_application_setting(admin_mode: true)
+        end
+
+        include_examples 'deactivates dormant users'
+        include_examples 'deactivates certain user types'
+        include_examples 'does not deactivate non-active users'
+        include_examples 'does not deactivate recently created users'
       end
     end
 
@@ -90,7 +119,9 @@
       it 'does nothing' do
         worker.perform
 
-        expect(deactivation_service).not_to have_received(:execute)
+        expect_any_instance_of(::Users::DeactivateService) do |deactivation_service|
+          expect(deactivation_service).not_to have_received(:execute)
+        end
       end
     end
   end