From 51243c59833d1838402fe70d877a03b4c9ddc285 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 23 Jan 2018 18:41:49 +0200 Subject: [PATCH] Extract contact Transferable concern #660 --- app/models/concerns/contact/transferable.rb | 13 ++++++ app/models/concerns/domain/transferable.rb | 32 +++++---------- app/models/contact.rb | 1 + app/models/epp/domain.rb | 4 +- test/models/contact_test.rb | 44 +++++++++++++++++++++ test/models/domain_test.rb | 10 ++++- 6 files changed, 79 insertions(+), 25 deletions(-) create mode 100644 app/models/concerns/contact/transferable.rb create mode 100644 test/models/contact_test.rb diff --git a/app/models/concerns/contact/transferable.rb b/app/models/concerns/contact/transferable.rb new file mode 100644 index 000000000..3805287ac --- /dev/null +++ b/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 diff --git a/app/models/concerns/domain/transferable.rb b/app/models/concerns/domain/transferable.rb index c7b36c4bd..161f513a2 100644 --- a/app/models/concerns/domain/transferable.rb +++ b/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 diff --git a/app/models/contact.rb b/app/models/contact.rb index 7edef09ea..85687cb6f 100644 --- a/app/models/contact.rb +++ b/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 diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index 8791a6126..4b944c9d3 100644 --- a/app/models/epp/domain.rb +++ b/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 diff --git a/test/models/contact_test.rb b/test/models/contact_test.rb new file mode 100644 index 000000000..017a50c0b --- /dev/null +++ b/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 diff --git a/test/models/domain_test.rb b/test/models/domain_test.rb index 3cbfe880d..f3baa2e4a 100644 --- a/test/models/domain_test.rb +++ b/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)