Merge pull request #1146 from internetee/refactor-contact-archivation

Refactor contact archivation

app/models/concerns/contact/archivable.rb

@@ -0,0 +1,68 @@
+module Concerns
+  module Contact
+    module Archivable
+      extend ActiveSupport::Concern
+
+      class_methods do
+        def archivable
+          unlinked.find_each.select(&:archivable?)
+        end
+      end
+
+      def archivable?(post: false)
+        inactive = inactive?
+
+        log("Found archivable contact id(#{id}), code (#{code})") if inactive && !post
+
+        inactive
+      end
+
+      def archive(verified: false, notify: true, extra_log: false)
+        unless verified
+          raise 'Contact cannot be archived' unless archivable?(post: true)
+        end
+
+        notify_registrar_about_archivation if notify
+        write_to_registrar_log if extra_log
+        destroy!
+      end
+
+      private
+
+      def notify_registrar_about_archivation
+        registrar.notifications.create!(
+          text: I18n.t('contact_has_been_archived',
+                       contact_code: code, orphan_months: Setting.orphans_contacts_in_months)
+        )
+      end
+
+      def inactive?
+        if DomainVersion.contact_unlinked_more_than?(contact_id: id, period: inactivity_period)
+          return true
+        end
+
+        DomainVersion.was_contact_linked?(id) ? false : created_at <= inactivity_period.ago
+      end
+
+      def inactivity_period
+        Setting.orphans_contacts_in_months.months
+      end
+
+      def log(msg)
+        @log ||= Logger.new(STDOUT)
+        @log.info(msg)
+      end
+
+      def write_to_registrar_log
+        registrar_name = registrar.accounting_customer_code
+        archive_path = ENV['contact_archivation_log_file_dir']
+        registrar_log_path = "#{archive_path}/#{registrar_name}.txt"
+        FileUtils.mkdir_p(archive_path) unless Dir.exist?(archive_path)
+
+        f = File.new(registrar_log_path, 'a+')
+        f.write("#{code}\n")
+        f.close
+      end
+    end
+  end
+end

app/models/contact.rb

@@ -7,6 +7,7 @@ class Contact < ApplicationRecord
   include Concerns::Contact::Transferable
   include Concerns::Contact::Identical
   include Concerns::Contact::Disclosable
+  include Concerns::Contact::Archivable
   include Concerns::EmailVerifable
 
   belongs_to :original, class_name: self.name
@@ -152,61 +153,17 @@ class Contact < ApplicationRecord
       res.reduce([]) { |o, v| o << { id: v[:id], display_key: "#{v.name} (#{v.code})" } }
     end
 
