Reuse identical contacts

#746

app/models/concerns/contact/identical.rb

@@ -0,0 +1,18 @@
+module Concerns::Contact::Identical
+  extend ActiveSupport::Concern
+
+  ATTRIBUTE_FILTER = %w[
+    name
+    ident
+    ident_type
+    ident_country_code
+    phone
+    email
+  ]
+  private_constant :ATTRIBUTE_FILTER
+
+  def identical(registrar)
+    self.class.where(attributes.slice(*ATTRIBUTE_FILTER)).where(registrar: registrar)
+      .where.not(id: id).take
+  end
+end

app/models/concerns/contact/transferable.rb

@@ -7,6 +7,8 @@ module Concerns::Contact::Transferable
   end
 
   def transfer(new_registrar)
+    return identical(new_registrar) if identical(new_registrar)
+
     new_contact = self.dup
     new_contact.registrar = new_registrar
     new_contact.original = self

app/models/concerns/domain/transferable.rb

@@ -52,7 +52,7 @@ module Concerns::Domain::Transferable
 
   def transfer_registrant(new_registrar)
     return if registrant.registrar == new_registrar
-    self.registrant = registrant.transfer(new_registrar)
+    self.registrant = registrant.transfer(new_registrar).becomes(Registrant)
   end
 
   def transfer_domain_contacts(new_registrar)

app/models/contact.rb

@@ -3,6 +3,7 @@ class Contact < ActiveRecord::Base
   include EppErrors
   include UserEvents
   include Concerns::Contact::Transferable
+  include Concerns::Contact::Identical
 
   belongs_to :original, class_name: self.name
   belongs_to :registrar, required: true

test/fixtures/contacts.yml

@@ -19,6 +19,8 @@ william:
   registrar: bestnames
   code: william-001
   auth_info: 6573d0
+  statuses:
+    - ok
 
 jane:
   name: Jane
@@ -42,6 +44,19 @@ acme_ltd:
   code: acme-ltd-001
   auth_info: 720b3c
 
+identical_to_william:
+  name: William
+  email: william@inbox.test
+  phone: '+555.555'
+  ident: 1234
+  ident_type: priv
+  ident_country_code: US
+  registrar: goodnames
+  code: william-002
+  auth_info: 5ab865
+  statuses:
+    - ok
+
 invalid:
   name: any
   code: any

test/integration/api/domain_transfers_test.rb

@@ -3,6 +3,7 @@ require 'test_helper'
 class APIDomainTransfersTest < ActionDispatch::IntegrationTest
   def setup
     @domain = domains(:shop)
+    @new_registrar = registrars(:goodnames)
     Setting.transfer_wait_time = 0 # Auto-approval
   end
 
@@ -29,10 +30,10 @@ class APIDomainTransfersTest < ActionDispatch::IntegrationTest
     assert @domain.transfers.last.approved?
   end
 
-  def test_changes_registrar
+  def test_assigns_new_registrar
     post '/repp/v1/domain_transfers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key }
     @domain.reload
-    assert_equal registrars(:goodnames), @domain.registrar
+    assert_equal @new_registrar, @domain.registrar
   end
 
   def test_regenerates_transfer_code
@@ -52,11 +53,20 @@ class APIDomainTransfersTest < ActionDispatch::IntegrationTest
   end
 
   def test_duplicates_registrant_admin_and_tech_contacts
-    assert_difference 'Contact.count', 3 do
+    assert_difference -> { @new_registrar.contacts.size }, 2 do
       post '/repp/v1/domain_transfers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key }
     end
   end
 
+  def test_reuses_identical_contact
+    request_params = { format: :json,
+                       data: { domainTransfers: [{ domainName: 'shop.test', transferCode: '65078d5' },
+                                                 { domainName: 'airport.test', transferCode: '55438j5' },
+                                                 { domainName: 'library.test', transferCode: '45118f5' }] } }
+    post '/repp/v1/domain_transfers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key }
+    assert_equal 1, @new_registrar.contacts.where(name: 'William').size
+  end
+
   def test_fails_if_domain_does_not_exist
     request_params = { format: :json,
                        data: { domainTransfers: [{ domainName: 'non-existent.test', transferCode: 'any' }] } }
