Improve DomainUpdate logic

app/controllers/epp/domains_controller.rb

@@ -37,13 +37,17 @@ module Epp
     end
 
     def update
-      authorize! :update, @domain, @password
+      authorize!(:update, @domain, @password)
 
-      updated = @domain.update(params[:parsed_frame], current_user)
-      (handle_errors(@domain) && return) unless updated
-
-      pending = @domain.epp_pending_update.present?
-      render_epp_response "/epp/domains/success#{'_pending' if pending}"
+      registrar_id = current_user.registrar.id
+      update_params = ::Deserializers::Xml::DomainUpdate.new(params[:parsed_frame], registrar_id).call
+      action = Actions::DomainUpdate.new(@domain, update_params, false)
+      if action.call
+        pending = @domain.epp_pending_update.present?
+        render_epp_response("/epp/domains/success#{'_pending' if pending}")
+      else
+        handle_errors(@domain)
+      end
     end
 
     def delete

app/models/actions/domain_update.rb

@@ -1,26 +1,71 @@
 module Actions
-  class DomainCreate
-    attr_reader :domain, :params
+  class DomainUpdate
+    attr_reader :domain, :params, :bypass_verify
 
-    def initialize(domain, params)
+    def initialize(domain, params, bypass_verify)
       @domain = domain
       @params = params
+      @bypass_verify = bypass_verify
     end
 
     def call
+      @changes_registrant = false
+
       validate_domain_integrity
-      verify_registrant_change # Should be last step
-      assign_admin_contact_changes if params[:contacts]
-      assign_tech_contact_changes if params[:contacts]
+      assign_new_registrant
+      assign_nameserver_modifications
+      assign_admin_contact_changes
+      assign_tech_contact_changes
+      assign_requested_statuses
+
+      commit
     end
 
     def validate_domain_integrity
+      domain.auth_info = params[:auth_info] if params[:auth_info]
+
       return unless domain.discarded?
 
       domain.add_epp_error('2304', nil, nil, 'Object status prohibits operation')
     end
 
+    def assign_new_registrant
+      return unless params[:registrant]
+
+      domain.add_epp_error('2306', nil, nil, %i[registrant cannot_be_missing]) unless params[:registrant][:code]
+      regt = Registrant.find_by(code: params[:registrant][:code])
+      if regt.present?
+        return if domain.registrant == regt
+
+        @changes_registrant = true if domain.registrant.ident != regt.ident
+        domain.registrant = regt
+      else
+        domain.add_epp_error('2303', 'registrant', params[:registrant_id], %i[registrant not_found])
+      end
+    end
+
+    def assign_nameserver_modifications
+      return unless params[:nameservers]
+
+      nameservers = []
+      params[:nameservers].select { |ns| ns[:action] == 'rem' }.each do |ns_attr|
+        ns = domain.nameservers.find_by_hash_params(ns_attr.except(:action)).first
+        domain.add_epp_error('2303', 'hostAttr', ns_attr[:hostname], %i[nameservers not_found]) and break unless ns
+        nameservers << { id: ns.id, _destroy: 1 }
+      end
+
+      params[:nameservers].select { |ns| ns[:action] == 'add' }.each do |ns_attr|
+        nameservers << ns_attr.except(:action)
+      end
+
+      return unless nameservers.present?
+
+      domain.nameservers_attributes = nameservers
+    end
+
     def assign_admin_contact_changes
+      return unless params[:contacts]
+
       props = []
       contacts = params[:contacts].select { |c| c[:type] == 'admin' }
 
@@ -31,138 +76,112 @@ module Actions
 
       contacts.select { |c| c[:action] == 'rem' }.each do |c|
         dc = domain.admin_domain_contacts.find_by(contact_code_cache: c[:code])
-        domain.add_epp_error('2303', 'contact', at[:contact_code_cache], [:domain_contacts, :not_found]) and break unless dc
-
-        props << { id: dc.id, _destroy: 1 }
+        if dc.present?
+          props << { id: dc.id, _destroy: 1 }
+        else
+          domain.add_epp_error('2303', 'contact', c[:code], %i[domain_contacts not_found])
+        end
       end
 
       contacts.select { |c| c[:action] == 'add' }.each do |c|
         contact = Epp::Contact.find_by_epp_code(c[:code])
