[Ovirt-devel] [PATCH server] Cloud UI layer to initiate actions on vms.

[Scott Seago]

Just a couple comments/questions, as I haven't tried it out yet. Looks 
good overall though -- will hopefully test it out later today.


Jason Guiditta wrote:
> 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           |   31 +++++---
>  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, 215 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
>  
>    # 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.
>   
It should probably eventually be handled via the svc_* calls. Once a 
permission-checking svc method is  called, @user will already be set for 
you.
>    def set_vars
>      @user = get_login_user
>    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..330dd58 100644
> --- a/src/app/views/cloud/instance/index.rhtml
> +++ b/src/app/views/cloud/instance/index.rhtml
> @@ -1,23 +1,30 @@
> +<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>
> +      <%# This form tag is terminated in _show.rhtml %>
>   
Which form tag? And above a lone closing </form> tag is removed from 
_show.rhtml
> +        <%=%>
> +        <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
>