[Ovirt-devel] [PATCH server] fixed bug in 'move items' that prevented moving hosts or storage. Also added separate handler for PartialSuccessException to facilitate better notification of partial successes.

[Scott Seago]

Signed-off-by: Scott Seago <sseago at redhat.com>
---
 src/app/controllers/application.rb         |   10 ++++++++++
 src/app/controllers/hardware_controller.rb |    2 +-
 src/app/models/host.rb                     |    3 +++
 src/app/models/storage_pool.rb             |    4 ++++
 src/app/services/application_service.rb    |    1 +
 src/app/services/hardware_pool_service.rb  |    5 +++--
 6 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/src/app/controllers/application.rb b/src/app/controllers/application.rb
index 6932a1f..4834915 100644
--- a/src/app/controllers/application.rb
+++ b/src/app/controllers/application.rb
@@ -50,6 +50,7 @@ class ApplicationController < ActionController::Base
   # to most specific
   rescue_from Exception, :with => :handle_general_error
   rescue_from PermissionError, :with => :handle_perm_error
+  rescue_from PartialSuccessError, :with => :handle_partial_success_error
 
   def choose_layout
     if(params[:component_layout])
@@ -124,6 +125,15 @@ class ApplicationController < ActionController::Base
                  :title => "Access denied")
   end
 
+  def handle_partial_success_error(error)
+    failures_arr = error.failures.collect do |resource, reason|
+      resource.display_name + ": " + reason
+    end
+    handle_error(:error => error, :status => :ok,
+                 :message => error.message + ": " + failures_arr.join(", "),
+                 :title => "Some actions failed")
+  end
+
   def handle_general_error(error)
     handle_error(:error => error, :status => :internal_server_error,
                  :title => "Internal Server Error")
diff --git a/src/app/controllers/hardware_controller.rb b/src/app/controllers/hardware_controller.rb
index 0b6cf9b..7ef08db 100644
--- a/src/app/controllers/hardware_controller.rb
+++ b/src/app/controllers/hardware_controller.rb
@@ -214,7 +214,7 @@ class HardwareController < PoolController
     edit_items(params[:target_pool_id], :svc_move_storage, :move)
   end
 
-  def edit_items(svc_method, target_pool_id, item_action)
+  def edit_items(target_pool_id, svc_method, item_action)
     alert = send(svc_method, params[:id], params[:resource_ids].split(","),
                  target_pool_id)
     render :json => { :success => true, :alert => alert,
diff --git a/src/app/models/host.rb b/src/app/models/host.rb
index 0665c3f..588137b 100644
--- a/src/app/models/host.rb
+++ b/src/app/models/host.rb
@@ -142,4 +142,7 @@ class Host < ActiveRecord::Base
      return vms.size == 0
   end
 
+  def not_movable_reason
+    return "Host has VMs"
+  end
 end
diff --git a/src/app/models/storage_pool.rb b/src/app/models/storage_pool.rb
index 92548bd..40333d0 100644
--- a/src/app/models/storage_pool.rb
+++ b/src/app/models/storage_pool.rb
@@ -165,4 +165,8 @@ class StoragePool < ActiveRecord::Base
     }
     return true
   end
+
+  def not_movable_reason
+    return "Storage in use"
+  end
 end
diff --git a/src/app/services/application_service.rb b/src/app/services/application_service.rb
index 7652996..f8085eb 100644
--- a/src/app/services/application_service.rb
+++ b/src/app/services/application_service.rb
@@ -34,6 +34,7 @@ module ApplicationService
     def initialize(msg, failures={}, successes=[])
       @failures = failures
       @successes = successes
+      super(msg)
     end
   end
 
diff --git a/src/app/services/hardware_pool_service.rb b/src/app/services/hardware_pool_service.rb
index c9aa70b..51519fa 100644
--- a/src/app/services/hardware_pool_service.rb
+++ b/src/app/services/hardware_pool_service.rb
@@ -69,7 +69,7 @@ module HardwarePoolService
     resources.each do |resource|
       begin
         if !resource.movable?
-          failed_resources[resource] = "Not Movable"
+          failed_resources[resource] = resource.not_movable_reason
         elsif ! resource.hardware_pool.can_modify(@user)
           failed_resources[resource] = "Failed permission check"
         else
@@ -82,7 +82,8 @@ module HardwarePoolService
       end
     end
     unless failed_resources.empty?
-      raise PartialSuccessError.new("Move #{item_class.table_name.humanize} only partially successful",
+      raise PartialSuccessError.new("Move failed for some " +
+                                    "#{item_class.table_name.humanize}",
                                     failed_resources, successful_resources)
     end
     return "Move #{item_class.table_name.humanize} successful."
-- 
1.6.0.6