-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 (@lfu, Tempfile creation (@gmcculloug)
- after review, simplify populate action (@lfu)

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?

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])

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,30 @@ 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)
+          # may contain dialog_label, then find and connect dialog (optionally)
+          dialog_label = assoc.last.first['attributes'].delete('dialog_label')
+          resource_action = create_object(*assoc).first
+          resource_action.dialog = Dialog.in_region(MiqRegion.my_region_number).find_by(:label => dialog_label) if
+              dialog_label && @connect_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

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

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: