Refactored pools destroy and multi_destroy

[Jirka Tomasek]

Pools destroy and multi_destroy to resemble deployments actions, added
validation when no pool is selected and javascript is turned off.

[PATCH] Refactored pools destroy and multi destroy to resemble deployments

[Jirka Tomasek]

From: Jiri Tomasek

---
 src/app/controllers/pools_controller.rb |   41 +++++++++++-------------------
 src/config/locales/en.yml               |    1 +
 2 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/src/app/controllers/pools_controller.rb b/src/app/controllers/pools_controller.rb
index 745eb5b..9f95d22 100644
--- a/src/app/controllers/pools_controller.rb
+++ b/src/app/controllers/pools_controller.rb
@@ -156,21 +156,15 @@ class PoolsController < ApplicationController
   end

def destroy
-    destroyed = []
-    failed = []
-    error_messages = []
-    Pool.find(ids_list('pools_selected')).each do |pool|
-      if pool.id == MetadataObject.lookup("self_service_default_pool").id
-        error_messages << "The default pool cannot be deleted"
-      elsif check_privilege(Privilege::MODIFY, pool) && pool.destroyable?
-        pool.destroy
-        destroyed << pool.name
-      else
-        failed << pool.name
-      end
+    pool = Pool.find(params[:id])
+    if pool.id == MetadataObject.lookup("self_service_default_pool").id
+      flash[:error] = "The default pool cannot be deleted"
+    elsif check_privilege(Privilege::MODIFY, pool) && pool.destroyable?
+      pool.destroy
+      flash[:success] = t('pools.index.pool_deleted', :list => pool.name, :count => 1)
+    else
+      flash[:error] = t('pools.index.pool_not_deleted', :list => pool.name, :count => 1)
     end
-    flash[:success] = t('pools.index.pool_deleted', :list => destroyed.to_sentence, :count => destroyed.size) if destroyed.present?
-    flash[:error] = t('pools.index.pool_not_deleted', :list => failed.to_sentence, :count => failed.size) if failed.present?
     respond_to do |format|
       # TODO - What is expected to be returned on an AJAX delete?
       format.js do
@@ -186,7 +180,7 @@ class PoolsController < ApplicationController
     destroyed = []
     failed = []
     error_messages = []
-    Pool.find(params[:pools_selected]).each do |pool|
+    Pool.find(params[:pools_selected] || []).each do |pool|
       # FIXME: remove this check when pools can be assigned to new users
       # default_pool cannot be deleted because metadata object has it tied
       # to id of 1 and deleting it prevents new users from being created
@@ -199,18 +193,13 @@ class PoolsController < ApplicationController
         failed << pool.name
       end
     end
-
-    unless destroyed.empty?
-      flash[:notice] = t('pools.index.pool_deleted', :count => destroyed.length, :list => destroyed.join(', '))
-    end
-    unless failed.empty?
-      error_messages << t('pools.index.pool_not_deleted', :count => failed.length, :list => failed.join(', '))
-    end
-    unless error_messages.empty?
-      flash[:error] = error_messages.join('<br />')
-    end
+    # If nothing is selected, display error message:
+    flash[:error] = t('pools.index.none_selected') if failed.blank? && destroyed.blank? && error_messages.blank?
+    flash[:success] = t('pools.index.pool_deleted', :count => destroyed.length, :list => destroyed.join(', ')) if destroyed.present?
+    error_messages << t('pools.index.pool_not_deleted', :count => failed.length, :list => failed.join(', ')) if failed.present?
+    flash[:error] = error_messages.join('<br />') unless error_messages.empty?
     respond_to do |format|
-      format.html { redirect_to pools_url }
+      format.html { redirect_to pools_url(:view => 'filter', :details_tab => 'pools') }
       format.json { render :json => {:success => destroyed, :errors => failed} }
     end
   end
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 8e9b61e..b92ded6 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -142,6 +142,7 @@ en:
       pool_not_deleted:
         one: "Pool %{list} was not deleted. There are instances associated with it."
         other: "Pools %{list} were not deleted. They have instances associated with them."
+      none_selected: "You must select one or more pools to delete."
   hardware_profiles:
     index:
       hardware_profile_name: Hardware Profile Name

[Hugh O. Brock]

On Fri, Jul 22, 2011 at 01:06:23PM +0200, [e-mail] wrote:
> Pools destroy and multi_destroy to resemble deployments actions, added
> validation when no pool is selected and javascript is turned off.

Gratuitous nag: Please don't forget to add [PATCH 0/n] to the subject line
of the patch description mail. Thanks!

--Hugh