Refactor force delete procedure

- Change `domains.force_delete_at` database column type to date,
rename to `force_delete_date`
- Save `force_delete_date` in application timezone
- Improve tests

Fixes #812, #253

app/models/concerns/domain/force_delete.rb

@@ -12,8 +12,7 @@ module Concerns::Domain::ForceDelete
 
     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
+    self.force_delete_date = Time.zone.today + Setting.redemption_grace_period.days + 1.day
-                             .beginning_of_day
     stop_all_pending_actions
     allow_deletion
     save(validate: false)
@@ -22,7 +21,7 @@ module Concerns::Domain::ForceDelete
   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/domain.rb

@@ -501,7 +501,7 @@ class Domain < ActiveRecord::Base
           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

@@ -84,7 +84,7 @@ class DomainCron
 
     c = 0
 
-    Domain.where('force_delete_at <= ?', Time.zone.now.end_of_day.utc).each do |x|
+    Domain.where('force_delete_date <= ?', 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

app/models/whois_record.rb

@@ -40,7 +40,8 @@ class WhoisRecord < ActiveRecord::Base
     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 @@ class DomainPresenter
   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>
+            <dt><%= Domain.human_attribute_name :force_delete_date %></dt>
-            <dd><%= l @domain.force_delete_at %></dd>
+            <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>
+            <dt><%= Domain.human_attribute_name :force_delete_date %></dt>
-            <dd><%= l(@domain.force_delete_at) %></dd>
+            <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:

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 @@ module Serializers
           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 @@ RSpec.describe DomainPresenter do
     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 @@ class SerializersRegistrantApiDomainTest < ActiveSupport::TestCase
               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 @@ require 'test_helper'
 class DomainForceDeleteTest < ActiveSupport::TestCase
   setup do
     @domain = domains(:shop)
+    @original_redemption_grace_period = Setting.redemption_grace_period
   end
 
-  def test_schedule_force_delete
+  teardown do
-    @original_redemption_grace_period = Setting.redemption_grace_period
+    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
+    assert_equal Date.parse('2010-08-05'), @domain.force_delete_date
-
-    travel_back
-    Setting.redemption_grace_period = @original_redemption_grace_period
   end
 
   def test_scheduling_force_delete_adds_corresponding_statuses
@@ -80,6 +82,17 @@ class DomainForceDeleteTest < ActiveSupport::TestCase
     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/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 @@ class RegistrantAreaDomainDetailsTest < ApplicationSystemTestCase
     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