Merge pull request #16753 from eclarizio/BZ1518942

Fix for failure to update service dialog - Couldn't find DialogField without an ID
(cherry picked from commit cb5544d)

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

app/models/dialog.rb

@@ -124,6 +124,8 @@ def content(target = nil, resource_action = nil, all_attributes = false)
   # Creates a new item without an ID,
   # Removes any items not passed in the content.
   def update_tabs(tabs)
+    association_list = dialog_import_service.build_association_list("dialog_tabs" => tabs)
+
     transaction do
       updated_tabs = []
       tabs.each do |dialog_tab|
@@ -134,11 +136,15 @@ def update_tabs(tabs)
             updated_tabs << tab
           end
         else
-          updated_tabs << DialogImportService.new.build_dialog_tabs('dialog_tabs' => [dialog_tab]).first
+          updated_tabs << dialog_import_service.build_dialog_tabs('dialog_tabs' => [dialog_tab]).first
         end
       end
       self.dialog_tabs = updated_tabs
     end
+
+    transaction do
+      dialog_import_service.build_associations(self, association_list)
+    end
   end
 
   def deep_copy(new_attributes = {})
@@ -162,4 +168,8 @@ def reject_if_has_resource_actions
       raise _("Dialog cannot be deleted because it is connected to other components.")
     end
   end
+
+  def dialog_import_service
+    @dialog_import_service ||= DialogImportService.new
+  end
 end

app/models/dialog_group.rb

@@ -23,7 +23,6 @@ def update_dialog_fields(fields)
           resource_action_fields = field.delete('resource_action') || {}
           update_resource_fields(resource_action_fields, dialog_field)
           dialog_field.update_attributes(field.except('id', 'href', 'dialog_group_id', 'dialog_field_responders'))
-          dialog_field.update_dialog_field_responders(field['dialog_field_responders'])
           updated_fields << dialog_field
         end
       else

lib/services/dialog_import_service.rb

@@ -91,11 +91,24 @@ def build_resource_actions(dialog)
   def import(dialog)
     @dialog_import_validator.determine_dialog_validity(dialog)
     new_dialog = Dialog.create(dialog.except('dialog_tabs'))
+    association_list = build_association_list(dialog)
     new_dialog.update!(dialog.merge('dialog_tabs' => build_dialog_tabs(dialog)))
+    build_associations(new_dialog, association_list)
     new_dialog
   end
 
-  private
+  def build_associations(dialog, association_list)
+    fields = dialog.dialog_fields
+    association_list.each do |association|
+      association.values.each do |values|
+        values.each do |responder|
+          next if fields.select { |field| field.name == responder }.empty?
+          DialogFieldAssociation.create(:trigger_id => fields.find { |field| field.name.include?(association.keys.first) }.id,
+                                        :respond_id => fields.find { |field| field.name == responder }.id)
+        end
+      end
+    end
+  end
 
   def build_association_list(dialog)
     associations = []
@@ -109,6 +122,8 @@ def build_association_list(dialog)
     associations
   end
 
+  private
+
   def create_import_file_upload(file_contents)
     ImportFileUpload.create.tap do |import_file_upload|
       import_file_upload.store_binary_data_as_yml(file_contents, "Service dialog import")
@@ -128,16 +143,9 @@ def import_from_dialogs(dialogs)
           "resource_actions" => build_resource_actions(dialog)
         )
       )
-      fields = new_or_existing_dialog.dialog_fields
-      (associations_to_be_created + build_old_association_list(fields).flatten).reject(&:blank?).each do |association|
-        association.values.each do |values|
-          values.each do |responder|
-            next if fields.select { |field| field.name == responder }.empty?
-            DialogFieldAssociation.create(:trigger_id => fields.find { |field| field.name.include?(association.keys.first) }.id,
-                                          :respond_id => fields.find { |field| field.name == responder }.id)
-          end
-        end
-      end
+      old_associations = build_old_association_list(new_or_existing_dialog.dialog_fields).flatten
+      association_list = (associations_to_be_created + old_associations).reject(&:blank?)
+      build_associations(new_or_existing_dialog, association_list)
     end
   rescue DialogFieldImporter::InvalidDialogFieldTypeError
     raise

spec/lib/services/dialog_import_service_spec.rb

@@ -454,6 +454,12 @@
       end.to change(Dialog, :count).by(1)
     end
 
+    it "creates field associations" do
+      expect do
+        dialog_import_service.import(dialogs.first)
+      end.to change(DialogFieldAssociation, :count).by(1)
+    end
+
     it 'will raise record invalid for invalid dialog' do
       dialog_import_service.import(dialogs.first)
 
@@ -462,4 +468,40 @@
       end.to raise_error(ActiveRecord::RecordInvalid, /Validation failed: Name is not unique within region/)
     end
   end
+
+  describe "#build_associations" do
+    let(:dialog) { instance_double("Dialog", :dialog_fields => dialog_fields) }
+    let(:field1) { instance_double("DialogField", :id => 123, :name => "field1") }
+    let(:field2) { instance_double("DialogField", :id => 321, :name => "field2") }
+    let(:dialog_fields) { [field1, field2] }
+    let(:association_list) { [{"field1" => %w(responder1 field2)}] }
+
+    it "creates dialog field associations" do
+      expect do
+        dialog_import_service.build_associations(dialog, association_list)
+      end.to change(DialogFieldAssociation, :count).by(1)
+    end
+  end
+
+  describe "#build_association_list" do
+    let(:dialog) do
+      {
+        "dialog_tabs" => [{
+          "dialog_groups" => [{
+            "dialog_fields" => [field1, field2, field3]
+          }]
+        }]
+      }
+    end
+
+    let(:field1) { {"name" => "field1", "dialog_field_responders" => %w(field2 field3)} }
+    let(:field2) { {"name" => "field2", "dialog_field_responders" => %w(field3)} }
+    let(:field3) { {"name" => "field3"} }
+
+    it "creates an association list of ids based on names" do
+      expect(dialog_import_service.build_association_list(dialog)).to eq(
+        [{"field1" => %w(field2 field3)}, {"field2" => %w(field3)}]
+      )
+    end
+  end
 end

spec/models/dialog_spec.rb

@@ -265,13 +265,15 @@
               'dialog_tab_id' => dialog_tab.first.compressed_id,
               'dialog_fields' =>
                                  [{
-                                   'id'              => dialog_field.first.id,
-                                   'dialog_group_id' => dialog_group.first.compressed_id
+                                   'id'                      => dialog_field.first.id,
+                                   'name'                    => dialog_field.first.name,
+                                   'dialog_group_id'         => dialog_group.first.compressed_id,
+                                   'dialog_field_responders' => %w(dialog_field2)
                                  }] },
             {
               'label'         => 'group 2',
               'dialog_fields' => [{
-                'name'  => 'dialog_field',
+                'name'  => 'dialog_field2',
                 'label' => 'field_label'
               }]
             }
@@ -301,6 +303,14 @@
         dialog.update_tabs(updated_content)
         expect(dialog.reload.dialog_tabs.count).to eq(2)
       end
+
+      it "creates associations with the correct ids" do
+        expect do
+          dialog.update_tabs(updated_content)
+        end.to change(DialogFieldAssociation, :count).by(1)
+        expect(DialogFieldAssociation.first.trigger_id).to eq(dialog_field.first.id)
+        expect(DialogFieldAssociation.first.respond_id).to eq(dialog_field.first.id + 1)
+      end
     end
 
     context 'with a dialog tab removed from the dialog tabs collection' do