From 4b046292408445b2fe2de986870b7b114a8c160d Mon Sep 17 00:00:00 2001 From: Erik Clarizio Date: Fri, 10 Aug 2018 14:39:11 -0700 Subject: [PATCH 1/2] Add specific funnel for provision_workflow Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1613935 --- app/models/dialog.rb | 2 ++ app/models/resource_action_workflow.rb | 3 ++ app/models/service_template.rb | 15 +++++---- spec/models/dialog_spec.rb | 23 +++++++++++++ spec/models/resource_action_workflow_spec.rb | 10 ++++++ spec/models/service_template_spec.rb | 34 +++++++++++++++++--- 6 files changed, 77 insertions(+), 10 deletions(-) diff --git a/app/models/dialog.rb b/app/models/dialog.rb index 0271fd79d0f..00122abceb6 100644 --- a/app/models/dialog.rb +++ b/app/models/dialog.rb @@ -89,6 +89,8 @@ def validate_field_data end def load_values_into_fields(values) + values = values.with_indifferent_access + dialog_field_hash.each_value do |field| field.dialog = self field.value = values[field.automate_key_name] || values[field.name] diff --git a/app/models/resource_action_workflow.rb b/app/models/resource_action_workflow.rb index 550ef80bf8c..237f8db1790 100644 --- a/app/models/resource_action_workflow.rb +++ b/app/models/resource_action_workflow.rb @@ -95,6 +95,9 @@ def load_dialog(resource_action, values, options) dialog.target_resource = @target if options[:display_view_only] dialog.init_fields_with_values_for_request(values) + elsif options[:provision_workflow] + dialog.initialize_value_context(values) + dialog.load_values_into_fields(values) elsif options[:refresh] || options[:submit_workflow] dialog.load_values_into_fields(values) elsif options[:reconfigure] diff --git a/app/models/service_template.rb b/app/models/service_template.rb index a62826fc260..e514af80407 100644 --- a/app/models/service_template.rb +++ b/app/models/service_template.rb @@ -379,7 +379,8 @@ def self.create_from_options(options) end private_class_method :create_from_options - def provision_request(user, options = nil, request_options = nil) + def provision_request(user, options = nil, request_options = {}) + request_options[:provision_workflow] = true result = order(user, options, request_options) raise result[:errors].join(", ") if result[:errors].any? result[:request] @@ -421,13 +422,15 @@ def order(user_or_id, options = nil, request_options = nil, schedule_time = nil) end end - def provision_workflow(user, dialog_options = nil, request_options = nil) + def provision_workflow(user, dialog_options = nil, request_options = {}) dialog_options ||= {} - request_options ||= {} + request_options.delete(:provision_workflow) if request_options[:submit_workflow] + ra_options = { - :target => self, - :initiator => request_options[:initiator], - :submit_workflow => request_options[:submit_workflow] + :target => self, + :initiator => request_options[:initiator], + :submit_workflow => request_options[:submit_workflow], + :provision_workflow => request_options[:provision_workflow] } ResourceActionWorkflow.new(dialog_options, user, provision_action, ra_options).tap do |wf| diff --git a/spec/models/dialog_spec.rb b/spec/models/dialog_spec.rb index 660e62a792a..1f46e6ee844 100644 --- a/spec/models/dialog_spec.rb +++ b/spec/models/dialog_spec.rb @@ -458,6 +458,29 @@ end end + describe "#load_values_into_fields" do + let(:dialog) { described_class.new(:dialog_tabs => [dialog_tab]) } + let(:dialog_tab) { DialogTab.new(:dialog_groups => [dialog_group]) } + let(:dialog_group) { DialogGroup.new(:dialog_fields => [dialog_field1]) } + let(:dialog_field1) { DialogField.new(:value => "123", :name => "field1") } + + context "string values" do + it "sets field value" do + vars = {"field1" => "10.8.99.248"} + dialog.load_values_into_fields(vars) + expect(dialog_field1.value).to eq("10.8.99.248") + end + end + + context "symbol values" do + it "sets field value" do + vars = {:field1 => "10.8.99.248"} + dialog.load_values_into_fields(vars) + expect(dialog_field1.value).to eq("10.8.99.248") + end + end + end + describe "#init_fields_with_values_for_request" do let(:dialog) { described_class.new(:dialog_tabs => [dialog_tab]) } let(:dialog_tab) { DialogTab.new(:dialog_groups => [dialog_group]) } diff --git a/spec/models/resource_action_workflow_spec.rb b/spec/models/resource_action_workflow_spec.rb index eea11b8babe..11e15e70eed 100644 --- a/spec/models/resource_action_workflow_spec.rb +++ b/spec/models/resource_action_workflow_spec.rb @@ -158,6 +158,16 @@ end end + context "when the options are set to a provision workflow request" do + let(:options) { {:provision_workflow => true} } + + it "initializes the value context and then loads the values into fields" do + expect(dialog).to receive(:initialize_value_context).with(values).ordered + expect(dialog).to receive(:load_values_into_fields).with(values).ordered + ResourceActionWorkflow.new(values, nil, resource_action, options) + end + end + context "when neither display_view_only nor refresh are true" do let(:options) { {} } diff --git a/spec/models/service_template_spec.rb b/spec/models/service_template_spec.rb index 12041095d6f..8e5a2b98614 100644 --- a/spec/models/service_template_spec.rb +++ b/spec/models/service_template_spec.rb @@ -880,20 +880,25 @@ context "#provision_request" do let(:arg1) { {'ordered_by' => 'fred'} } + context "with submit_workflow" do let(:arg2) { {:initiator => 'control', :submit_workflow => true} } it "provisions a service template without errors" do expect(resource_action_workflow).to receive(:validate_dialog).and_return([]) expect(resource_action_workflow).to receive(:make_request).and_return(miq_request) - expect(resource_action_workflow).to receive(:request_options=).with(:initiator => 'control', :submit_workflow => true) + expect(resource_action_workflow).to receive(:request_options=).with( + :initiator => 'control', :submit_workflow => true + ) expect(service_template.provision_request(user, arg1, arg2)).to eq(miq_request) end it "provisions a service template with errors" do expect(resource_action_workflow).to receive(:validate_dialog).and_return(%w(Error1 Error2)) - expect(resource_action_workflow).to receive(:request_options=).with(:initiator => 'control', :submit_workflow => true) + expect(resource_action_workflow).to receive(:request_options=).with( + :initiator => 'control', :submit_workflow => true + ) expect { service_template.provision_request(user, arg1, arg2) }.to raise_error(RuntimeError) end @@ -905,18 +910,39 @@ it "provisions a service template without errors" do expect(resource_action_workflow).to receive(:validate_dialog).and_return([]) expect(resource_action_workflow).to receive(:make_request).and_return(miq_request) - expect(resource_action_workflow).to receive(:request_options=).with(:initiator => 'control') + expect(resource_action_workflow).to receive(:request_options=).with( + :initiator => 'control', :provision_workflow => true + ) expect(service_template.provision_request(user, arg1, arg2)).to eq(miq_request) end it "provisions a service template with errors" do expect(resource_action_workflow).to receive(:validate_dialog).and_return(%w(Error1 Error2)) - expect(resource_action_workflow).to receive(:request_options=).with(:initiator => 'control') + expect(resource_action_workflow).to receive(:request_options=).with( + :initiator => 'control', :provision_workflow => true + ) expect { service_template.provision_request(user, arg1, arg2) }.to raise_error(RuntimeError) end end + + context "without any request options" do + it "provisions a service template without errors" do + expect(resource_action_workflow).to receive(:validate_dialog).and_return([]) + expect(resource_action_workflow).to receive(:make_request).and_return(miq_request) + expect(resource_action_workflow).to receive(:request_options=).with(:provision_workflow => true) + + expect(service_template.provision_request(user, arg1)).to eq(miq_request) + end + + it "provisions a service template with errors" do + expect(resource_action_workflow).to receive(:validate_dialog).and_return(%w(Error1 Error2)) + expect(resource_action_workflow).to receive(:request_options=).with(:provision_workflow => true) + + expect { service_template.provision_request(user, arg1) }.to raise_error(RuntimeError) + end + end end end From f15c18b8a8b81c57fb64d071426c6b0980a650fa Mon Sep 17 00:00:00 2001 From: Erik Clarizio Date: Mon, 13 Aug 2018 08:00:10 -0700 Subject: [PATCH 2/2] Allow nil values to not overwrite old values in #load_values_into_fields Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1613935 --- app/models/dialog.rb | 7 +++-- app/models/resource_action_workflow.rb | 2 +- app/models/service_template.rb | 2 +- spec/models/dialog_spec.rb | 33 ++++++++++++++++++++ spec/models/resource_action_workflow_spec.rb | 2 +- 5 files changed, 41 insertions(+), 5 deletions(-) diff --git a/app/models/dialog.rb b/app/models/dialog.rb index 00122abceb6..e64d057a550 100644 --- a/app/models/dialog.rb +++ b/app/models/dialog.rb @@ -88,12 +88,15 @@ def validate_field_data result end - def load_values_into_fields(values) + def load_values_into_fields(values, overwrite = true) values = values.with_indifferent_access dialog_field_hash.each_value do |field| field.dialog = self - field.value = values[field.automate_key_name] || values[field.name] + new_value = values[field.automate_key_name] || values[field.name] + new_value ||= field.value unless overwrite + + field.value = new_value end end diff --git a/app/models/resource_action_workflow.rb b/app/models/resource_action_workflow.rb index 237f8db1790..4f2f3d5b47b 100644 --- a/app/models/resource_action_workflow.rb +++ b/app/models/resource_action_workflow.rb @@ -97,7 +97,7 @@ def load_dialog(resource_action, values, options) dialog.init_fields_with_values_for_request(values) elsif options[:provision_workflow] dialog.initialize_value_context(values) - dialog.load_values_into_fields(values) + dialog.load_values_into_fields(values, false) elsif options[:refresh] || options[:submit_workflow] dialog.load_values_into_fields(values) elsif options[:reconfigure] diff --git a/app/models/service_template.rb b/app/models/service_template.rb index e514af80407..57f77d9a1e4 100644 --- a/app/models/service_template.rb +++ b/app/models/service_template.rb @@ -395,7 +395,7 @@ def queue_order(user_id, options, request_options) ) end - def order(user_or_id, options = nil, request_options = nil, schedule_time = nil) + def order(user_or_id, options = nil, request_options = {}, schedule_time = nil) user = user_or_id.kind_of?(User) ? user_or_id : User.find(user_or_id) workflow = provision_workflow(user, options, request_options) if schedule_time diff --git a/spec/models/dialog_spec.rb b/spec/models/dialog_spec.rb index 1f46e6ee844..a9c74d6bb82 100644 --- a/spec/models/dialog_spec.rb +++ b/spec/models/dialog_spec.rb @@ -470,6 +470,12 @@ dialog.load_values_into_fields(vars) expect(dialog_field1.value).to eq("10.8.99.248") end + + it "sets field value when prefixed with 'dialog'" do + vars = {"dialog_field1" => "10.8.99.248"} + dialog.load_values_into_fields(vars) + expect(dialog_field1.value).to eq("10.8.99.248") + end end context "symbol values" do @@ -478,6 +484,33 @@ dialog.load_values_into_fields(vars) expect(dialog_field1.value).to eq("10.8.99.248") end + + it "sets field value when prefixed with 'dialog'" do + vars = {:dialog_field1 => "10.8.99.248"} + dialog.load_values_into_fields(vars) + expect(dialog_field1.value).to eq("10.8.99.248") + end + end + + context "when the incoming values are missing keys" do + let(:dialog_group) { DialogGroup.new(:dialog_fields => [dialog_field1, dialog_field2]) } + let(:dialog_field2) { DialogField.new(:value => "321", :name => "field2") } + + context "when overwrite is true" do + it "sets nil values" do + vars = {:field1 => "10.8.99.248"} + dialog.load_values_into_fields(vars) + expect(dialog_field2.value).to eq(nil) + end + end + + context "when overwrite is false" do + it "does not set nil values" do + vars = {:field1 => "10.8.99.248"} + dialog.load_values_into_fields(vars, false) + expect(dialog_field2.value).to eq("321") + end + end end end diff --git a/spec/models/resource_action_workflow_spec.rb b/spec/models/resource_action_workflow_spec.rb index 11e15e70eed..8db9826fbbe 100644 --- a/spec/models/resource_action_workflow_spec.rb +++ b/spec/models/resource_action_workflow_spec.rb @@ -163,7 +163,7 @@ it "initializes the value context and then loads the values into fields" do expect(dialog).to receive(:initialize_value_context).with(values).ordered - expect(dialog).to receive(:load_values_into_fields).with(values).ordered + expect(dialog).to receive(:load_values_into_fields).with(values, false).ordered ResourceActionWorkflow.new(values, nil, resource_action, options) end end