From e5889e87e1c88ec4186e161e65c87c25d8152d86 Mon Sep 17 00:00:00 2001
From: Martin Lensment <mlensment@gmail.com>
Date: Thu, 12 Mar 2015 13:51:19 +0200
Subject: [PATCH] Prevent creating multiple new contacts

---
 app/models/epp/domain.rb |  6 ++-
 spec/epp/domain_spec.rb  | 90 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 88 insertions(+), 8 deletions(-)

diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb
index 834ce4091..a6421f627 100644
--- a/app/models/epp/domain.rb
+++ b/app/models/epp/domain.rb
@@ -417,7 +417,10 @@ class Epp::Domain < Domain
       owner_contact.update_column(:registrar_id, current_user.registrar_id)
     end
 
+    copied_ids = []
     contacts.each do |c|
+      next if copied_ids.include?(c.id)
+
       is_other_domains_contact = DomainContact.where('contact_id = ? AND domain_id != ?', c.id, id).count > 0
       if c.domains.count > 1 || is_other_domains_contact
         # create contact
@@ -427,7 +430,8 @@ class Epp::Domain < Domain
         oc.registrar_id = current_user.registrar_id
         oc.save!
 
-        domain_contacts.update_all({ contact_id: oc.id })
+        domain_contacts.where(contact_id: c.id).update_all({ contact_id: oc.id })
+        copied_ids << c.id
       else
         # transfer contact
         c.update_column(:registrar_id, current_user.registrar_id)
diff --git a/spec/epp/domain_spec.rb b/spec/epp/domain_spec.rb
index 573ffc14a..51a768526 100644
--- a/spec/epp/domain_spec.rb
+++ b/spec/epp/domain_spec.rb
@@ -952,14 +952,16 @@ describe 'EPP Domain', epp: true do
     end
 
     it 'transfers domain when domain contacts are some other domain contacts' do
-      c = Fabricate(:contact, registrar: @registrar1)
-      domain.tech_contacts << c
-      original_c_id = c.id
+      old_contact = Fabricate(:contact, registrar: @registrar1)
+      domain.tech_contacts << old_contact
+      domain.admin_contacts << old_contact
 
       d = Fabricate(:domain)
-      d.tech_contacts << c
-
+      d.tech_contacts << old_contact
+      d.admin_contacts << old_contact
       original_oc_id = domain.owner_contact.id
+      original_contact_count = Contact.count
+      original_domain_contact_count = DomainContact.count
 
       pw = domain.auth_info
       xml = domain_transfer_xml({
@@ -976,11 +978,85 @@ describe 'EPP Domain', epp: true do
       # all domain contacts should be under registrar2 now
       domain.reload
       domain.owner_contact.registrar_id.should == @registrar2.id
-      # owner_contact should be a new record
+      # owner_contact should not be a new record
       domain.owner_contact.id.should == original_oc_id
 
-      old_contact = Contact.find(original_c_id)
+      # old contact must not change
       old_contact.registrar_id.should == @registrar1.id
+
+      domain.contacts.each do |x|
+        x.registrar_id.should == @registrar2.id
+      end
+
+      new_contact = Contact.last
+      new_contact.name.should == old_contact.name
+
+      # there should be 2 references to the new contact
+      domain.domain_contacts.where(contact_id: new_contact.id).count.should == 2
+
+      # there should be only one new contact object
+      (original_contact_count + 1).should == Contact.count
+
+      # and no new references
+      original_domain_contact_count.should == DomainContact.count
+    end
+
+    it 'transfers domain when multiple domain contacts are some other domain contacts' do
+      old_contact = Fabricate(:contact, registrar: @registrar1)
+      old_contact_2 = Fabricate(:contact, registrar: @registrar1)
+
+      domain.tech_contacts << old_contact
+      domain.admin_contacts << old_contact
+      domain.tech_contacts << old_contact_2
+
+      d = Fabricate(:domain)
+      d.tech_contacts << old_contact
+      d.admin_contacts << old_contact_2
+
+      original_oc_id = domain.owner_contact.id
+      original_contact_count = Contact.count
+      original_domain_contact_count = DomainContact.count
+
+      pw = domain.auth_info
+      xml = domain_transfer_xml({
+        name: { value: domain.name },
+        authInfo: { pw: { value: pw } }
+      })
+
+      login_as :registrar2 do
+        response = epp_plain_request(xml, :xml)
+        response[:msg].should == 'Command completed successfully'
+        response[:result_code].should == '1000'
+      end
+
+      # all domain contacts should be under registrar2 now
+      domain.reload
+      domain.owner_contact.registrar_id.should == @registrar2.id
+      # owner_contact should not be a new record
+      domain.owner_contact.id.should == original_oc_id
+
+      # old contact must not change
+      old_contact.registrar_id.should == @registrar1.id
+
+      domain.contacts.each do |x|
+        x.registrar_id.should == @registrar2.id
+      end
+
+      new_contact, new_contact_2 = Contact.last(2)
+      new_contact.name.should == old_contact.name
+      new_contact_2.name.should == old_contact_2.name
+
+      # there should be 2 references to the new contact (admin + tech)
+      domain.domain_contacts.where(contact_id: new_contact.id).count.should == 2
+
+      # there should be 1 reference to the new contact 2 (tech)
+      domain.domain_contacts.where(contact_id: new_contact_2.id).count.should == 1
+
+      # there should be only two new contact objects
+      (original_contact_count + 2).should == Contact.count
+
+      # and no new references
+      original_domain_contact_count.should == DomainContact.count
     end
 
     it 'should not creates transfer without password' do