Delete discarded domains at the random moment within next 24h

#790

app/models/concerns/domain/deletable.rb

@@ -4,11 +4,21 @@ module Concerns::Domain::Deletable
   private
 
   def delete_later
-    run_at = rand(((24 * 60) - (DateTime.now.hour * 60 + DateTime.now.minute))).minutes.from_now
-    DomainDeleteJob.enqueue(id, run_at: run_at)
+    deletion_time = Time.zone.at(rand(deletion_time_span))
+    DomainDeleteJob.enqueue(id, run_at: deletion_time)
+    logger.info "Domain #{name} is scheduled to be deleted around #{deletion_time}"
   end
 
   def do_not_delete_later
-    QueJob.find_by!("args->>0 = '#{id}'", job_class: DomainDeleteJob.name).delete
+    QueJob.find_by!("args->>0 = '#{id}'", job_class: DomainDeleteJob.name).destroy!
+  end
+
+  def deletion_time_span
+    # 5 minutes to ensure we don't create a background job with past `run_at`
+    ((Time.zone.now + 5.minutes).to_i)..(deletion_deadline.to_i)
+  end
+
+  def deletion_deadline
+    delete_at + 24.hours
   end
 end

app/models/concerns/domain/discardable.rb

@@ -13,18 +13,22 @@ module Concerns::Domain::Discardable
   end
 
   def discard
+    raise 'Domain is already discarded' if discarded?
+
     statuses << DomainStatus::DELETE_CANDIDATE
-    # We don't validate deliberately since nobody is interested in fixing discarded domain
+    transaction do
       save(validate: false)
       delete_later
-    logger.info "Domain #{name} (ID: #{id}) is scheduled to be deleted"
+    end
   end
 
   def keep
     statuses.delete(DomainStatus::DELETE_CANDIDATE)
+    transaction do
       save(validate: false)
       do_not_delete_later
     end
+  end
 
   def discarded?
     statuses.include?(DomainStatus::DELETE_CANDIDATE)

test/integration/admin/domains/details_test.rb

@@ -7,20 +7,15 @@ class AdminAreaDomainDetailsTest < ActionDispatch::IntegrationTest
   end
 
   def test_discarded_domain_has_corresponding_label
+    travel_to Time.zone.parse('2010-07-05 10:30')
+    @domain.delete_at = Time.zone.parse('2010-07-05 10:00')
+
     visit admin_domain_url(@domain)
     assert_no_css 'span.label.label-warning', text: 'deleteCandidate'
+
     @domain.discard
+
     visit admin_domain_url(@domain)
     assert_css 'span.label.label-warning', text: 'deleteCandidate'
   end
-
-  def test_keep_a_domain
-    @domain.discard
-    visit edit_admin_domain_url(@domain)
-    click_link_or_button 'Remove deleteCandidate status'
-    @domain.reload
-    refute @domain.discarded?
-    assert_text 'deleteCandidate status has been removed'
-    assert_no_link 'Remove deleteCandidate status'
-  end
 end

test/integration/admin/domains_test.rb

@@ -1,13 +1,31 @@
 require 'test_helper'
 
 class AdminDomainsTestTest < ActionDispatch::IntegrationTest
-  setup do
+  def setup
     login_as users(:admin)
+    @domain = domains(:shop)
+  end
+
+  def teardown
+    travel_back
   end
 
   def test_shows_details
-    domain = domains(:shop)
-    visit admin_domain_path(domain)
-    assert_field nil, with: domain.transfer_code
+    visit admin_domain_path(@domain)
+    assert_field nil, with: @domain.transfer_code
+  end
+
+  def test_keep_a_domain
+    travel_to Time.zone.parse('2010-07-05 10:30')
+    @domain.delete_at = Time.zone.parse('2010-07-05 10:00')
+    @domain.discard
+
+    visit edit_admin_domain_url(@domain)
+    click_link_or_button 'Remove deleteCandidate status'
+    @domain.reload
+
+    assert_not @domain.discarded?
+    assert_text 'deleteCandidate status has been removed'
+    assert_no_link 'Remove deleteCandidate status'
   end
 end

test/integration/epp/domain/domain_delete_test.rb

@@ -1,6 +1,10 @@
 require 'test_helper'
 
 class EppDomainDeleteTest < ActionDispatch::IntegrationTest
+  def setup
+    @domain = domains(:shop)
+  end
+
   def test_bypasses_domain_and_registrant_and_contacts_validation
     request_xml = <<-XML
       <?xml version="1.0" encoding="UTF-8" standalone="no"?>
@@ -27,7 +31,9 @@ class EppDomainDeleteTest < ActionDispatch::IntegrationTest
   end
 
   def test_discarded_domain_cannot_be_deleted
-    domains(:shop).discard
+    travel_to Time.zone.parse('2010-07-05 10:30')
+    @domain.delete_at = Time.zone.parse('2010-07-05 10:00')
+    @domain.discard
 
     request_xml = <<-XML
       <?xml version="1.0" encoding="UTF-8" standalone="no"?>
