Refactor some dispute logic

app/controllers/admin/disputes_controller.rb

@@ -44,7 +44,7 @@ module Admin
 
     # DELETE /admin/disputes/1
     def delete
-      @dispute.update(closed: true)
+      @dispute.close
       redirect_to admin_disputes_url, notice: 'Dispute was successfully closed.'
     end
 

app/controllers/epp/domains_controller.rb

@@ -106,7 +106,7 @@ module Epp
       updated = @domain.update(params[:parsed_frame], current_user)
       (handle_errors(@domain) && return) unless updated
 
-      Dispute.active.close_by_domain(@domain.name) if @domain.disputed?
+      Dispute.close_by_domain(@domain.name) if @domain.disputed?
       pending = @domain.epp_pending_update.present?
       render_epp_response "/epp/domains/success#{'_pending' if pending}"
     end

app/models/dispute.rb

@@ -1,14 +1,11 @@
 class Dispute < ApplicationRecord
   validates :domain_name, :password, :starts_at, :expires_at, presence: true
-  before_validation :fill_empty_passwords
+  before_validation :fill_empty_passwords, :set_expiry_date
-  before_validation :set_expiry_date
   validate :validate_domain_name_format
   validate :validate_domain_name_period_uniqueness
   validate :validate_start_date
 
-  before_save :set_expiry_date
+  before_save :set_expiry_date, :sync_reserved_password, :generate_data
-  before_save :sync_reserved_password
-  before_save :generate_data
   after_destroy :remove_data
 
   scope :expired, -> { where('expires_at < ?', Time.zone.today) }
@@ -19,8 +16,6 @@ class Dispute < ApplicationRecord
 
   attr_readonly :domain_name
 
-  alias_attribute :name, :domain_name
-
   def domain
     Domain.find_by(name: domain_name)
   end
@@ -47,12 +42,9 @@ class Dispute < ApplicationRecord
   end
 
   def generate_data
-    return if starts_at > Time.zone.today
+    return if starts_at > Time.zone.today || expires_at < Time.zone.today
-    return if expires_at < Time.zone.today
 
-    domain = Domain.find_by_idn(domain_name)
     domain&.mark_as_disputed
-
     return if domain
 
     wr = Whois::Record.find_or_initialize_by(name: domain_name)
@@ -60,32 +52,31 @@ class Dispute < ApplicationRecord
     wr.save
   end
 
-  alias_method :update_whois_record, :generate_data
-
   def close
     return false unless update(closed: true)
     return if Dispute.active.where(domain_name: domain_name).any?
 
-    Domain.find_by_idn(domain_name)&.unmark_as_disputed
+    domain&.unmark_as_disputed
+    return true if domain
 
-    domain = DNS::DomainName.new(domain_name)
+    forward_to_auction_if_possible
-    if domain.available? && domain.auctionable?
-      domain.sell_at_auction
-      return true
-    else
-      whois_record = Whois::Record.find_or_initialize_by(name: domain_name)
-      return true if remove_whois_data(whois_record)
   end
 
-    false
+  def forward_to_auction_if_possible
+    domain = DNS::DomainName.new(domain_name)
+    return domain.sell_at_auction if domain.available? && domain.auctionable?
+
+    whois_record = Whois::Record.find_by(name: domain_name)
+    remove_whois_data(whois_record)
   end
 
   def remove_whois_data(record)
-    record.json['status'] = record.json['status'].delete_if { |status| status == 'disputed' }
+    return true unless record
-    if record.json['status'].blank?
+
-      return true if record.destroy && record.json['status'].blank?
+    record.json['status'].delete_if { |status| status == 'disputed' }
-    end
+    record.destroy && return if record.json['status'].blank?
-    record.save
+
+    save
   end
 
   def generate_json(record)

app/models/dns/domain_name.rb

@@ -29,8 +29,6 @@ module DNS
         :at_auction
       elsif awaiting_payment?
         :awaiting_payment
-      elsif disputed?
-        :disputed
       end
     end
 

app/models/domain.rb

@@ -284,21 +284,22 @@ class Domain < ApplicationRecord
   def server_holdable?
     return false if statuses.include?(DomainStatus::SERVER_HOLD)
     return false if statuses.include?(DomainStatus::SERVER_MANUAL_INZONE)
+
     true
   end
 
   def renewable?