-    def find_orphans
-      where('
-        NOT EXISTS(
-          select 1 from domains d where d.registrant_id = contacts.id
-        ) AND NOT EXISTS(
-          select 1 from domain_contacts dc where dc.contact_id = contacts.id
-        )
-      ')
-    end
-
-    def find_linked
-      where('
-        EXISTS(
-          select 1 from domains d where d.registrant_id = contacts.id
-        ) OR EXISTS(
-          select 1 from domain_contacts dc where dc.contact_id = contacts.id
-        )
-      ')
-    end
-
     def filter_by_states in_states
       states = Array(in_states).dup
       scope  = all
 
       # all contacts has state ok, so no need to filter by it
       scope = scope.where("NOT contacts.statuses && ?::varchar[]", "{#{(STATUSES - [OK, LINKED]).join(',')}}") if states.delete(OK)
-      scope = scope.find_linked if states.delete(LINKED)
+      scope = scope.linked if states.delete(LINKED)
       scope = scope.where("contacts.statuses @> ?::varchar[]", "{#{states.join(',')}}") if states.any?
       scope
     end
 
-    # To leave only new ones we need to check
-    # if contact was at any time used in domain.
-    # This can be checked by domain history.
-    # This can be checked by saved relations in children attribute
-    def destroy_orphans
-      STDOUT << "#{Time.zone.now.utc} - Destroying orphaned contacts\n" unless Rails.env.test?
-
-      counter = Counter.new
-      find_orphans.find_each do |contact|
-        ver_scope = []
-        %w(admin_contacts tech_contacts registrant).each do |type|
-          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.linked?
-
-        contact.destroy
-        counter.next
-        STDOUT << "#{Time.zone.now.utc} Contact.destroy_orphans: ##{contact.id} (#{contact.name})\n" unless Rails.env.test?
-      end
-
-      STDOUT << "#{Time.zone.now.utc} - Successfully destroyed #{counter} orphaned contacts\n" unless Rails.env.test?
-    end
-
     def admin_statuses
       [
         SERVER_UPDATE_PROHIBITED,
@@ -264,6 +221,23 @@ class Contact < ApplicationRecord
                                                                                   .country.alpha2)
     end
 
+    def linked
+      sql = <<-SQL
+        EXISTS(SELECT 1 FROM domains WHERE domains.registrant_id = contacts.id) OR
+        EXISTS(SELECT 1 FROM domain_contacts WHERE domain_contacts.contact_id =
+        contacts.id)
+      SQL
+
+      where(sql)
+    end
+
+    def unlinked
+      where('NOT EXISTS(SELECT 1 FROM domains WHERE domains.registrant_id = contacts.id)
+             AND
+             NOT EXISTS(SELECT 1 FROM domain_contacts WHERE domain_contacts.contact_id =
+             contacts.id)')
+    end
+
     def registrant_user_company_contacts(registrant_user)
       ident = registrant_user.companies.collect(&:registration_number)
 

app/models/inactive_contacts.rb

@@ -0,0 +1,20 @@
+class InactiveContacts
+  attr_reader :contacts
+
+  def initialize(contacts = Contact.archivable)
+    @contacts = contacts
+  end
+
+  def archive(verified: false)
+    contacts.each do |contact|
+      log("Archiving contact: id(#{contact.id}), code(#{contact.code})")
+      contact.archive(verified: verified)
+      yield contact if block_given?
+    end
+  end
+
+  def log(msg)
+    @log ||= Logger.new(STDOUT)
+    @log.info(msg)
+  end
+end

app/models/version/domain_version.rb

@@ -5,4 +5,41 @@ class DomainVersion < PaperTrail::Version
   self.sequence_name = :log_domains_id_seq
 
   scope :deleted, -> { where(event: 'destroy') }
+
+  def self.was_contact_linked?(contact_id)
+    sql = <<-SQL
+      SELECT
+        COUNT(*)
+      FROM
+        #{table_name}
+      WHERE
+        (children->'registrant') @> '#{contact_id}'
+        OR
+        (children->'admin_contacts') @> '#{contact_id}'
+        OR
+        (children->'tech_contacts') @> '#{contact_id}'
+    SQL
+
+    count_by_sql(sql).nonzero?
+  end
+
+  def self.contact_unlinked_more_than?(contact_id:, period:)
+    sql = <<-SQL
+      SELECT
+        COUNT(*)
+      FROM
+        #{table_name}
+      WHERE
+        created_at < TIMESTAMP WITH TIME ZONE '#{period.ago}'
+        AND (
+          (children->'registrant') @> '#{contact_id}'
+          OR
+          (children->'admin_contacts') @> '#{contact_id}'
+          OR
+          (children->'tech_contacts') @> '#{contact_id}'
+        )
+    SQL
+
+    count_by_sql(sql).nonzero?
+  end
 end

config/application.yml.sample

@@ -154,6 +154,7 @@ lhv_ca_file: # Needed only in dev mode
 lhv_dev_mode: 'false'
 
 epp_session_timeout_seconds: '300'
+contact_archivation_log_file_dir:
 
 # Since the keys for staging are absent from the repo, we need to supply them separate for testing.
 test:

config/locales/en.yml

@@ -640,6 +640,7 @@ en:
   cant_match_version: 'Impossible match version with request'
   user_not_authenticated: "user not authenticated"
   actions: Actions
+  contact_has_been_archived: 'Contact with code %{contact_code} has been archieved because it has been orphaned for longer than %{orphan_months} months.'
 
   number:
     currency:

config/schedule.rb

@@ -17,8 +17,8 @@ if @cron_group == 'registry'
     runner 'DNS::Zone.generate_zonefiles'
   end
 
-  every 6.months, at: '12:01am' do
+  every :day, at: '12:01am' do
-    runner 'Contact.destroy_orphans'
+    rake 'contacts:archive'
   end
 
   every :day, at: '12:10am' do

db/migrate/20200902131603_change_log_domains_children_type_to_jsonb.rb

@@ -0,0 +1,5 @@
+class ChangeLogDomainsChildrenTypeToJsonb < ActiveRecord::Migration[6.0]
+  def change
+    change_column :log_domains, :children, 'jsonb USING children::jsonb'
+  end
+end

db/structure.sql

@@ -1491,7 +1491,7 @@ CREATE TABLE public.log_domains (
     object_changes json,
     created_at timestamp without time zone,
     session character varying,
-    children json,
+    children jsonb,
     uuid character varying
 );
 
@@ -4903,6 +4903,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20200811074839'),
 ('20200812090409'),
 ('20200812125810'),
+('20200902131603'),
 ('20200908131554'),
 ('20200910085157'),
 ('20200910102028');

lib/tasks/contacts/archive.rake

@@ -0,0 +1,32 @@
+namespace :contacts do
+  desc 'Archives inactive contacts'
+
+  task :archive, %i[track_id initial_run] => [:environment] do |_t, args|
+    unlinked_contacts = contacts_start_point(args[:track_id])
+    initial_run = args[:initial_run] == true || args[:initial_run] == 'true'
+    counter = 0
+    log("Found #{unlinked_contacts.count} unlinked contacts. Starting to archive.")
+
+    unlinked_contacts.each do |contact|
+      next unless contact.archivable?
+
+      log("Archiving contact: id(#{contact.id}), code(#{contact.code})")
+      contact.archive(verified: true, notify: !initial_run, extra_log: initial_run)
+      counter += 1
+    end
+
+    log("Archived total: #{counter}")
+  end
+
+  def contacts_start_point(track_id = nil)
+    puts "Starting to find archivable contacts WHERE CONTACT_ID > #{track_id}" if track_id
+    return Contact.unlinked unless track_id
+
+    Contact.unlinked.where("id > #{track_id}")
+  end
+
+  def log(msg)
+    @log ||= Logger.new(STDOUT)
+    @log.info(msg)
+  end
+end

test/mailers/previews/contact_mailer_preview.rb

@@ -1,13 +1,7 @@
 class ContactMailerPreview < ActionMailer::Preview
   def email_changed
-    # Replace with `Contact.in_use` once https://github.com/internetee/registry/pull/1146 is merged
+    contact = Contact.linked
-    contact = Contact.where('EXISTS(SELECT 1 FROM domains WHERE domains.registrant_id = contacts.id)
+    contact = contact.where.not(email: nil, country_code: nil, code: nil).first
-                             OR
-                             EXISTS(SELECT 1 FROM domain_contacts WHERE domain_contacts.contact_id =
-                             contacts.id)')
-
-    contact = contact.where.not(email: nil, country_code: nil, ident_country_code: nil, code: nil)
-              .take
 
     ContactMailer.email_changed(contact: contact, old_email: 'old@inbox.test')
   end

test/models/contact/archivable_test.rb

@@ -0,0 +1,91 @@
+require 'test_helper'
+
+class ArchivableContactTest < ActiveSupport::TestCase
+  setup do
+    @contact = contacts(:john)
+  end
+
+  def test_contact_is_archivable_when_it_was_linked_and_inactivity_period_has_passed
+    DomainVersion.stub(:was_contact_linked?, true) do
+      DomainVersion.stub(:contact_unlinked_more_than?, true) do
+        assert @contact.archivable?
+      end
+    end
+  end
+
+  def test_contact_is_archivable_when_it_was_never_linked_and_inactivity_period_has_passed
+    Setting.orphans_contacts_in_months = 0
+    @contact.created_at = Time.zone.parse('2010-07-05 00:00:00')
+    travel_to Time.zone.parse('2010-07-05 00:00:01')
+
+    DomainVersion.stub(:was_contact_linked?, false) do
+      assert @contact.archivable?
+    end
+  end
+
+  def test_contact_is_not_archivable_when_it_was_never_linked_and_inactivity_period_has_not_passed
+    Setting.orphans_contacts_in_months = 5
+    @contact.created_at = Time.zone.parse('2010-07-05')
+    travel_to Time.zone.parse('2010-07-05')
+
+    DomainVersion.stub(:contact_unlinked_more_than?, false) do
+      assert_not @contact.archivable?
+    end
+  end
+
+  def test_contact_is_not_archivable_when_it_was_ever_linked_but_linked_within_inactivity_period
+    DomainVersion.stub(:was_contact_linked?, true) do
+      DomainVersion.stub(:contact_unlinked_more_than?, false) do
+        assert_not @contact.archivable?
+      end
+    end
+  end
+
+  def test_archives_contact
+    contact = archivable_contact
+
+    assert_difference 'Contact.count', -1 do
+      contact.archive
+    end
+  end
+
+  def test_unarchivable_contact_cannot_be_archived
+    contact = unarchivable_contact
+
+    e = assert_raises do
+      contact.archive
+    end
+    assert_equal 'Contact cannot be archived', e.message
+  end
+
+  def test_sends_poll_msg_to_registrar_after_archivation
+    contact = archivable_contact
+    registrar = contact.registrar
+    contact.archive
+
+    assert_equal(I18n.t(:contact_has_been_archived, contact_code: contact.code,
+                                                    orphan_months: Setting.orphans_contacts_in_months),
+                 registrar.notifications.last.text)
+  end
+
+  private
+
+  def archivable_contact
+    contact = contacts(:john)
+    Setting.orphans_contacts_in_months = 0
+    DomainVersion.delete_all
+
+    other_contact = contacts(:william)
+    assert_not_equal other_contact, contact
+    Domain.update_all(registrant_id: other_contact.id)
+
+    DomainContact.delete_all
+
+    contact
+  end
+
+  def unarchivable_contact
+    Setting.orphans_contacts_in_months = 1188
+    @contact
+  end
+end

test/models/contact_test.rb

@@ -238,6 +238,52 @@ class ContactTest < ActiveSupport::TestCase
     assert_not @contact.deletable?
   end
 
+  def test_linked_scope_returns_contact_that_acts_as_registrant
+    domains(:shop).update!(registrant: @contact.becomes(Registrant))
+    assert Contact.linked.include?(@contact), 'Contact should be included'
+  end
+
+  def test_linked_scope_returns_contact_that_acts_as_admin_contact
+    domains(:shop).admin_contacts = [@contact]
+    assert Contact.linked.include?(@contact), 'Contact should be included'
+  end
+
+  def test_linked_scope_returns_contact_that_acts_as_tech_contact
+    domains(:shop).tech_contacts = [@contact]
+    assert Contact.linked.include?(@contact), 'Contact should be included'
+  end
+
+  def test_linked_scope_skips_unlinked_contact
+    contact = unlinked_contact
+    assert_not Contact.linked.include?(contact), 'Contact should be excluded'
+  end
+
+  def test_unlinked_scope_returns_unlinked_contact
+    contact = unlinked_contact
+    assert Contact.unlinked.include?(contact), 'Contact should be included'
+  end
+
+  def test_unlinked_scope_skips_contact_that_is_linked_as_registrant
+    contact = unlinked_contact
+    domains(:shop).update_columns(registrant_id: contact.becomes(Registrant).id)
+
+    assert Contact.unlinked.exclude?(contact), 'Contact should be excluded'
+  end
+
+  def test_unlinked_scope_skips_contact_that_is_linked_as_admin_contact
+    contact = unlinked_contact
+    domains(:shop).admin_contacts = [contact]
+
+    assert Contact.unlinked.exclude?(contact), 'Contact should be excluded'
+  end
+
+  def test_unlinked_scope_skips_contact_that_is_linked_as_tech_contact
+    contact = unlinked_contact
+    domains(:shop).tech_contacts = [contact]
+
+    assert Contact.unlinked.exclude?(contact), 'Contact should be excluded'
+  end
+
   def test_normalizes_country_code
     Setting.address_processing = true
     contact = Contact.new(country_code: 'us')
@@ -306,9 +352,12 @@ class ContactTest < ActiveSupport::TestCase
   end
 
   def unlinked_contact
-    Domain.update_all(registrant_id: contacts(:william).id)
+    other_contact = contacts(:william)
+    assert_not_equal @contact, other_contact
+    Domain.update_all(registrant_id: other_contact.id)
     DomainContact.delete_all
-    contacts(:john)
+
+    @contact
   end
 
   def valid_contact

test/models/inactive_contacts_test.rb

@@ -0,0 +1,15 @@
+require 'test_helper'
+
+class InactiveContactsTest < ActiveSupport::TestCase
+  def test_archives_inactive_contacts
+    contact_mock = Minitest::Mock.new
+    contact_mock.expect(:archive, nil, [{verified: false}])
+    contact_mock.expect(:id, 'id')
+    contact_mock.expect(:code, 'code')
+
+    inactive_contacts = InactiveContacts.new([contact_mock])
+    inactive_contacts.archive
+
+    assert_mock contact_mock
+  end
+end

test/models/version/domain_version_test.rb

@@ -0,0 +1,105 @@
+require 'test_helper'
+
+class DomainVersionTest < ActiveSupport::TestCase
+  setup do
+    @domain_version = log_domains(:one)
+    @contact = contacts(:john)
+  end
+
+  def test_was_contact_linked_returns_true_when_contact_was_used_as_registrant
+    @domain_version.update!(children: { admin_contacts: [],
+                                        tech_contacts: [],
+                                        registrant: [@contact.id] })
+
+    assert DomainVersion.was_contact_linked?(@contact.id)
+  end
+
+  def test_was_contact_linked_returns_true_when_contact_was_used_as_admin_contact
+    @domain_version.update!(children: { admin_contacts: [@contact.id],
+                                        tech_contacts: [],
+                                        registrant: [] })
+
+    assert DomainVersion.was_contact_linked?(@contact.id)
+  end
+
+  def test_was_contact_linked_returns_true_when_contact_was_used_as_tech_contact
+    @domain_version.update!(children: { admin_contacts: [],
+                                        tech_contacts: [@contact.id],
+                                        registrant: [] })
+
+    assert DomainVersion.was_contact_linked?(@contact.id)
+  end
+
+  def test_was_contact_linked_returns_false_when_contact_was_not_used
+    @domain_version.update!(children: { admin_contacts: [],
+                                        tech_contacts: [],
+                                        registrant: [] })
+
+    assert_not DomainVersion.was_contact_linked?(@contact.id)
+  end
+
+  def test_contact_unlinked_more_than_returns_true_when_contact_was_linked_as_registrant_more_than_given_period
+    @domain_version.update!(created_at: Time.zone.parse('2010-07-04 00:00:00'),
+                            children: { admin_contacts: [],
+                                        tech_contacts: [],
+                                        registrant: [@contact.id] })
+    travel_to Time.zone.parse('2010-07-05 00:00:01')
+
+    assert DomainVersion.contact_unlinked_more_than?(contact_id: @contact.id, period: 1.day)
+  end
+
+  def test_contact_unlinked_more_than_given_period_as_admin_contact
+    @domain_version.update!(created_at: Time.zone.parse('2010-07-04 00:00:00'),
+                            children: { admin_contacts: [1, @contact.id],
+                                        tech_contacts: [],
+                                        registrant: [] })
+    travel_to Time.zone.parse('2010-07-05 00:00:01')
+
+    assert DomainVersion.contact_unlinked_more_than?(contact_id: @contact.id, period: 1.day)
+  end
+
+  def test_contact_unlinked_more_than_given_period_as_tech_contact
+    @domain_version.update!(created_at: Time.zone.parse('2010-07-04 00:00:00'),
+                            children: { admin_contacts: [],
+                                        tech_contacts: [1, @contact.id],
+                                        registrant: [] })
+    travel_to Time.zone.parse('2010-07-05 00:00:01')
+
+    assert DomainVersion.contact_unlinked_more_than?(contact_id: @contact.id, period: 1.day)
+  end
+
+  def test_contact_linked_within_given_period_as_registrant
+    @domain_version.update!(created_at: Time.zone.parse('2010-07-05'),
+                            children: { admin_contacts: [],
+                                        tech_contacts: [],
+                                        registrant: [@contact.id] })
+    travel_to Time.zone.parse('2010-07-05')
+
+    assert_not DomainVersion.contact_unlinked_more_than?(contact_id: @contact.id, period: 1.day)
+  end
+
+  def test_contact_linked_within_given_period_as_admin_contact
+    @domain_version.update!(created_at: Time.zone.parse('2010-07-05'),
+                            children: { admin_contacts: [1, @contact.id],
+                                        tech_contacts: [],
+                                        registrant: [] })
+    travel_to Time.zone.parse('2010-07-05')
+
+    assert_not DomainVersion.contact_unlinked_more_than?(contact_id: @contact.id, period: 1.day)
+  end
+
+  def test_contact_linked_within_given_period_as_tech_contact
+    @domain_version.update!(created_at: Time.zone.parse('2010-07-05'),
+                            children: { admin_contacts: [],
+                                        tech_contacts: [1, @contact.id],
+                                        registrant: [] })
+    travel_to Time.zone.parse('2010-07-05')
+
+    assert_not DomainVersion.contact_unlinked_more_than?(contact_id: @contact.id, period: 1.day)
+  end
+
+  def test_contact_was_never_linked
+    DomainVersion.delete_all
+    assert_not DomainVersion.contact_unlinked_more_than?(contact_id: @contact.id, period: 1.day)
+  end
+end

test/tasks/contacts/archive_test.rb

@@ -0,0 +1,37 @@
+require 'test_helper'
+
+class ArchiveContactsTaskTest < ActiveSupport::TestCase
+  def test_archives_inactive_contacts
+    eliminate_effect_of_all_contacts_except(archivable_contact)
+
+    assert_difference 'Contact.count', -1 do
+      capture_io { run_task }
+    end
+  end
+
+  private
+
+  def archivable_contact
+    contact = contacts(:john)
+    Setting.orphans_contacts_in_months = 0
+    DomainVersion.delete_all
+
+    other_contact = contacts(:william)
+    assert_not_equal other_contact, contact
+    Domain.update_all(registrant_id: other_contact.id)
+
+    DomainContact.delete_all
+
+    contact
+  end
+
+  def eliminate_effect_of_all_contacts_except(contact)
+    Contact.connection.disable_referential_integrity do
+      Contact.where("id != #{contact.id}").delete_all
+    end
+  end
+
+  def run_task
+    Rake::Task['contacts:archive'].execute
+  end
+end

test/test_helper.rb

@@ -41,6 +41,7 @@ EInvoice.provider = EInvoice::Providers::TestProvider.new
 class ActiveSupport::TestCase
   ActiveRecord::Migration.check_pending!
   fixtures :all
+  set_fixture_class log_domains: DomainVersion
 
   teardown do
     travel_back