Add check for existence of user for system_context

app/models/mixins/retirement_mixin.rb

@@ -253,12 +253,12 @@ def valid_zone?
   end
 
   def system_context_requester
-    if try(:evm_owner_id).present?
-      User.find(evm_owner_id)
-    else
-      $log.info("System context defaulting to admin user because owner of #{name} not set.")
-      User.super_admin
+    if evm_owner.blank?
+      $log.info("System context defaulting to admin user because owner of #{name} (#{self.class}) not set or owner no longer found in database.")
+      return User.super_admin
     end
+    $log.info("Setting retirement requester of #{name} to #{evm_owner_id}.")
+    evm_owner
   end
 
   def current_user

spec/models/orchestration_stack/retirement_management_spec.rb

@@ -1,6 +1,8 @@
 describe "Service Retirement Management" do
-  let!(:user) { FactoryBot.create(:user_miq_request_approver, :userid => "admin") }
+  let(:user) { FactoryBot.create(:user_miq_request_approver) }
   let(:orchestration_stack) { FactoryBot.create(:orchestration_stack) }
+  let(:stack_with_owner) { FactoryBot.create(:orchestration_stack, :evm_owner => user) }
+
   context "with zone/ems" do
     before do
       @miq_server = EvmSpecHelper.local_miq_server
@@ -9,14 +11,37 @@
       @stack = FactoryBot.create(:orchestration_stack, :ext_management_system => ems)
     end
 
-    it "#retirement_check" do
-      expect(MiqEvent).to receive(:raise_evm_event)
-      @stack.update_attributes(:retires_on => 90.days.ago, :retirement_warn => 60, :retirement_last_warn => nil)
-      expect(@stack.retirement_last_warn).to be_nil
-      @stack.retirement_check
-      @stack.reload
-      expect(@stack.retirement_last_warn).not_to be_nil
-      expect(@stack.retirement_requester).to eq(user.userid)
+    describe "#retirement_check" do
+      context "with user" do
+        it "uses user as requester" do
+          expect(MiqEvent).to receive(:raise_evm_event)
+          stack_with_owner.update_attributes(:retires_on => 90.days.ago, :retirement_warn => 60, :retirement_last_warn => nil)
+          expect(stack_with_owner.retirement_last_warn).to be_nil
+          stack_with_owner.retirement_check
+          stack_with_owner.reload
+          expect(stack_with_owner.retirement_last_warn).not_to be_nil
+          expect(stack_with_owner.retirement_requester).to eq(user.userid)
+        end
+      end
+
+      context "with deleted user" do
+        before do
+          # system_context_retirement relies on the presence of a user with this userid
+          FactoryBot.create(:user, :userid => 'admin', :role => 'super_administrator')
+          user.destroy
+          stack_with_owner.reload
+        end
+
+        it "uses admin as requester" do
+          expect(MiqEvent).to receive(:raise_evm_event)
+          stack_with_owner.update_attributes(:retires_on => 90.days.ago, :retirement_warn => 60, :retirement_last_warn => nil)
+          expect(stack_with_owner.retirement_last_warn).to be_nil
+          stack_with_owner.retirement_check
+          stack_with_owner.reload
+          expect(stack_with_owner.retirement_last_warn).not_to be_nil
+          expect(stack_with_owner.retirement_requester).to eq("admin")
+        end
+      end
     end
 
     it "#start_retirement" do

spec/models/service/retirement_management_spec.rb

@@ -1,40 +1,44 @@
 describe "Service Retirement Management" do
-  let!(:user) { FactoryBot.create(:user_miq_request_approver, :userid => 'admin') }
-  let(:service_without_owner) { FactoryBot.create(:service) }
-  let(:service3) { FactoryBot.create(:service) }
+  let(:user) { FactoryBot.create(:user_miq_request_approver) }
+  let(:service_with_owner) { FactoryBot.create(:service, :evm_owner => user) }
+
   before do
     @server = EvmSpecHelper.local_miq_server
-    @service = FactoryBot.create(:service, :evm_owner_id => user.id)
+    @service = FactoryBot.create(:service)
   end
 
   # shouldn't be running make_retire_request because it's the bimodal not from ui part
-  context "with user" do
-    it "#retirement_check" do
-      User.with_user(user) do
+  describe "#retirement_check" do
+    context "with user" do
+      it "#retirement_check" do
+        expect(MiqEvent).to receive(:raise_evm_event)
+        service_with_owner.update_attributes(:retires_on => 90.days.ago, :retirement_warn => 60, :retirement_last_warn => nil)
+        expect(service_with_owner.retirement_last_warn).to be_nil
+        service_with_owner.retirement_check
+        service_with_owner.reload
+        expect(service_with_owner.retirement_last_warn).not_to be_nil
+        expect(service_with_owner.retirement_requester).to eq(user.userid)
+      end
+    end
+
+    context "without user" do
+      before do
+        User.super_admin || FactoryBot.create(:user, :userid => 'admin', :role => 'super_administrator')
+        user.destroy
+      end
+      it "#retirement_check" do
         expect(MiqEvent).to receive(:raise_evm_event)
+
         @service.update_attributes(:retires_on => 90.days.ago, :retirement_warn => 60, :retirement_last_warn => nil)
         expect(@service.retirement_last_warn).to be_nil
         @service.retirement_check
         @service.reload
         expect(@service.retirement_last_warn).not_to be_nil
-        expect(@service.retirement_requester).to eq(user.userid)
+        expect(@service.retirement_requester).to eq('admin')
       end
     end
   end
 
