[PATCH aggregator] New Resources/Pools multi delete functionality

From: Ladislav Martincik <lmartinc(a)redhat.com&gt;

---
 src/app/controllers/resources/pools_controller.rb |    5 ++++
 src/app/models/pool.rb                            |    7 +----
 src/app/views/resources/pools/_list.haml          |   19 +++++++++++++++-
 src/config/routes.rb                              |    3 +-
 src/features/pool.feature                         |   14 ++++++++++++
 src/features/step_definitions/pool_steps.rb       |   23 +++++++++++++++++++++
 6 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/src/app/controllers/resources/pools_controller.rb
b/src/app/controllers/resources/pools_controller.rb
index 02ba8f4..4e627be 100644
--- a/src/app/controllers/resources/pools_controller.rb
+++ b/src/app/controllers/resources/pools_controller.rb
@@ -61,6 +61,11 @@ class Resources::PoolsController < ApplicationController
     end
   end
 
+  def multi_destroy
+    Pool.destroy(params[:pools_selected])
+    redirect_to resources_pools_url
+  end
+
   protected
 
   def load_pools
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 262f744..d35f578 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -25,7 +25,8 @@ class Pool < ActiveRecord::Base
   belongs_to :quota
   belongs_to :pool_family
 
-  has_many :images,  :dependent => :destroy
+  # NOTE: Commented out because images table doesn't have pool_id foreign key?!
+  #has_many :images,  :dependent => :destroy
   has_many :hardware_profiles,  :dependent => :destroy
 
   validates_presence_of :name
@@ -48,10 +49,6 @@ class Pool < ActiveRecord::Base
     end
   end
 
-  def images
-    Image.find(:all, :conditions => {:provider_id => nil})
-  end
-
   def hardware_profiles
     HardwareProfile.find(:all, :conditions => {:provider_id => nil})
   end
diff --git a/src/app/views/resources/pools/_list.haml
b/src/app/views/resources/pools/_list.haml
index 605dbd7..558ff19 100644
--- a/src/app/views/resources/pools/_list.haml
+++ b/src/app/views/resources/pools/_list.haml
@@ -1,17 +1,18 @@
 - form_tag do
   = link_to "New Pool", new_resources_pool_path, { :class =>
'button' }
+  = restful_submit_tag "Destroy", 'destroy',
multi_destroy_resources_pools_path, 'DELETE', :id => 'delete_button'
   %p
     Select:&nbsp;
     = link_to "All", @url_params.merge(:select => 'all')
     %span> ,&nbsp;
     = link_to "None", @url_params.merge(:select => 'none')
-  %table
+  %table#pools_table
     = sortable_table_header @header
     - @pools.each do |pool|
       %tr
         %td
           - selected = @url_params[:select] == 'all'
-          = check_box(:pool, "selected[#{pool.id}]", :checked => selected)
+          %input{:name => "pools_selected[]", :type =>
"checkbox", :value => pool.id, :id =>
"pool_checkbox_#{pool.id}", :checked => selected }
           = link_to pool.name, resources_pool_path(pool)
         %td
           = pool.quota.maximum_running_instances or 'unlimited'
@@ -20,3 +21,17 @@
           ='%'
         %td
           = pool.pool_family.name
+
+:javascript
+  $(document).ready(function () {
+    $('#delete_button').click(function(e) {
+      if ($("#pools_table input[@type=radio]:checked").length == 0) {
+        alert('Please select any pool to be deleted before clicking Delete
button.');
+        e.preventDefault();
+      } else {
+        if (!confirm("Are you sure you want to delete this user?")) {
+          e.preventDefault();
+        }
+      }
+    });
+  });
diff --git a/src/config/routes.rb b/src/config/routes.rb
index b513d80..6d9b5cf 100644
--- a/src/config/routes.rb
+++ b/src/config/routes.rb
@@ -33,7 +33,8 @@ ActionController::Routing::Routes.draw do |map|
   # -- just remember to delete public/index.html.
 
   map.namespace 'resources' do |r|
-    r.resources :pools, :deployments
+    r.resources :pools, :collection => { :multi_destroy => :delete }
+    r.resources :deployments
     r.resources :instances, :collection => {:start => :get, :stop => :get,
:select_template => :get, :remove_failed => :get}, :member => {:key => :get}
   end
 
diff --git a/src/features/pool.feature b/src/features/pool.feature
index a3329dd..d38238d 100644
--- a/src/features/pool.feature
+++ b/src/features/pool.feature
@@ -41,3 +41,17 @@ Feature: Manage Pools
     When I fill in "pool[name]" with "@%&*())_@!#!"
     And I press "Save"
     Then I should see "Name must only contain: numbers, letters, spaces, '_'
and '-'"
+
+  Scenario: Delete pools
+    Given there's no pools
+    And a pool "Amazon Startrek Pool" exists
+    And a pool "Redhat Voyager Pool" exists
+    And I am on the resources pools page
+    And there are 2 pools
+    When I check "Redhat Voyager Pool" pool
+    And I check "Amazon Startrek Pool" pool
+    And I press "Destroy"
+    Then there should only be 0 pools
+    And I should be on the resources pools page
+    And I should not see "Redhat Voyager Pool"
+    And I should not see "Amazon Startrek Pool"
diff --git a/src/features/step_definitions/pool_steps.rb
b/src/features/step_definitions/pool_steps.rb
index 9f4a19c..b719f38 100644
--- a/src/features/step_definitions/pool_steps.rb
+++ b/src/features/step_definitions/pool_steps.rb
@@ -8,14 +8,37 @@ Given /^I have Pool Creator permissions on a pool named
"([^\"]*)"$/ do |name|
   Factory(:pool_creator_permission, :user => @user, :permission_object => @pool)
 end
 
+Given /^there's no pools$/ do
+  Pool.delete_all
+end
+
+Given /^there are (\d+) pools$/ do |number|
+  Pool.count.should == number.to_i
+end
+
 Given /^there is not a pool named "([^\"]*)"$/ do |name|
   Pool.find_by_name(name).should be_nil
 end
 
+Given /^a pool "([^"]*)" exists$/ do |pool_name|
+  pool_family = PoolFamily.find_by_name('default') || Factory(:pool_family)
+  quota = Quota.first || Factory(:quota)
+  Pool.create!(:name => pool_name, :pool_family => pool_family, :quota =>
quota)
+end
+
 Then /^I should have a pool named "([^\"]*)"$/ do |name|
   Pool.find_by_name(name).should_not be_nil
 end
 
