EPP error refactor (once again)

app/models/concerns/epp_errors.rb

@@ -11,35 +11,21 @@ module EppErrors
         epp_errors << collect_child_errors(key)
       end
 
-      epp_errors << collect_parent_errors(key, values)
+      epp_errors << collect_parent_errors(values)
     end
 
     errors[:epp_errors] = epp_errors
     errors[:epp_errors].flatten!
   end
 
-  def collect_parent_errors(key, values)
+  def collect_parent_errors(values)
     epp_errors = []
     values = [values] if values.is_a?(String)
 
     values.each do |err|
-      if err.is_a?(Hash)
+      code, value = find_epp_code_and_value(err)
-        code = err[:code] || find_epp_code(err[:msg])
       next unless code
-        err_msg = { code: code, msg: err[:msg] }
+      epp_errors << { code: code, msg: err, value: value}
-        err_msg[:value] = { val: err[:val], obj: err[:obj] } if err[:val]
-        epp_errors << err_msg
-      else
-        next unless code = find_epp_code(err)
-        err = { code: code, msg: err }
-
-        # if the key represents relations, skip value
-        unless self.class.reflect_on_association(key)
-          err[:value] = { val: send(key), obj: self.class::EPP_ATTR_MAP[key] }
-        end
-
-        epp_errors << err
-      end
     end
     epp_errors
   end
@@ -52,24 +38,38 @@ module EppErrors
     epp_errors = []
     send(key).each do |x|
       x.errors.messages.each do |key, values|
-        epp_errors << x.collect_parent_errors(key, values)
+        epp_errors << x.collect_parent_errors(values)
       end
     end if multi.include?(macro)
 
     epp_errors
   end
 
-  def find_epp_code(msg)
+  def find_epp_code_and_value(msg)
     epp_code_map.each do |code, values|
       values.each do |x|
-        t = errors.generate_message(*x) if x.is_a?(Array)
+        msg_args, value = construct_msg_args_and_value(x)
-        t = x if x.is_a?(String)
+        t = errors.generate_message(*msg_args)
-        return code if t == msg
+        return [code, value] if t == msg
       end
     end
     nil
   end
 
+  def construct_msg_args_and_value(epp_error_args)
+    args = {}
+    args = epp_error_args.delete_at(-1) if epp_error_args.last.is_a?(Hash)
+    msg_args = epp_error_args
+
+    value = args.delete(:value) if args.key?(:value)
+
+    interpolation = args[:interpolation] || args
+
+    msg_args << interpolation
+
+    [msg_args, value]
+  end
+
   def add_epp_error(code, obj, val, msg)
     errors[:epp_errors] ||= []
     t = errors.generate_message(*msg) if msg.is_a?(Array)

app/models/contact.rb

@@ -101,8 +101,6 @@ class Contact < ActiveRecord::Base
       '2302' => [ # Object exists
         [:code, :epp_id_taken]
       ],
-      '2303' => # Object does not exist
-        [:not_found, :epp_obj_does_not_exist],
       '2305' => [ # Association exists
         [:domains, :exist]
       ],

app/models/domain_contact.rb

@@ -10,7 +10,7 @@ class DomainContact < ActiveRecord::Base
   def epp_code_map
     {
       '2302' => [
-        [:contact, :taken]
+        [:contact, :taken, {value: {obj: 'contact', val: contact.code}}]
       ]
     }
   end

app/models/domain_status.rb

@@ -23,16 +23,12 @@ class DomainStatus < ActiveRecord::Base
 
   STATUSES = [CLIENT_DELETE_PROHIBITED, SERVER_DELETE_PROHIBITED, CLIENT_HOLD, SERVER_HOLD, CLIENT_RENEW_PROHIBITED, SERVER_RENEW_PROHIBITED, CLIENT_TRANSFER_PROHIBITED, SERVER_TRANSFER_PROHIBITED, CLIENT_UPDATE_PROHIBITED, SERVER_UPDATE_PROHIBITED, INACTIVE, OK, PENDING_CREATE, PENDING_DELETE, PENDING_RENEW, PENDING_TRANSFER, PENDING_UPDATE]
 
-  EPP_ATTR_MAP = {
-    value: 'status'
-  }
-
   validates :value, uniqueness: { scope: :domain_id }
 
   def epp_code_map
     {
       '2302' => [ # Object exists
-        [:value, :taken]
+        [:value, :taken, { value: { obj: 'status', val: value } }]
       ]
     }
   end

app/models/epp/epp_domain.rb

@@ -1,15 +1,36 @@
 class Epp::EppDomain < Domain
   include EppErrors
 
-  EPP_ATTR_MAP = {
-    owner_contact: 'registrant',
-    name_dirty: 'name',
-    period: 'period'
-  }
-
   validate :validate_nameservers_count
   validate :validate_admin_contacts_count
 