@@ -51,5 +57,6 @@ class EppDomainDeleteTest < ActionDispatch::IntegrationTest
       post '/epp/command/delete', { frame: request_xml }, 'HTTP_COOKIE' => 'session=api_bestnames'
     end
     assert_equal '2105', Nokogiri::XML(response.body).at_css('result')[:code]
+    travel_back
   end
 end

test/integration/epp/domain/domain_update_test.rb

@@ -1,6 +1,10 @@
 require 'test_helper'
 
 class EppDomainUpdateTest < ActionDispatch::IntegrationTest
+  def setup
+    @domain = domains(:shop)
+  end
+
   def test_update_domain
     request_xml = <<-XML
       <?xml version="1.0" encoding="UTF-8" standalone="no"?>
@@ -21,13 +25,16 @@ class EppDomainUpdateTest < ActionDispatch::IntegrationTest
     XML
 
     post '/epp/command/update', { frame: request_xml }, 'HTTP_COOKIE' => 'session=api_bestnames'
-    assert_equal 'f0ff7d17b0', domains(:shop).transfer_code
+    @domain.reload
+    assert_equal 'f0ff7d17b0', @domain.transfer_code
     assert_equal '1000', Nokogiri::XML(response.body).at_css('result')[:code]
     assert_equal 1, Nokogiri::XML(response.body).css('result').size
   end
 
   def test_discarded_domain_cannot_be_updated
-    domains(:shop).discard
+    travel_to Time.zone.parse('2010-07-05 10:30')
+    @domain.delete_at = Time.zone.parse('2010-07-05 10:00')
+    @domain.discard
 
     request_xml = <<-XML
       <?xml version="1.0" encoding="UTF-8" standalone="no"?>
@@ -44,5 +51,6 @@ class EppDomainUpdateTest < ActionDispatch::IntegrationTest
 
     post '/epp/command/update', { frame: request_xml }, 'HTTP_COOKIE' => 'session=api_bestnames'
     assert_equal '2105', Nokogiri::XML(response.body).at_css('result')[:code]
+    travel_back
   end
 end

test/integration/epp/domain/transfer/request_test.rb

@@ -1,12 +1,17 @@
 require 'test_helper'
 
 class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest
-  setup do
+  def setup
     @domain = domains(:shop)
     @new_registrar = registrars(:goodnames)
+    @original_transfer_wait_time = Setting.transfer_wait_time
     Setting.transfer_wait_time = 0
   end
 
+  def teardown
+    Setting.transfer_wait_time = @original_transfer_wait_time
+  end
+
   def test_transfers_domain_at_once
     post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' }
     assert_equal '1000', Nokogiri::XML(response.body).at_css('result')[:code]
@@ -76,6 +81,8 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest
   end
 
   def test_discarded_domain_cannot_be_transferred
+    travel_to Time.zone.parse('2010-07-05 10:30')
+    @domain.delete_at = Time.zone.parse('2010-07-05 10:00')
     @domain.discard
 
     post '/epp/command/transfer', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=api_goodnames' }
@@ -83,6 +90,7 @@ class EppDomainTransferRequestTest < ActionDispatch::IntegrationTest
 
     assert_equal registrars(:bestnames), @domain.registrar
     assert_equal '2105', Nokogiri::XML(response.body).at_css('result')[:code]
+    travel_back
   end
 
   def test_same_registrar

test/models/domain/discardable_test.rb

@@ -1,30 +1,54 @@
 require 'test_helper'
 
 class DomainDiscardableTest < ActiveSupport::TestCase
-  setup do
+  def setup
+    travel_to Time.zone.parse('2010-07-05 10:30')
     @domain = domains(:shop)
+    @domain.delete_at = Time.zone.parse('2010-07-05 10:00')
   end
 
-  def test_discarding_a_domain
+  def teardown
+    travel_back
+  end
+
+  def test_discarding_a_domain_persists_the_state
     @domain.discard
     @domain.reload
     assert @domain.discarded?
   end
 
-  def test_discarding_a_domain_deletes_schedules_domain_deletion
+  def test_discarding_a_domain_schedules_deletion_at_random_time
     @domain.discard
-    assert QueJob.find_by("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name)
+    other_domain = domains(:airport)
+    other_domain.delete_at = Time.zone.parse('2010-07-05 10:00')
+    other_domain.discard
+
+    background_job = QueJob.find_by("args->>0 = '#{@domain.id}'", job_class: DomainDeleteJob.name)
+    other_background_job = QueJob.find_by("args->>0 = '#{other_domain.id}'",
+                                          job_class: DomainDeleteJob.name)
+    assert_not_equal background_job.run_at, other_background_job.run_at
   end
 
   def test_discarding_a_domain_bypasses_validation
     domain = domains(:invalid)
+    domain.delete_at = Time.zone.parse('2010-07-05 10:00')
     domain.discard
     domain.reload
     assert domain.discarded?
   end
 
+  def test_domain_cannot_be_discarded_repeatedly
+    @domain.discard
+
+    exception = assert_raises do
+      @domain.discard
+    end
+    assert_equal 'Domain is already discarded', exception.message
+  end
+
   def test_keeping_a_domain_bypasses_validation
     domain = domains(:invalid)
+    domain.delete_at = Time.zone.parse('2010-07-05 10:00')
     domain.discard
     domain.keep
     domain.reload