Contacts validation improvement

app/controllers/concerns/epp/common.rb

@@ -45,6 +45,7 @@ module Epp::Common
 
     # for debugging
     @errors << { code: '1', msg: 'handle_errors was executed when there were actually no errors' } if @errors.blank?
+
     render '/epp/error'
   end
 

app/helpers/epp/domains_helper.rb

@@ -35,16 +35,18 @@ module Epp::DomainsHelper
   end
 
   def update_domain
-    @domain = find_domain
+    Domain.transaction do
+      @domain = find_domain
 
-    handle_errors(@domain) and return unless @domain
-    handle_errors(@domain) and return unless @domain.parse_and_attach_domain_dependencies(@ph, parsed_frame.css('add'))
-    handle_errors(@domain) and return unless @domain.parse_and_detach_domain_dependencies(parsed_frame.css('rem'))
-    handle_errors(@domain) and return unless @domain.parse_and_update_domain_dependencies(parsed_frame.css('chg'))
-    handle_errors(@domain) and return unless @domain.parse_and_update_domain_attributes(parsed_frame.css('chg'))
-    handle_errors(@domain) and return unless @domain.save
+      handle_errors(@domain) and return unless @domain
+      handle_errors(@domain) and return unless @domain.parse_and_attach_domain_dependencies(@ph, parsed_frame.css('add'))
+      handle_errors(@domain) and return unless @domain.parse_and_detach_domain_dependencies(parsed_frame.css('rem'))
+      handle_errors(@domain) and return unless @domain.parse_and_update_domain_dependencies(parsed_frame.css('chg'))
+      handle_errors(@domain) and return unless @domain.parse_and_update_domain_attributes(parsed_frame.css('chg'))
+      handle_errors(@domain) and return unless @domain.save
 
-    render '/epp/domains/success'
+      render '/epp/domains/success'
+    end
   end
 
   ### HELPER METHODS ###

app/models/domain.rb

@@ -105,13 +105,13 @@ class Domain < ActiveRecord::Base
 
     return unless owner_contact
 
-    attach_contact(DomainContact::TECH, owner_contact) if tech_contacts.empty?
-    attach_contact(DomainContact::ADMIN, owner_contact) if admin_contacts.empty? if owner_contact.citizen?
+    attach_contact(DomainContact::TECH, owner_contact) if tech_contacts_count.zero?
+    attach_contact(DomainContact::ADMIN, owner_contact) if admin_contacts_count.zero? && owner_contact.citizen?
   end
 
   def attach_contact(type, contact)
-    tech_contacts << contact if type.to_sym == :tech
-    admin_contacts << contact if type.to_sym == :admin
+    domain_contacts.build(contact: contact, contact_type: DomainContact::TECH) if type.to_sym == :tech
+    domain_contacts.build(contact: contact, contact_type: DomainContact::ADMIN) if type.to_sym == :admin
   end
 
   def attach_nameservers(ns_list)
@@ -137,9 +137,9 @@ class Domain < ActiveRecord::Base
 
   def detach_contacts(contact_list)
     to_delete = []
-    contact_list.each do |_k, v|
+    contact_list.each do |k, v|
       v.each do |x|
-        contact = domain_contacts.joins(:contact).where(contacts: { code: x[:contact] })
+        contact = domain_contacts.joins(:contact).where(contacts: { code: x[:contact] }, contact_type: k.to_s)
         if contact.blank?
           add_epp_error('2303', 'contact', x[:contact], [:domain_contacts, :not_found])
         else
@@ -205,7 +205,7 @@ class Domain < ActiveRecord::Base
   end
 
   def validate_admin_contacts_count
-    errors.add(:admin_contacts, :blank) if admin_contacts.empty?
+    errors.add(:admin_contacts, :out_of_range) if admin_contacts_count.zero?
   end
 
   def validate_period
@@ -236,7 +236,7 @@ class Domain < ActiveRecord::Base
       ],
       '2306' => [ # Parameter policy error
         [:owner_contact, :blank],
-        [:admin_contacts, :blank]
+        [:admin_contacts, :out_of_range]
       ],
       '2004' => [ # Parameter value range error
         [:nameservers, :out_of_range,
@@ -261,6 +261,14 @@ class Domain < ActiveRecord::Base
     errors.empty?
   end
 
+  def tech_contacts_count
+    domain_contacts.select { |x| x.contact_type == DomainContact::TECH }.count
+  end
+
+  def admin_contacts_count
+    domain_contacts.select { |x| x.contact_type == DomainContact::ADMIN }.count
+  end
+
   class << self
     def convert_period_to_time(period, unit)
       return period.to_i.days if unit == 'd'

config/locales/en.yml

@@ -50,7 +50,7 @@ en:
             domain_contacts:
               not_found: 'Contact was not found'
             admin_contacts:
-              blank: 'Admin contact is missing'
+              out_of_range: 'Admin contacts count must be between 1 - infinity'
             nameservers:
               out_of_range: 'Nameservers count must be between %{min}-%{max}'
               not_found: 'Nameserver was not found'

spec/epp/domain_spec.rb

@@ -170,7 +170,7 @@ describe 'EPP Domain', epp: true do
 
         response = epp_request(xml, :xml)
         expect(response[:result_code]).to eq('2306')
-        expect(response[:msg]).to eq('Admin contact is missing')
+        expect(response[:msg]).to eq('Admin contacts count must be between 1 - infinity')
         expect(response[:clTRID]).to eq('ABC-12345')
 
         expect(Domain.count).to eq 0