Merge pull request #1239 from internetee/fix-contact-deletion

Fix contact deletion

app/models/contact.rb

@@ -12,6 +12,7 @@ class Contact < ActiveRecord::Base
   has_many :domains, through: :domain_contacts
   has_many :legal_documents, as: :documentable
   has_many :registrant_domains, class_name: 'Domain', foreign_key: 'registrant_id'
+  has_many :actions, dependent: :destroy
 
   has_paper_trail class_name: "ContactVersion", meta: { children: :children_log }
 
@@ -188,7 +189,7 @@ class Contact < ActiveRecord::Base
           ver_scope << "(children->'#{type}')::jsonb <@ json_build_array(#{contact.id})::jsonb"
         end
         next if DomainVersion.where("created_at > ?", Time.now - Setting.orphans_contacts_in_months.to_i.months).where(ver_scope.join(" OR ")).any?
-        next if contact.in_use?
+        next if contact.linked?
 
         contact.destroy
         counter.next
@@ -279,7 +280,7 @@ class Contact < ActiveRecord::Base
     calculated.delete(Contact::OK)
     calculated.delete(Contact::LINKED)
     calculated << Contact::OK     if calculated.empty?# && valid?
-    calculated << Contact::LINKED if in_use?
+    calculated << Contact::LINKED if linked?
 
     calculated.uniq
   end
@@ -345,7 +346,7 @@ class Contact < ActiveRecord::Base
   # no need separate method
   # should use only in transaction
   def destroy_and_clean frame
-    if in_use?
+    if linked?
       errors.add(:domains, :exist)
       return false
     end
@@ -513,7 +514,7 @@ class Contact < ActiveRecord::Base
     Country.new(ident_country_code)
   end
 
-  def in_use?
+  def linked?
     registrant_domains.any? || domain_contacts.any?
   end
 
@@ -552,4 +553,8 @@ class Contact < ActiveRecord::Base
   def registrant?
     registrant_domains.any?
   end
+
+  def deletable?
+    !linked?
+  end
 end

app/presenters/registrant_presenter.rb

@@ -8,7 +8,7 @@ class RegistrantPresenter
            :reg_no,
            :street, :city, :state, :zip, :country,
            :ident_country,
-           :in_use?,
+           :linked?,
            to: :registrant
 
   def initialize(registrant:, view:)

app/views/mailers/contact_mailer/email_changed.html.erb

@@ -10,7 +10,7 @@ uus aadress: <%= contact.email %>
 <br><br>
 E-posti aadressile saadetakse domeeni toimingutega seotud infot, sealhulgas kinnitustaotlused omanikuvahetuse ja domeeni kustutamise korral.
 <br><br>
-<% if contact.in_use? %>
+<% if contact.linked? %>
     Muudatusega seotud domeenid:<br>
     <%= contact.domain_names_with_roles(locale: :et, line_break: '<br>') %>
 <% end %>
@@ -31,7 +31,7 @@ new address: <%= contact.email %>
 <br><br>
 E-mail addresses are used to send important information regarding your registered domains including applications for approval of registrant change and domain deletion. Please make sure that the update and contact information are correct.
 <br><br>
-<% if contact.in_use? %>
+<% if contact.linked? %>
     Domains affected by this update:<br>
     <%= contact.domain_names_with_roles(line_break: '<br>') %>
 <% end %>

app/views/mailers/contact_mailer/email_changed.text.erb

@@ -10,7 +10,7 @@ uus aadress: <%= contact.email %>
 
 E-posti aadressile saadetakse domeeni toimingutega seotud infot, sealhulgas kinnitustaotlused omanikuvahetuse ja domeeni kustutamise korral.
 
-<% if contact.in_use? %>
+<% if contact.linked? %>
     Muudatusega seotud domeenid:
     <%= contact.domain_names_with_roles(locale: :et) %>
 <% end %>
@@ -32,7 +32,7 @@ new address: <%= contact.email %>
 
 E-mail addresses are used to send important information regarding your registered domains including applications for approval of registrant change and domain deletion. Please make sure that the update and contact information are correct.
 
-<% if contact.in_use? %>
+<% if contact.linked? %>
     Domains affected by this update:
     <%= contact.domain_names_with_roles %>
 <% end %>

spec/presenters/registrant_presenter_spec.rb

@@ -67,7 +67,7 @@ RSpec.describe RegistrantPresenter do
     zip
     id_code
     reg_no
-    in_use?
+    linked?
   )
 
   registrant_delegatable_attributes.each do |attr_name|

test/fixtures/contacts.yml

@@ -78,14 +78,6 @@ identical_to_william:
   auth_info: 5ab865
   uuid: c0a191d5-3793-4f0b-8f85-491612d0293e
 
