[PATCH] Changes in Deployment Launch

jtomasek at redhat.com jtomasek at redhat.com
Fri Jul 27 15:05:14 UTC 2012


From: Jiri Tomasek <jtomasek at redhat.com>

Removed disabling the Next button in New Deployment form, established validations
for Deployment in New Deployment form. Added proper back and continue buttons to
navigate through Deployment launch process.
Unified the workflow when launching Deployment from different places in the App.

Deployment Launch workflow teps:
1. Deployment details form
2. Launch time params
3. Deployment Launch Overview
---
 src/app/controllers/deployments_controller.rb      |   47 ++++++++++++--------
 src/app/models/deployable.rb                       |    2 +-
 src/app/models/deployment.rb                       |    8 ++--
 src/app/stylesheets/layout.scss                    |    6 ++-
 src/app/util/deployable_xml.rb                     |    8 +++-
 src/app/views/catalogs/_form.html.haml             |    2 +-
 src/app/views/deployables/show.html.haml           |    2 +-
 src/app/views/deployments/_launch_new.html.haml    |   34 ++++----------
 src/app/views/deployments/_overview.html.haml      |   34 +++++++-------
 .../deployments/launch_from_catalog.html.haml      |    2 +-
 .../views/deployments/launch_time_params.html.haml |    8 +++-
 src/app/views/pools/_pretty_list.html.haml         |    2 +-
 src/app/views/pools/show.html.haml                 |    2 +-
 src/config/locales/en.yml                          |    7 ++-
 src/features/deployables.feature                   |    2 +-
 15 files changed, 87 insertions(+), 79 deletions(-)

diff --git a/src/app/controllers/deployments_controller.rb b/src/app/controllers/deployments_controller.rb
index d9ec800..add9c4d 100644
--- a/src/app/controllers/deployments_controller.rb
+++ b/src/app/controllers/deployments_controller.rb
@@ -1,4 +1,3 @@
-#
 #   Copyright 2011 Red Hat, Inc.
 #
 #   Licensed under the Apache License, Version 2.0 (the "License");
@@ -37,18 +36,20 @@ class DeploymentsController < ApplicationController
     end
   end
 
-  # It is expected that params[:pool_id] will be set on requests into this method
+  # It is expected that params[:deployment][:pool_id] will be set on requests into this method
   def launch_new
-    @title = t 'deployments.new_deployment'
-    @pool = Pool.find(params[:pool_id]) or raise "Invalid pool"
+    @title = t('deployments.new_deployment')
+    @deployment = Deployment.new(params[:deployment])
+    @pool = Pool.find(@deployment.pool_id) or raise "Invalid pool"
     require_privilege(Privilege::CREATE, Deployment, @pool)
+
     unless @pool.enabled
       flash[:warning] = t 'deployments.flash.warning.disabled_pool'
       redirect_to pool_path(@pool) and return
     end
 
-    @deployment = Deployment.new(:pool_id => @pool.id)
     init_new_deployment_attrs
+
     respond_to do |format|
       format.html
       format.js { render :partial => 'launch_new' }
@@ -56,24 +57,26 @@ class DeploymentsController < ApplicationController
     end
   end
 
-
-
   def launch_time_params
     @title = t 'deployments.new_deployment'
-    unless params.has_key?(:deployable_id)
-      flash[:error] = t('deployments.flash.warning.deployable_not_selected')
-      redirect_to launch_new_deployments_path(:pool_id => params[:deployment][:pool_id]) and return
-    end
 
     @deployment = Deployment.new(params[:deployment])
     @pool = @deployment.pool
+
     init_new_deployment_attrs
+    @deployment.deployable_xml = DeployableXML.new(@deployable.xml)
+
+    unless @deployment.valid? and params.has_key?(:deployable_id)
+      @deployment.errors.add(:base, t('deployments.flash.warning.deployable_not_selected')) unless params.has_key?(:deployable_id)
+      render :launch_new and return
+    end
+
     require_privilege(Privilege::CREATE, Deployment, @pool)
     require_privilege(Privilege::USE, @deployable)
     img, img2, missing, d_errors = @deployable.get_image_details
     flash[:error] = d_errors unless d_errors.empty?
 
