Ovirt devel - Jun 2008 - [PATCH] initial support for "clear this VM" UI.

taskomatic bits are not in, so for now taskomatic should just fail these tasks
as 'unknown task type'

Another bit to work out -- what should the logic be for when to show the
'clear this VM' task? Do we need to hide the link when:
1) there's already a pending clear VM task for this host
2) vm is disabled and there are no VMs running
3) vm is unavailable

Signed-off-by: Scott Seago <sseago at redhat.com>
---
 wui/src/app/controllers/application.rb     |    6 ++-
 wui/src/app/controllers/host_controller.rb |   62 +++++++++++++++++++++-------
 wui/src/app/views/host/show.rhtml          |   14 ++++---
 3 files changed, 60 insertions(+), 22 deletions(-)

diff --git a/wui/src/app/controllers/application.rb
b/wui/src/app/controllers/application.rb
index a3a99a0..aa8fd7c 100644
--- a/wui/src/app/controllers/application.rb
+++ b/wui/src/app/controllers/application.rb
@@ -74,7 +74,11 @@ class ApplicationController < ActionController::Base
       unless (is_modify_action ? @can_modify : @can_control_vms)
         @redir_obj = @perm_obj unless @redir_obj
         flash[:notice] = 'You do not have permission to create or modify
this item '
-        if @redir_controller
+        if @json_hash
+          @json_hash[:success] = false
+          @json_hash[:alert] = flash[:notice]
+          render :json => @json_hash
+        elsif @redir_controller
           redirect_to :controller => @redir_controller, :action =>
'show', :id => @redir_obj
         else
           redirect_to :action => 'show', :id => @redir_obj
diff --git a/wui/src/app/controllers/host_controller.rb
b/wui/src/app/controllers/host_controller.rb
index 8eb26b9..9e60b1f 100644
--- a/wui/src/app/controllers/host_controller.rb
+++ b/wui/src/app/controllers/host_controller.rb
@@ -23,6 +23,8 @@ class HostController < ApplicationController
     render :action => 'list'
   end
 
+  before_filter :pre_action, :only => [:host_action, :enable, :disable,
:clear_vms]
+
   # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
   verify :method => :post, :only => [ :destroy, :create, :update ],
          :redirect_to => { :action => :list }
@@ -61,6 +63,17 @@ class HostController < ApplicationController
   def destroy
   end
 
+  def host_action
+    action = params[:action_type]
+    if["disable", "enable",
"clear_vms"].include?(action)
+      self.send(action)
+    else
+      @json_hash[:alert]="invalid operation #{action}"
+      @json_hash[:success]=false
+      render :json => @json_hash
+    end
+  end
+
   def disable
     set_disabled(1)
   end
@@ -70,23 +83,36 @@ class HostController < ApplicationController
 
   def set_disabled(value)
     operation = value == 1 ? "diabled" : "enabled"
-    @host = Host.find(params[:id])
-    set_perms(@host.hardware_pool)
-    unless @can_modify
-      alert= 'You do not have permission to edit this host'
-      success=false
-    else
-      begin
-        @host.is_disabled = value
-        @host.save
-        alert="Host was successfully #{operation}"
-        success=true
-      rescue
-        alert="Error setting host to #{operation}"
-        success=false
+    begin
+      @host.is_disabled = value
+      @host.save!
+      @json_hash[:alert]="Host was successfully #{operation}"
+      @json_hash[:success]=true
+    rescue
+      @json_hash[:alert]="Error setting host to #{operation}"
+      @json_hash[:success]=false
+    end
+    render :json => @json_hash
+  end
+
+  def clear_vms
+    begin
+      Host.transaction do
+        task = HostTask.new({ :user    => get_login_user,
+                              :host_id   => @host.id,
+                              :action  => HostTask::ACTION_CLEAR_VMS,
+                              :state   => Task::STATE_QUEUED})
+        task.save!
+        @host.is_disabled = true
+        @host.save!
       end
+      @json_hash[:alert]="Clear VMs action was successfully queued."
+      @json_hash[:success]=true
+    rescue
+      @json_hash[:alert]="Error in queueing Clear VMs action."
+      @json_hash[:success]=false
     end
-    render :json => { :object => "host", :success =>
success, :alert => alert }
+    render :json => @json_hash
   end
 
 
@@ -105,6 +131,12 @@ class HostController < ApplicationController
     flash[:notice] = 'Hosts may not be edited via the web UI'
     redirect_to :action=> 'show', :id => @host
   end
+  def pre_action
+    @host = Host.find(params[:id])
+    @perm_obj = @host.hardware_pool
+    @json_hash = { :object => :host }
+    authorize_admin
+  end
   def pre_show
     @host = Host.find(params[:id])
     @perm_obj = @host.hardware_pool