-        domain.add_epp_error('2303', 'contact', c[:code], [:domain_contacts, :not_found]) and break unless contact
-        domain.add_epp_error('2306', 'contact', c[:code], [:domain_contacts, :admin_contact_can_be_only_private_person]) and break if contact.org?
-
-        props << { id: contact.id, code: contact.code }
+        if contact.present?
+          if contact.org?
+            domain.add_epp_error('2306', 'contact', c[:code], %i[domain_contacts admin_contact_can_be_only_private_person])
+          else
+            props << { contact_id: contact.id, contact_code_cache: contact.code }
+          end
+        else
+          domain.add_epp_error('2303', 'contact', c[:code], %i[domain_contacts not_found])
+        end
       end
 
+      return unless props.present?
+
       domain.admin_domain_contacts_attributes = props
     end
 
     def assign_tech_contact_changes
-      props = []
-      contacts = params[:contacts].select { |c| c[:type] == 'admin' }
+      return unless params[:contacts]
 
-      if contacts.present? && domain.admin_change_prohibited?
-        domain.add_epp_error('2304', 'admin', DomainStatus::SERVER_ADMIN_CHANGE_PROHIBITED, I18n.t(:object_status_prohibits_operation))
+      props = []
+      contacts = params[:contacts].select { |c| c[:type] == 'tech' }
+
+      if contacts.present? && domain.tech_change_prohibited?
+        domain.add_epp_error('2304', 'tech', DomainStatus::SERVER_TECH_CHANGE_PROHIBITED, I18n.t(:object_status_prohibits_operation))
         return
       end
 
       contacts.select { |c| c[:action] == 'rem' }.each do |c|
-        dc = domain.admin_domain_contacts.find_by(contact_code_cache: c[:code])
-        domain.add_epp_error('2303', 'contact', at[:contact_code_cache], [:domain_contacts, :not_found]) and break unless dc
-
-        props << { id: dc.id, _destroy: 1 }
+        dc = domain.tech_domain_contacts.find_by(contact_code_cache: c[:code])
+        if dc.present?
+          props << { id: dc.id, _destroy: 1 }
+        else
+          domain.add_epp_error('2303', 'contact', c[:code], %i[domain_contacts not_found])
+        end
       end
 
       contacts.select { |c| c[:action] == 'add' }.each do |c|
         contact = Epp::Contact.find_by_epp_code(c[:code])
-        domain.add_epp_error('2303', 'contact', c[:code], [:domain_contacts, :not_found]) and break unless contact
-        domain.add_epp_error('2306', 'contact', c[:code], [:domain_contacts, :admin_contact_can_be_only_private_person]) and break if contact.org?
-
-        props << { id: contact.id, code: contact.code }
+        if contact.present?
+          props << { contact_id: contact.id, contact_code_cache: contact.code }
+        else
+          domain.add_epp_error('2303', 'contact', c[:code], %i[domain_contacts not_found])
+        end
       end
 
-      domain.admin_domain_contacts_attributes = props
+      return unless props.present?
+
+      domain.tech_domain_contacts_attributes = props
     end
 
-    def verify_registrant_change
-      return if !params[:registrant] || domain.registrant.code == params[:registrant][:code]
+    def assign_requested_statuses
+      return unless params[:statuses]
 
-      if domain.disputed?
-        domain.add_epp_error('2304', nil, nil, 'Required parameter missing; reservedpw element required for dispute domains') and return unless params[:reserved_pw]
-        dispute = Dispute.active.find_by(domain_name: name, password: params[:reserved_pw])
-        domain.add_epp_error('2202', nil, nil, 'Invalid authorization information; invalid reserved>pw value') and return unless dispute
-        Dispute.close_by_domain(name)
+      rem = []
+      add = []
+
+      invalid = false
+      params[:statuses].each do |s|
+        unless DomainStatus::CLIENT_STATUSES.include?(s[:status])
+          domain.add_epp_error('2303', 'status', s[:status], %i[domain_statuses not_found])
+          invalid = true
+        end
       end
 