-    unless @deployable && @deployable.xml && @deployment.valid_deployable_xml?(@deployable.xml) && d_errors.empty?
+    unless @deployable && @deployable.xml && @deployment.valid_deployable_xml? && d_errors.empty?
       render 'launch_new' and return
     end
 
@@ -95,14 +98,21 @@ class DeploymentsController < ApplicationController
       flash[:warning] = [flash[:warning]] if flash[:warning].kind_of? String
       flash[:warning]+=warnings
     end
-      
+
   end
 
   def overview
     @title = t 'deployments.new_deployment'
     @deployment = Deployment.new(params[:deployment])
     @pool = @deployment.pool
+
     init_new_deployment_attrs
+    @deployment.deployable_xml = DeployableXML.new(@deployable.xml)
+
+    if params.delete(:commit) == 'back'
+      render :launch_new and return
+    end
+
     require_privilege(Privilege::CREATE, Deployment, @pool)
     require_privilege(Privilege::USE, @deployable)
     @launch_parameters_encoded = Base64.encode64(ActiveSupport::JSON.encode(@deployment.launch_parameters))
@@ -110,7 +120,7 @@ class DeploymentsController < ApplicationController
     flash[:error] = d_errors unless d_errors.empty?
 
     respond_to do |format|
-      if @deployable.xml && @deployment.valid_deployable_xml?(@deployable.xml) && d_errors.empty?
+      if @deployable.xml && @deployment.valid_deployable_xml? && d_errors.empty?
         @errors = @deployment.check_assemblies_matches(current_session,
                                                        current_user)
         set_errors_flash(@errors)
@@ -138,9 +148,9 @@ class DeploymentsController < ApplicationController
     end
 
     init_new_deployment_attrs
+    @deployment.deployable_xml = DeployableXML.new(@deployable.xml)
+
     require_privilege(Privilege::USE, @deployable)
-    @deployment.deployable_xml = @deployable.xml
-    @deployment.owner = current_user
 
     if params.delete(:commit) == 'back'
       load_assemblies_services
@@ -185,7 +195,6 @@ class DeploymentsController < ApplicationController
     @deployment = Deployment.find(params[:id])
     @title = t('deployments.show.name', :name => @deployment.name)
     require_privilege(Privilege::VIEW, @deployment)
-    init_new_deployment_attrs
     save_breadcrumb(deployment_path(@deployment, :viewstate => viewstate_id), @deployment.name)
     @failed_instances = @deployment.failed_instances.list(sort_column(Instance), sort_direction)
     if filter_view?
@@ -440,8 +449,8 @@ class DeploymentsController < ApplicationController
         :include => :architecture,
         :conditions => {:provider_id => nil}
     )
-    @realm_choices = @realms.map{|r| [r.name, r.id]}
-    @realm_choices.unshift([I18n.t('deployments.launch_new.autoselect'), ''])
+
+    @deployment.owner = current_user
   end
 
   def load_assemblies_services
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb
index e1d80d5..82bcfe4 100644
--- a/src/app/models/deployable.rb
+++ b/src/app/models/deployable.rb
@@ -60,7 +60,7 @@ class Deployable < ActiveRecord::Base
   def valid_deployable_xml?
     begin
       deployable_xml = DeployableXML.new(xml)
-      if !deployable_xml.validate!
+      if !deployable_xml.valid?
         errors.add(:xml, I18n.t('catalog_entries.flash.warning.not_valid'))
       elsif !deployable_xml.unique_assembly_names?
         errors.add(:xml, I18n.t('catalog_entries.flash.warning.not_valid_duplicate_assembly_names'))
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 277f9b3..07f980b 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -107,7 +107,8 @@ class Deployment < ActiveRecord::Base
 
   def validate_xml
     begin
-      deployable_xml.validate!
+      deployable_xml_errors = deployable_xml.validate!
+      errors.add(:deployable_xml, deployable_xml_errors) unless deployable_xml_errors.empty?
     rescue DeployableXML::ValidationError => e
       errors.add(:deployable_xml, e.message)
     rescue Nokogiri::XML::SyntaxError => e
