Hey Steve,
ACK, works fine. I have a few suggestions about the code style (inline).
Feel free to ignore them and push anyway.
Thomas
On 04/01/2011 06:04 AM, Steve Linabery wrote:
From: Steve Linabery<slinabery(a)gmail.com>
Added multi_stop method to instances_controller.rb, changed form to send correct params,
edited routes.rb to match new path.
---
.../controllers/resources/instances_controller.rb | 55 ++++++++++++++------
src/app/views/resources/instances/_list.haml | 4 +-
src/config/routes.rb | 2 +-
src/features/instance.feature | 14 +++++
src/features/step_definitions/instance_steps.rb | 6 ++-
5 files changed, 61 insertions(+), 20 deletions(-)
diff --git a/src/app/controllers/resources/instances_controller.rb
b/src/app/controllers/resources/instances_controller.rb
index f857aa7..7b26383 100644
--- a/src/app/controllers/resources/instances_controller.rb
+++ b/src/app/controllers/resources/instances_controller.rb
@@ -1,6 +1,6 @@
class Resources::InstancesController< ApplicationController
before_filter :require_user, :except => [:can_start, :can_create]
- before_filter :load_instance, :only => [:show, :remove_failed, :key, :stop]
+ before_filter :load_instance, :only => [:show, :key]
before_filter :set_view_vars, :only => [:show, :index]
def index
@@ -106,27 +106,50 @@ class Resources::InstancesController< ApplicationController
redirect_to resources_instance_path(@instance)
end
- def stop
- unless @instance.valid_action?('stop')
- raise ActionError.new("stop is an invalid action.")
- end
+ def multi_stop
+ notices = ""
+ errors = ""
+ Instance.find(params[:instance_selected]).each do |instance|
+ begin
+ require_privilege(Privilege::USE,instance)
+ unless instance.valid_action?('stop')
+ raise ActionError.new("stop is an invalid action.")
+ end
- # not sure if task is used as everything goes through condor
- #permissons check here
- @task = @instance.queue_action(@current_user, 'stop')
- unless @task
- raise ActionError.new("stop cannot be performed on this instance.")
+ # not sure if task is used as everything goes through condor
+ #permissons check here
+ @task = instance.queue_action(@current_user, 'stop')
+ unless @task
+ raise ActionError.new("stop cannot be performed on this instance.")
+ end
+ condormatic_instance_stop(@task)
+ notices<< "#{instance.name}: stop action was successfully
queued.<br/>"
+ rescue Exception => err
+ errors<< "#{instance.name}: " + err +"<br/>"
+ end
end
- condormatic_instance_stop(@task)
- flash[:notice] = "#{(a)instance.name}: stop action was successfully
queued."
+ flash[:notice] = notices unless notices == ""
+ flash[:error] = errors unless errors == ""
Might I suggest using notices.blank? and errors.blank? here? I think
it's a tad more readable.
redirect_to resources_instances_path
end
+
You've added an empty line here. In case it was not intentional, could
you please remove it?
def remove_failed
- raise ActionError.new("remove failed cannot be performed on this
instance.") unless
- @instance.state == Instance::STATE_ERROR
- condormatic_instance_reset_error(@instance)
- flash[:notice] = "#{(a)instance.name}: remove failed action was successfully
queued."
+ notices = ""
+ errors = ""
+ Instance.find(params[:instance_selected]).each do |instance|
+ begin
+ require_privilege(Privilege::USE,instance)
+ raise ActionError.new("remove failed cannot be performed on this
instance.") unless
+ instance.state == Instance::STATE_ERROR
+ condormatic_instance_reset_error(instance)
+ notices<< "#{instance.name}: remove failed action was successfully
queued."
+ rescue Exception => err
+ errors<< "#{instance.name}: " + err +"<br/>"
+ end
+ end
+ flash[:notice] = notices unless notices == ""
+ flash[:error] = errors unless errors == ""
I think the `blank?` method fits here more as well.
redirect_to resources_instances_path
end
diff --git a/src/app/views/resources/instances/_list.haml
b/src/app/views/resources/instances/_list.haml
index b62d9e7..157dc97 100644
--- a/src/app/views/resources/instances/_list.haml
+++ b/src/app/views/resources/instances/_list.haml
@@ -1,5 +1,5 @@
- form_tag do
- = restful_submit_tag 'Stop', 'stop', stop_resources_instances_path,
'GET'
+ = restful_submit_tag 'Stop', 'stop',
multi_stop_resources_instances_path, 'GET'
= restful_submit_tag 'Create', 'new',
select_template_resources_instances_path, 'GET'
= restful_submit_tag 'Remove failed', 'remove_failed',
remove_failed_resources_instances_path, 'GET'
@@ -14,7 +14,7 @@
%tr
%td
- selected = params[:select] == 'all'
- %input{:checked => selected, :name => 'id', :type =>
'checkbox', :value => inst.id, :id => "inst_ids_#{inst.id}" }
+ %input{:name => 'instance_selected[]', :type =>
'checkbox', :value => inst.id, :id =>
"instance_checkbox_#{inst.id}", :checked => selected }
= link_to inst.name, resources_instance_path(inst)
%td= inst.state
%td= inst.template.name
diff --git a/src/config/routes.rb b/src/config/routes.rb
index 313dd25..88e3f71 100644
--- a/src/config/routes.rb
+++ b/src/config/routes.rb
@@ -35,7 +35,7 @@ ActionController::Routing::Routes.draw do |map|
map.namespace 'resources' do |r|
r.resources :pools, :collection => { :multi_destroy => :delete }
r.resources :deployments
- r.resources :instances, :collection => {:start => :get, :stop => :get,
:select_template => :get, :remove_failed => :get, :can_start => :get,
:can_create => :get }, :member => {:key => :get}
+ r.resources :instances, :collection => {:start => :get, :multi_stop =>
:get, :select_template => :get, :remove_failed => :get, :can_start => :get,
:can_create => :get }, :member => {:key => :get}
end
map.can_start_instance
'/resources/instances/:instance_id/can_start/:provider_account_id', :controller
=> 'resources/instances', :action => 'can_start', :conditions
=> { :method => :get }
diff --git a/src/features/instance.feature b/src/features/instance.feature
index 19b80bd..3b4e000 100644
--- a/src/features/instance.feature
+++ b/src/features/instance.feature
@@ -77,6 +77,20 @@ Feature: Mange Instances
Then I should be on the instances page
And I should see "mock1: stop action was successfully queued"
+ Scenario: Stop multiple instances
+ Given there is a "mock1" running instance
+ And there is a "mock2" running instance
+ And there is a "mock3" stopped instance
+ And I am on the instances page
+ When I check "mock1" instance
+ And I check "mock2" instance
+ And I check "mock3" instance
+ And I press "Stop"
+ Then I should be on the instances page
+ And I should see "mock1: stop action was successfully queued"
+ And I should see "mock2: stop action was successfully queued"
+ And I should see "mock3: stop is an invalid action"
+
@tag
Scenario: Search for instances
Given there are the following instances:
diff --git a/src/features/step_definitions/instance_steps.rb
b/src/features/step_definitions/instance_steps.rb
index 66e1282..b7daa18 100644
--- a/src/features/step_definitions/instance_steps.rb
+++ b/src/features/step_definitions/instance_steps.rb
@@ -47,6 +47,10 @@ Given /^there is a "([^"]*)" running instance$/ do
|name|
Factory :instance, :name => name, :state => Instance::STATE_RUNNING
end
+Given /^there is a "([^"]*)" stopped instance$/ do |name|
+ Factory :instance, :name => name, :state => Instance::STATE_STOPPED
+end
+
Given /^there is an uploaded image for a template$/ do
Factory :provider_image
end
@@ -66,7 +70,7 @@ end
When /^I check "([^"]*)" instance$/ do |name|
inst = Instance.find_by_name(name)
- check("inst_ids_#{inst.id}")
+ check("instance_checkbox_#{inst.id}")
end
Given /^there are the following instances:$/ do |table|