Improvements on validation

app/controllers/admin/domains_controller.rb

@@ -7,9 +7,7 @@ class Admin::DomainsController < ApplicationController
     @domain = Domain.new(owner_contact: owner_contact)
     params[:domain_owner_contact] = owner_contact
 
-    @domain.nameservers.build
-    @domain.domain_contacts.build
-    @domain.domain_statuses.build
+    build_associations
   end
 
   def create
@@ -19,7 +17,7 @@ class Admin::DomainsController < ApplicationController
       flash[:notice] = I18n.t('shared.domain_added')
       redirect_to [:admin, @domain]
     else
-      @domain.domain_statuses.build if @domain.domain_statuses.empty?
+      build_associations
       flash.now[:alert] = I18n.t('shared.failed_to_add_domain')
       render 'new'
     end
@@ -35,17 +33,16 @@ class Admin::DomainsController < ApplicationController
   end
 
   def edit
-    @domain.domain_statuses.build if @domain.domain_statuses.empty?
-
-    params[:registrar] = @domain.registrar
-    params[:domain_owner_contact] = @domain.owner_contact
+    build_associations
   end
 
   def update
     if @domain.update(domain_params)
+      flash[:notice] = I18n.t('shared.domain_updated')
       redirect_to [:admin, @domain]
     else
-      @domain.domain_statuses.build if @domain.domain_statuses.empty?
+      build_associations
+      flash[:alert] = I18n.t('shared.failed_to_update_domain')
       render 'edit'
     end
   end
@@ -76,11 +73,17 @@ class Admin::DomainsController < ApplicationController
       :owner_contact_typeahead,
       :registrar_typeahead,
       nameservers_attributes: [:id, :hostname, :ipv4, :ipv6, :_destroy],
-      domain_contacts_attributes: [:id, :contact_type, :contact_id, :typeahead_value, :_destroy],
+      domain_contacts_attributes: [:id, :contact_type, :contact_id, :value_typeahead, :_destroy],
       domain_statuses_attributes: [:id, :value, :description, :_destroy]
     )
   end
 
+  def build_associations
+    @domain.nameservers.build if @domain.nameservers.empty?
+    @domain.domain_contacts.build if @domain.domain_contacts.empty?
+    @domain.domain_statuses.build if @domain.domain_contacts.empty?
+  end
+
   def verify_deletion
     return if @domain.can_be_deleted?
     flash[:alert] = I18n.t('shared.domain_status_prohibits_deleting')

app/models/domain.rb

@@ -16,7 +16,8 @@ class Domain < ActiveRecord::Base
   end, through: :domain_contacts, source: :contact
 
   has_many :nameservers, dependent: :delete_all
-  accepts_nested_attributes_for :nameservers, allow_destroy: true
+  accepts_nested_attributes_for :nameservers, allow_destroy: true,
+    reject_if: proc { |attrs| attrs[:hostname].blank? }
 
   has_many :domain_statuses, dependent: :delete_all
   accepts_nested_attributes_for :domain_statuses, allow_destroy: true,
@@ -40,6 +41,7 @@ class Domain < ActiveRecord::Base
 
   validate :validate_period
   validate :validate_nameservers_count
+  validate :validate_admin_contacts_count, if: :new_record?
   validate :validate_nameservers_uniqueness, if: :new_record?
   validate :validate_tech_contacts_uniqueness, if: :new_record?
   validate :validate_admin_contacts_uniqueness, if: :new_record?
@@ -78,7 +80,7 @@ class Domain < ActiveRecord::Base
     sg = SettingGroup.domain_validation
     min, max = sg.setting(:ns_min_count).value.to_i, sg.setting(:ns_max_count).value.to_i
 
-    return if nameservers.length.between?(min, max)
+    return if nameservers.reject(&:marked_for_destruction?).length.between?(min, max)
     errors.add(:nameservers, :out_of_range, { min: min, max: max })
   end
 
@@ -88,9 +90,9 @@ class Domain < ActiveRecord::Base
 
   def validate_nameservers_uniqueness
     validated = []
-    nameservers.each do |ns|
+    nameservers.reject(&:marked_for_destruction?).each do |ns|
       next if ns.hostname.blank?
-      existing = nameservers.select { |x| x.hostname == ns.hostname }
+      existing = nameservers.reject(&:marked_for_destruction?).select { |x| x.hostname == ns.hostname }
       next unless existing.length > 1
       validated << ns.hostname
       errors.add(:nameservers, :invalid) if errors[:nameservers].blank?
@@ -99,12 +101,12 @@ class Domain < ActiveRecord::Base
   end
 
   def validate_tech_contacts_uniqueness
-    contacts = domain_contacts.select { |x| x.contact_type == DomainContact::TECH }
+    contacts = domain_contacts.reject(&:marked_for_destruction?).select { |x| x.contact_type == DomainContact::TECH }
     validate_domain_contacts_uniqueness(contacts)
   end
 
   def validate_admin_contacts_uniqueness
-    contacts = domain_contacts.select { |x| x.contact_type == DomainContact::ADMIN }
+    contacts = domain_contacts.reject(&:marked_for_destruction?).select { |x| x.contact_type == DomainContact::ADMIN }
     validate_domain_contacts_uniqueness(contacts)
   end
 
