From d034b348329a9a91dc4e4ef8b8b2cba80450bcad Mon Sep 17 00:00:00 2001 From: borisko123 Date: Thu, 7 May 2020 13:00:45 +0300 Subject: [PATCH] -Issue #20124 -fix error in spec example -the test result depends on file order -code style review -modified according to miq-bot request -fix rubocop offence -add new flag (for dialog), modify specs - after review, simplify export dialog, Tempfile creation --- lib/task_helpers/exports/custom_buttons.rb | 11 +- lib/task_helpers/imports.rb | 1 + lib/task_helpers/imports/custom_buttons.rb | 74 +++++++--- .../imports/custom_buttons_spec.rb | 113 ++++++++++++--- .../data/custom_buttons/CustomButtons.yaml | 4 + .../custom_buttons/CustomButtonsForUpdate.yml | 136 ++++++++++++++++++ ...CustomButtonsForUpdateOneButtonChanged.yml | 60 ++++++++ 7 files changed, 361 insertions(+), 38 deletions(-) create mode 100644 spec/lib/task_helpers/imports/data/custom_buttons/CustomButtonsForUpdate.yml create mode 100644 spec/lib/task_helpers/imports/data/custom_buttons/CustomButtonsForUpdateOneButtonChanged.yml diff --git a/lib/task_helpers/exports/custom_buttons.rb b/lib/task_helpers/exports/custom_buttons.rb index 6e5a98c853ca..3c8388237fbb 100644 --- a/lib/task_helpers/exports/custom_buttons.rb +++ b/lib/task_helpers/exports/custom_buttons.rb @@ -3,11 +3,15 @@ class Exports class CustomButtons class ExportArInstances EXCLUDE_ATTRS = %w(id created_on updated_on created_at updated_at dialog_id resource_id).freeze + def self.export_object(obj, hash) class_name = obj.class.name.underscore $log.info("Exporting #{obj.class.name}: #{obj.try('name')} (ID: #{obj&.id})") - (hash[class_name] ||= []) << item = { 'attributes' => build_attr_list(obj.try(:attributes)) } + attrs = build_attr_list(obj.attributes) + # handle dialog label + attrs["dialog_label"] = obj.dialog&.label if obj.respond_to?(:dialog) + (hash[class_name] ||= []) << item = {'attributes' => attrs} create_association_list(obj, item) descendant_list(obj, item) end @@ -18,8 +22,9 @@ def self.build_attr_list(attrs) def self.create_association_list(obj, item) associations = obj.class.try(:reflections) + if associations - associations = associations.collect { |model, assoc| { model => assoc.class.to_s.demodulize } }.select { |as| as.values.first != "BelongsToReflection" && as.keys.first != "all_relationships" } + associations = associations.collect { |model, assoc| {model => assoc.class.to_s.demodulize} }.select { |as| as.values.first != "BelongsToReflection" && as.keys.first != "all_relationships" } associations.each do |assoc| assoc.each do |a| next if obj.try(a.first.to_sym).blank? @@ -36,7 +41,7 @@ def self.descendant_list(obj, item) def export(options = {}) parent_id_list = [] - objects = CustomButton.where.not(:applies_to_class => %w(ServiceTemplate GenericObject)) + objects = CustomButton.in_region(MiqRegion.my_region_number).order(:id).where.not(:applies_to_class => %w[ServiceTemplate GenericObject]) export = objects.each_with_object({}) do |obj, export_hash| if obj.try(:parent).present? diff --git a/lib/task_helpers/imports.rb b/lib/task_helpers/imports.rb index 81eef8a7762b..7ffd9c2c8786 100644 --- a/lib/task_helpers/imports.rb +++ b/lib/task_helpers/imports.rb @@ -5,6 +5,7 @@ def self.parse_options options = Optimist.options(EvmRakeHelper.extract_command_options) do opt :source, 'Directory or file to import from', :type => :string, :required => true opt :overwrite, 'Overwrite existing object', :type => :boolean, :default => true + opt :connect_dialog_by_name, 'for custom buttons: in case dialog with exported name exist, connect it' end error = validate_source(options[:source]) diff --git a/lib/task_helpers/imports/custom_buttons.rb b/lib/task_helpers/imports/custom_buttons.rb index 5724d51b9bee..bbf40064fca2 100644 --- a/lib/task_helpers/imports/custom_buttons.rb +++ b/lib/task_helpers/imports/custom_buttons.rb @@ -3,12 +3,13 @@ class Imports class CustomButtons def import(options) return unless options[:source] + glob = File.file?(options[:source]) ? options[:source] : "#{options[:source]}/*.yaml" Dir.glob(glob) do |filename| $log.info("Importing Custom Buttons from: #{filename}") begin - import_custom_buttons(filename) + import_custom_buttons(filename, options[:connect_dialog_by_name]) rescue StandardError raise StandardError, "Error importing #{filename} at #{$@}" end @@ -18,14 +19,30 @@ def import(options) private class ImportArInstances - def self.import(obj_hash) - new.import(obj_hash) + def self.import(obj_hash, connect_dialog_by_name) + new.import(obj_hash, connect_dialog_by_name) end - def import(obj_hash) + def import(obj_hash, connect_dialog_by_name) + @connect_dialog = connect_dialog_by_name ActiveRecord::Base.transaction { obj_hash.each { |obj_def| create_object(*obj_def) } } end + def find_or_create_object(class_name, obj) + klass = class_name.camelize.constantize + attributes = obj['attributes'] + name = attributes.fetch('name', nil) + instance = klass.in_region(MiqRegion.my_region_number).find_by(:name => name) if name + # create new object + if instance # update existed object + instance.update!(attributes&.except('guid')) + populate_item(instance, obj, class_name) + else + item = klass.create!(attributes&.except('guid')) + populate_item(item, obj, class_name) + end + end + def create_object(class_name, obj_array) klass = class_name.camelize.constantize @@ -34,12 +51,7 @@ def create_object(class_name, obj_array) order = obj.fetch_path('attributes', 'set_data', :button_order) obj['attributes']['set_data'][:button_order] = nil if order.present? end - - klass.create!(obj['attributes']&.except('guid')).tap do |new_obj| - add_children(obj, new_obj) - add_associations(obj, new_obj) - try("#{class_name}_post", new_obj) - end + find_or_create_object(class_name, obj) end end @@ -52,10 +64,13 @@ def add_children(obj, new_obj) end def add_associations(obj, new_obj) - if obj['associations'].present? - obj['associations'].each do |assoc| - new_obj.send("#{assoc.first}=", create_object(*assoc).first) - end + return if obj['associations'].blank? + + # try to find existing resource_actions + resource_action = ResourceAction.find_by(:resource_id => new_obj.id, :resource_type => 'CustomButton') + obj['associations'].each do |assoc| + populate_action(new_obj, assoc) { create_object(*assoc).first } unless resource_action + populate_action(new_obj, assoc) { |args| resource_action.tap { |ra| ra.update!(args) } } if resource_action end end @@ -72,11 +87,38 @@ def check_user(new_obj) existing_user = User.find_by(:name => new_obj[:userid]) new_obj.update(:userid => existing_user.nil? ? "admin" : existing_user) end + + private + + def populate_item(item, obj, class_name) + item.tap do |i| + add_children(obj, i) + add_associations(obj, i) + try("#{class_name}_post", i) + end + end + + def populate_action(item, assoc) + args = assoc.second.first['attributes'] + dialog_label = args.delete('dialog_label') + + dialog = nil + # check if imported data has dialog data, if option 'connect_dialog_by_name' is set + if @connect_dialog + unless dialog_label.nil? + dialog = Dialog.in_region(MiqRegion.my_region_number).find_by(:label => dialog_label) + $log.info("Unable to locate dialog: [#{dialog_label}]") unless dialog + end + end + resource_action = yield args + resource_action['dialog_id'] = dialog.id if dialog + item.send("#{assoc.first}=", resource_action) + end end - def import_custom_buttons(filename) + def import_custom_buttons(filename, connect_dialog_by_name) custom_buttons = YAML.load_file(filename) - ImportArInstances.import(custom_buttons) + ImportArInstances.import(custom_buttons, connect_dialog_by_name) end end end diff --git a/spec/lib/task_helpers/imports/custom_buttons_spec.rb b/spec/lib/task_helpers/imports/custom_buttons_spec.rb index 61dc256c95e3..4c5f6ffe9074 100644 --- a/spec/lib/task_helpers/imports/custom_buttons_spec.rb +++ b/spec/lib/task_helpers/imports/custom_buttons_spec.rb @@ -5,7 +5,12 @@ let(:custom_button_set_name) { 'group1|Vm|' } let(:custom_button_set_description) { 'group1' } let(:resource_action_ae_namespace) { 'SYSTEM' } - let(:options) { {:source => source} } + let(:connect_dialog_by_name) { true } + let(:options) { {:source => source, :connect_dialog_by_name => connect_dialog_by_name} } + let(:custom_button_1_name) { 'button 1' } + let!(:test_dialog) { FactoryBot.create(:dialog, :label => 'dialog') } + let!(:test_dialog_2) { FactoryBot.create(:dialog, :label => 'dialog 2') } + let(:for_update_custom_button_file) { 'CustomButtonsForUpdate.yml' } describe "#import" do describe "when the source is a directory" do @@ -19,19 +24,18 @@ end context "with existing identical buttons" do - it 'should raise' do + it 'should NOT raise' do TaskHelpers::Imports::CustomButtons.new.import(options) - assert_raises_import_error + assert_imports_only_custom_button_set_one end end context "yaml import failure" do it 'should raise' do - file = Tempfile.new('foo.yaml', data_dir) - file.write("bad yaml here") - TaskHelpers::Imports::CustomButtons.new.import(options) - assert_raises_import_error - assert_imports_only_custom_button_set_one + Tempfile.create(%w[foo .yaml], data_dir) do |file| + file.write("bad yaml here") + assert_raises_import_error + end end end end @@ -41,29 +45,79 @@ context "without existing buttons" do context "only imports good yaml" do - it 'imports a specified file' do - TaskHelpers::Imports::CustomButtons.new.import(options) - assert_test_custom_button_set_present - assert_imports_only_custom_button_set_one + context "connect dialog" do + it 'imports a specified file' do + TaskHelpers::Imports::CustomButtons.new.import(options) + assert_test_custom_button_set_present + assert_imports_only_custom_button_set_one + assert_dialog_is_set(true) + end + end + context "NOT connect dialog" do + let(:connect_dialog_by_name) { false } + it 'imports a specified file' do + TaskHelpers::Imports::CustomButtons.new.import(options) + assert_test_custom_button_set_present + assert_imports_only_custom_button_set_one + assert_dialog_is_set(false) + end end end + end - context "doesn't import bad yaml" do - let(:source) { "#{data_dir}/#{bad_custom_button_file}" } - it 'does not imports a specified file' do - TaskHelpers::Imports::CustomButtons.new.import(options) - assert_imports_no_custom_buttons - end + context "doesn't import bad yaml" do + let(:source) { "#{data_dir}/#{bad_custom_button_file}" } + it 'does not imports a specified file' do + TaskHelpers::Imports::CustomButtons.new.import(options) + assert_imports_no_custom_buttons end end context "with existing identical buttons" do it 'should not import anything' do TaskHelpers::Imports::CustomButtons.new.import(options) - assert_raises_import_error assert_imports_only_custom_button_set_one end end + + context "updating" do + let(:source) { "#{data_dir}/#{custom_button_file}" } + context "updated with the same data" do + it "should remain the same" do + TaskHelpers::Imports::CustomButtons.new.import(options) + assert_imports_custom_buttons(3) + assert_button_data('#ff0000') + # call second time, in order to test update logic + TaskHelpers::Imports::CustomButtons.new.import(options) + assert_imports_custom_buttons(3) + assert_button_data('#ff0000') + end + end + context "updated with the group and button's color" do + let(:source) { "#{data_dir}/#{custom_button_file}" } + it "color should be changed" do + TaskHelpers::Imports::CustomButtons.new.import(options) + assert_imports_custom_buttons(3) + options[:source] = "#{data_dir}/#{for_update_custom_button_file}" + # call second time, in order to test update logic + TaskHelpers::Imports::CustomButtons.new.import(options) + assert_imports_custom_buttons(3) + assert_button_data('#aabbcc') + assert_button_set_data('#112233') + assert_resource_action_data + end + end + end + end + end + + def assert_dialog_is_set(connect) + btn1 = CustomButton.find_by(:name => custom_button_1_name) + expect(btn1).to be + if connect + expect(btn1.resource_action.dialog.id).to eq(test_dialog_2.id) + else + expect(btn1.resource_action.dialog).to be_nil end end @@ -86,4 +140,25 @@ def assert_imports_no_custom_buttons def assert_raises_import_error expect { TaskHelpers::Imports::CustomButtons.new.import(options) }.to raise_error(StandardError, /Error importing/) end + + def assert_imports_custom_buttons(count = 0) + expect(CustomButton.count).to eq(count) + end + + def assert_button_data(color) + b1 = CustomButton.find_by(:name => 'button 1') + expect(b1).to be + expect(b1.options[:button_color]).to eq(color) + end + + def assert_button_set_data(color) + cbs1 = CustomButtonSet.find_by(:name => 'group1|Vm|') + expect(cbs1).to be + expect(cbs1.set_data[:button_color]).to eq(color) + end + + def assert_resource_action_data + b1 = CustomButton.find_by(:name => 'button 1') + expect(b1.resource_action.action).to eq('my_action') + end end diff --git a/spec/lib/task_helpers/imports/data/custom_buttons/CustomButtons.yaml b/spec/lib/task_helpers/imports/data/custom_buttons/CustomButtons.yaml index c8732ffa14c3..e75fd41613fa 100644 --- a/spec/lib/task_helpers/imports/data/custom_buttons/CustomButtons.yaml +++ b/spec/lib/task_helpers/imports/data/custom_buttons/CustomButtons.yaml @@ -30,6 +30,7 @@ custom_button_set: visibility_expression: options: :button_icon: pficon pficon-cpu + :button_color: "#ff0000" :button_type: default :display: true :open_url: false @@ -57,6 +58,7 @@ custom_button_set: request: test1 configuration_template_id: configuration_template_type: + dialog_label: dialog 2 - attributes: guid: 3f50d617-851e-451f-95ae-a17fc548cb11 description: button 2 @@ -92,6 +94,7 @@ custom_button_set: request: test2 configuration_template_id: configuration_template_type: + dialog_label: - attributes: guid: d3cd608a-f476-48b7-aa25-a930ec046e00 description: multiselect @@ -132,3 +135,4 @@ custom_button_set: request: multiselect configuration_template_id: configuration_template_type: + dialog_label: diff --git a/spec/lib/task_helpers/imports/data/custom_buttons/CustomButtonsForUpdate.yml b/spec/lib/task_helpers/imports/data/custom_buttons/CustomButtonsForUpdate.yml new file mode 100644 index 000000000000..b8b7b43245d9 --- /dev/null +++ b/spec/lib/task_helpers/imports/data/custom_buttons/CustomButtonsForUpdate.yml @@ -0,0 +1,136 @@ +--- +custom_button_set: +- attributes: + name: group1|Vm| + description: group1 + set_type: CustomButtonSet + guid: ba4ff235-75eb-4fa4-a9f9-b854d4186c3a + read_only: + set_data: + :button_order: + - 2 + - 3 + - 10 + :button_icon: ff ff-class + :button_color: "#112233" + :display: true + :applies_to_class: Vm + :group_index: 1 + mode: + owner_type: + owner_id: + userid: + group_id: + children: + custom_button: + - attributes: + guid: f059931f-8703-4bcf-b876-e482d38ce8ea + description: button 1 + applies_to_class: Vm + visibility_expression: + options: + :button_icon: pficon pficon-cpu + :button_color: "#aabbcc" + :button_type: default + :display: true + :open_url: false + :display_for: single + :submit_how: one + userid: admin + wait_for_complete: + name: button 1 + visibility: + :roles: + - _ALL_ + applies_to_id: + enablement_expression: + disabled_text: + associations: + resource_action: + - attributes: + action: my_action + resource_type: CustomButton + ae_namespace: SYSTEM + ae_class: PROCESS + ae_instance: Request + ae_message: + ae_attributes: + request: test1 + configuration_template_id: + configuration_template_type: + dialog_label: dialog + - attributes: + guid: 3f50d617-851e-451f-95ae-a17fc548cb11 + description: button 2 + applies_to_class: Vm + visibility_expression: + options: + :button_icon: pficon pficon-home + :button_color: "#c03638" + :button_type: default + :display: true + :open_url: false + :display_for: single + :submit_how: one + userid: + wait_for_complete: + name: button 2 + visibility: + :roles: + - _ALL_ + applies_to_id: + enablement_expression: + disabled_text: + associations: + resource_action: + - attributes: + action: + resource_type: CustomButton + ae_namespace: SYSTEM + ae_class: PROCESS + ae_instance: Request + ae_message: + ae_attributes: + request: test2 + configuration_template_id: + configuration_template_type: + - attributes: + guid: d3cd608a-f476-48b7-aa25-a930ec046e00 + description: multiselect + applies_to_class: Vm + visibility_expression: !ruby/object:MiqExpression + exp: + "=": + field: Vm-power_state + value: 'on' + context_type: + options: + :button_icon: fa fa-users + :button_color: "#996633" + :button_type: default + :display: true + :open_url: false + :display_for: both + :submit_how: all + userid: admin + wait_for_complete: + name: multiselect + visibility: + :roles: + - _ALL_ + applies_to_id: + enablement_expression: + disabled_text: + associations: + resource_action: + - attributes: + action: + resource_type: CustomButton + ae_namespace: SYSTEM + ae_class: PROCESS + ae_instance: Request + ae_message: + ae_attributes: + request: multiselect + configuration_template_id: + configuration_template_type: diff --git a/spec/lib/task_helpers/imports/data/custom_buttons/CustomButtonsForUpdateOneButtonChanged.yml b/spec/lib/task_helpers/imports/data/custom_buttons/CustomButtonsForUpdateOneButtonChanged.yml new file mode 100644 index 000000000000..e3e490dfd0cb --- /dev/null +++ b/spec/lib/task_helpers/imports/data/custom_buttons/CustomButtonsForUpdateOneButtonChanged.yml @@ -0,0 +1,60 @@ +--- +custom_button_set: +- attributes: + name: group1|Vm| + description: group1 + set_type: CustomButtonSet + guid: ba4ff235-75eb-4fa4-a9f9-b854d4186c3a + read_only: + set_data: + :button_order: + - 2 + - 3 + - 10 + :button_icon: ff ff-class + :button_color: "#a341ab" + :display: true + :applies_to_class: Vm + :group_index: 1 + mode: + owner_type: + owner_id: + userid: + group_id: + children: + custom_button: + - attributes: + guid: f059931f-8703-4bcf-b876-e482d38ce8ea + description: button 1 + applies_to_class: Vm + visibility_expression: + options: + :button_icon: pficon pficon-cpu + :button_type: default + :display: true + :open_url: false + :display_for: single + :submit_how: one + userid: admin + wait_for_complete: + name: button 1 + visibility: + :roles: + - _ALL_ + applies_to_id: + enablement_expression: + disabled_text: + associations: + resource_action: + - attributes: + action: + resource_type: CustomButton + ae_namespace: SYSTEM + ae_class: PROCESS + ae_instance: Request + ae_message: + ae_attributes: + request: test1 + configuration_template_id: + configuration_template_type: + dialog_label: dialog