-  context "without user" do
-    it "#retirement_check" do
-      expect(MiqEvent).to receive(:raise_evm_event)
-      service_without_owner.update_attributes(:retires_on => 90.days.ago, :retirement_warn => 60, :retirement_last_warn => nil)
-      expect(service_without_owner.retirement_last_warn).to be_nil
-      service_without_owner.retirement_check
-      service_without_owner.reload
-      expect(service_without_owner.retirement_last_warn).not_to be_nil
-      expect(service_without_owner.retirement_requester).to eq(user.userid)
-      expect(MiqRequest.first.userid).to eq("admin")
-    end
-  end
-
   it "#start_retirement" do
     expect(@service.retirement_state).to be_nil
     @service.start_retirement
@@ -99,13 +103,13 @@
   end
 
   it "with one src_id" do
-    expect(ServiceRetireRequest).to receive(:make_request).with(nil, {:src_ids => [service3.id], :__request_type__ => "service_retire"}, user, true)
-    @service.class.to_s.demodulize.constantize.make_retire_request(service3.id, user)
+    expect(ServiceRetireRequest).to receive(:make_request).with(nil, {:src_ids => [@service.id], :__request_type__ => "service_retire"}, user, true)
+    @service.class.to_s.demodulize.constantize.make_retire_request(@service.id, user)
   end
 
-  it "with many src_ids" do
-    expect(ServiceRetireRequest).to receive(:make_request).with(nil, {:src_ids => [@service.id, service3.id, service_without_owner.id], :__request_type__ => "service_retire"}, user, true)
-    @service.class.to_s.demodulize.constantize.make_retire_request(@service.id, service3.id, service_without_owner.id, user)
+  it "with src_ids" do
+    expect(ServiceRetireRequest).to receive(:make_request).with(nil, {:src_ids => [@service.id, service_with_owner.id], :__request_type__ => "service_retire"}, user, true)
+    @service.class.to_s.demodulize.constantize.make_retire_request(@service.id, service_with_owner.id, user)
   end
 
   it "#retire date" do
@@ -147,12 +151,12 @@
   end
 
   it "#finish_retirement" do
-    message = "Service: [#{service3.name}], Retires On: [#{Time.zone.now.strftime("%x %R %Z")}], has been retired"
-    expect(service3).to receive(:raise_audit_event).with("service_retired", message, nil)
+    message = "Service: [#{@service.name}], Retires On: [#{Time.zone.now.strftime("%x %R %Z")}], has been retired"
+    expect(@service).to receive(:raise_audit_event).with("service_retired", message, nil)
 
-    service3.finish_retirement
+    @service.finish_retirement
 
-    expect(service3.retirement_state).to eq("retired")
+    expect(@service.retirement_state).to eq("retired")
   end
 
   it "#mark_retired" do

spec/models/vm/retirement_management_spec.rb

@@ -1,22 +1,46 @@
 describe "VM Retirement Management" do
-  let!(:user) { FactoryBot.create(:user_miq_request_approver) }
+  let(:user) { FactoryBot.create(:user_miq_request_approver) }
+  let(:vm_with_owner) { FactoryBot.create(:vm, :evm_owner => user) }
   let(:region) { FactoryBot.create(:miq_region, :region => ApplicationRecord.my_region_number) }
   let(:vm2) { FactoryBot.create(:vm) }
 
   before do
     @zone = EvmSpecHelper.local_miq_server.zone
     @ems = FactoryBot.create(:ems_vmware, :zone => @zone)
-    @vm = FactoryBot.create(:vm_vmware, :ems_id => @ems.id, :evm_owner => user)
-  end
+    @vm = FactoryBot.create(:vm_vmware, :ems_id => @ems.id)
+  end
+
+  describe "#retirement_check" do
+    context "with user" do
+      it "uses user as requester" do
+        expect(MiqEvent).to receive(:raise_evm_event)
+        vm_with_owner.update_attributes(:retires_on => 90.days.ago, :retirement_warn => 60, :retirement_last_warn => nil)
+        expect(vm_with_owner.retirement_last_warn).to be_nil
+        vm_with_owner.retirement_check
+        vm_with_owner.reload
+        expect(vm_with_owner.retirement_last_warn).not_to be_nil
+        expect(vm_with_owner.retirement_requester).to eq(user.userid)
+      end
+    end
 
-  it "#retirement_check" do
-    expect(MiqEvent).to receive(:raise_evm_event).once
-    @vm.update_attributes(:retires_on => 90.days.ago, :retirement_warn => 60, :retirement_last_warn => nil)
-    expect(@vm.retirement_last_warn).to be_nil
-    @vm.retirement_check
-    @vm.reload
-    expect(@vm.retirement_last_warn).not_to be_nil
-    expect(@vm.retirement_requester).to eq(user.userid)
+    context "without user" do
+      before do
+        # system_context_retirement relies on the presence of a user with this userid
+        FactoryBot.create(:user, :userid => 'admin', :role => 'super_administrator')
+        user.destroy
+        vm_with_owner.reload
+      end
+
+      it "uses admin as requester" do
+        expect(MiqEvent).to receive(:raise_evm_event)
+        vm_with_owner.update_attributes(:retires_on => 90.days.ago, :retirement_warn => 60, :retirement_last_warn => nil)
+        expect(vm_with_owner.retirement_last_warn).to be_nil
+        vm_with_owner.retirement_check
+        vm_with_owner.reload
+        expect(vm_with_owner.retirement_last_warn).not_to be_nil
+        expect(vm_with_owner.retirement_requester).to eq('admin')
+      end
+    end
   end
 
   it "#start_retirement" do