@@ -121,12 +123,12 @@ class Domain < ActiveRecord::Base
 
   def validate_domain_statuses_uniqueness
     validated = []
-    domain_statuses.each do |status|
+    domain_statuses.reject(&:marked_for_destruction?).each do |status|
       next if status.value.blank?
       existing = domain_statuses.select { |x| x.value == status.value }
       next unless existing.length > 1
       validated << status.value
-      errors.add(:'domain_statuses.value', 'duplicate')
+      errors.add(:domain_statuses, :invalid)
       status.errors.add(:value, :taken)
     end
   end
@@ -166,6 +168,14 @@ class Domain < ActiveRecord::Base
     collect_errors_with_keys([:name_dirty, :period, :period_unit, :registrar, :owner_contact])
   end
 
+  def contacts_tab_valid?
+    (
+      associations_valid?(:domain_contacts) &&
+      associations_valid?(:admin_contacts) &&
+      associations_valid?(:tech_contacts)
+    )
+  end
+
   def collect_errors_with_keys(keys)
     res = []
     errors.each { |attr, err| res << err if keys.include?(attr) }
@@ -196,11 +206,11 @@ class Domain < ActiveRecord::Base
   end
 
   def tech_contacts_count
-    domain_contacts.select { |x| x.contact_type == DomainContact::TECH }.count
+    domain_contacts.reject(&:marked_for_destruction?).select { |x| x.contact_type == DomainContact::TECH }.count
   end
 
   def admin_contacts_count
-    domain_contacts.select { |x| x.contact_type == DomainContact::ADMIN }.count
+    domain_contacts.reject(&:marked_for_destruction?).select { |x| x.contact_type == DomainContact::ADMIN }.count
   end
 
   class << self

app/models/domain_contact.rb

@@ -3,7 +3,7 @@ class DomainContact < ActiveRecord::Base
   belongs_to :contact
   belongs_to :domain
 
-  attr_accessor :typeahead_value
+  attr_accessor :value_typeahead
 
   def epp_code_map
     {
@@ -31,7 +31,7 @@ class DomainContact < ActiveRecord::Base
     contact_type == TECH
   end
 
-  def typeahead_value
-    @typeahead_value || contact.try(:name) || nil
+  def value_typeahead
+    @value_typeahead || contact.try(:name) || nil
   end
 end

app/views/admin/domains/_form.haml

@@ -9,7 +9,7 @@
         - li_class = @domain.associations_valid?(:nameservers) ? nil : 'error-tab'
         %li{class: li_class}
           %a{"data-toggle" => "tab", :href => "#nameservers-tab", :role => "tab"}= t('shared.nameservers')
-        - li_class = @domain.associations_valid?(:domain_contacts) ? nil : 'error-tab'
+        - li_class = @domain.contacts_tab_valid? ? nil : 'error-tab'
         %li{class: li_class}
           %a{"data-toggle" => "tab", :href => "#contacts-tab", :role => "tab"}= t('shared.contacts')
         - li_class = @domain.associations_valid?(:domain_statuses) ? nil : 'error-tab'

app/views/admin/domains/form_partials/_contacts.haml

@@ -1,5 +1,7 @@
 = render 'admin/shared/errors_array', errors: @domain.errors[:domain_contacts]
-- if @domain.errors[:domain_contacts].any?
+= render 'admin/shared/errors_array', errors: @domain.errors[:admin_contacts]
+= render 'admin/shared/errors_array', errors: @domain.errors[:tech_contacts]
+- if (@domain.errors[:domain_contacts] + @domain.errors[:admin_contacts] + @domain.errors[:tech_contacts]).any?
   %hr
 #domain_contacts
   = f.fields_for :domain_contacts do |contact_fields|
@@ -19,8 +21,8 @@
               = contact_fields.select :contact_type, options_for_select(DomainContact::TYPES, contact_fields.object.contact_type), {}, {class: 'form-control'}
           .col-md-6
             .form-group.has-feedback.js-typeahead-container
-              = contact_fields.label :typeahead_value
-              = contact_fields.text_field(:typeahead_value, class: 'form-control js-contact-typeahead', placeholder: t('shared.contact_code'), autocomplete: 'off')
+              = contact_fields.label :value_typeahead
+              = contact_fields.text_field(:value_typeahead, class: 'form-control js-contact-typeahead', placeholder: t('shared.contact_code'), autocomplete: 'off')
               %span.glyphicon.glyphicon-ok.form-control-feedback.js-typeahead-ok.hidden
               %span.glyphicon.glyphicon-remove.form-control-feedback.js-typeahead-remove
               = contact_fields.hidden_field(:contact_id, class: 'js-contact-id')

config/locales/en.yml

@@ -103,6 +103,8 @@ en:
               invalid: 'Status is invalid'
               not_found: 'Status was not found'
               taken: 'Status already exists on this domain'
+            registrar:
+              blank: 'Registrar is missing'
 
         domain:
           <<: *epp_domain_ar_attributes
@@ -261,3 +263,5 @@ en:
     domain_added: 'Domain added!'
     new_contact: 'New contact'
     add_another: 'Add another'
+    domain_updated: 'Domain updated!'
+    failed_to_update_domain: 'Failed to update domain'