[Ovirt-devel] [PATCH server] Cloud UI layer to initiate actions on vms.
Jason Guiditta
jguiditt at redhat.com
Mon Jun 29 17:52:30 UTC 2009
Signed-off-by: Jason Guiditta <jguiditt at redhat.com>
---
src/app/controllers/cloud/cloud_controller.rb | 20 +++++-
src/app/controllers/cloud/instance_controller.rb | 25 +++----
src/app/views/cloud/instance/_show.rhtml | 1 -
src/app/views/cloud/instance/index.rhtml | 30 +++++---
src/app/views/layouts/cloud/_notification.rhtml | 31 ++++++++
src/public/stylesheets/cloud/layout.css | 73 +++++++++++++++++++-
.../functional/cloud/instance_controller_test.rb | 65 +++++++++++++++++-
7 files changed, 214 insertions(+), 31 deletions(-)
create mode 100644 src/app/views/layouts/cloud/_notification.rhtml
diff --git a/src/app/controllers/cloud/cloud_controller.rb b/src/app/controllers/cloud/cloud_controller.rb
index 154d15d..19c56b6 100644
--- a/src/app/controllers/cloud/cloud_controller.rb
+++ b/src/app/controllers/cloud/cloud_controller.rb
@@ -26,9 +26,27 @@ class Cloud::CloudController < ApplicationController
protected
+ # Override the default in ApplicationController, cloud has its own
+ # template for rendering these errors
+ def html_error_page(title, msg)
+ flash[:error] = msg
+ redirect_to params
+ end
+
+ # Override the default in ApplicationController, cloud has its own
+ # way of handling these (hooked into VmService::svc_vm_actions
+ def handle_partial_success_error(error)
+ handle_error(:error => error, :status => :ok,
+ :message => {:summary => error.message,
+ :failures => error.failures,
+ :successes => error.successes},
+ :title => "Some actions failed")
+ end
# NOTE: This probably will/should be moved to use set_perms in
- # ApplicationService once that is ready to go.
+ # ApplicationService once that is ready to go. Only problem with that
+ # idea is that there is currently no before filter to make sure that
+ # gets called.
def set_vars
@user = get_login_user
end
diff --git a/src/app/controllers/cloud/instance_controller.rb b/src/app/controllers/cloud/instance_controller.rb
index a4d0fd8..130f30b 100644
--- a/src/app/controllers/cloud/instance_controller.rb
+++ b/src/app/controllers/cloud/instance_controller.rb
@@ -44,7 +44,7 @@ class Cloud::InstanceController < Cloud::CloudController
task_order = find_in_params_or_default(:task_order, "tasks.id")
@vm_details = Vm.find(ids) if ids
if @vm_details
- @tasks = VmTask.paginate(:conditions => ["task_target_id in (:ids)",{:ids => ids}],
+ @tasks = VmTask.paginate(:include => :vm, :conditions => ["task_target_id in (:ids)",{:ids => ids}],
:per_page => 5, :page => task_page, :order => task_order)
end
end
@@ -76,19 +76,18 @@ class Cloud::InstanceController < Cloud::CloudController
return params[key] && params[key] != "" ? params[key] : default
end
-# This redirects the user to a get url if they are just trying to view details for one or more
-# instances.
-# TODO: if the user is trying to submit an acton on selected instances, call the service
-# layer and display the :flash (might still want to do :get redirect to keep pagination/sorting
-# correct.
+# This implements the Post/Redirect/Get pattern, and redirects the user to
+# a get url so an action is not accidentally posted twice.
def handle_form
- case params[:submit_for_list]
- when "Show Selected"
- params.delete(:submit_for_list)
- redirect_to :action => "index", :params => params
- return
-# Do this if we have submitted an action on one or more vms.
-# svc_vm_action(params[:ids], params[:vm_action], params[:action_args])
+ if params[:submit_for_list]
+ submit_type = params[:submit_for_list]
+ params.delete(:submit_for_list)
+ if params[:ids].nil?
+ flash[:warning] = "You must select at least one instance to perform an action."
+ elsif submit_type != "Show Selected"
+ flash[:notice] = svc_vm_actions(params[:ids], submit_type.downcase << '_vm', params[:action_args])
+ end
+ redirect_to params
end
end
diff --git a/src/app/views/cloud/instance/_show.rhtml b/src/app/views/cloud/instance/_show.rhtml
index 001286e..186c4ba 100644
--- a/src/app/views/cloud/instance/_show.rhtml
+++ b/src/app/views/cloud/instance/_show.rhtml
@@ -1,7 +1,6 @@
<div>
<div id="detail_header">
<%= submit_tag 'Show Selected', :id => 'submit_for_list', :name => 'submit_for_list' %>
- </form>
<% if @vm_details %>
<h3>
<% if @vm_details.size == 1 %>
diff --git a/src/app/views/cloud/instance/index.rhtml b/src/app/views/cloud/instance/index.rhtml
index 757d67c..a787e39 100644
--- a/src/app/views/cloud/instance/index.rhtml
+++ b/src/app/views/cloud/instance/index.rhtml
@@ -1,23 +1,29 @@
+<form action="<%= url_for({:action => 'index'})%>" method="post">
<div id="toolbar">
- <!-- TODO: Make each li a submit button with same styling as current li.
- Handlng of this will be implemented in InstanceController::handle_form
- -->
<ul>
<li>New Instance</li>
<li>
Actions
- <ul>
- <% @actions.each {|action| %>
- <li><%= image_tag action[2]%><%= action[0] %></li>
- <% } %>
- </ul>
+ <%=%>
+ <ul>
+ <% @actions.each {|action| %>
+
+ <li>
+ <%= image_tag action[2]%>
+ <%= submit_tag action[0], :name => 'submit_for_list', :class => 'button_as_link' %>
+ </li>
+ <% } %>
+ </ul>
</li>
</ul>
</div>
-<form action="<%= url_for({:action => 'index'})%>" method="post"> <%# This form tag is terminated in _show.rhtml %>
<div id="list-view">
- <%= render :partial => 'list' %>
+ <% if !flash.empty? %>
+ <%= render :partial => '/layouts/cloud/notification' %>
+ <% end %>
+ <%= render :partial => 'list' %>
</div>
<div id="detail-view">
- <%= render :partial => 'show' %>
- </div>
\ No newline at end of file
+ <%= render :partial => 'show' %>
+ </div>
+</form>
\ No newline at end of file
diff --git a/src/app/views/layouts/cloud/_notification.rhtml b/src/app/views/layouts/cloud/_notification.rhtml
new file mode 100644
index 0000000..96042fc
--- /dev/null
+++ b/src/app/views/layouts/cloud/_notification.rhtml
@@ -0,0 +1,31 @@
+<div id="notification" <%if flash[:error]%>class="error-align"<%end%>">
+ <%= link_to "", params, :id => "close-notify" %>
+ <% if flash[:error] %>
+ <% if !flash[:error][:successes].empty? %>
+ <div class="success">
+ <h4>The following items were successful:</h4>
+ <ul>
+ <% flash[:error][:successes].each {|item| %>
+ <li><%= item %></li>
+ <% } %>
+ </ul>
+ </div>
+ <% end %>
+ <% if !flash[:error][:failures].empty? %>
+ <div class="warning"><h4><%= flash[:error][:summary]%></h4></div>
+ <div class="error">
+ <ul>
+ <% flash[:error][:failures].each {|key, value| %>
+ <li><%= key %>: <%= value %></li>
+ <% } %>
+ </ul>
+ </div>
+ <% end %>
+ <% end %>
+ <% if flash[:warning] %>
+ <div class="warning"><h4><%= flash[:warning] %></h4></div>
+ <% end %>
+ <% if flash[:notice] %>
+ <div class="success"><ul><li><%= flash[:notice] %></li></ul></div>
+ <% end %>
+</div>
\ No newline at end of file
diff --git a/src/public/stylesheets/cloud/layout.css b/src/public/stylesheets/cloud/layout.css
index 0fd158d..1d1fa07 100644
--- a/src/public/stylesheets/cloud/layout.css
+++ b/src/public/stylesheets/cloud/layout.css
@@ -28,6 +28,14 @@ html {min-width:640px;}
color: #337ACC;
}
+/* Make buttons look like links */
+.button_as_link {
+ background: none;
+ color: #666666;
+ border: none;
+ cursor: pointer;
+}
+
#navcontainer {
height:40px;
}
@@ -205,8 +213,10 @@ input#submit_for_list {
#toolbar li:hover {
background: #337ACC;
+}
+
+#toolbar li:hover > .button_as_link {
color: #FFFFFF;
- cursor: pointer;
}
#toolbar li.current { /* This is not really needed right now, but will be useful when we add filters. */
@@ -234,3 +244,64 @@ input#submit_for_list {
}
#toolbar li:hover ul {display: block;}
+
+/****** Notification styles ******/
+#notification {
+ -webkit-border-radius: 0.2em;
+ -moz-border-radius: 0.2em;
+ position: absolute;
+ z-index: 1;
+ top: 2em;
+ left: 35%;
+ margin: 0 auto;
+ padding: 1.5em;
+ background: #337ACC;
+ border: solid 0.2em;
+}
+
+.error-align {left: 25% !important;}
+
+#notification > div {
+ margin-bottom: 1em;
+}
+
+#notification h4 {
+ text-indent: 3em;
+}
+
+#close-notify {
+ position: absolute;
+ right: 0.2em;
+ top: 0.2em;
+ width:11px;
+ height:11px;
+ background: url(/images/closebox-11px.png) no-repeat;
+}
+
+.success {
+ width: 100%;
+ background:url(/images/icon_conf_success.png) left center no-repeat #CCFFCC;
+ border: solid 0.2em green;
+ -webkit-border-radius: 0.2em;
+ -moz-border-radius: 0.2em;
+}
+
+.success li, .error li {
+ margin-left:3em;
+}
+
+.error {
+ width: 100%;
+ background:url(/images/icon_conf_error.png) left center no-repeat #EC9999;
+ border: solid 0.2em red;
+ -webkit-border-radius: 0.2em;
+ -moz-border-radius: 0.2em;
+}
+
+.warning {
+ width: 100%;
+ background:url(/images/icon_conf_warning.png) left center no-repeat #FFFFCC;
+ border: solid 0.2em yellow;
+ -webkit-border-radius: 0.2em;
+ -moz-border-radius: 0.2em;
+}
diff --git a/src/test/functional/cloud/instance_controller_test.rb b/src/test/functional/cloud/instance_controller_test.rb
index 213d2e0..71d132b 100644
--- a/src/test/functional/cloud/instance_controller_test.rb
+++ b/src/test/functional/cloud/instance_controller_test.rb
@@ -1,6 +1,27 @@
-require 'test_helper'
+#
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Jason Guiditta <jguiditt at redhat.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA 02110-1301, USA. A copy of the GNU General Public License is
+# also available at http://www.gnu.org/copyleft/gpl.html.
+
+require File.dirname(__FILE__) + '/../../test_helper'
class Cloud::InstanceControllerTest < ActionController::TestCase
+
+ include ServiceModuleHelper
fixtures :vms, :tasks
def setup
@controller = Cloud::InstanceController.new
@@ -14,10 +35,48 @@ class Cloud::InstanceControllerTest < ActionController::TestCase
assert_template 'index'
assert_not_nil assigns(:vms)
assert_not_nil assigns(:user)
+ assert_equal({}, flash, "flash object is not empty!")
+ assert_equal(true, flash.empty?)
+ assert_select "#notification", false, "This page must contain no notification block"
end
- def test_should_redirect_if_request_for_selected
+ def test_should_redirect_if_form_submitted
post(:index,{:submit_for_list => 'Show Selected'})
- assert_response :redirect
+ assert_redirected_to :action => :index
+ #TODO: since this is a redirect, it returns a 302 and nil template,
+ # would be good to figure out how to get the actual result returned to
+ # browser so we can do an assert_select
+ end
+
+ def test_add_valid_task
+ post(:index,{:submit_for_list => 'Shutdown', :ids => [vms(:production_mysqld_vm).id]})
+ assert_equal('shutdown_vm successful.', flash[:notice])
+ assert_redirected_to :action => :index, :ids => vms(:production_mysqld_vm).id
+ end
+
+ def test_add_invalid_task
+ post(:index,{:submit_for_list => 'Shutdown',
+ :ids => [vms(:production_mysqld_vm).id,vms(:production_httpd_vm).id]})
+ expected_failures = {:summary => "Your request to shutdown_vm encountered the following errors: ",
+ :failures => {vms(:production_httpd_vm).description => "shutdown_vm cannot be performed on this vm."},
+ :successes => ["#{vms(:production_mysqld_vm).description}: shutdown_vm was successfully queued."]}
+ assert_equal(expected_failures, flash[:error])
+ assert_redirected_to :action => :index, :ids => [vms(:production_mysqld_vm).id,vms(:production_httpd_vm).id]
+ end
+
+ def test_add_task_no_perms_for_some_vms
+ post(:index,{:submit_for_list => 'Shutdown',
+ :ids => [vms(:production_mysqld_vm).id,vms(:corp_com_qa_postgres_vm).id]})
+ expected_failures = {:summary => "Your request to shutdown_vm encountered the following errors: ",
+ :failures => {vms(:corp_com_qa_postgres_vm).description => "You have insufficient privileges to perform action."},
+ :successes => ["#{vms(:production_mysqld_vm).description}: shutdown_vm was successfully queued."]}
+ assert_equal(expected_failures, flash[:error])
+ assert_redirected_to :action => :index, :ids => [vms(:production_mysqld_vm).id,vms(:corp_com_qa_postgres_vm).id]
+ end
+
+ # Make sure we get warning back if no instance was chosen to perform an action on.
+ def test_no_instance_chosen
+ post(:index,{:submit_for_list => 'Shutdown'})
+ assert_equal("You must select at least one instance to perform an action.", flash[:warning])
end
end
--
1.6.0.6
More information about the ovirt-devel
mailing list