Fix service bundle task creation

app/models/miq_request_task.rb

@@ -77,7 +77,7 @@ def update_request_status
     req_status = status.slice('Error', 'Timeout', 'Warn').keys.first || 'Ok'
 
     if req_state == "finished" && state != "finished"
-      req_state = (req_status == 'Ok') ? 'provisioned' : "finished"
+      req_state = (req_status == 'Ok') ? completed_state : "finished"
       $log.info("Child tasks finished but current task still processing. Setting state to: [#{req_state}]...")
     end
 
@@ -94,6 +94,10 @@ def update_request_status
     update_and_notify_parent(:state => req_state, :status => req_status, :message => display_message(msg))
   end
 
+  def completed_state
+    "provisioned"
+  end
+
   def execute_callback(state, message, _result)
     unless state.to_s.downcase == "ok"
       update_and_notify_parent(:state => "finished", :status => "Error", :message => "Error: #{message}")

app/models/miq_retire_task.rb

@@ -86,6 +86,10 @@ def mark_pending_items_as_finished
     end
   end
 
+  def completed_state
+    "retired"
+  end
+
   def self.display_name(number = 1)
     n_('Retire Task', 'Retire Tasks', number)
   end

app/models/service.rb

@@ -152,6 +152,10 @@ def request_type
     'service_reconfigure'
   end
 
+  def retireable?
+    parent.present? ? true : type.present?
+  end
+
   alias root_service root
   alias services children
   alias direct_service_children children
@@ -218,10 +222,6 @@ def composite?
     children.present?
   end
 
-  def retireable?
-    type.present?
-  end
-
   def atomic?
     children.empty?
   end

app/models/service_retire_task.rb

@@ -26,7 +26,7 @@ def task_active
   def after_request_task_create
     update_attributes(:description => get_description)
     parent_svc = Service.find_by(:id => options[:src_ids])
-    _log.info("- creating service tasks for service <#{self.class.name}:#{id}>")
+    _log.info("- creating service subtasks for service task <#{self.class.name}:#{id}>, service <#{parent_svc.id}>")
     create_retire_subtasks(parent_svc, self)
   end
 
@@ -42,22 +42,22 @@ def create_retire_subtasks(parent_service, parent_task)
       # Initial Options[:dialog] to an empty hash so we do not pass down dialog values to child services tasks
       nh['options'][:dialog] = {}
       new_task = create_task(svc_rsc, parent_service, nh, parent_task)
-      create_retire_subtasks(svc_rsc.resource, new_task) if svc_rsc.resource.kind_of?(Service)
+      new_task.after_request_task_create
       miq_request.miq_request_tasks << new_task
       new_task.tap(&:deliver_to_automate)
     end.compact!
   end
 
   def create_task(svc_rsc, parent_service, nh, parent_task)
-    resource_type = svc_rsc.resource.type.presence || svc_rsc.resource.class.name
+    resource_type = svc_rsc.resource.type.present? ? svc_rsc.resource.type : "Service"
     (resource_type.demodulize + "RetireTask").constantize.new(nh).tap do |task|
       task.options.merge!(
-        :src_id              => svc_rsc.resource.id,
+        :src_ids             => svc_rsc.resource.id,
         :service_resource_id => svc_rsc.id,
         :parent_service_id   => parent_service.id,
         :parent_task_id      => parent_task.id,
       )
-      task.request_type = resource_type.demodulize.downcase + "_retire"
+      task.request_type = resource_type.demodulize.underscore.downcase + "_retire"
       task.source = svc_rsc.resource
       parent_task.miq_request_tasks << task
       task.save!

app/models/vm_retire_task.rb

@@ -9,4 +9,8 @@ def self.base_model
   def self.model_being_retired
     Vm
   end
+
+  def after_request_task_create
+    update_attributes(:description => get_description)
+  end
 end

spec/models/mixins/ci_feature_mixin_spec.rb

@@ -19,23 +19,10 @@
       expect(service.service_resources.first.resource.retireable?).to eq(false)
     end
 
-    context "service" do
-      context "with type" do
-        let(:service1) { FactoryBot.create(:service_ansible_tower, :type => ServiceAnsibleTower) }
-        it "is retireable" do
-          FactoryBot.create(:service_resource, :service => service, :resource => service1)
+    it "service is retireable" do
+      FactoryBot.create(:service_resource, :service => service, :resource => FactoryBot.create(:service_ansible_tower, :type => ServiceAnsibleTower))
 
