Merge pull request #1124 from internetee/refactor-force-delete

Refactor force delete

app/models/concerns/domain/force_delete.rb

@@ -12,8 +12,7 @@ def schedule_force_delete
 
     preserve_current_statuses_for_force_delete
     add_force_delete_statuses
-    self.force_delete_at = (Time.zone.now + (Setting.redemption_grace_period.days + 1.day)).utc
-                             .beginning_of_day
+    self.force_delete_date = Time.zone.today + Setting.redemption_grace_period.days + 1.day
     stop_all_pending_actions
     allow_deletion
     save(validate: false)
@@ -22,7 +21,7 @@ def schedule_force_delete
   def cancel_force_delete
     restore_statuses_before_force_delete
     remove_force_delete_statuses
-    self.force_delete_at = nil
+    self.force_delete_date = nil
     save(validate: false)
   end
 

app/models/concerns/domain/releasable.rb

@@ -15,13 +15,17 @@ def release_domains
 
         def releasable_domains
           if release_to_auction
-            where('delete_at < ? AND ? != ALL(coalesce(statuses, array[]::varchar[]))',
+            where('(delete_at < ? OR force_delete_date <= ?)' \
+              ' AND ? != ALL(coalesce(statuses, array[]::varchar[]))',
                   Time.zone.now,
+                  Time.zone.today,
                   DomainStatus::SERVER_DELETE_PROHIBITED)
           else
-            where('delete_at < ? AND ? != ALL(coalesce(statuses, array[]::varchar[])) AND' \
+            where('(delete_at < ? OR force_delete_date <= ?)' \
+              ' AND ? != ALL(coalesce(statuses, array[]::varchar[])) AND' \
                   ' ? != ALL(COALESCE(statuses, array[]::varchar[]))',
                   Time.zone.now,
+                  Time.zone.today,
                   DomainStatus::SERVER_DELETE_PROHIBITED,
                   DomainStatus::DELETE_CANDIDATE)
           end

app/models/domain.rb

@@ -501,7 +501,7 @@ def admin_status_update(update)
           when DomainStatus::PENDING_DELETE
             self.delete_at = nil
           when DomainStatus::SERVER_MANUAL_INZONE # removal causes server hold to set
-            self.outzone_at = Time.zone.now if self.force_delete_at.present?
+            self.outzone_at = Time.zone.now if force_delete_scheduled?
           when DomainStatus::DomainStatus::EXPIRED # removal causes server hold to set
             self.outzone_at = self.expire_time + 15.day
           when DomainStatus::DomainStatus::SERVER_HOLD # removal causes server hold to set

app/models/domain_cron.rb

@@ -78,18 +78,4 @@ def self.start_redemption_grace_period
     STDOUT << "#{Time.zone.now.utc} - Successfully set server_hold to #{marked} of #{real} domains\n" unless Rails.env.test?
     marked
   end
-
-  def self.destroy_delete_candidates
-    STDOUT << "#{Time.zone.now.utc} - Destroying domains\n" unless Rails.env.test?
-
-    c = 0
-
-    Domain.where('force_delete_at <= ?', Time.zone.now.end_of_day.utc).each do |x|
-      DomainDeleteJob.enqueue(x.id, run_at: rand(((24*60) - (DateTime.now.hour * 60  + DateTime.now.minute))).minutes.from_now)
-      STDOUT << "#{Time.zone.now.utc} DomainCron.destroy_delete_candidates: job added by force delete time ##{x.id} (#{x.name})\n" unless Rails.env.test?
-      c += 1
-    end
-
-    STDOUT << "#{Time.zone.now.utc} - Job destroy added for #{c} domains\n" unless Rails.env.test?
-  end
 end

app/models/whois_record.rb

@@ -40,7 +40,8 @@ def generate_json
     h[:changed]    = domain.updated_at.try(:to_s, :iso8601)
     h[:expire]     = domain.valid_to.to_date.to_s
     h[:outzone]    = domain.outzone_at.try(:to_date).try(:to_s)
-    h[:delete]     = [domain.delete_at, domain.force_delete_at].compact.min.try(:to_date).try(:to_s)
+    h[:delete] = [domain.delete_at, domain.force_delete_date].compact.min.try(:to_date)
+                                                             .try(:to_s)
 
     h[:registrant] = registrant.name
     h[:registrant_kind] = registrant.kind

app/presenters/domain_presenter.rb

@@ -39,7 +39,7 @@ def delete_date
   end
 
   def force_delete_date
-    view.l(domain.force_delete_at, format: :date) if domain.force_delete_at
+    view.l(domain.force_delete_date) if domain.force_delete_scheduled?
   end
 
   def admin_contact_names

app/views/admin/domains/partials/_general.html.erb

@@ -31,8 +31,8 @@
             <dt><%= t('.delete_time') %></dt>
             <dd><%= l(@domain.delete_at) %></dd>
 
-            <dt><%= Domain.human_attribute_name :force_delete_at %></dt>
-            <dd><%= l @domain.force_delete_at %></dd>
+            <dt><%= Domain.human_attribute_name :force_delete_date %></dt>
+            <dd><%= l @domain.force_delete_date %></dd>
 
             <dt><%= t('.locked_by_registrant_at') %></dt>
             <dd><%= l(@domain.locked_by_registrant_at) %></dd>

app/views/registrant/domains/partials/_general.html.erb

@@ -31,8 +31,8 @@
             <dt><%= Domain.human_attribute_name :delete_at %></dt>
             <dd><%= l(@domain.delete_at) %></dd>
 
-            <dt><%= Domain.human_attribute_name :force_delete_at %></dt>
-            <dd><%= l(@domain.force_delete_at) %></dd>
+            <dt><%= Domain.human_attribute_name :force_delete_date %></dt>
+            <dd><%= l @domain.force_delete_date %></dd>
         </dl>
     </div>
 </div>

config/locales/admin/domains.en.yml

@@ -52,7 +52,6 @@ en:
         general:
           outzone_time: Outzone time
           delete_time: Delete time
-          force_delete_time: Force delete time
           locked_by_registrant_at: Registry lock time
 
         admin_contacts:

config/schedule.rb

@@ -38,10 +38,6 @@
     runner 'Certificate.update_crl'
   end
 
-  every 42.minutes do
-    runner 'DomainCron.destroy_delete_candidates'
-  end
-
   every 45.minutes do
     runner 'DomainCron.start_expire_period'
   end

db/migrate/20190322152123_change_domains_force_delete_at_to_date.rb

@@ -0,0 +1,5 @@
+class ChangeDomainsForceDeleteAtToDate < ActiveRecord::Migration
+  def change
+    change_column :domains, :force_delete_at, :date
+  end
+end

db/migrate/20190322152529_rename_domains_force_delete_at_to_force_delete_date.rb

@@ -0,0 +1,5 @@
+class RenameDomainsForceDeleteAtToForceDeleteDate < ActiveRecord::Migration
+  def change
+    rename_column :domains, :force_delete_at, :force_delete_date
+  end
+end

db/structure.sql

@@ -936,7 +936,7 @@ CREATE TABLE public.domains (
     registrant_verification_asked_at timestamp without time zone,
     registrant_verification_token character varying,
     pending_json jsonb,
-    force_delete_at timestamp without time zone,
+    force_delete_date date,
     statuses character varying[],
     reserved boolean DEFAULT false,
     status_notes public.hstore,
@@ -4935,3 +4935,7 @@ INSERT INTO schema_migrations (version) VALUES ('20190311111718');
 
 INSERT INTO schema_migrations (version) VALUES ('20190312211614');
 
+INSERT INTO schema_migrations (version) VALUES ('20190322152123');
+
+INSERT INTO schema_migrations (version) VALUES ('20190322152529');
+

doc/registrant-api/v1/domain.md

@@ -71,7 +71,7 @@ Content-Type: application/json
     "pending_json":{
 
     },
-    "force_delete_at":null,
+    "force_delete_date":null,
     "statuses":[
       "ok"
     ],
@@ -187,7 +187,7 @@ Content-Type: application/json
     "pending_json":{
 
     },
-    "force_delete_at":null,
+    "force_delete_date":null,
     "statuses":[
       "ok"
     ],
@@ -295,7 +295,7 @@ Content-Type: application/json
   "pending_json":{
 
   },
-  "force_delete_at":null,
+  "force_delete_date":null,
   "statuses":[
     "ok"
   ],

doc/registrant-api/v1/registry_lock.md

@@ -71,7 +71,7 @@ Content-Type: application/json
   "pending_json":{
 
   },
-  "force_delete_at":null,
+  "force_delete_date":null,
   "statuses":[
     "serverUpdateProhibited",
     "serverDeleteProhibited",
@@ -216,7 +216,7 @@ Content-Type: application/json
   "pending_json":{
 
   },
-  "force_delete_at":null,
+  "force_delete_date":null,
   "statuses":[
     "ok"
   ],

doc/repp/v1/domain.md

@@ -57,7 +57,7 @@ Content-Type: application/json
       "registrant_verification_token": null,
       "pending_json": {
       },
-      "force_delete_at": null,
+      "force_delete_date": null,
       "statuses": [
         "ok"
       ],

lib/serializers/registrant_api/domain.rb

@@ -40,7 +40,7 @@ def to_json
           registrant_verification_asked_at: domain.registrant_verification_asked_at,
           registrant_verification_token: domain.registrant_verification_token,
           pending_json: domain.pending_json,
-          force_delete_at: domain.force_delete_at,
+          force_delete_date: domain.force_delete_date,
           statuses: domain.statuses,
           locked_by_registrant_at: domain.locked_by_registrant_at,
           reserved: domain.reserved,

spec/presenters/domain_presenter_spec.rb

@@ -63,16 +63,16 @@
     subject(:force_delete_date) { presenter.force_delete_date }
 
     context 'when present' do
-      let(:domain) { instance_double(Domain, force_delete_at: '05.07.2010') }
+      let(:domain) { instance_double(Domain, force_delete_date: '05.07.2010', force_delete_scheduled?: true) }
 
       it 'returns localized date' do
-        expect(view).to receive(:l).with('05.07.2010', format: :date).and_return('delete date')
+        expect(view).to receive(:l).with('05.07.2010').and_return('delete date')
         expect(force_delete_date).to eq('delete date')
       end
     end
 
     context 'when absent' do
-      let(:domain) { instance_double(Domain, force_delete_at: nil) }
+      let(:domain) { instance_double(Domain, force_delete_date: nil, force_delete_scheduled?: false) }
 
       specify { expect(force_delete_date).to be_nil }
     end

test/fixtures/domains.yml

@@ -8,7 +8,7 @@ shop:
   valid_to: <%= Time.zone.parse('2010-07-05').to_s(:db) %>
   outzone_at: <%= Time.zone.parse('2010-07-06').to_s(:db) %>
   delete_at: <%= Time.zone.parse('2010-07-07').to_s(:db) %>
-  force_delete_at: <%= Time.zone.parse('2010-07-08').to_s(:db) %>
+  force_delete_date: 2010-07-08
   period: 1
   period_unit: m
   uuid: 1b3ee442-e8fe-4922-9492-8fcb9dccc69c

test/lib/serializers/registrant_api/domain_test.rb

@@ -73,7 +73,7 @@ def test_other_fields_are_also_present
               registrant tech_contacts admin_contacts transfer_code name_dirty name_puny period
               period_unit creator_str updator_str legacy_id legacy_registrar_id legacy_registrant_id
               outzone_at delete_at registrant_verification_asked_at
-              registrant_verification_token pending_json force_delete_at statuses
+              registrant_verification_token pending_json force_delete_date statuses
               locked_by_registrant_at reserved status_notes nameservers]
 
     assert_equal(keys, @json.keys & keys)

test/models/domain/force_delete_test.rb

@@ -3,21 +3,23 @@
 class DomainForceDeleteTest < ActiveSupport::TestCase
   setup do
     @domain = domains(:shop)
+    @original_redemption_grace_period = Setting.redemption_grace_period
   end
 
-  def test_schedule_force_delete
-    @original_redemption_grace_period = Setting.redemption_grace_period
+  teardown do
+    Setting.redemption_grace_period = @original_redemption_grace_period
+  end
+
+  def test_schedules_force_delete
+    assert_not @domain.force_delete_scheduled?
     Setting.redemption_grace_period = 30
-    travel_to Time.zone.parse('2010-07-05 00:00')
+    travel_to Time.zone.parse('2010-07-05')
 
     @domain.schedule_force_delete
     @domain.reload
 
     assert @domain.force_delete_scheduled?
-    assert_equal Time.zone.parse('2010-08-04 03:00'), @domain.force_delete_at
-
-    travel_back
-    Setting.redemption_grace_period = @original_redemption_grace_period
+    assert_equal Date.parse('2010-08-05'), @domain.force_delete_date
   end
 
   def test_scheduling_force_delete_adds_corresponding_statuses
@@ -80,6 +82,17 @@ def test_force_delete_cannot_be_scheduled_when_a_domain_is_discarded
     end
   end
 
+  def test_cancels_force_delete
+    @domain.update_columns(statuses: [DomainStatus::FORCE_DELETE], force_delete_date: '2010-07-05')
+    assert @domain.force_delete_scheduled?
+
+    @domain.cancel_force_delete
+    @domain.reload
+
+    assert_not @domain.force_delete_scheduled?
+    assert_nil @domain.force_delete_date
+  end
+
   def test_cancelling_force_delete_bypasses_validation
     @domain = domains(:invalid)
     @domain.schedule_force_delete

test/models/domain/releasable/auctionable_test.rb

@@ -40,6 +40,15 @@ def test_skips_auction_when_domains_is_reserved
     assert_not @domain.domain_name.at_auction?
   end
 
+  def test_sells_domains_with_scheduled_force_delete_procedure_at_auction
+    @domain.update!(force_delete_date: '2010-07-05')
+    travel_to Time.zone.parse('2010-07-05')
+
+    Domain.release_domains
+
+    assert @domain.domain_name.at_auction?
+  end
+
   def test_deletes_registered_domain
     @domain.update!(delete_at: Time.zone.parse('2010-07-05 07:59'))
     travel_to Time.zone.parse('2010-07-05 08:00')

test/models/domain/releasable/discardable_test.rb

@@ -15,6 +15,16 @@ def test_discards_domains_with_past_delete_at
     assert @domain.discarded?
   end
 
+  def test_discards_domains_with_scheduled_force_delete_procedure
+    @domain.update!(force_delete_date: '2010-07-05')
+    travel_to Time.zone.parse('2010-07-05')
+
+    Domain.release_domains
+    @domain.reload
+
+    assert @domain.discarded?
+  end
+
   def test_ignores_domains_with_delete_at_in_the_future_or_now
     @domain.update!(delete_at: Time.zone.parse('2010-07-05 08:00'))
     travel_to Time.zone.parse('2010-07-05 08:00')

test/system/registrant_area/domains/details_test.rb

@@ -7,7 +7,10 @@ class RegistrantAreaDomainDetailsTest < ApplicationSystemTestCase
   end
 
   def test_general_data
+    @domain.update_columns(force_delete_date: '2010-07-08', statuses: [DomainStatus::FORCE_DELETE])
+
     visit registrant_domain_url(@domain)
+
     assert_text 'Name shop.test'
     assert_text "Registered at #{l Time.zone.parse('2010-07-04')}"
     assert_link 'Best Names', href: registrant_registrar_path(@domain.registrar)
@@ -18,7 +21,7 @@ def test_general_data
     assert_text "Valid to #{l Time.zone.parse('2010-07-05')}"
     assert_text "Outzone at #{l Time.zone.parse('2010-07-06')}"
     assert_text "Delete at #{l Time.zone.parse('2010-07-07')}"
-    assert_text "Force delete at #{l Time.zone.parse('2010-07-08')}"
+    assert_text "Force delete date #{l Date.parse('2010-07-08')}"
   end
 
   def test_registrant