Improve DomainCreate wrapper for EPP

app/controllers/epp/domains_controller.rb

@@ -26,82 +26,14 @@ module Epp
     end
 
     def create
-      authorize! :create, Epp::Domain
+      authorize!(:create, Epp::Domain)
 
-      domain_params = ::Deserializers::Xml::DomainCreate.new(params[:parsed_frame], current_user.registrar.id)
+      registrar_id = current_user.registrar.id
-      puts "Ayy lmao"
+      @domain = Epp::Domain.new
-      puts domain_params.call
+      data = ::Deserializers::Xml::DomainCreate.new(params[:parsed_frame], registrar_id).call
+      action = Actions::DomainCreate.new(@domain, data)
 
-      if Domain.release_to_auction
+      action.call ? render_epp_response('/epp/domains/create') : handle_errors(@domain)
-        request_domain_name = params[:parsed_frame].css('name').text.strip.downcase
-        domain_name = DNS::DomainName.new(SimpleIDN.to_unicode(request_domain_name))
-
-        if domain_name.at_auction?
-          epp_errors << {
-            code: '2306',
-            msg: 'Parameter value policy error: domain is at auction',
-          }
-          handle_errors
-          return
-        elsif domain_name.awaiting_payment?
-          epp_errors << {
-            code: '2003',
-            msg: 'Required parameter missing; reserved>pw element required for reserved domains',
-          }
-          handle_errors
-          return
-        elsif domain_name.pending_registration?
-          registration_code = params[:parsed_frame].css('reserved > pw').text
-
-          if registration_code.empty?
-            epp_errors << {
-              code: '2003',
-              msg: 'Required parameter missing; reserved>pw element is required',
-            }
-            handle_errors
-            return
-          end
-
-          unless domain_name.available_with_code?(registration_code)
-            epp_errors << {
-              code: '2202',
-              msg: 'Invalid authorization information; invalid reserved>pw value',
-            }
-            handle_errors
-            return
-          end
-        end
-      end
-
-      @domain = Epp::Domain.new_from_epp(params[:parsed_frame], current_user)
-      handle_errors(@domain) and return if @domain.errors.any?
-      @domain.valid?
-      @domain.errors.delete(:name_dirty) if @domain.errors[:puny_label].any?
-      handle_errors(@domain) and return if @domain.errors.any?
-      handle_errors and return unless balance_ok?('create') # loads pricelist in this method
-
-      ActiveRecord::Base.transaction do
-        @domain.add_legal_file_to_new(params[:parsed_frame])
-
-        if @domain.save # TODO: Maybe use validate: false here because we have already validated the domain?
-          current_user.registrar.debit!({
-                                          sum: @domain_pricelist.price.amount,
-                                          description: "#{I18n.t('create')} #{@domain.name}",
-                                          activity_type: AccountActivity::CREATE,
-                                          price: @domain_pricelist
-                                        })
-
-          if Domain.release_to_auction && domain_name.pending_registration?
-            active_auction = Auction.find_by(domain: domain_name.to_s,
-                                             status: Auction.statuses[:payment_received])
-            active_auction.domain_registered!
-          end
-          Dispute.close_by_domain(@domain.name)
-          render_epp_response '/epp/domains/create'
-        else
-          handle_errors(@domain)
-        end
-      end
     end
 
     def update

app/models/actions/domain_create.rb

@@ -8,48 +8,58 @@ module Actions
     end
 
     def call
-      validate_domain_integrity
-      assign_registrant
       assign_domain_attributes
+      validate_domain_integrity
+      return false if domain.errors[:epp_errors].any?
+
+      assign_registrant
       assign_nameservers
       assign_admin_contacts
       assign_tech_contacts
+      domain.attach_default_contacts
       assign_expiry_time
 
-      return domain unless domain.save
-
       commit
     end
 
     # Check if domain is eligible for new registration
     def validate_domain_integrity
-      return if Domain.release_to_auction
+      return unless Domain.release_to_auction
 
-      dn = DNS::DomainName.new(SimpleIDN.to_unicode(params[:name]))
+      dn = DNS::DomainName.new(SimpleIDN.to_unicode(params[:name].strip.downcase))
-      domain.add_epp_error(2306, nil, nil, 'Parameter value policy error: domain is at auction') if dn.at_auction?
+      if dn.at_auction?
-      domain.add_epp_error(2003, nil, nil, 'Required parameter missing; reserved>pw element required for reserved domains') if dn.awaiting_payment?
+        domain.add_epp_error('2306', nil, nil, 'Parameter value policy error: domain is at auction')
-      return unless dn.pending_registration?
+        return
-
+      elsif dn.awaiting_payment?
-      domain.add_epp_error(2003, nil, nil, 'Required parameter missing; reserved>pw element is required') if params[:reserved_pw].empty?
+        domain.add_epp_error('2003', nil, nil, 'Required parameter missing; reserved>pw element required for reserved domains')
-      domain.add_epp_errpr(2202, nil, nil, 'Invalid authorization information; invalid reserved>pw value') unless dn.available_with_code?(params[:reserved_pw])
+        return
+      elsif dn.pending_registration?
+        if params[:reserved_pw].blank?
+          domain.add_epp_error('2003', nil, nil, 'Required parameter missing; reserved>pw element is required')
+        else
+          unless dn.available_with_code?(params[:reserved_pw])
+            domain.add_epp_error('2202', nil, nil, 'Invalid authorization information; invalid reserved>pw value')
+          end
+        end
+      end
     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', code, %i[registrant not_found]) and return unless regt
