Domain creating refactor

app/controllers/concerns/epp/common.rb

@@ -35,11 +35,16 @@ module Epp::Common
   def handle_errors(obj=nil)
     if obj
       obj.construct_epp_errors
-      @errors = obj.errors[:epp_errors]
+      @errors += obj.errors[:epp_errors]
     end
     render '/epp/error'
   end
 
+  def append_errors(obj)
+    obj.construct_epp_errors
+    @errors += obj.errors[:epp_errors]
+  end
+
   def xml_attrs_present?(ph, attributes)
     attributes.each do |x|
       epp_errors << {code: '2003', msg: I18n.t('errors.messages.required_parameter_missing', key: x.last)} unless has_attribute(ph, x)

app/helpers/epp/domains_helper.rb

@@ -1,27 +1,12 @@
 module Epp::DomainsHelper
   def create_domain
-    ph = params_hash['epp']['command']['create']['create']
-
-    unless xml_attrs_present?(ph, [['name'], ['ns'], ['authInfo'], ['contact'], ['registrant']])
-      render '/epp/error' and return
-    end
-
-    @domain = Domain.new(domain_create_params(ph))
-
-    if owner_contact_id = Contact.find_by(code: ph[:registrant]).try(:id)
-      @domain.owner_contact_id = owner_contact_id
-    else
-      epp_errors << {code: '2303', msg: I18n.t('errors.messages.epp_registrant_not_found'), value: {obj: 'registrant', val: ph[:registrant]}}
-      render '/epp/error' and return
-    end
-
     Domain.transaction do
-      if @domain.save && @domain.attach_contacts(domain_contacts) && @domain.attach_nameservers(domain_nameservers)
+      @domain = Domain.new(domain_create_params)
+
+      handle_errors(@domain) and return unless @domain.attach_objects(@ph, params[:frame])
+      handle_errors(@domain) and return unless @domain.save
+
       render '/epp/domains/create'
-      else
-        handle_errors(@domain)
-        raise ActiveRecord::Rollback
-      end
     end
   end
 
@@ -44,11 +29,31 @@ module Epp::DomainsHelper
   ### HELPER METHODS ###
   private
 
+  ## CREATE
+  def validate_domain_create_request
+    @ph = params_hash['epp']['command']['create']['create']
+    xml_attrs_present?(@ph, [['name'], ['ns'], ['authInfo'], ['contact'], ['registrant']])
+  end
+
+  def domain_create_params
+    {
+      name: @ph[:name],
+      registrar_id: current_epp_user.registrar.try(:id),
+      registered_at: Time.now,
+      period: @ph[:period].to_i,
+      valid_from: Date.today,
+      valid_to: Date.today + @ph[:period].to_i.years,
+      auth_info: @ph[:authInfo][:pw]
+    }
+  end
+
+  ## RENEW
   def validate_domain_renew_request
     @ph = params_hash['epp']['command']['renew']['renew']
     xml_attrs_present?(@ph, [['name'], ['curExpDate'], ['period']])
   end
 
+  ## SHARED
   def find_domain
     domain = Domain.find_by(name: @ph[:name])
     unless domain
@@ -56,38 +61,4 @@ module Epp::DomainsHelper
     end
     domain
   end
-
-  def domain_create_params(ph)
-    {
-      name: ph[:name],
-      registrar_id: current_epp_user.registrar.try(:id),
-      registered_at: Time.now,
-      period: ph[:period].to_i,
-      valid_from: Date.today,
-      valid_to: Date.today + ph[:period].to_i.years,
-      auth_info: ph[:authInfo][:pw]
-    }
-  end
-
-  def domain_contacts
-    parsed_frame = Nokogiri::XML(params[:frame]).remove_namespaces!
-
-    res = {}
-    Contact::CONTACT_TYPES.each do |ct|
-      res[ct.to_sym] ||= []
-      parsed_frame.css("contact[type='#{ct}']").each do |x|
-        res[ct.to_sym] << Hash.from_xml(x.to_s).with_indifferent_access
-      end
-    end
-
-    res
-  end
-
-  def domain_nameservers
-    ph = params_hash['epp']['command']['create']['create']['ns']
-    return [] unless ph
-    return ph[:hostObj] if ph[:hostObj]
-    return ph[:hostAttr] if ph[:hostAttr]
-    []
-  end
 end

app/models/domain.rb

@@ -34,9 +34,13 @@ class Domain < ActiveRecord::Base
   validates :name_dirty, domain_name: true, uniqueness: true
   validates :period, numericality: { only_integer: true, greater_than: 0, less_than: 100 }
   validates :name, :owner_contact, presence: true
-  validate :validate_period
+
   validates_associated :nameservers
 
+  validate :validate_period
+  validate :validate_nameservers_count
+  validate :validate_admin_contacts_count
+
   def name=(value)
     value.strip!
     write_attribute(:name, SimpleIDN.to_unicode(value))