-          expect(service.service_resources.first.resource.retireable?).to eq(true)
-        end
-      end
-
-      context "without type" do
-        it "is not retireable" do
-          FactoryBot.create(:service_resource, :service => service, :resource => FactoryBot.create(:service))
-
-          expect(service.service_resources.first.resource.retireable?).to eq(false)
-        end
-      end
+      expect(service.service_resources.first.resource.retireable?).to eq(true)
     end
   end
 end

spec/models/service_retire_task_spec.rb

@@ -8,17 +8,6 @@
   let(:approver) { FactoryBot.create(:user_miq_request_approver) }
   let(:zone) { FactoryBot.create(:zone, :name => "fred") }
 
-  shared_context "service_bundle" do
-    let(:zone) { FactoryBot.create(:small_environment) }
-    let(:service_c1) { FactoryBot.create(:service, :service => service) }
-
-    before do
-      allow(MiqServer).to receive(:my_server).and_return(zone.miq_servers.first)
-      @miq_request = FactoryBot.create(:service_retire_request, :requester => user)
-      @miq_request.approve(approver, reason)
-    end
-  end
-
   it "should initialize properly" do
     expect(service_retire_task).to have_attributes(:state => 'pending', :status => 'Ok')
   end
@@ -69,48 +58,27 @@
     end
 
     context "bundled service retires all children" do
-      include_context "service_bundle"
-      let(:vm1) { FactoryBot.create(:vm_vmware) }
-      let(:service_c2) { FactoryBot.create(:service, :service => service_c1) }
+      let(:service_c1) { FactoryBot.create(:service) }
 
       before do
-        service_c1 << vm
-        service_c2 << vm1
-        service.save
-        service_c1.save
-        service_c2.save
+        service.add_resource!(service_c1)
+        service.add_resource!(FactoryBot.create(:service_template))
+        @miq_request = FactoryBot.create(:service_retire_request, :requester => user)
+        @miq_request.approve(approver, reason)
+        @service_retire_task = FactoryBot.create(:service_retire_task, :source => service, :miq_request => @miq_request, :options => {:src_ids => [service.id] })
       end
 
-      it "creates subtask" do
-        @service_retire_task = FactoryBot.create(:service_retire_task, :source => service, :miq_request_task_id => nil, :miq_request_id => @miq_request.id, :options => {:src_ids => [service.id] })
-        service.service_resources << FactoryBot.create(:service_resource, :resource_type => "VmOrTemplate", :service_id => service_c1.id, :resource_id => vm.id)
-        service.service_resources << FactoryBot.create(:service_resource, :resource_type => "VmOrTemplate", :service_id => service_c1.id, :resource_id => vm1.id)
-        service.service_resources << FactoryBot.create(:service_resource, :resource_type => "Service", :service_id => service_c1.id, :resource_id => service_c1.id)
-        service.service_resources << FactoryBot.create(:service_resource, :resource_type => "ServiceTemplate", :service_id => service_c1.id, :resource_id => service_c1.id)
-
+      it "creates subtask for services but not templates" do
         @service_retire_task.after_request_task_create
-        expect(VmRetireTask.count).to eq(2)
-        expect(VmRetireTask.all.pluck(:message)).to eq(["Automation Starting", "Automation Starting"])
-        expect(ServiceRetireTask.count).to eq(1)
+
+        expect(ServiceRetireTask.count).to eq(2)
         expect(ServiceRetireRequest.count).to eq(1)
       end
 
       it "doesn't creates subtask for ServiceTemplates" do
-        @service_retire_task = FactoryBot.create(:service_retire_task, :source => service, :miq_request_task_id => nil, :miq_request_id => @miq_request.id, :options => {:src_ids => [service.id] })
-        service.service_resources << FactoryBot.create(:service_resource, :resource_type => "ServiceTemplate", :service_id => service_c1.id, :resource_id => service_c1.id)
-
         @service_retire_task.after_request_task_create
-        expect(ServiceRetireTask.count).to eq(1)
-        expect(ServiceRetireRequest.count).to eq(1)
-      end
 
-      it "doesn't creates subtask for service resources whose resources are nil" do
-        @service_retire_task = FactoryBot.create(:service_retire_task, :source => service, :miq_request_task_id => nil, :miq_request_id => @miq_request.id, :options => {:src_ids => [service.id] })
-        service.service_resources << FactoryBot.create(:service_resource, :resource_type => "ServiceTemplate", :service_id => service_c1.id, :resource => nil)
-
-        @service_retire_task.after_request_task_create
-        expect(ServiceRetireTask.count).to eq(1)
-        expect(ServiceRetireRequest.count).to eq(1)
+        expect(ServiceRetireTask.count).to eq(2)
       end
     end
   end