+      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]
+      domain.name = params[:name].strip.downcase
       domain.registrar = Registrar.find(params[:registrar_id])
       domain.period = params[:period]
       domain.period_unit = params[:period_unit]
-      domain.reserved_pw = params[:reserved_pw] if params[:transfer_code]
+      domain.transfer_code = params[:transfer_code] if params[:transfer_code].present?
-      domain.transfer_code = params[:transfer_code] if params[:transfer_code]
+      domain.reserved_pw = params[:reserved_pw] if params[:reserved_pw].present?
       domain.dnskeys_attributes = params[:dnskeys_attributes]
     end
 
@@ -61,7 +71,7 @@ module Actions
       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
+        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
       end
 
@@ -72,7 +82,7 @@ module Actions
       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
+        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
       end
 
@@ -87,8 +97,8 @@ module Actions
     end
 
     def debit_registrar
-      domain_pricelist = domain.pricelist('create', domain.period.try(:to_i), domain.period_unit)
+      @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
+      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)
@@ -97,12 +107,12 @@ module Actions
       end
 
       domain.registrar.debit!({ sum: @domain_pricelist.price.amount, price: @domain_pricelist,
-                                description: "#{I18n.t('create')} #{@domain.name}",
+                                description: "#{I18n.t('create')} #{domain.name}",
                                 activity_type: AccountActivity::CREATE })
     end
 
     def process_auction_and_disputes
-      dn = DNS::DomainName.new(SimpleIDN.to_unicode(domain.name))
+      dn = DNS::DomainName.new(SimpleIDN.to_unicode(params[:name]))
       Dispute.close_by_domain(domain.name)
       return unless Domain.release_to_auction && dn.pending_registration?
 
@@ -123,12 +133,17 @@ module Actions
     end
 
     def commit
-      ActiveRecord::Base.transaction do
+      unless domain.valid?
-        debit_registrar
+        domain.errors.delete(:name_dirty) if domain.errors[:puny_label].any?
-        process_auction_and_disputes
+        return false if domain.errors.any?
-
-        domain.save
       end
+      # @domain.add_legal_file_to_new(params[:parsed_frame])
+      debit_registrar
+
+      return false if domain.errors.any?
+
+      process_auction_and_disputes
+      domain.save
     end
   end
 end

app/models/epp/domain.rb

@@ -37,25 +37,6 @@ class Epp::Domain < Domain
     ok
   end
 
-  class << self
-    def new_from_epp(frame, current_user)
-      domain_create = ::Deserializers::Xml::DomainCreate.new(frame, current_user.registrar.id)
-      puts "DOMAIN CREATE ACTION"
-      puts domain_create
-
-      domain = Epp::Domain.new
-      domain.attributes = domain.attrs_from(frame, current_user)
-      domain.attach_default_contacts
-
-      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
-
-      domain
-    end
-  end
-
   def epp_code_map
     {
       '2002' => [ # Command use error

lib/deserializers/xml/domain.rb

@@ -15,7 +15,7 @@ module Deserializers
           registrar_id: registrar,
           registrant_id: if_present('registrant'),
           reserved_pw: if_present('reserved > pw'),
-          period: Integer(frame.css('period').text) || 1,
+          period: frame.css('period').text.present? ? Integer(frame.css('period').text) : 1,
           period_unit: frame.css('period').first ? frame.css('period').first[:unit] : 'y',
         }
 

lib/deserializers/xml/domain_create.rb

@@ -32,11 +32,11 @@ module Deserializers
       end
 
       def admin_contacts
-        frame.css('contact').map { |c| c.text if c['type'] == 'admin' }
+        frame.css('contact').select { |c| c['type'] == 'admin' }.map(&:text)
       end
 
       def tech_contacts
-        frame.css('contact').map { |c| c.text if c['type'] == 'tech' }
+        frame.css('contact').select { |c| c['type'] == 'tech' }.map(&:text)
       end
 
       def dns_keys

test/integration/repp/v1/domains/transfer_info_test.rb

@@ -28,6 +28,7 @@ class ReppV1DomainsTransferInfoTest < ActionDispatch::IntegrationTest
   def test_respects_domain_authorization_code
     headers = @auth_headers
     headers['Auth-Code'] = 'jhfgifhdg'
+    @domain.update!(registrar: registrars(:goodnames))
 
     get "/repp/v1/domains/#{@domain.name}/transfer_info", headers: headers
     json = JSON.parse(response.body, symbolize_names: true)

test/integration/repp/v1/retained_domains_test.rb

@@ -77,7 +77,7 @@ class ReppV1RetainedDomainsTest < ActionDispatch::IntegrationTest
                           status: 'disputed',
                           punycode_name: 'reserved.test' }]
 
-    assert_equal response_json[:domains], expected_objects
+    assert_equal response_json[:domains].to_set, expected_objects.to_set
   end
 
   def test_etags_cache