diff --git a/app/models/concerns/contact/identical.rb b/app/models/concerns/contact/identical.rb new file mode 100644 index 0000000000..1c490b75d8 --- /dev/null +++ b/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 diff --git a/app/models/concerns/contact/transferable.rb b/app/models/concerns/contact/transferable.rb index 8da98fc57d..14c1cac3c7 100644 --- a/app/models/concerns/contact/transferable.rb +++ b/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 diff --git a/app/models/concerns/domain/transferable.rb b/app/models/concerns/domain/transferable.rb index f2e7736c2e..56e77f34d2 100644 --- a/app/models/concerns/domain/transferable.rb +++ b/app/models/concerns/domain/transferable.rb @@ -52,7 +52,7 @@ def transfer_contacts(new_registrar) 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) diff --git a/app/models/contact.rb b/app/models/contact.rb index 10b71b55b5..d01d42634c 100644 --- a/app/models/contact.rb +++ b/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 diff --git a/test/fixtures/contacts.yml b/test/fixtures/contacts.yml index eaf4401b7d..4c82f6f560 100644 --- a/test/fixtures/contacts.yml +++ b/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 diff --git a/test/integration/api/domain_transfers_test.rb b/test/integration/api/domain_transfers_test.rb index b90b59be40..2e344e45e4 100644 --- a/test/integration/api/domain_transfers_test.rb +++ b/test/integration/api/domain_transfers_test.rb @@ -3,6 +3,7 @@ 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 @@ def test_approves_automatically_if_auto_approval_is_enabled 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 @@ def test_notifies_old_registrar 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 @@ def test_fails_if_transfer_code_is_wrong 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 diff --git a/test/integration/epp/domain/transfer/request_test.rb b/test/integration/epp/domain/transfer/request_test.rb index 03c5e7daf3..b516cbe767 100644 --- a/test/integration/epp/domain/transfer/request_test.rb +++ b/test/integration/epp/domain/transfer/request_test.rb @@ -3,6 +3,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest def setup @domain = domains(:shop) + @new_registrar = registrars(:goodnames) Setting.transfer_wait_time = 0 end @@ -24,10 +25,10 @@ def test_approves_automatically_if_auto_approval_is_enabled '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 @@ def test_notifies_old_registrar 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 @@ def test_wrong_transfer_code 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 diff --git a/test/models/contact/identical_test.rb b/test/models/contact/identical_test.rb new file mode 100644 index 0000000000..10fbc3a66b --- /dev/null +++ b/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 diff --git a/test/models/contact/transfer_test.rb b/test/models/contact/transfer_test.rb index d9379ecb82..a695c112a3 100644 --- a/test/models/contact/transfer_test.rb +++ b/test/models/contact/transfer_test.rb @@ -35,18 +35,23 @@ def test_overrides_default_auth_info 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)