+  def epp_code_map # rubocop:disable Metrics/MethodLength
+    domain_validation_sg = SettingGroup.domain_validation
+
+    {
+      '2302' => [ # Object exists
+        [:name_dirty, :taken, { value: { obj: 'name', val: name_dirty } }],
+        [:name_dirty, :reserved, { value: { obj: 'name', val: name_dirty } }]
+      ],
+      '2306' => [ # Parameter policy error
+        [:owner_contact, :blank],
+        [:admin_contacts, :out_of_range]
+      ],
+      '2004' => [ # Parameter value range error
+        [:nameservers, :out_of_range,
+          {
+            min: domain_validation_sg.setting(:ns_min_count).value,
+            max: domain_validation_sg.setting(:ns_max_count).value
+          }
+        ],
+        [:period, :out_of_range, { value: { obj: 'period', val: period } }]
+      ],
+      '2200' => [
+        [:auth_info, :wrong_pw]
+      ]
+    }
+  end
+
   def parse_and_attach_domain_dependencies(parsed_frame)
     attach_owner_contact(self.class.parse_owner_contact_from_frame(parsed_frame))
     attach_contacts(self.class.parse_contacts_from_frame(parsed_frame))
@@ -216,38 +237,11 @@ class Epp::EppDomain < Domain
     add_epp_error('2306', 'curExpDate', cur_exp_date, I18n.t('errors.messages.epp_exp_dates_do_not_match'))
   end
 
-  def epp_code_map # rubocop:disable Metrics/MethodLength
-    domain_validation_sg = SettingGroup.domain_validation
-
-    {
-      '2302' => [ # Object exists
-        [:name_dirty, :taken],
-        [:name_dirty, :reserved]
-      ],
-      '2306' => [ # Parameter policy error
-        [:owner_contact, :blank],
-        [:admin_contacts, :out_of_range]
-      ],
-      '2004' => [ # Parameter value range error
-        [:nameservers, :out_of_range,
-          {
-            min: domain_validation_sg.setting(:ns_min_count).value,
-            max: domain_validation_sg.setting(:ns_max_count).value
-          }
-        ],
-        [:period, :out_of_range]
-      ],
-      '2200' => [
-        [:auth_info, :wrong_pw]
-      ]
-    }
-  end
-
   ## SHARED
 
   # For domain transfer
   def authenticate(pw)
-    errors.add(:auth_info, { msg: errors.generate_message(:auth_info, :wrong_pw) }) if pw != auth_info
+    errors.add(:auth_info, :wrong_pw) if pw != auth_info
     errors.empty?
   end
 

app/models/nameserver.rb

@@ -1,10 +1,6 @@
 class Nameserver < ActiveRecord::Base
   include EppErrors
 
-  EPP_ATTR_MAP = {
-    hostname: 'hostObj'
-  }
-
   belongs_to :registrar
   belongs_to :domain
 
@@ -17,12 +13,12 @@ class Nameserver < ActiveRecord::Base
   def epp_code_map
     {
       '2302' => [
-        [:hostname, :taken]
+        [:hostname, :taken, { value: { obj: 'hostObj', val: hostname } }]
       ],
       '2005' => [
-        [:hostname, :invalid],
+        [:hostname, :invalid, { value: { obj: 'hostObj', val: hostname } }],
-        [:ipv4, :invalid],
+        [:ipv4, :invalid, { value: { obj: 'hostAddr', val: ipv4 } }],
-        [:ipv6, :invalid]
+        [:ipv6, :invalid, { value: { obj: 'hostAddr', val: ipv6 } }]
       ]
     }
   end

config/locales/en.yml

@@ -142,7 +142,7 @@ en:
 
   errors:
     messages:
-      #taken: 'Status already exists on this domain'
+      taken: 'Status already exists on this domain'
       blank: 'is missing'
       epp_domain_reserved: 'Domain name is reserved or restricted'
       epp_obj_does_not_exist: 'Object does not exist'

spec/epp/domain_spec.rb

@@ -437,13 +437,16 @@ describe 'EPP Domain', epp: true do
 
         response = epp_request(xml, :xml)
         expect(response[:results][0][:result_code]).to eq('2302')
-        expect(response[:results][0][:msg]).to eq('Nameserver already exists on this domain')
+        expect(response[:results][0][:msg]).to eq('Contact already exists on this domain!')
-        expect(response[:results][0][:value]).to eq('ns1.example.com')
+        expect(response[:results][0][:value]).to eq('mak21')
+        expect(response[:results][1][:result_code]).to eq('2302')
         expect(response[:results][1][:msg]).to eq('Nameserver already exists on this domain')
-        expect(response[:results][2][:msg]).to eq('Status already exists on this domain')
+        expect(response[:results][1][:value]).to eq('ns1.example.com')
-        expect(response[:results][2][:value]).to eq('clientHold')
+        expect(response[:results][2][:msg]).to eq('Nameserver already exists on this domain')
         expect(response[:results][3][:msg]).to eq('Status already exists on this domain')
-        expect(response[:results][3][:value]).to eq('clientUpdateProhibited')
+        expect(response[:results][3][:value]).to eq('clientHold')
+        expect(response[:results][4][:msg]).to eq('Status already exists on this domain')
+        expect(response[:results][4][:value]).to eq('clientUpdateProhibited')
         expect(d.domain_statuses.count).to eq(2)
       end