+When /^(?:|I )check "([^"]*)" pool$/ do |pool_name|
+  pool = Pool.find_by_name(pool_name)
+  check("pool_checkbox_#{pool.id}")
+end
+
+Then /^there should only be (\d+) pools$/ do |number|
+  Pool.count.should == number.to_i
+end
+
 Then /^I should see the following:$/ do |table|
   table.raw.each do |array|
     array.each do |text|
-- 
1.7.3.2

[Tomas Sedovic]

ACK with a small nit inline.


On 01/06/2011 01:27 PM, lmartinc(a)redhat.com wrote:
>  From: Ladislav Martincik<lmartinc(a)redhat.com>
>
>  ---
>    src/app/controllers/resources/pools_controller.rb |    5 ++++
>    src/app/models/pool.rb                            |    7 +----
>    src/app/views/resources/pools/_list.haml          |   19 +++++++++++++++-
>    src/config/routes.rb                              |    3 +-
>    src/features/pool.feature                         |   14 ++++++++++++
>    src/features/step_definitions/pool_steps.rb       |   23 +++++++++++++++++++++
>    6 files changed, 63 insertions(+), 8 deletions(-)
>
>  diff --git a/src/app/controllers/resources/pools_controller.rb
> b/src/app/controllers/resources/pools_controller.rb
>  index 02ba8f4..4e627be 100644
>  --- a/src/app/controllers/resources/pools_controller.rb
>  +++ b/src/app/controllers/resources/pools_controller.rb
>  @@ -61,6 +61,11 @@ class Resources::PoolsController<  ApplicationController
>        end
>      end
>
>  +  def multi_destroy
>  +    Pool.destroy(params[:pools_selected])
>  +    redirect_to resources_pools_url
>  +  end
>  +
>      protected
>
>      def load_pools
>  diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
>  index 262f744..d35f578 100644
>  --- a/src/app/models/pool.rb
>  +++ b/src/app/models/pool.rb
>  @@ -25,7 +25,8 @@ class Pool<  ActiveRecord::Base
>      belongs_to :quota
>      belongs_to :pool_family
>
>  -  has_many :images,  :dependent =>  :destroy
>  +  # NOTE: Commented out because images table doesn't have pool_id foreign key?!
>  +  #has_many :images,  :dependent =>  :destroy
>      has_many :hardware_profiles,  :dependent =>  :destroy
>
>      validates_presence_of :name
>  @@ -48,10 +49,6 @@ class Pool<  ActiveRecord::Base
>        end
>      end
>
>  -  def images
>  -    Image.find(:all, :conditions =>  {:provider_id =>  nil})
>  -  end
>  -
>      def hardware_profiles
>        HardwareProfile.find(:all, :conditions =>  {:provider_id =>  nil})
>      end
>  diff --git a/src/app/views/resources/pools/_list.haml
> b/src/app/views/resources/pools/_list.haml
>  index 605dbd7..558ff19 100644
>  --- a/src/app/views/resources/pools/_list.haml
>  +++ b/src/app/views/resources/pools/_list.haml
>  @@ -1,17 +1,18 @@
>    - form_tag do
>      = link_to "New Pool", new_resources_pool_path, { :class => 
> 'button' }
>  +  = restful_submit_tag "Destroy", 'destroy',
> multi_destroy_resources_pools_path, 'DELETE', :id =>  'delete_button'
>      %p
>        Select:&nbsp;
>        = link_to "All", @url_params.merge(:select =>  'all')
>        %span>  ,&nbsp;
>        = link_to "None", @url_params.merge(:select =>  'none')
>  -  %table
>  +  %table#pools_table
>        = sortable_table_header @header
>        - @pools.each do |pool|
>          %tr
>            %td
>              - selected = @url_params[:select] == 'all'
>  -          = check_box(:pool, "selected[#{pool.id}]", :checked => 
> selected)
>  +          %input{:name =>  "pools_selected[]", :type => 
> "checkbox", :value =>  pool.id, :id => 
> "pool_checkbox_#{pool.id}", :checked =>  selected }
>              = link_to pool.name, resources_pool_path(pool)
>            %td
>              = pool.quota.maximum_running_instances or 'unlimited'
>  @@ -20,3 +21,17 @@
>              ='%'
>            %td
>              = pool.pool_family.name
>  +
>  +:javascript
>  +  $(document).ready(function () {
>  +    $('#delete_button').click(function(e) {
>  +      if ($("#pools_table input[@type=radio]:checked").length == 0) {
>  +        alert('Please select any pool to be deleted before clicking Delete
> button.');
>  +        e.preventDefault();
>  +      } else {
>  +        if (!confirm("Are you sure you want to delete this user?")) {
>  +          e.preventDefault();
>  +        }
>  +      }
>  +    });
>  +  });
>  diff --git a/src/config/routes.rb b/src/config/routes.rb
>  index b513d80..6d9b5cf 100644
>  --- a/src/config/routes.rb
>  +++ b/src/config/routes.rb
>  @@ -33,7 +33,8 @@ ActionController::Routing::Routes.draw do |map|
>      # -- just remember to delete public/index.html.
>
>      map.namespace 'resources' do |r|
>  -    r.resources :pools, :deployments
>  +    r.resources :pools, :collection =>  { :multi_destroy =>  :delete }
>  +    r.resources :deployments
>        r.resources :instances, :collection =>  {:start =>  :get, :stop =>  :get,
> :select_template =>  :get, :remove_failed =>  :get}, :member =>  {:key => 
> :get}
>      end
>
>  diff --git a/src/features/pool.feature b/src/features/pool.feature
>  index a3329dd..d38238d 100644
>  --- a/src/features/pool.feature
>  +++ b/src/features/pool.feature
>  @@ -41,3 +41,17 @@ Feature: Manage Pools
>        When I fill in "pool[name]" with "@%&*())_@!#!"
>        And I press "Save"
>        Then I should see "Name must only contain: numbers, letters, spaces,
> '_' and '-'"
>  +
>  +  Scenario: Delete pools
>  +    Given there's no pools 
I think "there are no pools" is a bit better when it comes to English 
grammar.

>  +    And a pool "Amazon Startrek Pool" exists
>  +    And a pool "Redhat Voyager Pool" exists
>  +    And I am on the resources pools page
>  +    And there are 2 pools
>  +    When I check "Redhat Voyager Pool" pool
>  +    And I check "Amazon Startrek Pool" pool
>  +    And I press "Destroy"
>  +    Then there should only be 0 pools
>  +    And I should be on the resources pools page
>  +    And I should not see "Redhat Voyager Pool"
>  +    And I should not see "Amazon Startrek Pool"
>  diff --git a/src/features/step_definitions/pool_steps.rb
> b/src/features/step_definitions/pool_steps.rb
>  index 9f4a19c..b719f38 100644
>  --- a/src/features/step_definitions/pool_steps.rb
>  +++ b/src/features/step_definitions/pool_steps.rb
>  @@ -8,14 +8,37 @@ Given /^I have Pool Creator permissions on a pool named
> "([^\"]*)"$/ do |name|
>      Factory(:pool_creator_permission, :user =>  @user, :permission_object => 
> @pool)
>    end
>
>  +Given /^there's no pools$/ do 
Same here, please change it to "there are no pools".

>  +  Pool.delete_all
>  +end
>  +
>  +Given /^there are (\d+) pools$/ do |number|
>  +  Pool.count.should == number.to_i
>  +end
>  +
>    Given /^there is not a pool named "([^\"]*)"$/ do |name|
>      Pool.find_by_name(name).should be_nil
>    end
>
>  +Given /^a pool "([^"]*)" exists$/ do |pool_name|
>  +  pool_family = PoolFamily.find_by_name('default') || Factory(:pool_family)
>  +  quota = Quota.first || Factory(:quota)
>  +  Pool.create!(:name =>  pool_name, :pool_family =>  pool_family, :quota => 
> quota)
>  +end
>  +
>    Then /^I should have a pool named "([^\"]*)"$/ do |name|
>      Pool.find_by_name(name).should_not be_nil
>    end
>
>  +When /^(?:|I )check "([^"]*)" pool$/ do |pool_name|
>  +  pool = Pool.find_by_name(pool_name)
>  +  check("pool_checkbox_#{pool.id}")
>  +end
>  +
>  +Then /^there should only be (\d+) pools$/ do |number|
>  +  Pool.count.should == number.to_i
>  +end
>  +
>    Then /^I should see the following:$/ do |table|
>      table.raw.each do |array|
>        array.each do |text|