-      return unless params[:registrant][:verified] && Setting.request_confirmation_on_registrant_change_enabled
-
-      domain.registrant_verification_asked!(frame.to_s, current_user.id)
-    end
-
-    def assign_registrant
-      domain.add_epp_error('2306', nil, nil, %i[registrant cannot_be_missing]) and return unless params[:registrant_id]
-
-      regt = Registrant.find_by(code: params[:registrant_id])
-      domain.add_epp_error('2303', 'registrant', params[:registrant_id], %i[registrant not_found]) and return unless regt
-
-      domain.registrant = regt
-    end
-
-    def assign_domain_attributes
-      domain.name = params[:name].strip.downcase
-      domain.registrar = Registrar.find(params[:registrar_id])
-      domain.period = params[:period]
-      domain.period_unit = params[:period_unit]
-      domain.transfer_code = params[:transfer_code] if params[:transfer_code].present?
-      domain.reserved_pw = params[:reserved_pw] if params[:reserved_pw].present?
-      domain.dnskeys_attributes = params[:dnskeys_attributes]
-    end
-
-    def assign_nameservers
-      domain.nameservers_attributes = params[:nameservers_attributes]
-    end
-
-    def assign_admin_contacts
-      attrs = []
-      params[:admin_domain_contacts_attributes].each do |c|
-        contact = Contact.find_by(code: c)
-        domain.add_epp_error('2303', 'contact', c, %i[domain_contacts not_found]) unless contact.present?
-        attrs << { contact_id: contact.id, contact_code_cache: contact.code } if contact
+      params[:statuses].select { |s| s[:action] == 'rem' }.each do |s|
+        if domain.statuses.include?(s[:status])
+          rem << s[:status]
+        else
+          domain.add_epp_error('2303', 'status', s[:status], %i[domain_statuses not_found])
+          invalid = true
+        end
       end
 
-      domain.admin_domain_contacts_attributes = attrs
+      params[:statuses].select { |s| s[:action] == 'add' }.each { |s| add << s[:status] }
+      return if invalid
+
+      domain.statuses = domain.statuses - rem + add
     end
 
-    def assign_tech_contacts
-      attrs = []
-      params[:tech_domain_contacts_attributes].each do |c|
-        contact = Contact.find_by(code: c)
-        domain.add_epp_error('2303', 'contact', c, %i[domain_contacts not_found]) unless contact.present?
-        attrs << { contact_id: contact.id, contact_code_cache: contact.code } if contact
+    def verify_registrant_change?
+      return false unless @changes_registrant
+      return false if params[:registrant][:verified] == true
+      return true unless domain.disputed?
+
+      if params[:reserved_pw]
+        dispute = Dispute.active.find_by(domain_name: domain.name, password: params[:reserved_pw])
+        if dispute
+          Dispute.close_by_domain(domain.name)
+          return false
+        else
+          domain.add_epp_error('2202', nil, nil, 'Invalid authorization information; invalid reserved>pw value')
+        end
+      else
+        domain.add_epp_error('2304', nil, nil, 'Required parameter missing; reservedpw element required for dispute domains')
       end
 
-      domain.tech_domain_contacts_attributes = attrs
-    end
-
-    def assign_expiry_time
-      period = domain.period.to_i
-      plural_period_unit_name = (domain.period_unit == 'm' ? 'months' : 'years').to_sym
-      expire_time = (Time.zone.now.advance(plural_period_unit_name => period) + 1.day).beginning_of_day
-      domain.expire_time = expire_time
-    end
-
-    def debit_registrar
-      @domain_pricelist ||= domain.pricelist('create', domain.period.try(:to_i), domain.period_unit)
-      if @domain_pricelist.try(:price) && domain.registrar.balance < @domain_pricelist.price.amount
-        domain.add_epp_error(2104, nil, nil, I18n.t('billing_failure_credit_balance_low'))
-        return
-      elsif !@domain_pricelist.try(:price)
-        domain.add_epp_error(2104, nil, nil, I18n.t(:active_price_missing_for_this_operation))
-        return
-      end
-
-      domain.registrar.debit!({ sum: @domain_pricelist.price.amount, price: @domain_pricelist,
-                                description: "#{I18n.t('create')} #{domain.name}",
-                                activity_type: AccountActivity::CREATE })
-    end
-
-    def process_auction_and_disputes
-      dn = DNS::DomainName.new(SimpleIDN.to_unicode(params[:name]))
-      Dispute.close_by_domain(domain.name)
-      return unless Domain.release_to_auction && dn.pending_registration?
-
-      auction = Auction.find_by(domain: domain.name, status: Auction.statuses[:payment_received])
-      auction.domain_registered!
+      true
     end
 
     def maybe_attach_legal_doc
@@ -178,16 +197,18 @@ module Actions
     end
 
     def commit
