Extract contact Transferable concern

#660

app/models/concerns/contact/transferable.rb

@@ -0,0 +1,13 @@
+module Concerns::Contact::Transferable
+  extend ActiveSupport::Concern
+
+  def transfer(new_registrar)
+    new_contact = self.dup
+    new_contact.registrar = new_registrar
+    new_contact.generate_code
+    new_contact.original = self
+    new_contact.remove_address unless self.class.address_processing?
+    new_contact.save!
+    new_contact
+  end
+end

app/models/concerns/domain/transferable.rb

@@ -17,7 +17,7 @@ module Concerns::Domain::Transferable
       transfer_to: new_registrar
     )
 
-    transfer_contacts(new_registrar.id)
+    transfer_contacts(new_registrar)
   end
 
   private
@@ -26,25 +26,25 @@ module Concerns::Domain::Transferable
     self.transfer_code = SecureRandom.hex
   end
 
-  def transfer_contacts(registrar_id)
-    transfer_registrant(registrar_id)
-    transfer_domain_contacts(registrar_id)
+  def transfer_contacts(new_registrar)
+    transfer_registrant(new_registrar)
+    transfer_domain_contacts(new_registrar)
   end
 
-  def transfer_registrant(registrar_id)
-    return if registrant.registrar_id == registrar_id
-    self.registrant_id = copy_and_transfer_contact(registrant_id, registrar_id).id
+  def transfer_registrant(new_registrar)
+    return if registrant.registrar == new_registrar
+    self.registrant = registrant.transfer(new_registrar)
   end
 
-  def transfer_domain_contacts(registrar_id)
+  def transfer_domain_contacts(new_registrar)
     copied_ids = []
     contacts.each do |c|
-      next if copied_ids.include?(c.id) || c.registrar_id == registrar_id
+      next if copied_ids.include?(c.id) || c.registrar == new_registrar
 
       if registrant_id_was == c.id # registrant was copied previously, do not copy it again
         oc = OpenStruct.new(id: registrant_id)
       else
-        oc = copy_and_transfer_contact(c.id, registrar_id)
+        oc = c.transfer(new_registrar)
       end
 
       domain_contacts.where(contact_id: c.id).update_all({ contact_id: oc.id }) # n+1 workaround
@@ -52,17 +52,5 @@ module Concerns::Domain::Transferable
     end
   end
 
-  def copy_and_transfer_contact(contact_id, registrar_id)
-    c = Contact.find(contact_id) # n+1 workaround
-    oc = c.deep_clone
-    oc.code = nil
-    oc.registrar_id = registrar_id
-    oc.original = c
-    oc.generate_code
-    oc.remove_address unless Contact.address_processing?
-    oc.save!(validate: false)
-    oc
-  end
-
   alias_method :regenerate_transfer_code, :generate_transfer_code
 end

app/models/contact.rb

@@ -2,6 +2,7 @@ class Contact < ActiveRecord::Base
   include Versions # version/contact_version.rb
   include EppErrors
   include UserEvents
+  include Concerns::Contact::Transferable
 
   belongs_to :original, class_name: self.name
   belongs_to :registrar, required: true

app/models/epp/domain.rb

@@ -679,7 +679,7 @@ class Epp::Domain < Domain
       end
 
       if dt.approved?
-        transfer_contacts(current_user.registrar_id)
+        transfer_contacts(current_user.registrar)
         dt.notify_losing_registrar(old_contact_codes, old_registrant_code)
         regenerate_transfer_code
         self.registrar = current_user.registrar
@@ -709,7 +709,7 @@ class Epp::Domain < Domain
         transferred_at: Time.zone.now
       )
 
-      transfer_contacts(pt.transfer_to_id)
+      transfer_contacts(pt.transfer_to)
       regenerate_transfer_code
       self.registrar = pt.transfer_to
 

test/models/contact_test.rb

@@ -0,0 +1,44 @@
+require 'test_helper'
+
+class ContactTest < ActiveSupport::TestCase
+  def setup
+    @contact = contacts(:john)
+    @new_registrar = registrars(:goodnames)
+  end
+
+  def test_validates
+    assert @contact.valid?
+  end
+
+  def test_transfer_keeps_original_contact_untouched
+    original_hash = @contact.to_json
+    new_contact = @contact.transfer(@new_registrar)
+    refute_equal original_hash, new_contact.to_json
+  end
+
+  def test_transfer_creates_new_contact
+    assert_difference 'Contact.count' do
+      @contact.transfer(@new_registrar)
+    end
+  end
+
+  def test_transfer_changes_registrar
+    new_contact = @contact.transfer(@new_registrar)
+    assert_equal @new_registrar, new_contact.registrar
+  end
+
+  def test_transfer_links_to_original
+    new_contact = @contact.transfer(@new_registrar)
+    assert_equal @contact, new_contact.original
+  end
+
+  def test_transfer_regenerates_new_code
+    new_contact = @contact.transfer(@new_registrar)
+    refute_equal @contact.code, new_contact.code
+  end
+
+  def test_transfer_regenerates_auth_info
+    new_contact = @contact.transfer(@new_registrar)
+    refute_equal @contact.auth_info, new_contact.auth_info
+  end
+end

test/models/domain_test.rb

@@ -31,7 +31,7 @@ class DomainTest < ActiveSupport::TestCase
     assert_equal original_transfer_code, @domain.transfer_code
   end
 
-  def test_transfers_domain
+  def test_changes_registrar
     old_transfer_code = @domain.transfer_code
     new_registrar = registrars(:goodnames)
     @domain.transfer(new_registrar)
@@ -40,6 +40,14 @@ class DomainTest < ActiveSupport::TestCase
     refute_same @domain.transfer_code, old_transfer_code
   end
 
+  def test_regenerates_transfer_code
+    old_transfer_code = @domain.transfer_code
+    new_registrar = registrars(:goodnames)
+    @domain.transfer(new_registrar)
+
+    refute_same @domain.transfer_code, old_transfer_code
+  end
+
   def test_transfer_creates_domain_transfer_object
     new_registrar = registrars(:goodnames)