On 08/13/2012 04:18 PM, jtomasek(a)redhat.com wrote:
From: Jiri Tomasek <jtomasek(a)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(-)
Hi Jirka,
could you please resend this patch? It probably remained forgotten and
it is not applicable now.
Thanks, Jan
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((a)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((a)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?((a)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((a)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?((a)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((a)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 374b4e3..0d91cf0 100644
--- a/src/app/models/deployable.rb
+++ b/src/app/models/deployable.rb
@@ -61,7 +61,7 @@ class Deployable < ActiveRecord::Base
def validate_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 fa8605f..c57754d 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 cb01cf5..80db118 100644
--- a/src/app/util/deployable_xml.rb
+++ b/src/app/util/deployable_xml.rb
@@ -239,7 +239,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((a)root.document) || []
# ...and validate the assembly
assemblies.each do |assembly|
@@ -249,7 +249,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 =>
"#{(a)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((a)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 151a84e..745b85f 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -308,7 +308,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"
@@ -355,7 +355,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
@@ -388,7 +388,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:"
@@ -413,6 +413,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