Pull up methods

#660

.reek

@@ -103,13 +103,11 @@ UncommunicativeVariableName:
   - Epp::Contact#attrs_from
   - Epp::Contact#check_availability
   - Epp::Domain#check_availability
-  - Epp::Domain#copy_and_transfer_contact
   - Epp::Domain#domain_contact_attrs_from
   - Epp::Domain#domain_status_list_from
   - Epp::Domain#domain_statuses_attrs
   - Epp::Domain#nameservers_from
   - Epp::Domain#parse_period_unit_from_frame
-  - Epp::Domain#transfer_domain_contacts
   - Epp::Domain#validate_contacts
   - Invoice#cancel_overdue_invoices
   - Legacy::Db
@@ -317,7 +315,6 @@ DuplicateMethodCall:
   - Epp::Domain#parse_legal_document_from_frame
   - Epp::Domain#query_transfer
   - Epp::Domain#renew
-  - Epp::Domain#transfer_domain_contacts
   - Epp::Domain#update
   - Epp::Domain#validate_contacts
   - Epp::Domain#validate_exp_dates
@@ -795,7 +792,6 @@ TooManyStatements:
   - Epp::Domain#approve_transfer
   - Epp::Domain#attrs_from
   - Epp::Domain#check_availability
-  - Epp::Domain#copy_and_transfer_contact
   - Epp::Domain#destroy_attrs
   - Epp::Domain#dnskeys_attrs
   - Epp::Domain#domain_contact_attrs_from
@@ -811,7 +807,6 @@ TooManyStatements:
   - Epp::Domain#reject_transfer
   - Epp::Domain#renew
   - Epp::Domain#transfer
-  - Epp::Domain#transfer_domain_contacts
   - Epp::Domain#update
   - Epp::Domain#validate_contacts
   - Invoice#cancel
@@ -881,7 +876,6 @@ UtilityFunction:
   - EppErrors#construct_msg_args_and_value
   - Versions#user_from_id_role_username
   - Depp::Keyrelay#expiry
-  - Epp::Domain#copy_and_transfer_contact
   - Epp::Domain#nameservers_from
   - Epp::Domain::DnsSecKeys#mark
   - Epp::Domain::DnsSecKeys#xm_copy
@@ -956,7 +950,6 @@ FeatureEnvy:
   - Epp::Domain#destroy_attrs
   - Epp::Domain#domain_contact_attrs_from
   - Epp::Domain#domain_status_list_from
-  - Epp::Domain#transfer_domain_contacts
   - LegalDocument#calc_checksum
   - Nameserver#find_by_hash_params
   - RegistrantUser#find_or_create_by_idc_data

app/models/concerns/domain/transferable.rb

@@ -6,8 +6,18 @@ module Concerns::Domain::Transferable
   end
 
   def transfer(new_registrar)
+    old_registrar = registrar
+
     self.registrar = new_registrar
     regenerate_transfer_code
+
+    domain_transfers.create!(
+      transfer_requested_at: Time.zone.now,
+      transfer_from: old_registrar,
+      transfer_to: new_registrar
+    )
+
+    transfer_contacts(new_registrar.id)
   end
 
   private
@@ -16,5 +26,43 @@ module Concerns::Domain::Transferable
     self.transfer_code = SecureRandom.hex
   end
 
+  def transfer_contacts(registrar_id)
+    transfer_registrant(registrar_id)
+    transfer_domain_contacts(registrar_id)
+  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
+  end
+
+  def transfer_domain_contacts(registrar_id)
+    copied_ids = []
+    contacts.each do |c|
+      next if copied_ids.include?(c.id) || c.registrar_id == registrar_id
+
+      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)
+      end
+
+      domain_contacts.where(contact_id: c.id).update_all({ contact_id: oc.id }) # n+1 workaround
+      copied_ids << c.id
+    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.copy_from_id = c.id
+    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/epp/domain.rb

@@ -641,46 +641,6 @@ class Epp::Domain < Domain
     end
   end
 
-  # TODO: Eager load problems here. Investigate how it's possible not to query contact again
-  # Check if versioning works with update_column
-  def transfer_contacts(registrar_id)
-    transfer_registrant(registrar_id)
-    transfer_domain_contacts(registrar_id)
-  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.copy_from_id = c.id
-    oc.generate_code
-    oc.remove_address unless Contact.address_processing?
-    oc.save!(validate: false)
-    oc
-  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
-  end
-
-  def transfer_domain_contacts(registrar_id)
-    copied_ids = []
-    contacts.each do |c|
-      next if copied_ids.include?(c.id) || c.registrar_id == registrar_id
-
-      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)
-      end
-
-      domain_contacts.where(contact_id: c.id).update_all({ contact_id: oc.id }) # n+1 workaround
-      copied_ids << c.id
-    end
-  end
-
   # rubocop: enable Metrics/PerceivedComplexity
   # rubocop: enable Metrics/CyclomaticComplexity
   # rubocop: disable Metrics/MethodLength

test/models/domain_test.rb

@@ -39,4 +39,20 @@ class DomainTest < ActiveSupport::TestCase
     assert_equal new_registrar, @domain.registrar
     refute_same @domain.transfer_code, old_transfer_code
   end
+
+  def test_transfer_creates_domain_transfer_object
+    new_registrar = registrars(:goodnames)
+
+    assert_difference 'DomainTransfer.count' do
+      @domain.transfer(new_registrar)
+    end
+  end
+
+  def test_transfer_copies_contacts
+    new_registrar = registrars(:goodnames)
+
+    assert_difference 'Contact.count', 2 do
+      @domain.transfer(new_registrar)
+    end
+  end
 end