@@ -304,10 +305,9 @@ class Deployment < ActiveRecord::Base
                    :order => (order_field || 'name') +' '+ (order_dir || 'asc'))
   end
 
-  def valid_deployable_xml?(xml)
+  def valid_deployable_xml?
     begin
-      self.deployable_xml = DeployableXML.new(xml)
-      deployable_xml.validate!
+      deployable_xml.valid?
       true
     rescue
       errors.add(:base, I18n.t("deployments.errors.not_valid_deployable_xml", :msg => "#{$!.message}"))
diff --git a/src/app/stylesheets/layout.scss b/src/app/stylesheets/layout.scss
index 1f154ac..3ff0280 100644
--- a/src/app/stylesheets/layout.scss
+++ b/src/app/stylesheets/layout.scss
@@ -2266,7 +2266,11 @@ form.generic.horizontal{
   span.text_instead_input{
     @include display_inline_block;
     font-weight: bold;
-    padding: 6px 0px;
+    padding: 4px 0px 3px 0px;
+    &.em{
+      font-size: 16px;
+      padding: 5px 0px 4px 0px;
+    }
   }
 
 }
diff --git a/src/app/util/deployable_xml.rb b/src/app/util/deployable_xml.rb
index cdbbc39..90d4863 100644
--- a/src/app/util/deployable_xml.rb
+++ b/src/app/util/deployable_xml.rb
@@ -213,7 +213,7 @@ class DeployableXML
 
   def validate!
     # load the relaxNG file and validate
-    return false if @root.nil?
+    return I18n.t("deployments.errors.empty_deployable_xml") if @root.nil?
     errors = relax.validate(@root.document) || []
     # ...and validate the assembly
     assemblies.each do |assembly|
@@ -223,7 +223,11 @@ class DeployableXML
         errors << e.message
       end
     end
-    errors.empty?
+    errors
+  end
+
+  def valid?
+    self.validate!.empty?
   end
 
   def unique_assembly_names?
diff --git a/src/app/views/catalogs/_form.html.haml b/src/app/views/catalogs/_form.html.haml
index 111569c..0c76c04 100644
--- a/src/app/views/catalogs/_form.html.haml
+++ b/src/app/views/catalogs/_form.html.haml
@@ -2,7 +2,7 @@
   = render 'layouts/error_messages', :object => @catalog
 %fieldset
   .field
-    = form.label :name
+    = form.label :name, :class => 'em'
     .input
       = form.text_field :name, :class => 'em long'
   .field
diff --git a/src/app/views/deployables/show.html.haml b/src/app/views/deployables/show.html.haml
index a7ee021..7a19a5b 100644
--- a/src/app/views/deployables/show.html.haml
+++ b/src/app/views/deployables/show.html.haml
@@ -7,7 +7,7 @@
         = link_to t('.edit_xml'), edit_polymorphic_path([@catalog, @deployable], :edit_xml => true), :class => 'button', :id => 'edit_xml_button'
         = button_to t('.delete'), polymorphic_path([@catalog, @deployable]), :method => 'delete', :confirm => "#{t'catalog_entries.show.confirm_delete'}", :class => 'button danger', :id => 'delete'
 
-    - launch_deployment_button_path = launch_time_params_deployments_path(:deployable_id => @deployable.id, :deployment => { :name => @deployable.name, :pool_id => @catalog.pool })
+    - launch_deployment_button_path = launch_new_deployments_path(:deployable_id => @deployable.id, :deployment => { :name => @deployable.name, :pool_id => @catalog.pool} )
     - if @catalog.present? && @deployable_errors.empty? && @pushed_count > 0
       = link_to t('.launch'), launch_deployment_button_path, :id => :launch_deployment_button, :'data-path' => launch_deployment_button_path
     -else
diff --git a/src/app/views/deployments/_launch_new.html.haml b/src/app/views/deployments/_launch_new.html.haml
index 6cfce80..6e3d612 100644
--- a/src/app/views/deployments/_launch_new.html.haml
+++ b/src/app/views/deployments/_launch_new.html.haml
@@ -1,10 +1,8 @@
 = if request.xhr?
   = render :partial => '/layouts/new_notification'
 %header.page-header
