Scott Seago
2008-May-27 16:20 UTC
[Ovirt-devel] [PATCH] added confirmation dialogs for remaining actions
-------------- next part -------------- A non-text attachment was scrubbed... Name: confirmation-messages.patch Type: text/x-patch Size: 16857 bytes Desc: not available URL: <http://listman.redhat.com/archives/ovirt-devel/attachments/20080527/b144c339/attachment.bin>
Scott Seago
2008-May-27 17:34 UTC
[Ovirt-devel] [PATCH] added confirmation dialogs for remaining actions
Scott Seago wrote:> > ------------------------------------------------------------------------ > > _______________________________________________ > Ovirt-devel mailing list > Ovirt-devel at redhat.com > https://www.redhat.com/mailman/listinfo/ovirt-develMissed an error in the patch...
Scott Seago
2008-May-27 17:35 UTC
[Ovirt-devel] [PATCH] added confirmation dialogs for remaining actions
Scott Seago wrote:> > ------------------------------------------------------------------------ > > _______________________________________________ > Ovirt-devel mailing list > Ovirt-devel at redhat.com > https://www.redhat.com/mailman/listinfo/ovirt-develTrying again with the patch sufficiently mangled... -------------- next part -------------- A non-text attachment was scrubbed... Name: confirmation-messages-2.patch Type: text/x-patch Size: 1007 bytes Desc: not available URL: <http://listman.redhat.com/archives/ovirt-devel/attachments/20080527/05ade5c6/attachment.bin>
Hugh O. Brock
2008-May-27 17:49 UTC
[Ovirt-devel] [PATCH] added confirmation dialogs for remaining actions
On Tue, May 27, 2008 at 12:20:04PM -0400, Scott Seago wrote:>> >From cd827b580ca56c8b552acf21cb2d3f457dcfbe0a Mon Sep 17 00:00:00 2001 > From: Scott Seago <sseago at redhat.com> > Date: Tue, 27 May 2008 12:17:51 -0400 > Subject: [PATCH] added confirmation dialogs for remaining actions > > > Signed-off-by: Scott Seago <sseago at redhat.com> > --- > wui/src/app/controllers/hardware_controller.rb | 52 ++++++++++++++++------ > wui/src/app/controllers/permission_controller.rb | 38 ++++++++++----- > wui/src/app/controllers/resources_controller.rb | 19 +++++--- > wui/src/app/controllers/storage_controller.rb | 18 +++++--- > wui/src/app/controllers/vm_controller.rb | 17 +++++--- > wui/src/app/views/hardware/move.rhtml | 5 ++- > wui/src/app/views/hardware/show_hosts.rhtml | 5 ++- > wui/src/app/views/hardware/show_storage.rhtml | 10 +++- > wui/src/app/views/hardware/show_vms.rhtml | 5 ++- > wui/src/app/views/host/addhost.html.erb | 5 ++- > wui/src/app/views/layouts/_side_toolbar.rhtml | 2 +- > wui/src/app/views/resources/show_vms.rhtml | 5 ++- > wui/src/app/views/storage/addstorage.html.erb | 5 ++- > wui/src/app/views/user/_show.rhtml | 12 ++++- > 14 files changed, 141 insertions(+), 57 deletions(-) > > diff --git a/wui/src/app/controllers/hardware_controller.rb b/wui/src/app/controllers/hardware_controller.rb > index a052885..ad857f1 100644 > --- a/wui/src/app/controllers/hardware_controller.rb > +++ b/wui/src/app/controllers/hardware_controller.rb > @@ -310,11 +310,17 @@ class HardwareController < ApplicationController > def add_hosts > host_ids_str = params[:host_ids] > host_ids = host_ids_str.split(",").collect {|x| x.to_i} > - > - @pool.transaction do > - @pool.move_hosts(host_ids, @pool.id) > + > + begin > + @pool.transaction do > + @pool.move_hosts(host_ids, @pool.id) > + end > + render :json => { :object => "host", :success => true, > + :alert => "Hosts were successfully added to this Hardware pool." } > + rescue > + render :json => { :object => "host", :success => false, > + :alert => "Error adding Hosts to this Hardware pool." } > end > - render :text => "added hosts (#{host_ids.join(', ')})" > end > > #FIXME: we need permissions checks. user must have permission on src pool > @@ -325,10 +331,16 @@ class HardwareController < ApplicationController > host_ids_str = params[:resource_ids] > host_ids = host_ids_str.split(",").collect {|x| x.to_i} > > - @pool.transaction do > - @pool.move_hosts(host_ids, target_pool_id) > + begin > + @pool.transaction do > + @pool.move_hosts(host_ids, target_pool_id) > + end > + render :json => { :object => "host", :success => true, > + :alert => "Hosts were successfully moved." } > + rescue > + render :json => { :object => "host", :success => false, > + :alert => "Error moving hosts." } > end > - render :text => "added hosts (#{host_ids.join(', ')})" > end > > #FIXME: we need permissions checks. user must have permission on src pool > @@ -338,10 +350,16 @@ class HardwareController < ApplicationController > storage_pool_ids_str = params[:storage_pool_ids] > storage_pool_ids = storage_pool_ids_str.split(",").collect {|x| x.to_i} > > - @pool.transaction do > - @pool.move_storage(storage_pool_ids, @pool.id) > + begin > + @pool.transaction do > + @pool.move_storage(storage_pool_ids, @pool.id) > + end > + render :json => { :object => "storage_pool", :success => true, > + :alert => "Storage Pools were successfully added to this Hardware pool." } > + rescue > + render :json => { :object => "storage_pool", :success => false, > + :alert => "Error adding storage pools to this Hardware pool." } > end > - render :text => "added storage (#{storage_pool_ids.join(', ')})" > end > > #FIXME: we need permissions checks. user must have permission on src pool > @@ -351,11 +369,17 @@ class HardwareController < ApplicationController > target_pool_id = params[:target_pool_id] > storage_pool_ids_str = params[:resource_ids] > storage_pool_ids = storage_pool_ids_str.split(",").collect {|x| x.to_i} > - > - @pool.transaction do > - @pool.move_storage(storage_pool_ids, target_pool_id) > + > + begin > + @pool.transaction do > + @pool.move_storage(storage_pool_ids, target_pool_id) > + end > + render :json => { :object => "storage_pool", :success => true, > + :alert => "Storage Pools were successfully moved." } > + rescue > + render :json => { :object => "storage_pool", :success => false, > + :alert => "Error moving storage pools." } > end > - render :text => "added storage (#{storage_pool_ids.join(', ')})" > end > > def destroy > diff --git a/wui/src/app/controllers/permission_controller.rb b/wui/src/app/controllers/permission_controller.rb > index 1787a27..14358de 100644 > --- a/wui/src/app/controllers/permission_controller.rb > +++ b/wui/src/app/controllers/permission_controller.rb > @@ -74,15 +74,21 @@ class PermissionController < ApplicationController > role = params[:user_role] > permission_ids_str = params[:permission_ids] > permission_ids = permission_ids_str.split(",").collect {|x| x.to_i} > - > - Permission.transaction do > - permissions = Permission.find(:all, :conditions => "id in (#{permission_ids.join(', ')})") > - permissions.each do |permission| > - permission.user_role = role > - permission.save! > + > + begin > + Permission.transaction do > + permissions = Permission.find(:all, :conditions => "id in (#{permission_ids.join(', ')})") > + permissions.each do |permission| > + permission.user_role = role > + permission.save! > + end > end > + render :json => { :object => "permission", :success => true, > + :alert => "User roles were successfully updated." } > + rescue > + render :json => { :object => "permission", :success => false, > + :alert => "Error updating user roles: #{$!}" } > end > - render :text => "deleted user permissions (#{permission_ids.join(', ')})" > end > > #FIXME: we need permissions checks. user must have permission. We also need to fail > @@ -90,14 +96,20 @@ class PermissionController < ApplicationController > def delete > permission_ids_str = params[:permission_ids] > permission_ids = permission_ids_str.split(",").collect {|x| x.to_i} > - > - Permission.transaction do > - permissions = Permission.find(:all, :conditions => "id in (#{permission_ids.join(', ')})") > - permissions.each do |permission| > - permission.destroy > + > + begin > + Permission.transaction do > + permissions = Permission.find(:all, :conditions => "id in (#{permission_ids.join(', ')})") > + permissions.each do |permission| > + permission.destroy > + end > end > + render :json => { :object => "permission", :success => true, > + :alert => "User roles were successfully deleted." } > + rescue > + render :json => { :object => "permission", :success => false, > + :alert => "Error deleting user roles." } > end > - render :text => "deleted user permissions (#{permission_ids.join(', ')})" > end > > def destroy > diff --git a/wui/src/app/controllers/resources_controller.rb b/wui/src/app/controllers/resources_controller.rb > index febbbb6..c891447 100644 > --- a/wui/src/app/controllers/resources_controller.rb > +++ b/wui/src/app/controllers/resources_controller.rb > @@ -127,14 +127,21 @@ class ResourcesController < ApplicationController > def delete > vm_pool_ids_str = params[:vm_pool_ids] > vm_pool_ids = vm_pool_ids_str.split(",").collect {|x| x.to_i} > - > - VmResourcePool.transaction do > - pools = VmResourcePool.find(:all, :conditions => "id in (#{vm_pool_ids.join(', ')})") > - pools.each do |pool| > - pool.destroy > + vm_pool_names = [] > + begin > + VmResourcePool.transaction do > + pools = VmResourcePool.find(:all, :conditions => "id in (#{vm_pool_ids.join(', ')})") > + pools.each do |pool| > + vm_pool_names << pool.name > + pool.destroy > + end > end > + render :json => { :object => "vm_resource_pool", :success => true, > + :alert => "Virtual Machine Pools #{vm_pool_names.join(', ')} were successfully deleted." } > + rescue > + render :json => { :object => "vm_resource_pool", :success => false, > + :alert => "Error in deleting Virtual Machine Pools."} > end > - render :text => "deleted vm pools (#{vm_pool_ids.join(', ')})" > end > > def destroy > diff --git a/wui/src/app/controllers/storage_controller.rb b/wui/src/app/controllers/storage_controller.rb > index 618acb4..853472a 100644 > --- a/wui/src/app/controllers/storage_controller.rb > +++ b/wui/src/app/controllers/storage_controller.rb > @@ -155,14 +155,20 @@ class StorageController < ApplicationController > def delete_pools > storage_pool_ids_str = params[:storage_pool_ids] > storage_pool_ids = storage_pool_ids_str.split(",").collect {|x| x.to_i} > - > - StoragePool.transaction do > - storage = StoragePool.find(:all, :conditions => "id in (#{storage_pool_ids.join(', ')})") > - storage.each do |storage_pool| > - storage_pool.destroy > + > + begin > + StoragePool.transaction do > + storage = StoragePool.find(:all, :conditions => "id in (#{storage_pool_ids.join(', ')})") > + storage.each do |storage_pool| > + storage_pool.destroy > + end > end > + render :json => { :object => "storage_pool", :success => true, > + :alert => "Storage Pools were successfully deleted." } > + rescue > + render :json => { :object => "storage_pool", :success => true, > + :alert => "Error deleting storage pools." } > end > - render :text => "deleted storage (#{storage_pool_ids.join(', ')})" > end > > def vm_action > diff --git a/wui/src/app/controllers/vm_controller.rb b/wui/src/app/controllers/vm_controller.rb > index ac29beb..8e6d7f8 100644 > --- a/wui/src/app/controllers/vm_controller.rb > +++ b/wui/src/app/controllers/vm_controller.rb > @@ -102,13 +102,18 @@ class VmController < ApplicationController > vm_ids_str = params[:vm_ids] > vm_ids = vm_ids_str.split(",").collect {|x| x.to_i} > > - Vm.transaction do > - vms = Vm.find(:all, :conditions => "id in (#{vm_ids.join(', ')})") > - vms.each do |vm| > - vm.destroy > - end > + begin > + Vm.transaction do > + vms = Vm.find(:all, :conditions => "id in (#{vm_ids.join(', ')})") > + vms.each do |vm| > + vm.destroy > + end > + end render :json => { :object => "vm", :success => true, > + :alert => "Virtual Machines were successfully deleted." } > + rescue > + render :json => { :object => "vm", :success => false, > + :alert => "Error deleting virtual machines." } > end > - render :text => "deleted vms (#{vm_ids.join(', ')})" > end > > def destroy > diff --git a/wui/src/app/views/hardware/move.rhtml b/wui/src/app/views/hardware/move.rhtml > index fb0bf51..190fc03 100644 > --- a/wui/src/app/views/hardware/move.rhtml > +++ b/wui/src/app/views/hardware/move.rhtml > @@ -19,7 +19,10 @@ > function(data,status){ > $("#<%= @resource_type %>_grid").flexReload() > jQuery(document).trigger('close.facebox'); > - }); > + if (data.alert) { > + alert(data.alert); > + } > + }, 'json'); > } > $('#move_to_new_pool').click(function(){ > $('#window').fadeOut('fast'); > diff --git a/wui/src/app/views/hardware/show_hosts.rhtml b/wui/src/app/views/hardware/show_hosts.rhtml > index 7f126a2..ae0e8af 100644 > --- a/wui/src/app/views/hardware/show_hosts.rhtml > +++ b/wui/src/app/views/hardware/show_hosts.rhtml > @@ -28,7 +28,10 @@ > { resource_ids: hosts.toString(), target_pool_id: <%= Pool.root.id %> }, > function(data,status){ > $("#hosts_grid").flexReload() > - }); > + if (data.alert) { > + alert(data.alert); > + } > + }, 'json'); > } > } > function hosts_select(selected_rows) > diff --git a/wui/src/app/views/hardware/show_storage.rhtml b/wui/src/app/views/hardware/show_storage.rhtml > index f673e07..3d547db 100644 > --- a/wui/src/app/views/hardware/show_storage.rhtml > +++ b/wui/src/app/views/hardware/show_storage.rhtml > @@ -30,7 +30,10 @@ > { resource_ids: storage.toString(), target_pool_id: <%= Pool.root.id %> }, > function(data,status){ > $("#storage_grid").flexReload() > - }); > + if (data.alert) { > + alert(data.alert); > + } > + }, 'json'); > } > } > function delete_storage() > @@ -41,7 +44,10 @@ > { storage_pool_ids: storage.toString() }, > function(data,status){ > $("#storage_grid").flexReload() > - }); > + if (data.alert) { > + alert(data.alert); > + } > + }, 'json'); > } > } > function storage_select(selected_rows) > diff --git a/wui/src/app/views/hardware/show_vms.rhtml b/wui/src/app/views/hardware/show_vms.rhtml > index 44b6f50..285f2c2 100644 > --- a/wui/src/app/views/hardware/show_vms.rhtml > +++ b/wui/src/app/views/hardware/show_vms.rhtml > @@ -17,7 +17,10 @@ > { vm_pool_ids: vm_pools.toString() }, > function(data,status){ > $("#vmpools_grid").flexReload() > - }); > + if (data.alert) { > + alert(data.alert); > + } > + }, 'json'); > } > } > function vmpools_select(selected_rows) > diff --git a/wui/src/app/views/host/addhost.html.erb b/wui/src/app/views/host/addhost.html.erb > index f485d8f..69e0e66 100644 > --- a/wui/src/app/views/host/addhost.html.erb > +++ b/wui/src/app/views/host/addhost.html.erb > @@ -34,7 +34,10 @@ > function(data,status){ > jQuery(document).trigger('close.facebox'); > $("#hosts_grid").flexReload() > - }); > + if (data.alert) { > + alert(data.alert); > + } > + }, 'json'); > } > } > </script> > diff --git a/wui/src/app/views/layouts/_side_toolbar.rhtml b/wui/src/app/views/layouts/_side_toolbar.rhtml > index 1613799..000cb66 100644 > --- a/wui/src/app/views/layouts/_side_toolbar.rhtml > +++ b/wui/src/app/views/layouts/_side_toolbar.rhtml > @@ -10,11 +10,11 @@ > <%= image_tag "icon_add_vmpool.png", :title=>"Add Virtual Machine Pool" %> > </a> > </div> > +<% end -%> > <div class="toolbar" style="float:left;"> > <%= link_to image_tag ("icon_delete.gif", :title=>"Delete Selected Pool"), > { :action => 'destroy', :id => pool }, > { :method => :post, :confirm => 'Are you sure you want to delete - %s?' % pool.name} %> > -<% end -%> > </div> > > <div class="toolbar"></div> > diff --git a/wui/src/app/views/resources/show_vms.rhtml b/wui/src/app/views/resources/show_vms.rhtml > index 8e951bf..34764a6 100644 > --- a/wui/src/app/views/resources/show_vms.rhtml > +++ b/wui/src/app/views/resources/show_vms.rhtml > @@ -31,7 +31,10 @@ > { vm_ids: vms.toString() }, > function(data,status){ > $("#vms_grid").flexReload() > - }); > + if (data.alert) { > + alert(data.alert); > + } > + }, 'json'); > } > } > function vm_actions(action) > diff --git a/wui/src/app/views/storage/addstorage.html.erb b/wui/src/app/views/storage/addstorage.html.erb > index cf75ae5..a747502 100644 > --- a/wui/src/app/views/storage/addstorage.html.erb > +++ b/wui/src/app/views/storage/addstorage.html.erb > @@ -34,7 +34,10 @@ > function(data,status){ > jQuery(document).trigger('close.facebox'); > $("#storage_grid").flexReload() > - }); > + if (data.alert) { > + alert(data.alert); > + } > + }, 'json'); > } > } > </script> > diff --git a/wui/src/app/views/user/_show.rhtml b/wui/src/app/views/user/_show.rhtml > index 03ecec6..d1c0ec1 100644 > --- a/wui/src/app/views/user/_show.rhtml > +++ b/wui/src/app/views/user/_show.rhtml > @@ -18,18 +18,24 @@ > { permission_ids: permissions.toString() }, > function(data,status){ > $("#users_grid").flexReload() > - }); > + if (data.alert) { > + alert(data.alert); > + } > + }, 'json'); > } > } > function update_users(role) > { > permissions = get_selected_users() > - if (validate_selected(permissions, "users)) { > + if (validate_selected(permissions, "users")) { > $.post('<%= url_for :controller => "permission", :action => "update_roles" %>', > { permission_ids: permissions.toString(), user_role: role }, > function(data,status){ > $("#users_grid").flexReload() > - }); > + if (data.alert) { > + alert(data.alert); > + } > + }, 'json'); > } > } > </script> > -- > 1.5.4.1Tested, works with the patch that follows (confirmation-dialog-2.patch)... therefore ACKed and committed --Hugh