From 660eef414e439dd8f9b78484bf902ca3d1a49ff6 Mon Sep 17 00:00:00 2001 From: Martin Lensment Date: Tue, 16 Sep 2014 17:58:51 +0300 Subject: [PATCH] Refactor --- .../admin/domain_contacts_controller.rb | 16 ++++++++++++---- app/models/domain.rb | 5 +++-- app/models/domain_contact.rb | 11 +++++++++++ app/views/admin/domain_contacts/new.haml | 7 ++++++- .../domains/partials/_admin_contacts.haml | 2 +- .../admin/domains/partials/_tech_contacts.haml | 10 +++++----- config/locales/en.yml | 18 +++++++++++++++--- spec/features/domain_management_spec.rb | 4 ++-- 8 files changed, 55 insertions(+), 18 deletions(-) diff --git a/app/controllers/admin/domain_contacts_controller.rb b/app/controllers/admin/domain_contacts_controller.rb index 1ce42f6aa..f0ba8092f 100644 --- a/app/controllers/admin/domain_contacts_controller.rb +++ b/app/controllers/admin/domain_contacts_controller.rb @@ -3,13 +3,20 @@ class Admin::DomainContactsController < ApplicationController before_action :set_domain_contact, only: [:destroy] def new - @domain_contact = @domain.domain_contacts.build + @domain_contact = @domain.domain_contacts.build(contact_type: params[:type]) end def create - @domain.adding_admin_contact = true @domain_contact = @domain.domain_contacts.build(domain_contact_params) + unless @domain_contact.contact + flash.now[:alert] = I18n.t('shared.contact_was_not_found') + render 'new' and return + end + + @domain.adding_admin_contact = true if @domain_contact.admin? + @domain.adding_admin_contact = true if @domain_contact.tech? + if @domain.save flash[:notice] = I18n.t('shared.contact_added') redirect_to [:admin, @domain] @@ -20,13 +27,14 @@ class Admin::DomainContactsController < ApplicationController end def destroy - @domain.deleting_admin_contact = true @domain.domain_contacts.select { |x| x == @domain_contact }[0].mark_for_destruction + @domain.deleting_admin_contact = true if @domain_contact.admin? + @domain.deleting_tech_contact = true if @domain_contact.tech? if @domain.save flash[:notice] = I18n.t('shared.contact_deleted') else - flash[:alert] = I18n.t('shared.fail') + flash[:alert] = @domain.errors.first[1] end redirect_to [:admin, @domain] diff --git a/app/models/domain.rb b/app/models/domain.rb index 95bb29400..ce23c5a4b 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -6,8 +6,6 @@ class Domain < ActiveRecord::Base has_many :domain_contacts, dependent: :delete_all, autosave: true - accepts_nested_attributes_for :domain_contacts, allow_destroy: true - has_many :tech_contacts, -> do where(domain_contacts: { contact_type: DomainContact::TECH }) end, through: :domain_contacts, source: :contact @@ -50,6 +48,9 @@ class Domain < ActiveRecord::Base attr_accessor :deleting_nameserver validate :validate_nameserver_min_count, if: :deleting_nameserver + attr_accessor :adding_tech_contact + validate :validate_tech_contacts_max_count, if: :adding_tech_contact + def name=(value) value.strip! write_attribute(:name, SimpleIDN.to_unicode(value)) diff --git a/app/models/domain_contact.rb b/app/models/domain_contact.rb index d0b0050d0..8a53fc56a 100644 --- a/app/models/domain_contact.rb +++ b/app/models/domain_contact.rb @@ -6,6 +6,17 @@ class DomainContact < ActiveRecord::Base ADMIN = 'admin' TYPES = [TECH, ADMIN] + # TODO: Fix EPP problems + validates :contact, uniqueness: { scope: [:domain_id, :contact_type] } + scope :admin, -> { where(contact_type: ADMIN) } scope :tech, -> { where(contact_type: TECH) } + + def admin? + contact_type == ADMIN + end + + def tech? + contact_type == TECH + end end diff --git a/app/views/admin/domain_contacts/new.haml b/app/views/admin/domain_contacts/new.haml index e31f61a4c..67b835a83 100644 --- a/app/views/admin/domain_contacts/new.haml +++ b/app/views/admin/domain_contacts/new.haml @@ -7,6 +7,11 @@ = link_to(t('shared.back_to_domain'), [:admin, @domain], class: 'btn btn-default') %hr = form_for([:admin, @domain, @domain_contact]) do |f| + = render 'admin/shared/errors', object: @domain + / = render 'admin/shared/errors', object: f.object + + - if @domain.errors.any? + %hr .row .col-md-6 .form-group @@ -14,7 +19,7 @@ = f.select :contact_type, options_for_select(DomainContact::TYPES, @domain_contact.contact_type), {}, {class: 'form-control'} .col-md-6 .form-group.has-feedback - = f.label :contact + = label_tag :contact = text_field_tag(:contact, params[:contact], 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 diff --git a/app/views/admin/domains/partials/_admin_contacts.haml b/app/views/admin/domains/partials/_admin_contacts.haml index 6b7731902..23aed96a6 100644 --- a/app/views/admin/domains/partials/_admin_contacts.haml +++ b/app/views/admin/domains/partials/_admin_contacts.haml @@ -4,7 +4,7 @@ .pull-left = t('shared.admin_contacts') .pull-right - = link_to(t('shared.add'), new_admin_domain_admin_contact_path(@domain), class: 'btn btn-primary btn-xs') + = link_to(t('shared.add'), new_admin_domain_domain_contact_path(@domain, type: DomainContact::ADMIN), class: 'btn btn-primary btn-xs') .table-responsive %table.table.table-hover.table-bordered.table-condensed %thead diff --git a/app/views/admin/domains/partials/_tech_contacts.haml b/app/views/admin/domains/partials/_tech_contacts.haml index e8e82ae93..28a722940 100644 --- a/app/views/admin/domains/partials/_tech_contacts.haml +++ b/app/views/admin/domains/partials/_tech_contacts.haml @@ -4,7 +4,7 @@ .pull-left = t('shared.tech_contacts') .pull-right - = link_to(t('shared.add'), new_admin_domain_tech_contact_path(@domain), class: 'btn btn-primary btn-xs') + = link_to(t('shared.add'), new_admin_domain_domain_contact_path(@domain, type: DomainContact::TECH), class: 'btn btn-primary btn-xs') .table-responsive %table.table.table-hover.table-bordered.table-condensed %thead @@ -13,13 +13,13 @@ %th{class: 'col-xs-5'}= t('shared.email') %th{class: 'col-xs-3'}= t('shared.action') %tbody - - @domain.tech_contacts.each do |x| + - @domain.domain_contacts.tech.each do |x| %tr - %td= link_to(x, root_path) - %td= x.email + %td= link_to(x.contact, root_path) + %td= x.contact.email %td = link_to(t('shared.edit'), root_path, class: 'btn btn-primary btn-xs') - = link_to(t('shared.detach'), admin_domain_tech_contact_path(@domain, x), method: :delete, data: { confirm: t('shared.are_you_sure') }, class: 'btn btn-warning btn-xs') + = link_to(t('shared.detach'), admin_domain_domain_contact_path(@domain, x), method: :delete, data: { confirm: t('shared.are_you_sure') }, class: 'btn btn-warning btn-xs') - if @domain.errors.messages[:tech_contacts] %tfoot - @domain.errors.messages[:tech_contacts].each do |x| diff --git a/config/locales/en.yml b/config/locales/en.yml index 871173ae1..c2b04d2ba 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -51,6 +51,7 @@ en: activerecord: errors: models: + contact: attributes: code: @@ -67,6 +68,7 @@ en: blank: "Required parameter missing - ident" domains: exist: 'Object association prohibits operation' + epp_domain: &epp_domain_ar_attributes attributes: name_dirty: @@ -98,8 +100,10 @@ en: invalid: 'Status is invalid' not_found: 'Status was not found' taken: 'Status already exists on this domain' + domain: <<: *epp_domain_ar_attributes + nameserver: attributes: hostname: @@ -109,14 +113,22 @@ en: invalid: 'IPv4 is invalid' ipv6: invalid: 'IPv6 is invalid' + + domain_contact: + attributes: + contact: + taken: 'Contact already exists on this domain!' + setting: attributes: code: taken: 'Code already exists' + domain_status: attributes: setting_id: taken: 'Status already exists on this domain' + attributes: epp_domain: &epp_domain_attributes name: 'Domain name' @@ -130,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' @@ -211,9 +223,8 @@ en: edit_domain: 'Edit domain' contact_was_not_found: 'Contact was not found!' contact_already_exists: 'Contact already exists on this domain!' + failed_to_add_contact: 'Failed to add contact!' contact_added: 'Contact added!' - new_tech_contact: 'New tech contact' - new_admin_contact: 'New admin contact' contact_detached: 'Contact detached!' failed_to_detach_contact: 'Failed to detach contact!' new_domain_status: 'New domain status' @@ -225,3 +236,4 @@ en: status_deleted: 'Status deleted!' failed_to_delete_status: 'Failed to delete status!' tech_contact: 'Tech contact' + new_domain_contact: 'New contact' diff --git a/spec/features/domain_management_spec.rb b/spec/features/domain_management_spec.rb index dcf1f4938..0a48079ae 100644 --- a/spec/features/domain_management_spec.rb +++ b/spec/features/domain_management_spec.rb @@ -103,14 +103,14 @@ feature 'Domain management', type: :feature do within('#tech_contacts') { click_on 'Add' } c = Contact.last - fill_in('Tech contact', with: c.code, fill_options: { blur: false }) + fill_in('Contact', with: c.code, fill_options: { blur: false }) # TODO: Wait for poltergeist to support blur option, then uncomment these lines: # expect(page).to have_text(c.code) # click_on(c.code) # expect(find_field('Tech contact').value).to eq(c.code) # temporary solution: - page.execute_script("$('#contact_id').val('#{c.id}')") + page.execute_script("$('#domain_contact_contact_id').val('#{c.id}')") click_on 'Save'