Validate all contacts on domain create / update #2825

app/models/domain.rb

@@ -9,6 +9,7 @@ class Domain < ActiveRecord::Base
 
   belongs_to :registrar
   belongs_to :registrant
+  # TODO: should we user validates_associated :registrant here?
 
   has_many :admin_domain_contacts
   accepts_nested_attributes_for :admin_domain_contacts, allow_destroy: true
@@ -271,6 +272,7 @@ class Domain < ActiveRecord::Base
     end
 
     # rubocop:disable Rails/FindEach
+    # rubocop:disable Metrics/AbcSize
     def destroy_delete_candidates
       STDOUT << "#{Time.zone.now.utc} - Destroying domains\n" unless Rails.env.test?
 
@@ -289,6 +291,7 @@ class Domain < ActiveRecord::Base
 
       STDOUT << "#{Time.zone.now.utc} - Successfully destroyed #{c} domains\n" unless Rails.env.test?
     end
+    # rubocop: enable Metrics/AbcSize
     # rubocop:enable Rails/FindEach
     # rubocop: enable Metrics/LineLength
   end

app/models/epp/domain.rb

@@ -9,11 +9,24 @@ class Epp::Domain < Domain
     false
   end
 
-  before_validation :validate_contacts
+  after_validation :validate_contacts
   def validate_contacts
-    return if contacts.map(&:valid?).all?
-    add_epp_error('2304', nil, nil, I18n.t(:object_status_prohibits_operation))
-    false
+    ok = true
+    if new_record?
+      ac = admin_domain_contacts.map(&:contact)
+      tc = tech_domain_contacts.map(&:contact)
+    else
+      ac = contacts
+      tc = []
+    end
+    # validate registrant here as well
+    ([registrant] + ac + tc).each do |x|
+      unless x.valid?
+        add_epp_error('2304', nil, nil, I18n.t(:contact_is_not_valid, value: x.code))
+        ok = false
+      end
+    end
+    ok
   end
 
   before_save :link_contacts

config/locales/en.yml

@@ -918,3 +918,4 @@ en:
   mail_templates: Mail Templates
   new_mail_template: New mail template
   failure: "It was not saved"
+  contact_is_not_valid: 'Contact %{value} is not valid, please fix the invalid contact'

spec/epp/domain_spec.rb

@@ -1435,7 +1435,7 @@ describe 'EPP Domain', epp: true do
       original_contacts_codes.sort.should == domain.contacts.pluck(:code).sort
     end
 
-    fit 'transfers domain contact should populate copy_from_id' do
+    it 'transfers domain contact should populate copy_from_id' do
       d = Fabricate(:domain)
       d.tech_contacts << domain.registrant