Nameservers refactor

app/models/concerns/epp_errors.rb

@@ -21,7 +21,8 @@ module EppErrors
 
     values.each do |err|
       if err.is_a?(Hash)
-        next unless code = find_epp_code(err[:msg])
+        code = err[:code] || find_epp_code(err[:msg])
+        next unless code
         err_msg = { code: code, msg: err[:msg] }
         err_msg[:value] = { val: err[:val], obj: err[:obj] } if err[:val]
         epp_errors << err_msg

app/models/domain.rb

@@ -23,7 +23,8 @@ class Domain < ActiveRecord::Base
     where(domain_contacts: { contact_type: DomainContact::ADMIN })
   end, through: :domain_contacts, source: :contact
 
-  has_and_belongs_to_many :nameservers
+  has_many :domain_nameservers
+  has_many :nameservers, through: :domain_nameservers
 
   has_many :domain_statuses, -> {
     joins(:setting).where(settings: { setting_group_id: SettingGroup.domain_statuses.id })
@@ -121,7 +122,18 @@ class Domain < ActiveRecord::Base
 
   def attach_nameservers(ns_list)
     ns_list.each do |ns_attrs|
+      existing = nameservers.select { |x| x.hostname == ns_attrs[:hostname] }
+
       nameservers.build(ns_attrs)
+
+      next if existing.empty?
+
+      errors.add(:nameservers, {
+        code: '2302',
+        obj: 'hostObj',
+        val: ns_attrs[:hostname],
+        msg: errors.generate_message(:nameservers, :taken)
+      })
     end
   end
 

app/models/domain_nameserver.rb

@@ -0,0 +1,4 @@
+class DomainNameserver < ActiveRecord::Base
+  belongs_to :domain
+  belongs_to :nameserver
+end

app/models/nameserver.rb

@@ -6,7 +6,9 @@ class Nameserver < ActiveRecord::Base
   }
 
   belongs_to :registrar
