[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