-      unless domain.valid?
-        domain.errors.delete(:name_dirty) if domain.errors[:puny_label].any?
-        return false if domain.errors.any?
+      return false if domain.errors[:epp_errors].any?
+      return false unless domain.valid?
+
+      if verify_registrant_change? && Setting.request_confirmation_on_registrant_change_enabled
+        return if bypass_verify
+
+        domain.registrant_verification_asked!(params.to_s, params[:registrar_id])
       end
-      # @domain.add_legal_file_to_new(params[:parsed_frame])
-      debit_registrar
 
-      return false if domain.errors.any?
+      return false if domain.errors[:epp_errors].any?
+      return false unless domain.valid?
 
-      process_auction_and_disputes
       domain.save
     end
   end

app/models/epp/domain.rb

@@ -1,6 +1,7 @@
 require 'deserializers/xml/legal_document'
 require 'deserializers/xml/nameserver'
 require 'deserializers/xml/domain_create'
+require 'deserializers/xml/domain_update'
 class Epp::Domain < Domain
   include EppErrors
 
@@ -116,6 +117,7 @@ class Epp::Domain < Domain
   def attrs_from(frame, current_user, action = nil)
     at = {}.with_indifferent_access
 
+    # KORRAS
     registrant_frame = frame.css('registrant').first
     code = registrant_frame.try(:text)
     if code.present?
@@ -131,7 +133,7 @@ class Epp::Domain < Domain
     else
       add_epp_error('2306', nil, nil, [:registrant, :cannot_be_missing])
     end if registrant_frame
-
+   # KORRAS
 
     at[:name] = frame.css('name').text if new_record? # Done
     at[:registrar_id] = current_user.registrar.try(:id) # Done
@@ -434,7 +436,7 @@ class Epp::Domain < Domain
     at[:tech_domain_contacts_attributes] += at_add[:tech_domain_contacts_attributes]
     at[:dnskeys_attributes] += at_add[:dnskeys_attributes]
     at[:statuses] =
-      statuses - domain_statuses_attrs(frame.css('rem'), 'rem') + domain_statuses_attrs(frame.css('add'), 'add')
+statuses - domain_statuses_attrs(frame.css('rem'), 'rem') + domain_statuses_attrs(frame.css('add'), 'add')
 
     if errors.empty? && verify
       self.upid = current_user.registrar.id if current_user.registrar

lib/deserializers/xml/domain_update.rb

@@ -14,7 +14,8 @@ module Deserializers
 
       def call
         obj = { domain: frame.css('name')&.text, registrant: registrant, contacts: contacts,
-                auth_info: if_present('pw'), nameservers: nameservers, dns_keys: dns_keys }
+                auth_info: if_present('authInfo > pw'), nameservers: nameservers, dns_keys: dns_keys,
+                registrar_id: registrar, statuses: statuses, reserved_pw: if_present('reserved > pw') }
 
         obj.reject { |_key, val| val.blank? }
       end
@@ -41,14 +42,14 @@ module Deserializers
       def nameservers
         nameservers = []
         frame.css('add > ns > hostAttr').each do |ns|
-          nsrv = { nameserver: ns.css('hostName').text, host_addr: [], action: 'add' }
-          ns.css('hostAddr').each { |ha| nsrv[:host_addr] << { proto: ha.attr('ip').to_s.downcase, addr: ha.text } }
+          nsrv = Deserializers::Xml::Nameserver.new(ns).call
+          nsrv[:action] = 'add'
           nameservers << nsrv
         end
 
         frame.css('rem > ns > hostAttr').each do |ns|
-          nsrv = { nameserver: ns.css('hostName').text, host_addr: [], action: 'rem' }
-          ns.css('hostAddr').each { |ha| nsrv[:host_addr] << { proto: ha.attr('ip').to_s.downcase, addr: ha.text } }
+          nsrv = Deserializers::Xml::Nameserver.new(ns).call
+          nsrv[:action] = 'rem'
           nameservers << nsrv
         end
 
@@ -66,6 +67,17 @@ module Deserializers
         added + removed
       end
 
+      def statuses
+        return unless frame.css('status').present?
+
+        statuses = []
+
+        frame.css('add > status').each { |entry| statuses << { status: entry.attr('s').to_s, action: 'add' } }
+        frame.css('rem > status').each { |entry| statuses << { status: entry.attr('s').to_s, action: 'rem' } }
+
+        statuses
+      end
+
       def if_present(css_path)
         return if frame.css(css_path).blank?