-  .obj_actions
-    = link_to t('cancel'), pool_path(@pool), :class => 'button danger', :id => 'cancel_deployment_button'
   %h1.deployments
-    = @title
+    = t('deployments.new_deployment')
     %span= t('deployments.launch_new.to_pool', :name => "#{@pool.name.capitalize}")
 
 %section.content-section.deployment
@@ -13,34 +11,35 @@
       = t 'deployments.deployment_details'
 
   .content
-    = form_for :deployment, {:url => launch_time_params_deployments_path, :method => :post, :html => {:class => 'generic'}} do
+    = form_for @deployment, {:url => launch_time_params_deployments_path, :method => :post, :html => {:class => 'generic'}} do |f|
       - if @deployment.errors.any?
         = render 'layouts/error_messages', :object => @deployment
-      = hidden_field :deployment, :pool_id
+      = f.hidden_field :pool_id
       %fieldset
         .field
           %label{:for => 'deployment_name'}
             = t 'deployments.deployment_name'
             %span
               = t 'deployments.deployment_name_subtext'
-          = text_field(:deployment, :name, :class => 'em long')
+          = f.text_field :name, :class => 'em long'
           %span#name_avail_indicator
 
       %fieldset
         .field
           %label{:for => 'deployable_id'}
-            = t('catalog_entries.index.deployable')
-          = select_tag :deployable_id, options_for_select(@deployables.map {|d| [d.name, d.id]}, params[:deployable_id])
+            = t('catalog_entries.index.catalog_entry')
+          = select_tag :deployable_id, options_from_collection_for_select(@deployables, :id, :name, params[:deployable_id])
 
       %fieldset
         .field
           %label{:for => 'frontend_realm_id'}
             = t('realms.index.realm')
-          = select :deployment, :frontend_realm_id, @realm_choices
-          %div#realm-description
+          = f.collection_select :frontend_realm_id, @realms, :id, :name, :include_blank => t('deployments.launch_new.autoselect')
+          %span#realm-description
 
       %fieldset.options
-        = submit_tag t('.next'), :class => 'button', :id => "next_button", :disabled => true
+        = link_to t('cancel'), pool_path(@pool), :class => 'button danger', :id => 'cancel_deployment_button'
+        = submit_tag t('.continue'), :class => 'button primary', :id => "next_button"
 
 
 :javascript