@@ -44,12 +48,30 @@ class Domain < ActiveRecord::Base
     write_attribute(:name_dirty, value)
   end
 
+  def attach_objects(ph, frame)
+    attach_owner_contact(ph[:registrant])
+    attach_contacts(self.class.parse_contacts_from_frame(frame))
+    attach_nameservers(self.class.parse_nameservers_from_params(ph[:ns]))
+
+    errors.empty?
+  end
+
+  def attach_owner_contact(code)
+    self.owner_contact = Contact.find_by(code: code)
+
+    errors.add(:owner_contact, {
+      obj: 'registrant',
+      val: code,
+      msg: I18n.t('errors.messages.epp_registrant_not_found')
+    }) unless owner_contact
+  end
+
   def attach_contacts(contacts)
     contacts.each do |k, v|
       v.each do |x|
-        contact = Contact.find_by(code: x[:contact])
-        attach_contact(k, contact) and next if contact
-
+        if contact = Contact.find_by(code: x[:contact])
+          attach_contact(k, contact)
+        else
           # Detailed error message with value to display in EPP response
           errors.add(:domain_contacts, {
             obj: 'contact',
@@ -58,35 +80,24 @@ class Domain < ActiveRecord::Base
           })
         end
       end
+    end
 
     attach_contact(Contact::CONTACT_TYPE_TECH, owner_contact) if tech_contacts.empty?
 
     if owner_contact.citizen?
       attach_contact(Contact::CONTACT_TYPE_ADMIN, owner_contact) if admin_contacts.empty?
     end
-
-    validate_admin_contacts_count
-
-    errors.empty?
   end
 
   def attach_contact(type, contact)
-    domain_contacts.create(
-      contact: contact,
-      contact_type: type
-    )
+    tech_contacts << contact if type.to_sym == :tech
+    admin_contacts << contact if type.to_sym == :admin
   end
 
   def attach_nameservers(ns_list)
     ns_list.each do |ns|
       attach_nameserver(ns)
     end
-
-    save
-
-    validate_nameservers_count
-
-    errors.empty?
   end
 
   def attach_nameserver(ns)
@@ -107,12 +118,12 @@ class Domain < ActiveRecord::Base
     self.nameservers.build(attrs)
   end
 
+  ### VALIDATIONS ###
+
   def validate_nameservers_count
-    unless nameservers.count.between?(1, 13)
+    unless nameservers.length.between?(1, 13)
       errors.add(:nameservers, :out_of_range, {min: 1, max: 13})
     end
-
-    errors.empty?
   end
 
   def validate_admin_contacts_count
@@ -156,6 +167,27 @@ class Domain < ActiveRecord::Base
   end
 
   class << self
+    def parse_contacts_from_frame(frame)
+      parsed_frame = Nokogiri::XML(frame).remove_namespaces!
+
+      res = {}
+      Contact::CONTACT_TYPES.each do |ct|
+        res[ct.to_sym] ||= []
+        parsed_frame.css("contact[type='#{ct}']").each do |x|
+          res[ct.to_sym] << Hash.from_xml(x.to_s).with_indifferent_access
+        end
+      end
+
+      res
+    end
+
+    def parse_nameservers_from_params(ph)
+      return [] unless ph
+      return ph[:hostObj] if ph[:hostObj]
+      return ph[:hostAttr] if ph[:hostAttr]
+      []
+    end
+
     def check_availability(domains)
       domains = [domains] if domains.is_a?(String)
 

app/models/domain_contact.rb

@@ -1,4 +1,6 @@
 class DomainContact < ActiveRecord::Base
   belongs_to :contact
   belongs_to :domain
+
+  scope :tech, -> {where(contact_type: :tech)}
 end

spec/fabricators/contact_fabricator.rb

@@ -3,7 +3,7 @@ Fabricator(:contact) do
   phone '+372.12345678'
   email Faker::Internet.email
   ident '37605030299'
-  code "sh#{Faker::Number.number(4)}"
+  code { "sh#{Faker::Number.number(4)}" }
   ident_type 'op'
   address
 end

spec/fabricators/domain_fabricator.rb

@@ -4,4 +4,6 @@ Fabricator(:domain) do
   period 1
   period_unit 'y'
   owner_contact(fabricator: :contact)
+  nameservers(count: 3)
+  admin_contacts(count: 1) { Fabricate(:contact) }
 end

spec/fabricators/nameserver_fabricator.rb

@@ -0,0 +1,3 @@
+Fabricator(:nameserver) do
+  hostname { "ns.#{Faker::Internet.domain_word}.ee" }
+end

spec/models/domain_spec.rb

@@ -41,7 +41,9 @@ describe Domain do
     expect(d.errors.messages).to match_array({
       name: ['is missing'],
       period: ['is not a number'],
-      owner_contact: ["Registrant is missing"]
+      owner_contact: ["Registrant is missing"],
+      admin_contacts: ["Admin contact is missing"],
+      nameservers: ["Nameservers count must be between 1-13"]
     })
   end