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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 35 additions & 22 deletions app/models/dialog/orchestration_template_service_dialog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ def add_field(parameter, group, position, name_prefix = nil)

checkbox = parameter.constraints.detect { |c| c.kind_of?(OrchestrationTemplate::OrchestrationParameterBoolean) }
return create_checkbox(parameter, group, position, name_prefix) if checkbox

textarea = parameter.constraints.detect { |c| c.kind_of?(OrchestrationTemplate::OrchestrationParameterMultiline) }
return create_textarea(parameter, group, position, name_prefix) if textarea
end

create_textbox(parameter, group, position, name_prefix)
Expand All @@ -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.

:dynamic => true,
:display => "edit",
:required => parameter.required,
Expand All @@ -76,34 +79,29 @@ def create_dropdown_list(parameter, group, position, dropdown, name_prefix)
values = dropdown.allowed_values
dropdown_list = values.kind_of?(Hash) ? values.to_a : values.collect { |v| [v, v] }
group.dialog_fields.build(
:type => "DialogFieldDropDownList",
:name => "#{name_prefix}#{parameter.name}",
:data_type => parameter.data_type || "string",
:display => "edit",
:required => parameter.required,
:values => dropdown_list,
:default_value => parameter.default_value || dropdown_list.first,
:label => parameter.label,
:description => parameter.description,
:reconfigurable => parameter.reconfigurable,
:position => position,
: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?

:display => "edit",
:force_multi_value => dropdown.allow_multiple,
:required => parameter.required,
:values => dropdown_list,
:default_value => parameter.default_value || dropdown_list.first,
:label => parameter.label,
:description => parameter.description,
:reconfigurable => parameter.reconfigurable,
:position => position,
:dialog_group => group
)
end

def create_textbox(parameter, group, position, name_prefix)
if parameter.data_type == 'string' || parameter.data_type == 'integer'
data_type = parameter.data_type
field_type = 'DialogFieldTextBox'
else
data_type = 'string'
field_type = 'DialogFieldTextAreaBox'
end
data_type = parameter.data_type.casecmp('integer').zero? ? 'integer' : 'string'
if parameter.constraints
pattern = parameter.constraints.detect { |c| c.kind_of?(OrchestrationTemplate::OrchestrationParameterPattern) }
end
group.dialog_fields.build(
:type => field_type,
:type => 'DialogFieldTextBox',
:name => "#{name_prefix}#{parameter.name}",
:data_type => data_type,
:display => "edit",
Expand All @@ -120,14 +118,29 @@ def create_textbox(parameter, group, position, name_prefix)
)
end

def create_textarea(parameter, group, position, name_prefix)
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. 👍

:display => "edit",
:required => parameter.required,
:default_value => parameter.default_value,
:label => parameter.label,
:description => parameter.description,
:reconfigurable => parameter.reconfigurable,
:position => position,
:dialog_group => group
)
end

