[PATCH] Moving the service parameter type attr off the parameter tag onto the value tag because the type only applys when we collect a value.
Michal Fojtik
mfojtik at redhat.com
Fri May 25 12:36:16 UTC 2012
On 05/25/12, Dan Radez wrote:
Hi Dan, some minor code nits ;-)
-- Michal
> ---
> src/app/controllers/deployables_controller.rb | 17 +++++++++++++++++
> src/app/controllers/deployments_controller.rb | 8 ++++++++
> src/app/models/deployable.rb | 17 +++++++++++++++++
> src/app/util/deployable-rng.xml | 19 ++++++++++++++++++-
> src/app/util/deployable_xml.rb | 6 ++++++
> src/app/views/layouts/_new_notification.html.haml | 6 +++++-
> src/config/locales/en.yml | 1 +
> 7 files changed, 72 insertions(+), 2 deletions(-)
>
> diff --git a/src/app/controllers/deployables_controller.rb b/src/app/controllers/deployables_controller.rb
> index 543cb53..434a41e 100644
> --- a/src/app/controllers/deployables_controller.rb
> +++ b/src/app/controllers/deployables_controller.rb
> @@ -152,6 +152,14 @@ class DeployablesController < ApplicationController
> redirect_to catalog_path(@selected_catalogs.first)
> end
> end
> +
> + # check that type attrs on service params are used properly
> + if warnings = @deployable.check_service_params_types
> + flash[:warning] = [] if not flash[:warning]
flash[:warning] ||= []
> + flash[:warning] = Array flash[:warning] if flash[:warning].class == String
flash[:warning] = [flash[:warning]] if flash[:warning].kind_of? String
> + flash[:warning] = flash[:warning] + warnings
flash[:warning]+=warnings
> + end
> +
> rescue => e
> if @deployable.errors.empty?
> logger.error e.message
> @@ -184,6 +192,13 @@ class DeployablesController < ApplicationController
> params[:deployable].delete(:owner_id) if params[:deployable]
>
> if @deployable.update_attributes(params[:deployable])
> + # check that type attrs on service params are used properly
> + if warnings = @deployable.check_service_params_types
> + flash[:warning] = [] if not flash[:warning]
> + flash[:warning] = Array flash[:warning] if flash[:warning].class == String
> + flash[:warning] = flash[:warning] + warnings
> + end
> +
> flash[:notice] = t"catalog_entries.flash.notice.updated"
> redirect_to polymorphic_path([@catalog, @deployable])
> else
> @@ -285,4 +300,6 @@ class DeployablesController < ApplicationController
> nil
> end
> end
> +
> +
> end
> diff --git a/src/app/controllers/deployments_controller.rb b/src/app/controllers/deployments_controller.rb
> index 169b1b6..dcabb79 100644
> --- a/src/app/controllers/deployments_controller.rb
> +++ b/src/app/controllers/deployments_controller.rb
> @@ -86,6 +86,14 @@ class DeploymentsController < ApplicationController
> @additional_quota = count_additional_quota(@deployment)
> render 'overview' and return
> end
> +
> + # check that type attrs on service params are used properly
> + if warnings = @deployable.check_service_params_types
> + flash[:warning] = [] if not flash[:warning]
> + flash[:warning] = Array flash[:warning] if flash[:warning].class == String
> + flash[:warning] = flash[:warning] + warnings
> + end
> +
> end
>
> def overview
> diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb
> index 201123f..02ad25f 100644
> --- a/src/app/models/deployable.rb
> +++ b/src/app/models/deployable.rb
> @@ -216,6 +216,23 @@ class Deployable < ActiveRecord::Base
> self[:pool_family_id] = catalogs.first.pool_family_id
> end
>
> +
> + def check_service_params_types
> + warnings = []
> + deployable_xml = DeployableXML.new(xml)
> + deployable_xml.assemblies.each do |assembly|
> + assembly.services.each do |service|
> + service.parameters.each do |param|
> + if param.type_warning
> + warnings << I18n.translate("deployables.flash.warning.param_type_attr",
> + :service_name => service.name,
> + :param_name => param.name)
> + end
> + end
> + end
> + end
> + end
> +
> private
>
> def self.apply_search_filter(search)
> diff --git a/src/app/util/deployable-rng.xml b/src/app/util/deployable-rng.xml
> index a8b01db..01e62f7 100644
> --- a/src/app/util/deployable-rng.xml
> +++ b/src/app/util/deployable-rng.xml
> @@ -131,6 +131,9 @@
> <define name="configuration-parameter-definition">
> <element name="parameter">
> <attribute name="name"><text/></attribute>
> + <!-- type is depricated here, it's proper use is on the value
> + tag. it can be removed is 1.2 -->
> + <optional>
> <attribute name="type">
> <choice>
> <!-- only supporting scalar and password types for now -->
> @@ -140,9 +143,23 @@
> <value>password</value>
> </choice>
> </attribute>
> + </optional>
> <choice>
> <empty/>
> - <element name="value"><text/></element>
> + <element name="value">
> + <text/>
> + <optional>
> + <attribute name="type">
> + <choice>
> + <!-- only supporting scalar and password types for now -->
> + <value>scalar</value>
> + <!-- password types simply indicate that the UI collecting the
> + value should obscure the input field's value -->
> + <value>password</value>
> + </choice>
> + </attribute>
> + </optional>
> + </element>
> <element name="reference">
> <attribute name="assembly"><text/></attribute>
> <choice>
> diff --git a/src/app/util/deployable_xml.rb b/src/app/util/deployable_xml.rb
> index 568e8c6..b299ba2 100644
> --- a/src/app/util/deployable_xml.rb
> +++ b/src/app/util/deployable_xml.rb
> @@ -30,6 +30,12 @@ class ParameterXML
> end
>
> def type
> + value_node['type'] || @root['type']
> + end
> +
> + def type_warning
> + # providing a place to see the old placement of the type
> + # attr directly so we can check it and throw warnings.
> @root['type']
> end
>
> diff --git a/src/app/views/layouts/_new_notification.html.haml b/src/app/views/layouts/_new_notification.html.haml
> index 0244d2b..d807020 100644
> --- a/src/app/views/layouts/_new_notification.html.haml
> +++ b/src/app/views/layouts/_new_notification.html.haml
> @@ -27,7 +27,11 @@
> %div.heading
> =image_tag 'flash_warning_icon.png', :alt => 'Warnings'
> %ul.flashes
> - %li= flash[:warning]
> + -if flash[:warning].kind_of?(Array)
> + - flash[:warning].each do |k, v|
> + %li= [k, v.present? ? ": #{v}" : '']
> + -else
> + %li= flash[:warning]
>
> -if flash[:error]
> -if flash[:error].kind_of?(String)
> diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> index 64e7012..dc2fac7 100644
> --- a/src/config/locales/en.yml
> +++ b/src/config/locales/en.yml
> @@ -992,6 +992,7 @@ en:
> wrong_environment: "The deployable '%{deployable}' contains an image (UUID %{uuid}) in the wrong environment ('%{wrong_env}' should be '%{environment}') and cannot be used."
> warning:
> failed: "Deployable was not created: %{message}"
> + param_type_attr: "Deprecation Warning: Parameter '%{param_name}' in service '%{service_name}' has a type attribute. Remove the type attribute or move it to a child value tag."
> notice:
> deleted:
> one: "Deployable %{deleted} removed successfully!"
> --
> 1.7.10.1
>
--
Michal Fojtik
Sr. Software Engineer, Deltacloud API (http://deltacloud.org)
More information about the aeolus-devel
mailing list