diff --git a/wui/src/app/views/host/show.rhtml
b/wui/src/app/views/host/show.rhtml
index 0903ac0..619a126 100644
--- a/wui/src/app/views/host/show.rhtml
+++ b/wui/src/app/views/host/show.rhtml
@@ -4,22 +4,24 @@
 <%- content_for :action_links do -%>
   <%if @can_modify -%>
     <%if @host.is_disabled.nil? or @host.is_disabled == 0  -%>
-      <a href="#" onClick="toggle_host(false)">
+      <a href="#"
onClick="host_action('disable')">
         <%= image_tag "icon_suspend.png" %> Disable Host
       </a> 
     <% else -%>
-      <a href="#" onClick="toggle_host(true)">
+      <a href="#"
onClick="host_action('enable')">
         <%= image_tag "icon_start.png" %> Enable Host
       </a> 
     <% end -%>
+    <a href="#"
onClick="host_action('clear_vms')">
+      <%= image_tag "icon_x.png" %> Clear VMs
+    </a> 
   <%- end -%>
 <%- end -%>
 <script type="text/javascript">
-  function toggle_host(enable)
+  function host_action(action)
   {
-    enable_url = '<%= url_for :controller => "host",
:action => "enable", :id => @host %>'
-    disable_url = '<%= url_for :controller => "host",
:action => "disable", :id => @host %>'
-      $.post(enable ? enable_url : disable_url,
+      $.post('<%= url_for :controller => "host", :action
=> 'host_action', :id => @host%>',
+             { action_type: action },
               function(data,status){
                 refresh_summary('hosts_selection', 
                             '<%= url_for :controller =>
"host",
-- 
1.5.4.1

When I apply this patch my host list looks funky (screenshot attached).
The columns in the table are not lined up with the headers and after the
first row, the ids show instead of the checkboxes.  I havent figured out
where this is coming from yet, but can help debug if you like.  Also,
small thing I noticed.  Here:

?@@ -70,23 +83,36 @@ class HostController < ApplicationController
 
   def set_disabled(value)
     operation = value == 1 ? "diabled" : "enabled"

s/diabled/disabled

We should probably start thinking about a better way to set our error
messages, as the current approach is not very i18n-friendly.

-j
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hosts-list.png
Type: image/png
Size: 207384 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/ovirt-devel/attachments/20080627/48d20f16/attachment.png>

On Mon, 2008-06-23 at 12:02 -0400, Scott Seago wrote:
> taskomatic bits are not in, so for now taskomatic should just fail these
tasks as 'unknown task type'
> 
> Another bit to work out -- what should the logic be for when to show the
'clear this VM' task? Do we need to hide the link when:
> 1) there's already a pending clear VM task for this host
> 2) vm is disabled and there are no VMs running
> 3) vm is unavailable
> 
> Signed-off-by: Scott Seago <sseago at redhat.com>
> ---
>  wui/src/app/controllers/application.rb     |    6 ++-
>  wui/src/app/controllers/host_controller.rb |   62
+++++++++++++++++++++-------
>  wui/src/app/views/host/show.rhtml          |   14 ++++---
>  3 files changed, 60 insertions(+), 22 deletions(-)
> 
> diff --git a/wui/src/app/controllers/application.rb
b/wui/src/app/controllers/application.rb
> index a3a99a0..aa8fd7c 100644
> --- a/wui/src/app/controllers/application.rb
> +++ b/wui/src/app/controllers/application.rb
> @@ -74,7 +74,11 @@ class ApplicationController < ActionController::Base
>        unless (is_modify_action ? @can_modify : @can_control_vms)
>          @redir_obj = @perm_obj unless @redir_obj
>          flash[:notice] = 'You do not have permission to create or
modify this item '
> -        if @redir_controller
> +        if @json_hash
> +          @json_hash[:success] = false
> +          @json_hash[:alert] = flash[:notice]
> +          render :json => @json_hash
> +        elsif @redir_controller
>            redirect_to :controller => @redir_controller, :action =>
'show', :id => @redir_obj
>          else
>            redirect_to :action => 'show', :id => @redir_obj
> diff --git a/wui/src/app/controllers/host_controller.rb
b/wui/src/app/controllers/host_controller.rb
> index 8eb26b9..9e60b1f 100644
> --- a/wui/src/app/controllers/host_controller.rb
> +++ b/wui/src/app/controllers/host_controller.rb
> @@ -23,6 +23,8 @@ class HostController < ApplicationController
>      render :action => 'list'
>    end
>  
> +  before_filter :pre_action, :only => [:host_action, :enable, :disable,
:clear_vms]
> +
>    # GETs should be safe (see
http://www.w3.org/2001/tag/doc/whenToUseGet.html)
>    verify :method => :post, :only => [ :destroy, :create, :update ],
>           :redirect_to => { :action => :list }
> @@ -61,6 +63,17 @@ class HostController < ApplicationController
>    def destroy
>    end
>  
> +  def host_action
> +    action = params[:action_type]
> +    if["disable", "enable",
"clear_vms"].include?(action)
> +      self.send(action)
> +    else
> +      @json_hash[:alert]="invalid operation #{action}"
> +      @json_hash[:success]=false
> +      render :json => @json_hash
> +    end
> +  end
> +
>    def disable
>      set_disabled(1)
>    end
> @@ -70,23 +83,36 @@ class HostController < ApplicationController
>  
>    def set_disabled(value)
>      operation = value == 1 ? "diabled" : "enabled"
> -    @host = Host.find(params[:id])
> -    set_perms(@host.hardware_pool)
> -    unless @can_modify
> -      alert= 'You do not have permission to edit this host'
> -      success=false
> -    else
> -      begin
> -        @host.is_disabled = value
> -        @host.save
> -        alert="Host was successfully #{operation}"
> -        success=true
> -      rescue
> -        alert="Error setting host to #{operation}"
> -        success=false
> +    begin
> +      @host.is_disabled = value
> +      @host.save!
> +      @json_hash[:alert]="Host was successfully #{operation}"
> +      @json_hash[:success]=true
> +    rescue
> +      @json_hash[:alert]="Error setting host to #{operation}"
> +      @json_hash[:success]=false
> +    end
> +    render :json => @json_hash
> +  end
> +
> +  def clear_vms
> +    begin
> +      Host.transaction do
> +        task = HostTask.new({ :user    => get_login_user,
> +                              :host_id   => @host.id,
> +                              :action  => HostTask::ACTION_CLEAR_VMS,
> +                              :state   => Task::STATE_QUEUED})
> +        task.save!
> +        @host.is_disabled = true
> +        @host.save!
>        end
> +      @json_hash[:alert]="Clear VMs action was successfully
queued."
> +      @json_hash[:success]=true
> +    rescue
> +      @json_hash[:alert]="Error in queueing Clear VMs action."
> +      @json_hash[:success]=false
>      end
> -    render :json => { :object => "host", :success =>
success, :alert => alert }
> +    render :json => @json_hash
>    end
>  
> 
> @@ -105,6 +131,12 @@ class HostController < ApplicationController
>      flash[:notice] = 'Hosts may not be edited via the web UI'
>      redirect_to :action=> 'show', :id => @host
>    end
> +  def pre_action
> +    @host = Host.find(params[:id])
> +    @perm_obj = @host.hardware_pool
> +    @json_hash = { :object => :host }
> +    authorize_admin
> +  end
>    def pre_show
>      @host = Host.find(params[:id])
>      @perm_obj = @host.hardware_pool
> diff --git a/wui/src/app/views/host/show.rhtml
b/wui/src/app/views/host/show.rhtml
> index 0903ac0..619a126 100644
> --- a/wui/src/app/views/host/show.rhtml
> +++ b/wui/src/app/views/host/show.rhtml
> @@ -4,22 +4,24 @@
>  <%- content_for :action_links do -%>
>    <%if @can_modify -%>
>      <%if @host.is_disabled.nil? or @host.is_disabled == 0  -%>
> -      <a href="#" onClick="toggle_host(false)">
> +      <a href="#"
onClick="host_action('disable')">
>          <%= image_tag "icon_suspend.png" %> Disable Host
>        </a> 
>      <% else -%>
> -      <a href="#" onClick="toggle_host(true)">
> +      <a href="#"
onClick="host_action('enable')">
>          <%= image_tag "icon_start.png" %> Enable Host
>        </a> 
>      <% end -%>
> +    <a href="#"
onClick="host_action('clear_vms')">
> +      <%= image_tag "icon_x.png" %> Clear VMs
> +    </a> 
>    <%- end -%>
>  <%- end -%>
>  <script type="text/javascript">
> -  function toggle_host(enable)
> +  function host_action(action)
>    {
> -    enable_url = '<%= url_for :controller => "host",
:action => "enable", :id => @host %>'
> -    disable_url = '<%= url_for :controller => "host",
:action => "disable", :id => @host %>'
> -      $.post(enable ? enable_url : disable_url,
> +      $.post('<%= url_for :controller => "host",
:action => 'host_action', :id => @host%>',
> +             { action_type: action },
>                function(data,status){
>                  refresh_summary('hosts_selection', 
>                              '<%= url_for :controller =>
"host",
Except for the 'diabled' typo, ACK.  Turns out this is an existing bug
against FF3 (not filed yet, will try to get one added this afternoon).
Works fine on FF2

-j