def create_checkbox(parameter, group, position, name_prefix)
group.dialog_fields.build(
:type => "DialogFieldCheckBox",
:name => "#{name_prefix}#{parameter.name}",
:data_type => "boolean",
:display => "edit",
:default_value => parameter.default_value,
:options => {:protected => parameter.hidden?},
:label => parameter.label,
:description => parameter.description,
:reconfigurable => parameter.reconfigurable,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class OrchestrationTemplate
class OrchestrationParameterAllowed < OrchestrationParameterConstraint
attr_accessor :allowed_values
attr_accessor :allow_multiple
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class OrchestrationTemplate
class OrchestrationParameterMultiline < OrchestrationParameterConstraint
end
end
119 changes: 71 additions & 48 deletions spec/models/dialog/orchestration_template_service_dialog_spec.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
describe Dialog::OrchestrationTemplateServiceDialog do
let(:orchestration_template) { FactoryGirl.create(:orchestration_template) }
let(:orchestration_template) do
FactoryGirl.create(:orchestration_template).tap do |template|
allow(template).to receive(:parameter_groups).and_return(param_groups)
end
end
let(:param_groups) { create_parameters(param_options) }
let(:dialog) { described_class.create_dialog("test", orchestration_template) }

describe ".create_dialog" do
it "creates a dialog from a template without parameters" do
allow(orchestration_template).to receive(:parameter_groups).and_return([])
dialog = described_class.create_dialog("test", orchestration_template)
let(:param_groups) { [] }

it "creates a dialog from a template without parameters" do
expect(dialog).to have_attributes(
:label => "test",
:buttons => "submit,cancel"
Expand All @@ -18,52 +23,81 @@

describe "creation of dropdown parameter fields" do
context "when allowed values are given" do
let(:param_options) do
constraint = OrchestrationTemplate::OrchestrationParameterAllowed.new(:allowed_values => %w(val1 val2), :allow_multiple => true)
{:default_value => 'val1', :constraints => [constraint]}
end

it "creates a dropdown field with pairs of values" do
# Create a simple dialog with one dropdown parameter.
constraint = OrchestrationTemplate::OrchestrationParameterAllowed.new(:allowed_values => %w(val1 val2))
param_groups = create_dropdown_param(constraint)
allow(orchestration_template).to receive(:parameter_groups).and_return(param_groups)
dialog = subject.create_dialog("test", orchestration_template)

# Get the dropdown field
field = dropdown_field(dialog)
# Ensure the allowed values are properly stored.
assert_field(field, DialogFieldDropDownList, :name => "param_dropdown", :default_value => "val1", :values => [%w(val1 val1), %w(val2 val2)], :reconfigurable => true)
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)
end
end

context "when a hash of allowed values is given" do
it "creates pairs from hashes" do
# Create a simple dialog with one dropdown parameter.
let(:param_options) do
constraint = OrchestrationTemplate::OrchestrationParameterAllowed.new(:allowed_values => {"key1" => "val1", "key2" => "val2"})
param_groups = create_dropdown_param(constraint)
allow(orchestration_template).to receive(:parameter_groups).and_return(param_groups)
dialog = subject.create_dialog("test", orchestration_template)

# Get the dropdown field
field = dropdown_field(dialog)
# Ensure the allowed values are properly stored.
assert_field(field, DialogFieldDropDownList, :name => "param_dropdown", :default_value => "val1", :values => [[nil, "<Choose>"], %w(key1 val1), %w(key2 val2)])
{:default_value => 'val1', :constraints => [constraint]}
end

it "creates pairs from hashes" do
assert_field(test_field(dialog),
DialogFieldDropDownList,
:name => "param_user",
:default_value => "val1",
:values => [[nil, "<Choose>"], %w(key1 val1), %w(key2 val2)])
end
end

context "when automate method is given" do
it "creates a dropdown field requested resource_action" do
# Create a simple dialog with one dropdown parameter.
let(:param_options) do
constraint = OrchestrationTemplate::OrchestrationParameterAllowedDynamic.new(:fqname => "/Path/To/Method")
param_groups = create_dropdown_param(constraint)
allow(orchestration_template).to receive(:parameter_groups).and_return(param_groups)
dialog = subject.create_dialog("test", orchestration_template)
{:constraints => [constraint]}
end

# Get the dropdown field
field = dropdown_field(dialog)
# Ensure the field is properly defined with the resource action.
it "creates a dropdown field requested resource_action" do
field = test_field(dialog)
expect(field.resource_action.fqname).to eq("/Path/To/Method")
assert_field(field, DialogFieldDropDownList, :name => "param_dropdown")
assert_field(field, DialogFieldDropDownList, :name => "param_user")
end
end
end

context "creation of checkbox parameter field" do
let(:param_options) do
constraint = OrchestrationTemplate::OrchestrationParameterBoolean.new
{:default_value => true, :constraints => [constraint]}
end

it "creates a checkbox field" do
assert_field(test_field(dialog), DialogFieldCheckBox, :data_type => 'boolean', :default_value => 't')
end
end

describe "creation of textarea parameter field" do
let(:param_options) do
constraint = OrchestrationTemplate::OrchestrationParameterMultiline.new
{:constraints => [constraint]}
end

it "creates a textarea field" do
assert_field(test_field(dialog), DialogFieldTextAreaBox, :name => 'param_user')
end
end

describe "creation of textbox parameter field" do
let(:param_options) { {:data_type => 'integer', :default_value => 2} }

it "creates a textarea field" do
assert_field(test_field(dialog), DialogFieldTextBox, :data_type => 'integer', :default_value => '2')
end
end

def assert_stack_tab(tab)
assert_tab_attributes(tab)

Expand Down Expand Up @@ -99,7 +133,7 @@ def assert_field(field, clss, attributes)
expect(field).to have_attributes(attributes)
end

def dropdown_field(dialog)
def test_field(dialog)
# First ensure that the dialog is properly constructed.
tabs = dialog.dialog_tabs
expect(tabs.size).to eq(1)
Expand All @@ -110,19 +144,8 @@ def dropdown_field(dialog)
fields[0]
end

def create_dropdown_param(constraint)
[OrchestrationTemplate::OrchestrationParameterGroup.new(
:label => "group",
:parameters => [
OrchestrationTemplate::OrchestrationParameter.new(
:name => "dropdown",
:label => "Drop down",
:data_type => "string",
:default_value => "val1",
:required => true,
:constraints => [constraint]
)
]
)]
def create_parameters(options)
options.reverse_merge!(:name => 'user', :label => 'User Parameter', :data_type => 'string', :required => true)
[OrchestrationTemplate::OrchestrationParameterGroup.new(:label => 'group', :parameters => [OrchestrationTemplate::OrchestrationParameter.new(options)])]
end
end