-  has_and_belongs_to_many :domains
+
+  has_many :domain_nameservers
+  has_many :domains, through: :domain_nameservers
 
   validates :hostname, format: { with: /\A(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\z/ }
   validates :ipv4, format: { with: /\A(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\z/, allow_nil: true }

config/locales/en.yml

@@ -53,6 +53,7 @@ en:
             nameservers:
               out_of_range: 'Nameservers count must be between %{min}-%{max}'
               not_found: 'Nameserver was not found'
+              taken: 'Nameserver already exists on this domain'
             period:
               out_of_range: 'Period must add up to 1, 2 or 3 years'
             auth_info:

db/migrate/20140826082057_create_domain_nameservers.rb

@@ -0,0 +1,8 @@
+class CreateDomainNameservers < ActiveRecord::Migration
+  def change
+    create_table :domain_nameservers, id: false do |t|
+      t.integer :domain_id
+      t.integer :nameserver_id
+    end
+  end
+end

db/migrate/20140826103454_drop_domains_nameservers.rb

@@ -0,0 +1,5 @@
+class DropDomainsNameservers < ActiveRecord::Migration
+  def change
+    drop_table :domains_nameservers
+  end
+end

db/schema.rb

@@ -11,181 +11,181 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20_140_819_103_517) do
+ActiveRecord::Schema.define(version: 20140826103454) do
 
   # These are extensions that must be enabled in order to support this database
-  enable_extension 'plpgsql'
+  enable_extension "plpgsql"
 
-  create_table 'addresses', force: true do |t|
-    t.integer 'contact_id'
-    t.integer 'country_id'
-    t.string 'city'
-    t.string 'street'
-    t.string 'zip'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
-    t.string 'street2'
-    t.string 'street3'
+  create_table "addresses", force: true do |t|
+    t.integer  "contact_id"
+    t.integer  "country_id"
+    t.string   "city"
+    t.string   "street"
+    t.string   "zip"
+    t.datetime "created_at"
+    t.datetime "updated_at"
+    t.string   "street2"
+    t.string   "street3"
   end
 
-  create_table 'contacts', force: true do |t|
-    t.string 'code'
-    t.string 'name'
-    t.string 'type'
-    t.string 'reg_no'
-    t.string 'phone'
-    t.string 'email'
-    t.string 'fax'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
-    t.string 'ident'
-    t.string 'ident_type'
-    t.string 'org_name'
-    t.integer 'created_by_id'
-    t.integer 'updated_by_id'
-    t.string 'auth_info'
+  create_table "contacts", force: true do |t|
+    t.string   "code"
+    t.string   "name"
+    t.string   "type"
+    t.string   "reg_no"
+    t.string   "phone"
+    t.string   "email"
+    t.string   "fax"
+    t.datetime "created_at"
+    t.datetime "updated_at"
+    t.string   "ident"
+    t.string   "ident_type"
+    t.string   "org_name"
+    t.integer  "created_by_id"
+    t.integer  "updated_by_id"
+    t.string   "auth_info"
   end
 
-  create_table 'countries', force: true do |t|
-    t.string 'iso'
-    t.string 'name'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
+  create_table "countries", force: true do |t|
+    t.string   "iso"
+    t.string   "name"
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
-  create_table 'domain_contacts', force: true do |t|
-    t.integer 'contact_id'
-    t.integer 'domain_id'
-    t.string 'contact_type'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
+  create_table "domain_contacts", force: true do |t|
+    t.integer  "contact_id"
+    t.integer  "domain_id"
+    t.string   "contact_type"
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
-  create_table 'domain_statuses', force: true do |t|
-    t.integer 'domain_id'
-    t.integer 'setting_id'
-    t.string 'description'
+  create_table "domain_nameservers", id: false, force: true do |t|
+    t.integer "domain_id"
+    t.integer "nameserver_id"
   end
 
-  create_table 'domains', force: true do |t|
-    t.string 'name'
-    t.integer 'registrar_id'
-    t.datetime 'registered_at'
-    t.string 'status'
-    t.datetime 'valid_from'
-    t.datetime 'valid_to'
-    t.integer 'owner_contact_id'
-    t.integer 'admin_contact_id'
-    t.integer 'technical_contact_id'
-    t.integer 'ns_set_id'
-    t.string 'auth_info'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
-    t.string 'name_dirty'
-    t.string 'name_puny'
-    t.integer 'period'
-    t.string 'period_unit',          limit: 1
+  create_table "domain_statuses", force: true do |t|
+    t.integer "domain_id"
+    t.integer "setting_id"
+    t.string  "description"
   end
 
-  create_table 'domains_nameservers', force: true do |t|
-    t.integer 'domain_id'
-    t.integer 'nameserver_id'
+  create_table "domains", force: true do |t|
+    t.string   "name"
+    t.integer  "registrar_id"
+    t.datetime "registered_at"
+    t.string   "status"
+    t.datetime "valid_from"
+    t.datetime "valid_to"
+    t.integer  "owner_contact_id"
+    t.integer  "admin_contact_id"
+    t.integer  "technical_contact_id"
+    t.integer  "ns_set_id"
+    t.string   "auth_info"
+    t.datetime "created_at"
+    t.datetime "updated_at"
+    t.string   "name_dirty"
+    t.string   "name_puny"
+    t.integer  "period"
+    t.string   "period_unit",          limit: 1
   end
 
-  create_table 'epp_sessions', force: true do |t|
-    t.string 'session_id'
-    t.text 'data'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
+  create_table "epp_sessions", force: true do |t|
+    t.string   "session_id"
+    t.text     "data"
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
-  add_index 'epp_sessions', ['session_id'], name: 'index_epp_sessions_on_session_id', unique: true, using: :btree
-  add_index 'epp_sessions', ['updated_at'], name: 'index_epp_sessions_on_updated_at', using: :btree
+  add_index "epp_sessions", ["session_id"], name: "index_epp_sessions_on_session_id", unique: true, using: :btree
+  add_index "epp_sessions", ["updated_at"], name: "index_epp_sessions_on_updated_at", using: :btree
 
-  create_table 'epp_users', force: true do |t|
-    t.integer 'registrar_id'
-    t.string 'username'
-    t.string 'password'
-    t.boolean 'active',       default: false
-    t.text 'csr'
-    t.text 'crt'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
+  create_table "epp_users", force: true do |t|
+    t.integer  "registrar_id"
+    t.string   "username"
+    t.string   "password"
+    t.boolean  "active",       default: false
+    t.text     "csr"
+    t.text     "crt"
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
-  create_table 'nameservers', force: true do |t|
-    t.string 'hostname'
-    t.string 'ipv4'
-    t.integer 'ns_set_id'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
-    t.string 'ipv6'
+  create_table "nameservers", force: true do |t|
+    t.string   "hostname"
+    t.string   "ipv4"
+    t.integer  "ns_set_id"
+    t.datetime "created_at"
+    t.datetime "updated_at"
+    t.string   "ipv6"
   end
 
-  create_table 'nameservers_ns_sets', force: true do |t|
-    t.integer 'nameserver_id'
-    t.integer 'ns_set_id'
+  create_table "nameservers_ns_sets", force: true do |t|
+    t.integer "nameserver_id"
+    t.integer "ns_set_id"
   end
 
-  create_table 'ns_sets', force: true do |t|
-    t.string 'code'
-    t.integer 'registrar_id'
-    t.string 'auth_info'
-    t.string 'report_level'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
+  create_table "ns_sets", force: true do |t|
+    t.string   "code"
+    t.integer  "registrar_id"
+    t.string   "auth_info"
+    t.string   "report_level"
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
-  create_table 'registrars', force: true do |t|
-    t.string 'name'
-    t.string 'reg_no'
-    t.string 'vat_no'
-    t.string 'address'
-    t.integer 'country_id'
-    t.string 'billing_address'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
+  create_table "registrars", force: true do |t|
+    t.string   "name"
+    t.string   "reg_no"
+    t.string   "vat_no"
+    t.string   "address"
+    t.integer  "country_id"
+    t.string   "billing_address"
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
-  create_table 'reserved_domains', force: true do |t|
-    t.string 'name'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
+  create_table "reserved_domains", force: true do |t|
+    t.string   "name"
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
-  create_table 'rights', force: true do |t|
-    t.string 'code'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
+  create_table "rights", force: true do |t|
+    t.string   "code"
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
-  create_table 'rights_roles', force: true do |t|
-    t.integer 'right_id'
-    t.integer 'role_id'
+  create_table "rights_roles", force: true do |t|
+    t.integer "right_id"
+    t.integer "role_id"
   end
 
-  create_table 'roles', force: true do |t|
-    t.string 'name'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
+  create_table "roles", force: true do |t|
+    t.string   "name"
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
-  create_table 'setting_groups', force: true do |t|
-    t.string 'code'
+  create_table "setting_groups", force: true do |t|
+    t.string "code"
   end
 
-  create_table 'settings', force: true do |t|
-    t.integer 'setting_group_id'
-    t.string 'code'
-    t.string 'value'
+  create_table "settings", force: true do |t|
+    t.integer "setting_group_id"
+    t.string  "code"
+    t.string  "value"
   end
 
-  create_table 'users', force: true do |t|
-    t.string 'username'
-    t.string 'password'
-    t.integer 'role_id'
-    t.datetime 'created_at'
-    t.datetime 'updated_at'
+  create_table "users", force: true do |t|
+    t.string   "username"
+    t.string   "password"
+    t.integer  "role_id"
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
 end

spec/epp/domain_spec.rb

@@ -272,10 +272,9 @@ describe 'EPP Domain', epp: true do
 
         response = epp_request('domains/update_add_objects.xml')
         expect(response[:results][0][:result_code]).to eq('2302')
-        expect(response[:results][0][:msg]).to eq('Status already exists on this domain')
-        expect(response[:results][0][:value]).to eq('clientHold')
-        expect(response[:results][1][:msg]).to eq('Status already exists on this domain')
-        expect(response[:results][1][:value]).to eq('clientUpdateProhibited')
+        expect(response[:results][0][:msg]).to eq('Nameserver already exists on this domain')
+        expect(response[:results][0][:value]).to eq('ns1.example.com')
+        expect(response[:results][1][:msg]).to eq('Nameserver already exists on this domain')
         expect(d.domain_statuses.count).to eq(2)
       end
 
@@ -311,6 +310,16 @@ describe 'EPP Domain', epp: true do
         expect(response[:results][2][:value]).to eq('clientHold')
       end
 
+      it 'does not add duplicate objects to domain' do
+        Fabricate(:contact, code: 'mak21')
+        epp_request('domains/update_add_objects.xml')
+        response = epp_request('domains/update_add_objects.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][:value]).to eq('ns1.example.com')
+      end
+
       it 'updates a domain' do
         Fabricate(:contact, code: 'mak21')
         epp_request('domains/update_add_objects.xml')