Pending delete improvements #2623

app/models/domain.rb

@@ -330,7 +330,6 @@ class Domain < ActiveRecord::Base
   end
 
   def server_holdable?
-    return false if outzone_at > Time.zone.now
     return false if statuses.include?(DomainStatus::SERVER_HOLD)
     return false if statuses.include?(DomainStatus::SERVER_MANUAL_INZONE)
     true
@@ -613,7 +612,6 @@ class Domain < ActiveRecord::Base
     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)
@@ -642,7 +640,7 @@ class Domain < ActiveRecord::Base
   end
 
   def pending_update_prohibited?
-    (statuses & [
+    (statuses_was & [
       DomainStatus::CLIENT_UPDATE_PROHIBITED,
       DomainStatus::SERVER_UPDATE_PROHIBITED,
       DomainStatus::PENDING_CREATE,
@@ -666,17 +664,24 @@ class Domain < ActiveRecord::Base
   end
 
   def pending_delete_prohibited?
-    (statuses & [
+    (statuses_was & [
       DomainStatus::CLIENT_DELETE_PROHIBITED,
       DomainStatus::SERVER_DELETE_PROHIBITED,
+      DomainStatus::CLIENT_UPDATE_PROHIBITED,
+      DomainStatus::SERVER_UPDATE_PROHIBITED,
       DomainStatus::PENDING_CREATE,
-      DomainStatus::PENDING_UPDATE,
-      DomainStatus::PENDING_DELETE,
       DomainStatus::PENDING_RENEW,
-      DomainStatus::PENDING_TRANSFER
+      DomainStatus::PENDING_TRANSFER,
+      DomainStatus::PENDING_UPDATE,
+      DomainStatus::PENDING_DELETE
     ]).present?
   end
 
+  # let's use positive method names
+  def pending_deletable?
+    !pending_delete_prohibited?
+  end
+
   def set_pending_delete
     if pending_delete_prohibited?
       logger.info "DOMAIN STATUS UPDATE ISSUE ##{id}: PENDING_DELETE not allowed to set. [#{statuses}]"
@@ -685,13 +690,25 @@ class Domain < ActiveRecord::Base
     statuses << DomainStatus::PENDING_DELETE
   end
 
+  def set_server_hold
+    statuses << DomainStatus::SERVER_HOLD
+  end
+
+  # rubocop: disable Metrics/CyclomaticComplexity
+  # rubocop: disable Metrics/PerceivedComplexity
   def manage_automatic_statuses
     if statuses.empty? && valid?
       statuses << DomainStatus::OK
     elsif statuses.length > 1 || !valid?
       statuses.delete(DomainStatus::OK)
     end
+
+    p_d = statuses.include?(DomainStatus::PENDING_DELETE)
+    s_h = (statuses & [DomainStatus::SERVER_MANUAL_INZONE, DomainStatus::SERVER_HOLD]).empty?
+    statuses << DomainStatus::SERVER_HOLD if p_d && s_h
   end
+  # rubocop: enable Metrics/CyclomaticComplexity
+  # rubocop: enable Metrics/PerceivedComplexity
 
   def children_log
     log = HashWithIndifferentAccess.new

app/models/epp/domain.rb

@@ -472,6 +472,8 @@ class Epp::Domain < Domain
     )
   end
 
+  # rubocop: disable Metrics/PerceivedComplexity
+  # rubocop: disable Metrics/CyclomaticComplexity
   def epp_destroy(frame, user_id, verify = true)
     return false unless valid?
 
@@ -485,9 +487,19 @@ class Epp::Domain < Domain
       manage_automatic_statuses
       true # aka 1001 pending_delete
     else
-      set_expired!
+      throw :epp_error, {
+        code: '2304',
+        msg: I18n.t(:object_status_prohibits_operation)
+      } unless pending_deletable?
+
+      self.delete_at = Time.zone.now + Setting.redemption_grace_period.days
+      set_pending_delete
+      set_server_hold if server_holdable?
+      save(validate: false)
     end
   end
+  # rubocop: enable Metrics/PerceivedComplexity
+  # rubocop: enable Metrics/CyclomaticComplexity
 
   ### RENEW ###
 

app/views/admin/domains/partials/_general.haml

@@ -21,3 +21,12 @@
 
       %dt= t(:valid_to)
       %dd= l(@domain.valid_to)
+
+      %dt= t(:outzone_at)
+      %dd= l(@domain.outzone_at)
+
+      %dt= t(:delete_at)
+      %dd= l(@domain.delete_at)
+
+      %dt= t(:force_delete_at)
+      %dd= l(@domain.force_delete_at)

spec/epp/domain_spec.rb

@@ -2014,7 +2014,7 @@ describe 'EPP Domain', epp: true do
           _anonymus: [
             { contact: { value: 'FIXED:MAK21', attrs: { type: 'tech' } } },
             { status: { value: 'Payment overdue.', attrs: { s: 'clientHold', lang: 'en' } } },
-            { status: { value: '', attrs: { s: 'clientUpdateProhibited' } } }
+            { status: { value: '', attrs: { s: 'clientRenewProhibited' } } }
           ]
         ]
       }, {
@@ -2044,6 +2044,7 @@ describe 'EPP Domain', epp: true do
       Fabricate(:contact, code: 'FIXED:MAK21')
 
       response = epp_plain_request(xml)
+
       response[:results][0][:result_code].should == '1000'
 
       d = Domain.last
@@ -2056,7 +2057,7 @@ describe 'EPP Domain', epp: true do
 
       d.statuses.count.should == 2
       d.statuses.include?('clientHold').should == true
-      d.statuses.include?('clientUpdateProhibited').should == true
+      d.statuses.include?('clientRenewProhibited').should == true
 
       d.dnskeys.count.should == 2
 
@@ -2238,7 +2239,7 @@ describe 'EPP Domain', epp: true do
           _anonymus: [
             { contact: { value: 'FIXED:CITIZEN_1234', attrs: { type: 'tech' } } },
             { status: { value: 'Payment overdue.', attrs: { s: 'clientHold', lang: 'en' } } },
-            { status: { value: '', attrs: { s: 'clientUpdateProhibited' } } }
+            { status: { value: '', attrs: { s: 'clientRenewProhibited' } } }
           ]
         ]
       }, {
@@ -2305,7 +2306,7 @@ describe 'EPP Domain', epp: true do
       d.dnskeys.count.should == 1
 
       d.statuses.count.should == 1
-      d.statuses.first.should == 'clientUpdateProhibited'
+      d.statuses.first.should == 'clientRenewProhibited'
 
       rem_ns = d.nameservers.find_by(hostname: 'ns1.example.com')
       rem_ns.should be_falsey

spec/models/domain_spec.rb

@@ -454,8 +454,12 @@ describe Domain do
 
       @domain.set_pending_delete
       @domain.save
-      @domain.statuses.should == ['pendingDelete']
+      @domain.statuses.should == ['pendingDelete', 'serverHold']
       @domain.pending_delete?.should == true
+      @domain.statuses = ['serverManualInzone']
+      @domain.save
+      @domain.set_pending_delete
+      @domain.statuses.sort.should == ['pendingDelete', 'serverManualInzone'].sort
       @domain.statuses = DomainStatus::OK # restore
     end