Fixes following agrare and carbonin guidance

ManageIQ · Oct 2, 2018 · 04ca79a · 04ca79a
1 parent 659be65
commit 04ca79a
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 25 deletions.
diff --git a/db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb b/db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb
@@ -5,29 +5,25 @@ class MiqRequestTask < ActiveRecord::Base
     belongs_to :conversion_host, :class_name => "AddConversionHostIdToMiqRequestTasks::ConversionHost"
   end
 
-  class ServiceTemplateTransformationPlanTask < MiqRequestTask
-  end
-
   class ConversionHost < ActiveRecord::Base
     self.inheritance_column = :_type_disabled
-    has_many :service_template_transformation_plan_tasks, :class_name => "AddConversionHostIdToMiqRequestTasks::MiqRequestTask"
   end
 
   class Host < ActiveRecord::Base
     self.inheritance_column = :_type_disabled
-    has_many :tags, :class_name => "AddConversionHostIdToMiqRequestTasks::Tag"
   end
 
   def up
     add_column :miq_request_tasks, :conversion_host_id, :bigint
 
-    ServiceTemplateTransformationPlanTask.all.reject { |task| task.options[:transformation_host_id].nil? }.each do |task|
-      host = Host.find_by(:id => task.options[:transformation_host_id])
-      task.conversion_host = ConversionHost.where(:id => task.options[:transformation_host_id]).first_or_create do |ch|
+    MiqRequestTask.where(:type => 'ServiceTemplateTransformationPlanTask').each do |task|
+      host_id = task.options[:transformation_host_id]
+      next unless host_id
+      host = Host.find_by(:id => host_id)
+      next if host.nil?
+      task.conversion_host = ConversionHost.find_or_create_by!(:resource_id => host_id) do |ch|
         ch.name                     = host.name
         ch.resource_type            = host.type
-        ch.resource_id              = host.id
-        ch.address                  = host.ipaddress
         ch.vddk_transport_supported = true
         ch.ssh_transport_supported  = false
       end
@@ -37,8 +33,9 @@ def up
   end
 
   def down
-    ServiceTemplateTransformationPlanTask.all.select { |task| task.conversion_host.present? }.each do |task|
-      task.options[:transformation_host_id] = task.conversion_host.id
+    MiqRequestTask.where(:type => 'ServiceTemplateTransformationPlanTask').each do |task|
+      next unless task.conversion_host
+      task.options[:transformation_host_id] = task.conversion_host.resource_id
       ConversionHost.find(task.conversion_host.id).destroy!
       task.save!
     end

diff --git a/spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb b/spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb
@@ -7,40 +7,37 @@
 
   migration_context :up do
     it "creates conversion host" do
-      task = task_stub.create!
       host = host_stub.create!
-      conversion_host = conversion_host_stub.create!(
-        :resource_id   => host.id,
-        :resource_type => host.type
+      task = task_stub.create!(
+        :type    => 'ServiceTemplateTransformationPlanTask',
+        :options => { :transformation_host_id => host.id }
       )
-      task.options = { :transformation_host_id => conversion_host.id }
-      task.save
 
       migrate
 
       task.reload
       expect(task.options).to eq({})
-      expect(task.conversion_host).to eq(conversion_host)
+      expect(AddConversionHostIdToMiqRequestTasks::ConversionHost.find_by(:resource_id => host.id)).not_to be_nil
+      expect(task.conversion_host).to eq(AddConversionHostIdToMiqRequestTasks::ConversionHost.find_by(:resource_id => host.id))
     end
   end
 
   migration_context :down do
     it "updates task.options" do
-      task = task_stub.create!
       host = host_stub.create!
       conversion_host = conversion_host_stub.create!(
         :resource_id   => host.id,
         :resource_type => host.type
       )
-      conversion_host.save
-      task.conversion_host = conversion_host
-      task.save
+      task = task_stub.create!(
+        :type    => 'ServiceTemplateTransformationPlanTask',
+        :conversion_host => conversion_host
+      )
 
       migrate
 
       task.reload
-      expect(task.attributes).not_to include('conversion_host_id')
-      expect(task.options[:transformation_host_id]).to eq(conversion_host.id)
+      expect(task.options[:transformation_host_id]).to eq(host.id)
       expect { conversion_host.reload }.to raise_error(ActiveRecord::RecordNotFound)
     end
   end