@@ -49,23 +48,10 @@
   // Might be nice to build something in application.js to guard against this, and that also
   // introduces a ~250ms buffer so we don't do a lookup on every single keystroke...
   $(document).ready(function () {
-    $('#deployable_id').change(function(){
-      if($(this).val() !== 'other'){
-        $('#deployable-url-section').hide();
-      }else{
-        $('#deployable-url-section').show();
-      }
-    });
     $('#deployment_name').blur(function(e) {
       e.preventDefault();
       $.get('#{check_name_deployments_path}', {name: $('#deployment_name').val() }, function(data) {
         $('#name_avail_indicator').html(data == "false" ? "#{t'deployments.launch_new.already_in_use'}" : "#{t'deployments.launch_new.name_available'}");
-        if(data == "true" && $('#next_button').is(':disabled')){
-         $('#next_button').removeAttr('disabled');
-        }else if(data == "true" && $('#next_button').is_not(':disabled')){
-        }else{
-         $('#next_button').attr("disabled", "disabled");
-        }
       });
     });
 
diff --git a/src/app/views/deployments/_overview.html.haml b/src/app/views/deployments/_overview.html.haml
index d4a90fd..69669d7 100644
--- a/src/app/views/deployments/_overview.html.haml
+++ b/src/app/views/deployments/_overview.html.haml
@@ -1,25 +1,26 @@
 - if @deployment.errors.any?
   = render 'layouts/error_messages', :object => @deployment
-= form_for @deployment, :html => {:class => 'generic horizontal'} do
-  = hidden_field :deployment, :pool_id
-  = hidden_field :deployment, :frontend_realm_id
+= form_for @deployment, :html => {:class => 'generic horizontal'} do |f|
+  = f.hidden_field :pool_id
+  = f.hidden_field :name
+  = f.hidden_field :frontend_realm_id
   = hidden_field_tag :launch_parameters_encoded, @launch_parameters_encoded
   = hidden_field_tag :deployable_id, params[:deployable_id]
 
   %header.page-header
-    %h1.pools= t('.pool_name', :pool => @pool.name)
+    /%h1.pools= t('.pool_name', :pool => @pool.name)
+    %h1.deployments= t('.header', :deployment => @deployment.name, :pool => @pool.name)
 
   %section.content-section
     %header.align-center
-      %h2= t('.header', :deployment => @deployment.name, :pool => @pool.name)
       %p= t('.confirmation', :deployment => @deployment.name, :pool => @pool.name, :quota => number_to_percentage(@additional_quota, :precision => 0))
 
     .content
       %fieldset
         .field
-          = label_tag :name, t('.name'), :class => 'em'
+          = f.label :name, t('.name'), :class => 'em'
           .input
-            = text_field :deployment, :name, :class => 'em long'
+            %span.text_instead_input.em= @deployment.name
 
         - unless @deployment.deployable_xml.description.blank?
           .field
@@ -36,15 +37,19 @@
 
         - if @realms.any?
           .field
-            = label_tag :realm, t('realms.index.realm')
+            = f.label :frontend_realm_id
             .input
-              = select :deployment, :frontend_realm_id, @realm_choices
-              %div#realm-description
+              - if @deployment.frontend_realm_id
+                %span.text_instead_input= @deployment.frontend_realm.name
+                /= select :deployment, :frontend_realm_id, @realm_choices
+                %div#realm-description= @deployment.frontend_realm.description
+              - else
+                %span.text_instead_input= t('deployments.launch_new.autoselect')
 
         .field
-          = label_tag :launch_with_errors , t('.launch_with_errors')
+          = f.label :launch_with_errors , t('.launch_with_errors')
           .input
-            = check_box :deployment, :partial_launch
+            = f.check_box :partial_launch
 
         .field
           = label_tag :image_ids, t('.image_ids')
@@ -58,8 +63,3 @@
         = submit_tag t(:back), :class => 'button', :value => 'back'
         = submit_tag t('.launch'), :id => 'launch_deployment_button', :disabled => (@errors && @errors.any?)
 -# Quota checking to go here as well, once I understand how it works with new code
-
-:javascript
-  $(document).ready(function () {
-    Conductor.fetchAjaxDescription($('#deployment_frontend_realm_id'), $('#realm-description'), "#{realms_url}/");
-  });
diff --git a/src/app/views/deployments/launch_from_catalog.html.haml b/src/app/views/deployments/launch_from_catalog.html.haml
index cc2cf12..0f7f12a 100644
--- a/src/app/views/deployments/launch_from_catalog.html.haml
+++ b/src/app/views/deployments/launch_from_catalog.html.haml
@@ -17,7 +17,7 @@
               %p= deployable.description
             .controls
               = link_to t(".details"),"#", :class => "collapse"
-              = button_to t(".select"), launch_time_params_deployments_path(:deployment => { :name => deployable.name, :pool_id => @catalog.pool.id }, :deployable_id => deployable.id ), :class=> "button pill"
+              = button_to t(".select"), launch_new_deployments_path(:deployment => { :name => deployable.name, :pool_id => @catalog.pool.id }, :deployable_id => deployable.id ), :class=> "button pill"
           .details.collapsible.collapsed
             %p= deployable.description
             %table
diff --git a/src/app/views/deployments/launch_time_params.html.haml b/src/app/views/deployments/launch_time_params.html.haml
index 78cbd69..84a173c 100644
--- a/src/app/views/deployments/launch_time_params.html.haml
+++ b/src/app/views/deployments/launch_time_params.html.haml
@@ -1,7 +1,8 @@
 = render :partial => 'layouts/nav_history'
 %header.page-header
   %h1.deployments
-    = t('pools.header_show.pool_name', :name => @deployment.pool.name)
+    = t('deployments.new_deployment')
+    %span= t('deployments.launch_new.to_pool', :name => @pool.name)
 
 %section.content-section.deployments.launch-time-params
   %h2= t('.configure_launch_time_params')
@@ -43,7 +44,10 @@
 
       #service-headers
 
-    = submit_tag t('.finalize'), :class => 'button primary', :id => 'submit_params'
+    %fieldset.options.align-center
+      = link_to t(:cancel), pool_path(@pool), :class => 'button danger', :id => 'cancel_deployment_button'
+      = submit_tag t(:back), :class => 'button', :value => 'back'
+      = submit_tag t('.continue'), :class => 'button primary', :id => 'submit_params'
 
 :javascript
   function checkEnteredParams() {
diff --git a/src/app/views/pools/_pretty_list.html.haml b/src/app/views/pools/_pretty_list.html.haml
index 4e16b69..0177499 100644
--- a/src/app/views/pools/_pretty_list.html.haml
+++ b/src/app/views/pools/_pretty_list.html.haml
@@ -15,7 +15,7 @@
           = link_to(pool_details_label, pool_path(pool), :class =>'pool_details')
         %li
           - if check_privilege(Privilege::CREATE, Deployment, pool)
-            = link_to t('deployments.new_deployment'), launch_new_deployments_path(:pool_id => pool.id), :class => 'button primary', :id => 'new_deployment_button'
+            = link_to t('deployments.new_deployment'), launch_new_deployments_path(:deployment => { :pool_id => pool.id }), :class => 'button primary', :id => 'new_deployment_button'
         %li.catalog_link
           = render :partial => 'layouts/catalog_dropdown', :locals => {:catalogs => pool.catalogs}
 
diff --git a/src/app/views/pools/show.html.haml b/src/app/views/pools/show.html.haml
index 03317b1..db7c774 100644
--- a/src/app/views/pools/show.html.haml
+++ b/src/app/views/pools/show.html.haml
@@ -3,7 +3,7 @@
   .obj_actions
     - if check_privilege(Privilege::CREATE, Deployment, @pool)
       = link_to t('deployments.new_deployment'),
-                launch_new_deployments_path(:pool_id => @pool.id),
+                launch_new_deployments_path(:deployment => {:pool_id => @pool.id}),
                 :class => 'button primary',
                 :id => 'new_deployment_button'
     .catalog_link
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index e0c0cba..481c587 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -307,7 +307,7 @@ en:
     launch_time_params:
       title: "Launch-time parameters"
       configure_launch_time_params: "Configure launch-time parameters for your deployment:"
-      finalize: Finalize
+      continue: "Continue"
       reference: "Reference to %{assembly}'s return parameter: %{parameter}"
     overview:
       pool_name: "%{pool} Pool"
@@ -354,7 +354,7 @@ en:
       to_pool: "to %{name} Pool"
       already_in_use: That name is already in use
       name_available: Name available
-      next: Next
+      continue: "Continue"
       load_definition: Load definition from XML
       other: other
       autoselect: Auto-select
@@ -387,7 +387,7 @@ en:
       warning:
         disabled_pool: "Cannot launch a Deployment in this Pool. The pool has been disabled."
         failed_to_launch: "Deployment launch failed!"
-        deployable_not_selected: "You need to select deployable"
+        deployable_not_selected: "Please select Deployable. It might need to get created first."
       error:
         not_launched: "Some assemblies will not be launched:"
         failed_to_launch_assemblies: "Failed to launch following assemblies:"
@@ -412,6 +412,7 @@ en:
       cannot_stop: "stop cannot be performed on this instance."
       all_stopped: "All instances must be stopped or running"
       not_valid_deployable_xml: "seems to be not valid deployable XML: %{msg}"
+      empty_deployable_xml: "is empty"
       match_not_found: "Match not found: %{errors}"
     preset_filters:
       all_deployments: All Deployments
diff --git a/src/features/deployables.feature b/src/features/deployables.feature
index f63670c..92bab6b 100644
--- a/src/features/deployables.feature
+++ b/src/features/deployables.feature
@@ -47,7 +47,7 @@ Feature: Manage Catalog Entries
     And there is "front_hwp2" conductor hardware profile
     And I am on testdepl's catalog entry page
     When I follow "launch_deployment_button"
-    Then I should be on the launch time params deployments page
+    Then I should be on the launch new deployments page
 
   Scenario: Delete deployables
     Given there is a "default" catalog
-- 
1.7.7.6




More information about the aeolus-devel mailing list