@@ -71,7 +81,7 @@ class APIDomainTransfersTest < ActionDispatch::IntegrationTest
                        data: { domainTransfers: [{ domainName: 'shop.test', transferCode: 'wrong' }] } }
     post '/repp/v1/domain_transfers', request_params, { 'HTTP_AUTHORIZATION' => http_auth_key }
     assert_response 400
-    refute_equal registrars(:goodnames), @domain.registrar
+    refute_equal @new_registrar, @domain.registrar
     assert_equal ({ errors: [{ title: 'shop.test transfer code is wrong' }] }),
                  JSON.parse(response.body, symbolize_names: true)
   end

test/integration/epp/domain/transfer/request_test.rb

@@ -3,6 +3,7 @@ require 'test_helper'
 class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest
   def setup
     @domain = domains(:shop)
+    @new_registrar = registrars(:goodnames)
     Setting.transfer_wait_time = 0
   end
 
@@ -24,10 +25,10 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest
       'https://epp.tld.ee/schema/domain-eis-1.0.xsd').text
   end
 
-  def test_changes_registrar
+  def test_assigns_new_registrar
     post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' }
     @domain.reload
-    assert_equal registrars(:goodnames), @domain.registrar
+    assert_equal @new_registrar, @domain.registrar
   end
 
   def test_regenerates_transfer_code
@@ -48,7 +49,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest
   end
 
   def test_duplicates_registrant_admin_and_tech_contacts
-    assert_difference 'Contact.count', 3 do
+    assert_difference -> { @new_registrar.contacts.size }, 2 do
       post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' }
     end
   end
@@ -106,7 +107,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest
 
     post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' }
     @domain.reload
-    refute_equal registrars(:goodnames), @domain.registrar
+    refute_equal @new_registrar, @domain.registrar
     assert_equal '2201', Nokogiri::XML(response.body).at_css('result')[:code]
   end
 

test/models/contact/identical_test.rb

@@ -0,0 +1,30 @@
+require 'test_helper'
+
+class ContactIdenticalTest < ActiveSupport::TestCase
+  def setup
+    @contact = contacts(:william)
+    @identical = contacts(:identical_to_william)
+  end
+
+  def test_identical
+    assert_equal @identical, @contact.identical(@identical.registrar)
+  end
+
+  def test_not_identical
+    filter_attributes = %i[
+      name
+      ident
+      ident_type
+      ident_country_code
+      phone
+      email
+    ]
+
+    filter_attributes.each do |attribute|
+      previous_value = @identical.public_send(attribute)
+      @identical.update_attribute(attribute, 'other')
+      assert_nil @contact.identical(@identical.registrar)
+      @identical.update_attribute(attribute, previous_value)
+    end
+  end
+end

test/models/contact/transfer_test.rb

@@ -35,18 +35,23 @@ class ContactTransferTest < ActiveSupport::TestCase
   end
 
   def test_keeps_original_contact_untouched
-    original_hash = @contact.to_json
+    original_hash = @contact.attributes
     @contact.transfer(@new_registrar)
     @contact.reload
-    assert_equal original_hash, @contact.to_json
+    assert_equal original_hash, @contact.attributes
   end
 
   def test_creates_new_contact
-    assert_difference 'Contact.count' do
+    assert_difference -> { @new_registrar.contacts.count } do
       @contact.transfer(@new_registrar)
     end
   end
 
+  def test_reuses_identical_contact
+    identical = contacts(:identical_to_william)
+    assert_equal identical, contacts(:william).transfer(@new_registrar)
+  end
+
   def test_bypasses_validation
     @contact = contacts(:invalid)