CV2-6141: apply PR comments

app/models/relationship.rb

@@ -159,13 +159,7 @@ def create_or_update_parent_id
   def self.create_unless_exists(source_id, target_id, relationship_type, options = {})
     # Verify that the target is not part of another source; in this case, we should return the existing relationship.
     r = Relationship.where(target_id: target_id).where.not(source_id: source_id).last
-    # should update options if exists and at least on field is blank
-    if !r.nil? && !options.blank? && r.model.blank?
-      options.each do |key, value|
-        r.send("#{key}=", value) if r.respond_to?("#{key}=")
-      end
-      r.save!
-    end
+    r.update_options(options) unless r.nil?
     return r unless r.nil?
     # otherwise we should get the existing relationship based on source, target and type
     r = Relationship.where(source_id: source_id, target_id: target_id).where('relationship_type = ?', relationship_type.to_yaml).last
@@ -203,6 +197,8 @@ def self.create_unless_exists(source_id, target_id, relationship_type, options =
         exception_message = e.message
         exception_class = e.class.name
       end
+    else
+      r.update_options(options)
     end
     if r.nil?
       Rails.logger.error("[Relationship::create_unless_exists] returning nil: source_id #{source_id}, target_id #{target_id}, relationship_type #{relationship_type}.")
@@ -234,6 +230,16 @@ def self.sync_statuses(item, status)
     end
   end
 
+  def update_options(options)
+    # should update options if exists and at least on field has a value
+    if self.model.blank? && !options.values.compact.empty?
+      options.each do |key, value|
+        self.send("#{key}=", value) if self.respond_to?("#{key}=")
+      end
+      self.save!
+    end
+  end
+
   protected
 
   def update_elasticsearch_parent(action = 'create_or_update')

test/models/relationship_test.rb

@@ -326,9 +326,17 @@ def setup
     target = create_project_media team: t
     r = nil
     assert_difference 'Relationship.count' do
-      r = Relationship.create_unless_exists(source.id, target.id, Relationship.confirmed_type, { user_id: u.id })
+      r = Relationship.create_unless_exists(source.id, target.id, Relationship.confirmed_type)
     end
-    assert_equal u.id, r.user_id
+    # should update options if relationship already exists
+    assert_nil r.model
+    another_source = create_project_media team: t
+    r2 = nil
+    assert_no_difference 'Relationship.count' do
+      r2 = Relationship.create_unless_exists(another_source.id, target.id, Relationship.confirmed_type, { model: 'elasticsearch' })
+    end
+    assert_equal r, r2
+    assert_equal 'elasticsearch', r2.model
     r2 = nil
     assert_no_difference 'Relationship.count' do
       r2 = Relationship.create_unless_exists(source.id, target.id, Relationship.confirmed_type)