Improve domain expiring logic #2909

app/models/domain.rb

@@ -230,7 +230,7 @@ class Domain < ActiveRecord::Base
       domains = Domain.where('valid_to <= ?', Time.zone.now)
       domains.each do |domain|
         next unless domain.expirable?
-        domain.set_expired
+        domain.set_graceful_expired
         STDOUT << "#{Time.zone.now.utc} Domain.start_expire_period: ##{domain.id} #{domain.changes}\n" unless Rails.env.test?
         domain.save(validate: false)
       end
@@ -320,7 +320,12 @@ class Domain < ActiveRecord::Base
 
   def expirable?
     return false if valid_to > Time.zone.now
-    !statuses.include?(DomainStatus::EXPIRED)
+
+    if statuses.include?(DomainStatus::EXPIRED) && outzone_at.present? && delete_at.present?
+      return false
+    end
+
+    true
   end
 
   def server_holdable?
@@ -545,8 +550,6 @@ class Domain < ActiveRecord::Base
     self.registered_at = Time.zone.now
     self.valid_from = Time.zone.now
     self.valid_to = valid_from + self.class.convert_period_to_time(period, period_unit)
-    self.outzone_at = valid_to + Setting.expire_warning_period.days
-    self.delete_at = outzone_at + Setting.redemption_grace_period.days
   end
 
   # rubocop:disable Metrics/AbcSize
@@ -603,6 +606,13 @@ class Domain < ActiveRecord::Base
     save(validate: false)
   end
 
+  def set_graceful_expired
+    self.outzone_at = valid_to + Setting.expire_warning_period.days
+    self.delete_at = outzone_at + Setting.redemption_grace_period.days
+    statuses << DomainStatus::EXPIRED
+  end
+
+  # TODO: This looks odd - outzone_at and delete_at will be the same value?
   def set_expired
     # TODO: currently valid_to attribute update logic is open
     # self.valid_to = valid_from + self.class.convert_period_to_time(period, period_unit)
@@ -675,7 +685,6 @@ class Domain < ActiveRecord::Base
   end
 
   def manage_automatic_statuses
-    # domain_statuses.create(value: DomainStatus::DELETE_CANDIDATE) if delete_candidateable?
     if statuses.empty? && valid?
       statuses << DomainStatus::OK
     elsif statuses.length > 1 || !valid?

app/models/epp/domain.rb

@@ -490,8 +490,8 @@ class Epp::Domain < Domain
 
     p = self.class.convert_period_to_time(period, unit)
     self.valid_to = valid_to + p
-    self.outzone_at = valid_to + Setting.expire_warning_period.days
+    self.outzone_at = nil
-    self.delete_at = outzone_at + Setting.redemption_grace_period.days
+    self.delete_at = nil
     self.period = period
     self.period_unit = unit
 

spec/epp/domain_spec.rb

@@ -2426,10 +2426,8 @@ describe 'EPP Domain', epp: true do
 
       domain.reload
       domain.valid_to.should be_within(1).of(exp_date + 1.year)
-      domain.outzone_at.should be_within(1).of(exp_date + 1.year + Setting.expire_warning_period.days)
+      domain.outzone_at.should be_nil
-      domain.delete_at.should be_within(1).of(
+      domain.delete_at.should be_nil
-        exp_date + 1.year + Setting.expire_warning_period.days + Setting.redemption_grace_period.days
-      )
 
       @registrar1.balance.should == old_balance - 15.0
       @registrar1.cash_account.account_activities.count.should == old_activities + 1
@@ -2485,10 +2483,8 @@ describe 'EPP Domain', epp: true do
 
       domain.reload
       domain.valid_to.should be_within(1).of(exp_date + 1.year)
-      domain.outzone_at.should be_within(1).of(exp_date + 1.year + Setting.expire_warning_period.days)
+      domain.outzone_at.should be_nil
-      domain.delete_at.should be_within(1).of(
+      domain.delete_at.should be_nil
-        exp_date + 1.year + Setting.expire_warning_period.days + Setting.redemption_grace_period.days
-      )
 
       @registrar1.balance.should == old_balance - 15.0
       @registrar1.cash_account.account_activities.count.should == old_activities + 1
@@ -2764,10 +2760,8 @@ describe 'EPP Domain', epp: true do
 
       domain.reload
       domain.valid_to.should be_within(5).of(old_valid_to + 1.year)
-      domain.outzone_at.should be_within(5).of(old_valid_to + 1.year + Setting.expire_warning_period.days)
+      domain.outzone_at.should be_nil
-      domain.delete_at.should be_within(5).of(
+      domain.delete_at.should be_nil
-        old_valid_to + 1.year + Setting.expire_warning_period.days + Setting.redemption_grace_period.days
-      )
     end
 
     it 'does not renew foreign domain' do

spec/models/domain_spec.rb

@@ -82,10 +82,8 @@ describe Domain do
     it 'should have correct validity dates' do
       valid_to = Time.zone.now + 1.year
       @domain.valid_to.should be_within(5).of(valid_to)
-      @domain.outzone_at.should be_within(5).of(valid_to + Setting.expire_warning_period.days)
+      @domain.outzone_at.should be_nil
-      @domain.delete_at.should be_within(5).of(
+      @domain.delete_at.should be_nil
-        valid_to + Setting.expire_warning_period.days + Setting.redemption_grace_period.days
-      )
     end
 
     it 'should validate uniqueness of tech contacts' do
@@ -146,18 +144,39 @@ describe Domain do
       Domain.start_expire_period
       @domain.statuses.include?(DomainStatus::EXPIRED).should == false
 
-      @domain.valid_to = Time.zone.now - 10.days
+      old_valid_to = Time.zone.now - 10.days
+      @domain.valid_to = old_valid_to
       @domain.save
 
       Domain.start_expire_period
       @domain.reload
       @domain.statuses.include?(DomainStatus::EXPIRED).should == true
+      @domain.outzone_at.should be_within(5).of(old_valid_to + Setting.expire_warning_period.days)
+      @domain.delete_at.should be_within(5).of(
+        old_valid_to + Setting.expire_warning_period.days + Setting.redemption_grace_period.days
+      )
 
       Domain.start_expire_period
       @domain.reload
       @domain.statuses.include?(DomainStatus::EXPIRED).should == true
     end
 
+    it 'should start redemption grace period' do
+      old_valid_to = Time.zone.now - 10.days
+      @domain.valid_to = old_valid_to
+      @domain.statuses = [DomainStatus::EXPIRED]
+      @domain.outzone_at, @domain.delete_at = nil, nil
+      @domain.save
+
+      Domain.start_expire_period
+      @domain.reload
+      @domain.statuses.include?(DomainStatus::EXPIRED).should == true
+      @domain.outzone_at.should be_within(5).of(old_valid_to + Setting.expire_warning_period.days)
+      @domain.delete_at.should be_within(5).of(
+        old_valid_to + Setting.expire_warning_period.days + Setting.redemption_grace_period.days
+      )
+    end
+
     it 'should start redemption grace period' do
       Domain.start_redemption_grace_period
       @domain.reload