Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service dialog generation rely only on OrchestrationParameterConstraint #16047

Merged
merged 1 commit into from
Sep 29, 2017

Conversation

bzwei
Copy link
Contributor

@bzwei bzwei commented Sep 26, 2017

Before we tried to determine the dialog field type, specifically text box or text area, by OrchestrationParameter#data_type. It turned out it is not reliable because every provider has its own data types.

With this work OrchestrationParameterConstraintMultiline is introduced for selecting text area. We will need to enhance each provider's template parsing code to utilize this new constraint. More PRs to follow.

Spec test has been enhanced to share code.

Also enable multiple selection of a dropdown.

https://bugzilla.redhat.com/show_bug.cgi?id=1489908

@bzwei
Copy link
Contributor Author

bzwei commented Sep 26, 2017

@miq-bot add_label enhancement
@miq-bot add_label bug
@miq-bot assign @gmcculloug

:dialog_group => group
:type => "DialogFieldDropDownList",
:name => "#{name_prefix}#{parameter.name}",
:data_type => "string",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. Why are drop-downs always strings now?

@@ -59,7 +62,7 @@ def create_dynamic_dropdown_list(parameter, group, position, dynamic_dropdown, n
group.dialog_fields.build(
:type => "DialogFieldDropDownList",
:name => "#{name_prefix}#{parameter.name}",
:data_type => parameter.data_type || "string",
:data_type => "string",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bzwei Does this mean we would no longer support dropdown-list with an 'integer' type? I would image that is something we would still need.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameter.data_type is provider specific type. It can be any value or any spelling. We will convert the string back to required value per provider before sending the provisioning request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is only for orchestration and only the drop-downs I am ok with this as long as you are processing the values before sending to the provider. Previously I was thinking of this issue we ran into (https://bugzilla.redhat.com/show_bug.cgi?id=1478367) where we were passing the value to Azure deployment as an integer instead of a string.

group.dialog_fields.build(
:type => 'DialogFieldTextAreaBox',
:name => "#{name_prefix}#{parameter.name}",
:data_type => 'string',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TextAreaBox I agree only makes sense for strings. 👍

:values => [%w(val1 val1), %w(val2 val2)],
:reconfigurable => true,
:force_multi_value => true
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubocop errors can be resolved with this formatting:

        assert_field(test_field(dialog),
                     DialogFieldDropDownList,
                     :name              => "param_user",
                     :default_value     => "val1",
                     :values            => [%w(val1 val1), %w(val2 val2)],
                     :reconfigurable    => true,
                     :force_multi_value => true)

:name => "param_user",
:default_value => "val1",
:values => [[nil, "<Choose>"], %w(key1 val1), %w(key2 val2)]
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assert_field call needs the same formatting changes.

@miq-bot
Copy link
Member

miq-bot commented Sep 28, 2017

Checked commit bzwei@cb31d23 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 0 offenses detected
Everything looks fine. 🍰

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants