Merge pull request #1761 from internetee/1760-move-domain-update-confirm-to-interactor

Move DomainUpdateConfirm to Interactor

.codeclimate.yml

@@ -20,6 +20,9 @@ plugins:
     channel: eslint-5
   fixme:
     enabled: true
+    checks:
+      TODO:
+        enabled: false
   rubocop:
     enabled: true
     channel: rubocop-0-74

app/interactions/domains/update_confirm/base.rb

@@ -0,0 +1,53 @@
+module Domains
+  module UpdateConfirm
+    class Base < ActiveInteraction::Base
+      object :domain,
+             class: Domain,
+             description: 'Domain to confirm update'
+      string :action
+      string :initiator,
+             default: nil
+
+      validates :domain, :action, presence: true
+      validates :action, inclusion: { in: [RegistrantVerification::CONFIRMED,
+                                           RegistrantVerification::REJECTED] }
+
+      def raise_errors!(domain)
+        return unless domain.errors.any?
+
+        message = "domain #{domain.name} failed with errors #{domain.errors.full_messages}"
+        throw message
+      end
+
+      def notify_registrar(message_key)
+        domain.registrar.notifications.create!(
+          text: "#{I18n.t(message_key)}: #{domain.name}",
+          attached_obj_id: domain.id,
+          attached_obj_type: domain.class.to_s
+        )
+      end
+
+      def preclean_pendings
+        domain.registrant_verification_token = nil
+        domain.registrant_verification_asked_at = nil
+      end
+
+      def clean_pendings!
+        domain.is_admin = true
+        domain.registrant_verification_token = nil
+        domain.registrant_verification_asked_at = nil
+        domain.pending_json = {}
+        clear_statuses
+        domain.save
+      end
+
+      def clear_statuses
+        domain.statuses.delete(DomainStatus::PENDING_DELETE_CONFIRMATION)
+        domain.statuses.delete(DomainStatus::PENDING_UPDATE)
+        domain.statuses.delete(DomainStatus::PENDING_DELETE)
+        domain.status_notes[DomainStatus::PENDING_UPDATE] = ''
+        domain.status_notes[DomainStatus::PENDING_DELETE] = ''
+      end
+    end
+  end
+end

app/interactions/domains/update_confirm/process_action.rb

@@ -0,0 +1,17 @@
+module Domains
+  module UpdateConfirm
+    class ProcessAction < Base
+      def execute
+        ::PaperTrail.request.whodunnit = "interaction - #{self.class.name} - #{action} by"\
+          " #{initiator}"
+
+        case action
+        when RegistrantVerification::CONFIRMED
+          compose(ProcessUpdateConfirmed, inputs)
+        when RegistrantVerification::REJECTED
+          compose(ProcessUpdateRejected, inputs)
+        end
+      end
+    end
+  end
+end

app/interactions/domains/update_confirm/process_update_confirmed.rb

@@ -0,0 +1,34 @@
+module Domains
+  module UpdateConfirm
+    class ProcessUpdateConfirmed < Base
+      def execute
+        ActiveRecord::Base.transaction do
+          old_registrant = domain.registrant
+          notify_registrar(:poll_pending_update_confirmed_by_registrant)
+
+          apply_pending_update!
+          raise_errors!(domain)
+          RegistrantChange.new(domain: domain, old_registrant: old_registrant).confirm
+        end
+      end
+
+      def apply_pending_update!
+        preclean_pendings
+        update_domain
+        clean_pendings!
+
+        WhoisRecord.find_by(domain_id: domain.id).save # need to reload model
+      end
+
+      # rubocop:disable Metrics/AbcSize
+      def update_domain
+        user = ApiUser.find(domain.pending_json['current_user_id'])
+        frame = Nokogiri::XML(domain.pending_json['frame'])
+        domain.upid = user.registrar.id if user.registrar
+        domain.up_date = Time.zone.now
+        domain.update(frame, user, false)
+      end
+      # rubocop:enable Metrics/AbcSize
+    end
+  end
+end

app/interactions/domains/update_confirm/process_update_rejected.rb

@@ -0,0 +1,18 @@
+module Domains
+  module UpdateConfirm
+    class ProcessUpdateRejected < Base
+      def execute
+        ActiveRecord::Base.transaction do
+          RegistrantChangeMailer.rejected(domain: domain,
+                                          registrar: domain.registrar,
+                                          registrant: domain.registrant).deliver_now
+
+          notify_registrar(:poll_pending_update_rejected_by_registrant)
+
+          preclean_pendings
+          clean_pendings!
+        end
+      end
+    end
+  end
+end

app/jobs/application_job.rb

@@ -0,0 +1,2 @@
+class ApplicationJob < ActiveJob::Base
+end

app/jobs/domain_update_confirm_job.rb

@@ -1,37 +1,10 @@
-class DomainUpdateConfirmJob < Que::Job
+class DomainUpdateConfirmJob < ApplicationJob
-  def run(domain_id, action, initiator = nil)
+  queue_as :default
-    ::PaperTrail.request.whodunnit = "job - #{self.class.name} - #{action} by #{initiator}"
-    # it's recommended to keep transaction against job table as short as possible.
-    ActiveRecord::Base.transaction do
-      domain = Epp::Domain.find(domain_id)
-      domain.is_admin = true
-      case action
-      when RegistrantVerification::CONFIRMED
-        old_registrant = domain.registrant
-        domain.notify_registrar(:poll_pending_update_confirmed_by_registrant)
-        raise_errors!(domain)
 
-        domain.apply_pending_update!
+  def perform(domain_id, action, initiator = nil)
-        raise_errors!(domain)
+    domain = Epp::Domain.find(domain_id)
-
+    Domains::UpdateConfirm::ProcessAction.run(domain: domain,
-        domain.clean_pendings!
+                                              action: action,
-        raise_errors!(domain)
+                                              initiator: initiator)
-        RegistrantChange.new(domain: domain, old_registrant: old_registrant).confirm
-      when RegistrantVerification::REJECTED
-        RegistrantChangeMailer.rejected(domain: domain,
-                                        registrar: domain.registrar,
-                                        registrant: domain.registrant).deliver_now
-
-        domain.notify_registrar(:poll_pending_update_rejected_by_registrant)
-
-        domain.preclean_pendings
-        domain.clean_pendings!
-      end
-      destroy # it's best to destroy the job in the same transaction
-    end
-  end
-
-  def raise_errors!(domain)
-    throw "domain #{domain.name} failed with errors #{domain.errors.full_messages}" if domain.errors.any?
   end
 end

app/models/domain.rb

@@ -327,6 +327,7 @@ class Domain < ApplicationRecord
   end
 
   def notify_registrar(message_key)
+    # TODO: To be deleted with DomainDeleteConfirm refactoring
     registrar.notifications.create!(
       text: "#{I18n.t(message_key)}: #{name}",
       attached_obj_id: id,
@@ -335,11 +336,13 @@ class Domain < ApplicationRecord
   end
 
   def preclean_pendings
+    # TODO: To be deleted with refactoring
     self.registrant_verification_token = nil
     self.registrant_verification_asked_at = nil
   end
 
   def clean_pendings!
+    # TODO: To be deleted with refactoring
     preclean_pendings
     self.pending_json = {}
     statuses.delete(DomainStatus::PENDING_DELETE_CONFIRMATION)

app/models/epp/domain.rb

@@ -508,25 +508,6 @@ class Epp::Domain < Domain
     errors.empty? && super(at)
   end
 
-  def apply_pending_update!
-    preclean_pendings
-    user  = ApiUser.find(pending_json['current_user_id'])
-    frame = Nokogiri::XML(pending_json['frame'])
-
-    self.statuses.delete(DomainStatus::PENDING_UPDATE)
-    self.upid = user.registrar.id if user.registrar
-    self.up_date = Time.zone.now
-
-    return unless update(frame, user, false)
-    clean_pendings!
-
-    save!
-
-    WhoisRecord.find_by(domain_id: id).save # need to reload model
-
-    true
-  end
-
   def apply_pending_delete!
     preclean_pendings
     statuses.delete(DomainStatus::PENDING_DELETE_CONFIRMATION)

app/models/registrant_verification.rb

@@ -18,13 +18,13 @@ class RegistrantVerification < ApplicationRecord
   def domain_registrant_change_confirm!(initiator)
     self.action_type = DOMAIN_REGISTRANT_CHANGE
     self.action = CONFIRMED
-    DomainUpdateConfirmJob.enqueue domain.id, CONFIRMED, initiator if save
+    DomainUpdateConfirmJob.perform_later domain.id, CONFIRMED, initiator if save
   end
 
   def domain_registrant_change_reject!(initiator)
     self.action_type = DOMAIN_REGISTRANT_CHANGE
     self.action = REJECTED
-    DomainUpdateConfirmJob.run domain.id, REJECTED, initiator if save
+    DomainUpdateConfirmJob.perform_later domain.id, REJECTED, initiator if save
   end
 
   def domain_registrant_delete_confirm!(initiator)

test/jobs/domain_update_confirm_job_test.rb

@@ -20,7 +20,7 @@ class DomainUpdateConfirmJobTest < ActiveSupport::TestCase
   end
 
   def test_rejected_registrant_verification_notifies_registrar
-    DomainUpdateConfirmJob.enqueue(@domain.id, RegistrantVerification::REJECTED)
+    DomainUpdateConfirmJob.perform_now(@domain.id, RegistrantVerification::REJECTED)
 
     last_registrar_notification = @domain.registrar.notifications.last
     assert_equal(last_registrar_notification.attached_obj_id, @domain.id)
@@ -28,7 +28,7 @@ class DomainUpdateConfirmJobTest < ActiveSupport::TestCase
   end
 
   def test_accepted_registrant_verification_notifies_registrar
-    DomainUpdateConfirmJob.enqueue(@domain.id, RegistrantVerification::CONFIRMED)
+    DomainUpdateConfirmJob.perform_now(@domain.id, RegistrantVerification::CONFIRMED)
 
     last_registrar_notification = @domain.registrar.notifications.last
     assert_equal(last_registrar_notification.attached_obj_id, @domain.id)
@@ -44,10 +44,11 @@ class DomainUpdateConfirmJobTest < ActiveSupport::TestCase
     @domain.update(pending_json: @domain.pending_json)
 
     @domain.reload
-    DomainUpdateConfirmJob.enqueue(@domain.id, RegistrantVerification::CONFIRMED)
+    DomainUpdateConfirmJob.perform_now(@domain.id, RegistrantVerification::CONFIRMED)
     @domain.reload
 
     assert_equal @domain.registrant.code, @new_registrant.code
+    assert @domain.statuses.include? DomainStatus::OK
   end
 
   def test_clears_pending_update_after_denial
@@ -58,10 +59,84 @@ class DomainUpdateConfirmJobTest < ActiveSupport::TestCase
     @domain.pending_json['frame'] = epp_xml
     @domain.update(pending_json: @domain.pending_json)
 
-    DomainUpdateConfirmJob.enqueue(@domain.id, RegistrantVerification::REJECTED)
+    DomainUpdateConfirmJob.perform_now(@domain.id, RegistrantVerification::REJECTED)
     @domain.reload
 
     assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE_CONFIRMATION
     assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE
   end
+
+  def test_protects_statuses_after_denial
+    epp_xml = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n<epp>\n  <command>\n    <update>\n      <update>\n        <name>#{@domain.name}</name>\n" \
+    "        <chg>\n          <registrant>#{@new_registrant.code}</registrant>\n        </chg>\n      </update>\n    </update>\n    <extension>\n      <update/>\n" \
+    "      <extdata>\n        <legalDocument type=\"pdf\">#{@legal_doc_path}</legalDocument>\n      </extdata>\n" \
+    "    </extension>\n    <clTRID>20alla-1594199756</clTRID>\n  </command>\n</epp>\n"
+    @domain.pending_json['frame'] = epp_xml
+    @domain.update(pending_json: @domain.pending_json)
+    @domain.update(statuses: [DomainStatus::DELETE_CANDIDATE, DomainStatus::DISPUTED])
+
+    DomainUpdateConfirmJob.perform_now(@domain.id, RegistrantVerification::REJECTED)
+    @domain.reload
+
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE_CONFIRMATION
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE
+    assert @domain.statuses.include? DomainStatus::DELETE_CANDIDATE
+    assert @domain.statuses.include? DomainStatus::DISPUTED
+  end
+
+  def test_protects_statuses_after_confirm
+    epp_xml = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n<epp>\n  <command>\n    <update>\n      <update>\n        <name>#{@domain.name}</name>\n" \
+    "        <chg>\n          <registrant>#{@new_registrant.code}</registrant>\n        </chg>\n      </update>\n    </update>\n    <extension>\n      <update/>\n" \
+    "      <extdata>\n        <legalDocument type=\"pdf\">#{@legal_doc_path}</legalDocument>\n      </extdata>\n" \
+    "    </extension>\n    <clTRID>20alla-1594199756</clTRID>\n  </command>\n</epp>\n"
+    @domain.pending_json['frame'] = epp_xml
+    @domain.update(pending_json: @domain.pending_json)
+    @domain.update(statuses: [DomainStatus::DELETE_CANDIDATE, DomainStatus::DISPUTED])
+
+    DomainUpdateConfirmJob.perform_now(@domain.id, RegistrantVerification::CONFIRMED)
+    @domain.reload
+
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE_CONFIRMATION
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE
+    assert @domain.statuses.include? DomainStatus::DELETE_CANDIDATE
+    assert @domain.statuses.include? DomainStatus::DISPUTED
+  end
+
+  def test_clears_pending_update_and_inactive_after_denial
+    epp_xml = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n<epp>\n  <command>\n    <update>\n      <update>\n        <name>#{@domain.name}</name>\n" \
+    "        <chg>\n          <registrant>#{@new_registrant.code}</registrant>\n        </chg>\n      </update>\n    </update>\n    <extension>\n      <update/>\n" \
+    "      <extdata>\n        <legalDocument type=\"pdf\">#{@legal_doc_path}</legalDocument>\n      </extdata>\n" \
+    "    </extension>\n    <clTRID>20alla-1594199756</clTRID>\n  </command>\n</epp>\n"
+    @domain.pending_json['frame'] = epp_xml
+    @domain.update(pending_json: @domain.pending_json)
+    @domain.update(statuses: [DomainStatus::PENDING_UPDATE])
+    @domain.nameservers.destroy_all
+    @domain.reload
+
+    DomainUpdateConfirmJob.perform_now(@domain.id, RegistrantVerification::REJECTED)
+    @domain.reload
+
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE_CONFIRMATION
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE
+    assert_not @domain.statuses.include? DomainStatus::PENDING_UPDATE
+    assert @domain.statuses.include? DomainStatus::INACTIVE
+  end
+
+  def test_clears_pending_update_and_sets_ok_after_denial
+    epp_xml = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>\n<epp>\n  <command>\n    <update>\n      <update>\n        <name>#{@domain.name}</name>\n" \
+    "        <chg>\n          <registrant>#{@new_registrant.code}</registrant>\n        </chg>\n      </update>\n    </update>\n    <extension>\n      <update/>\n" \
+    "      <extdata>\n        <legalDocument type=\"pdf\">#{@legal_doc_path}</legalDocument>\n      </extdata>\n" \
+    "    </extension>\n    <clTRID>20alla-1594199756</clTRID>\n  </command>\n</epp>\n"
+    @domain.pending_json['frame'] = epp_xml
+    @domain.update(pending_json: @domain.pending_json)
+    @domain.update(statuses: [DomainStatus::OK, DomainStatus::PENDING_UPDATE])
+
+    DomainUpdateConfirmJob.perform_now(@domain.id, RegistrantVerification::REJECTED)
+    @domain.reload
+
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE_CONFIRMATION
+    assert_not @domain.statuses.include? DomainStatus::PENDING_DELETE
+    assert_not @domain.statuses.include? DomainStatus::PENDING_UPDATE
+    assert @domain.statuses.include? DomainStatus::OK
+  end
 end