-not_in_use:
-  name: Useless
-  email: useless@inbox.test
-  registrar: bestnames
-  code: not-in-use
-  auth_info: e75a2a
-  uuid: ca613cc5-a8dc-48c1-8d32-d3c6a0b6c952
-
 invalid:
   name: any
   code: invalid

test/integration/epp/contact/delete/base_test.rb

@@ -1,15 +1,11 @@
 require 'test_helper'
 
 class EppContactDeleteBaseTest < ActionDispatch::IntegrationTest
-  def setup
+  def test_deletes_contact
-    @contact = contacts(:john)
+    contact = deletable_contact
-  end
-
-  def test_deletes_a_contact_that_is_not_in_use
-    @contact = contacts(:not_in_use)
 
     # https://github.com/internetee/registry/issues/415
-    @contact.update_columns(code: @contact.code.upcase)
+    contact.update_columns(code: contact.code.upcase)
 
     request_xml = <<-XML
       <?xml version="1.0" encoding="UTF-8" standalone="no"?>
@@ -17,7 +13,7 @@ class EppContactDeleteBaseTest < ActionDispatch::IntegrationTest
         <command>
           <delete>
             <contact:delete xmlns:contact="https://epp.tld.ee/schema/contact-ee-1.1.xsd">
-              <contact:id>not-in-use</contact:id>
+              <contact:id>#{contact.code}</contact:id>
             </contact:delete>
           </delete>
         </command>
@@ -32,11 +28,12 @@ class EppContactDeleteBaseTest < ActionDispatch::IntegrationTest
     assert_equal 1, response_xml.css('result').size
   end
 
-  def test_contact_that_is_in_use_cannot_be_deleted
+  def test_undeletable_cannot_be_deleted
-    assert_equal 'john-001', @contact.code
+    contact = contacts(:john)
+    assert_not contact.deletable?
 
     # https://github.com/internetee/registry/issues/415
-    @contact.update_columns(code: @contact.code.upcase)
+    contact.update_columns(code: contact.code.upcase)
 
     request_xml = <<-XML
       <?xml version="1.0" encoding="UTF-8" standalone="no"?>
@@ -44,7 +41,7 @@ class EppContactDeleteBaseTest < ActionDispatch::IntegrationTest
         <command>
           <delete>
             <contact:delete xmlns:contact="https://epp.tld.ee/schema/contact-ee-1.1.xsd">
-              <contact:id>john-001</contact:id>
+              <contact:id>#{contact.code}</contact:id>
             </contact:delete>
           </delete>
         </command>
@@ -57,4 +54,12 @@ class EppContactDeleteBaseTest < ActionDispatch::IntegrationTest
     response_xml = Nokogiri::XML(response.body)
     assert_equal '2305', response_xml.at_css('result')[:code]
   end
+
+  private
+
+  def deletable_contact
+    Domain.update_all(registrant_id: contacts(:william))
+    DomainContact.delete_all
+    contacts(:john)
+  end
 end

test/models/contact/contact_test.rb

@@ -13,18 +13,23 @@ class ContactTest < ActiveSupport::TestCase
     assert contacts(:invalid).invalid?
   end
 
-  def test_in_use_if_acts_as_a_registrant
+  def test_linked_when_in_use_as_registrant
+    Domain.update_all(registrant_id: @contact)
     DomainContact.delete_all
-    assert @contact.in_use?
+
+    assert @contact.linked?
   end
 
-  def test_in_use_if_acts_as_a_domain_contact
+  def test_linked_when_in_use_as_domain_contact
     Domain.update_all(registrant_id: contacts(:william))
-    assert @contact.in_use?
+    DomainContact.update_all(contact_id: @contact)
+
+    assert @contact.linked?
   end
 
-  def test_not_in_use_if_acts_as_neither_registrant_nor_domain_contact
+  def test_unlinked_when_not_in_use_as_either_registrant_or_domain_contact
-    refute contacts(:not_in_use).in_use?
+    contact = unlinked_contact
+    assert_not contact.linked?
   end
 
   def test_managed_when_identity_codes_match
@@ -38,4 +43,22 @@ class ContactTest < ActiveSupport::TestCase
     user = RegistrantUser.new(registrant_ident: 'US-12345')
     assert_not contact.managed_by?(user)
   end
+
+  def test_deletable_when_not_linked
+    contact = unlinked_contact
+    assert contact.deletable?
+  end
+
+  def test_undeletable_when_linked
+    assert @contact.linked?
+    assert_not @contact.deletable?
+  end
+
+  private
+
+  def unlinked_contact
+    Domain.update_all(registrant_id: contacts(:william))
+    DomainContact.delete_all
+    contacts(:john)
+  end
 end