-    if Setting.days_to_renew_domain_before_expire != 0
+    blocking_statuses = [DomainStatus::DELETE_CANDIDATE, DomainStatus::PENDING_RENEW,
+                         DomainStatus::PENDING_TRANSFER, DomainStatus::DISPUTED,
+                         DomainStatus::PENDING_UPDATE, DomainStatus::PENDING_DELETE,
+                         DomainStatus::PENDING_DELETE_CONFIRMATION]
+    return false if statuses.include_any? blocking_statuses
+    return true unless Setting.days_to_renew_domain_before_expire != 0
+
     # if you can renew domain at days_to_renew before domain expiration
     if (expire_time.to_date - Time.zone.today) + 1 > Setting.days_to_renew_domain_before_expire
       return false
     end
-    end
-
-    return false if statuses.include_any?(DomainStatus::DELETE_CANDIDATE, DomainStatus::PENDING_RENEW,
-                                          DomainStatus::PENDING_TRANSFER, DomainStatus::PENDING_DELETE,
-                                          DomainStatus::PENDING_UPDATE, DomainStatus::PENDING_DELETE_CONFIRMATION,
-                                          DomainStatus::DISPUTED)
 
     true
   end

app/models/domain_status.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 class DomainStatus < ApplicationRecord
   include EppErrors
   belongs_to :domain
@@ -80,18 +82,18 @@ class DomainStatus < ApplicationRecord
     PENDING_UPDATE, SERVER_MANUAL_INZONE, SERVER_REGISTRANT_CHANGE_PROHIBITED,
     SERVER_ADMIN_CHANGE_PROHIBITED, SERVER_TECH_CHANGE_PROHIBITED, FORCE_DELETE,
     DELETE_CANDIDATE, EXPIRED, DISPUTED
-  ]
+  ].freeze
 
   CLIENT_STATUSES = [
     CLIENT_DELETE_PROHIBITED, CLIENT_HOLD, CLIENT_RENEW_PROHIBITED, CLIENT_TRANSFER_PROHIBITED,
     CLIENT_UPDATE_PROHIBITED
-  ]
+  ].freeze
 
   SERVER_STATUSES = [
     SERVER_DELETE_PROHIBITED, SERVER_HOLD, SERVER_RENEW_PROHIBITED, SERVER_TRANSFER_PROHIBITED,
     SERVER_UPDATE_PROHIBITED, SERVER_MANUAL_INZONE, SERVER_REGISTRANT_CHANGE_PROHIBITED,
     SERVER_ADMIN_CHANGE_PROHIBITED, SERVER_TECH_CHANGE_PROHIBITED
-  ]
+  ].freeze
 
   UPDATE_PROHIBIT_STATES = [
       DomainStatus::PENDING_DELETE_CONFIRMATION,

app/models/epp/domain.rb

@@ -491,10 +491,12 @@ class Epp::Domain < Domain
       end
     end
 
+    unverified_registrant_params = frame.css('registrant').present? &&
+                                   frame.css('registrant').attr('verified').to_s.downcase != 'yes'
+
     if !same_registrant_as_current && errors.empty? && verify &&
        Setting.request_confrimation_on_registrant_change_enabled &&
-       frame.css('registrant').present? &&
+       unverified_registrant_params
-       frame.css('registrant').attr('verified').to_s.downcase != 'yes'
       registrant_verification_asked!(frame.to_s, current_user.id) unless disputed?
     end
 

test/jobs/dispute_status_update_job_test.rb

@@ -20,7 +20,7 @@ class DisputeStatusUpdateJobTest < ActiveSupport::TestCase
     assert_includes whois_record.json['status'], 'disputed'
   end
 
-  def test_unregistered_domain_whois_data_is_deleted
+  def test_on_expiry_unregistered_domain_is_sent_to_auction
     dispute = disputes(:active)
     dispute.update!(starts_at: Time.zone.today - 3.years - 1.day)
 
@@ -30,7 +30,7 @@ class DisputeStatusUpdateJobTest < ActiveSupport::TestCase
     assert dispute.closed
 
     whois_record = Whois::Record.find_by(name: dispute.domain_name)
-    assert whois_record.nil?
+    assert_equal ['AtAuction'], whois_record.json['status']
   end
 
   def test_registered_domain_whois_data_is_added