Merge pull request #583 from internetee/refactor-settings

Refactor settings

app/assets/stylesheets/admin/admin.sass

@@ -50,9 +50,6 @@ strong
 blockquote
   margin: 1em 40px
 
-dfn
-  font-style: italic
-
 mark
   background: #ff0
   color: #000
@@ -94,12 +91,6 @@ sup
 sub
   bottom: -0.25em
 
-.title-row
-  margin-bottom: 22px
-
-.app-nav
-  padding-top: 7px
-
 .general-tab
   padding-top: 30px
   padding-right: 20px
@@ -131,16 +122,6 @@ sub
       font-weight: bold
       color: #333
 
-
-.subactions
-  h4
-    margin-bottom: 20px
-    margin-top: 10px
-  min-height: 600px
-
-.sidebar
-  min-height: 400px
-
 .content
   margin-right: 240px
   margin-left: 0
@@ -148,15 +129,6 @@ sub
 h4
   margin: 0
 
-.top-actions
-  margin-top: 12px
-  margin-bottom: 16px
-  float: right
-
-.domify
-  td
-    vertical-align: middle !important
-
 body.login
   padding-top: 40px
   padding-bottom: 40px
@@ -196,12 +168,6 @@ body.login
     .form-control:focus
       z-index: 2
 
-.error-tab > a
-  color: #a94442 !important
-
-.edit-highlight
-  background-color: #E7E7E7
-
 .navbar-brand
   line-height: 12px
   padding-top: 20px

app/assets/stylesheets/shared/general.sass

@@ -1,12 +1,6 @@
 body > .container
   min-height: 800px
 
-.error-tab > a
-  color: #a94442 !important
-
-.edit-highlight
-  background-color: #E7E7E7
-
 .navbar-brand
   line-height: 12px
   padding-top: 20px

app/controllers/admin/settings_controller.rb

@@ -1,7 +1,6 @@
 module Admin
   class SettingsController < BaseController
     load_and_authorize_resource
-    before_action :set_setting_group, only: [:show, :update]
 
     def index
       @settings = Setting.unscoped
@@ -14,7 +13,7 @@ module Admin
           Setting[k] = v
         end
 
-        flash[:notice] = I18n.t('records_updated')
+        flash[:notice] = t('.saved')
         redirect_to [:admin, :settings]
       else
         flash[:alert] = @errors.values.uniq.join(", ")
@@ -22,74 +21,16 @@ module Admin
       end
     end
 
-    def show;
-    end
-
-    def update
-      if @setting_group.update(setting_group_params)
-        flash[:notice] = I18n.t('setting_updated')
-        redirect_to [:admin, @setting_group]
-      else
-        flash[:alert] = I18n.t('failed_to_update_setting')
-        render 'show'
-      end
-    end
-
     private
 
-    def set_setting_group
-      @setting_group = SettingGroup.find(params[:id])
-    end
-
-    def setting_group_params
-      params.require(:setting_group).permit(settings_attributes: [:value, :id])
-    end
-
-    def casted_settings # rubocop:disable Metrics/MethodLength
+    def casted_settings
       settings = {}
 
-      ints = [
-        :admin_contacts_min_count,
-        :admin_contacts_max_count,
-        :tech_contacts_min_count,
-        :tech_contacts_max_count,
-        :orphans_contacts_in_months,
-        :ds_digest_type,
-        :dnskeys_min_count,
-        :dnskeys_max_count,
-        :ns_min_count,
-        :ns_max_count,
-        :transfer_wait_time,
-        :invoice_number_min,
-        :invoice_number_max,
-        :days_to_keep_business_registry_cache,
-        :days_to_keep_invoices_active,
-        :days_to_keep_overdue_invoices_active,
-        :days_to_renew_domain_before_expire,
-        :expire_warning_period,
-        :redemption_grace_period,
-        :expire_pending_confirmation
-      ]
-
-      floats = [:registry_vat_prc, :minimum_deposit]
-
-      booleans = [
-        :ds_data_allowed,
-        :key_data_allowed,
-        :client_side_status_editing_enabled,
-        :registrar_ip_whitelist_enabled,
-        :api_ip_whitelist_enabled,
-        :request_confrimation_on_registrant_change_enabled,
-        :request_confirmation_on_domain_deletion_enabled,
-        :nameserver_required,
-        :address_processing
-      ]
-
       params[:settings].each do |k, v|
         settings[k] = v
-        settings[k] = v.to_i if ints.include?(k.to_sym)
-        settings[k] = v.to_f if floats.include?(k.to_sym)
-        settings[k] = (v == 'true' ? true : false) if booleans.include?(k.to_sym)
+        settings[k] = v.to_i if Setting.integer_settings.include?(k.to_sym)
+        settings[k] = v.to_f if Setting.float_settings.include?(k.to_sym)
+        settings[k] = (v == 'true' ? true : false) if Setting.boolean_settings.include?(k.to_sym)
       end
 
       settings

app/models/setting.rb

@@ -20,4 +20,50 @@ class Setting < RailsSettings::CachedSettings
 
     return errors
   end
+
+  def self.integer_settings
+    %i[
+      admin_contacts_min_count
+      admin_contacts_max_count
+      tech_contacts_min_count
+      tech_contacts_max_count
+      orphans_contacts_in_months
+      ds_digest_type
+      dnskeys_min_count
+      dnskeys_max_count
+      ns_min_count
+      ns_max_count
+      transfer_wait_time
+      invoice_number_min
+      invoice_number_max
+      days_to_keep_business_registry_cache
+      days_to_keep_invoices_active
+      days_to_keep_overdue_invoices_active
+      days_to_renew_domain_before_expire
+      expire_warning_period
+      redemption_grace_period
+      expire_pending_confirmation
+    ]
+  end
+
+  def self.float_settings
+    %i[
+      registry_vat_prc
+      minimum_deposit
+    ]
+  end
+
+  def self.boolean_settings
+    %i[
+      ds_data_allowed
+      key_data_allowed
+      client_side_status_editing_enabled
+      registrar_ip_whitelist_enabled
+      api_ip_whitelist_enabled
+      request_confrimation_on_registrant_change_enabled
+      request_confirmation_on_domain_deletion_enabled
+      nameserver_required
+      address_processing
+    ]
+  end
 end

app/views/admin/base/_menu.haml

@@ -11,7 +11,7 @@
     - if can?(:access, :settings_menu)
       %li.dropdown
         %a.dropdown-toggle{"data-toggle" => "dropdown", href: "#"}
-          = t(:settings)
+          = t('.settings')
           %span.caret
         %ul.dropdown-menu{role: "menu"}
           %li.dropdown-header= t('.users')
@@ -30,7 +30,7 @@
           %li= link_to t('.contact_history'), admin_contact_versions_path
           %li.divider
           %li.dropdown-header= t(:system)
-          %li= link_to t(:settings), admin_settings_path
+          %li= link_to t('.settings'), admin_settings_path
           %li= link_to t('.zones'), admin_zones_path
           %li= link_to t('.blocked_domains'), admin_blocked_domains_path
           %li= link_to t('.reserved_domains'), admin_reserved_domains_path

app/views/admin/settings/_setting_row.haml

@@ -1,9 +1,9 @@
 - value = Setting.send(var)
 %tr{class: (@errors && @errors.has_key?(var.to_s) && "danger")}
-  %td= t(var)
+  %td.col-md-6= var.to_s.humanize
   - if [TrueClass, FalseClass].include?(value.class)
-    %td
-      = hidden_field_tag("[settings][#{var}]", '')
+    %td.col-md-6
+      = hidden_field_tag("[settings][#{var}]", '', id: nil)
       = check_box_tag("[settings][#{var}]", true, value)
   - else
-    %td= text_field_tag("[settings][#{var}]", value)
+    %td.col-md-6= text_field_tag("[settings][#{var}]", value, class: 'form-control')

app/views/admin/settings/index.haml

@@ -1,15 +1,13 @@
-= render 'shared/title', name: t(:settings)
+.page-header
+  .h1
+    = t('.title')
 
 = form_tag [:admin, :settings] do
   .panel.panel-default
-    .panel-heading.clearfix
-      = t(:domain_validation_rules)
+    .panel-heading
+      = t('.domain_validation')
     .table-responsive
       %table.table.table-hover.table-bordered.table-condensed
-        %thead
-          %tr
-            %th{class: 'col-xs-6'}= t(:setting)
-            %th{class: 'col-xs-6'}= t(:value)
         %tbody
           = render 'setting_row', var: :admin_contacts_min_count
           = render 'setting_row', var: :admin_contacts_max_count
@@ -26,14 +24,10 @@
           = render 'setting_row', var: :expire_pending_confirmation
 
   .panel.panel-default
-    .panel-heading.clearfix
-      = t(:domain_expiring)
+    .panel-heading
+      = t('.domain_expiration')
     .table-responsive
       %table.table.table-hover.table-bordered.table-condensed
-        %thead
-          %tr
-            %th{class: 'col-xs-6'}= t(:setting)
-            %th{class: 'col-xs-6'}= t(:value)
         %tbody
           = render 'setting_row', var: :days_to_renew_domain_before_expire
           = render 'setting_row', var: :expire_warning_period
@@ -41,14 +35,10 @@
           = render 'setting_row', var: :expiration_reminder_mail
 
   .panel.panel-default
-    .panel-heading.clearfix
-      = t(:other)
+    .panel-heading
+      = t('.other')
     .table-responsive
       %table.table.table-hover.table-bordered.table-condensed
-        %thead
-          %tr
-            %th{class: 'col-xs-6'}= t(:setting)
-            %th{class: 'col-xs-6'}= t(:value)
         %tbody
           = render 'setting_row', var: :transfer_wait_time
           = render 'setting_row', var: :ds_digest_type
@@ -61,14 +51,10 @@
           = render 'setting_row', var: :address_processing
 
   .panel.panel-default
-    .panel-heading.clearfix
-      = t(:billing_settings)
+    .panel-heading
+      = t('.billing')
     .table-responsive
       %table.table.table-hover.table-bordered.table-condensed
-        %thead
-          %tr
-            %th{class: 'col-xs-6'}= t(:setting)
-            %th{class: 'col-xs-6'}= t(:value)
         %tbody
           = render 'setting_row', var: :invoice_number_min
           = render 'setting_row', var: :invoice_number_max
@@ -91,14 +77,10 @@
           = render 'setting_row', var: :registry_swift
 
   .panel.panel-default
-    .panel-heading.clearfix
-      = t(:registry_settings)
+    .panel-heading
+      = t('.contacts')
     .table-responsive
       %table.table.table-hover.table-bordered.table-condensed
-        %thead
-          %tr
-            %th{class: 'col-xs-6'}= t(:setting)
-            %th{class: 'col-xs-6'}= t(:value)
         %tbody
           = render 'setting_row', var: :registry_juridical_name
           = render 'setting_row', var: :registry_reg_no
@@ -113,4 +95,4 @@
 
   .row
     .col-md-12.text-right
-      %button.btn.btn-primary=t(:save)
+      = submit_tag(t('.save_btn'), class: 'btn btn-success', name: nil)

config/locales/admin/menu.en.yml

@@ -2,6 +2,7 @@ en:
   admin:
     base:
       menu:
+        settings: Settings
         users: Users
         api_users: API users
         admin_users: Admin users

config/locales/admin/settings.en.yml

@@ -0,0 +1,14 @@
+en:
+  admin:
+    settings:
+      index:
+        title: Settings
+        domain_validation: Domain validation
+        other: Other
+        domain_expiration: Domain expiration
+        billing: Billing
+        contacts: Contacts
+        save_btn: Save
+
+      create:
+        saved: Settings have been successfully updated

config/locales/en.yml

@@ -264,13 +264,11 @@ en:
       required_ident_attribute_missing: "Required ident attribute missing: %{key}"
 
   code: 'Code'
-  value: 'Value'
   action: 'Action'
   edit: 'Edit'
   save: 'Save'
   log_out: 'Log out (%{user})'
   system: 'System'
-  settings: 'Settings'
   domains: 'Domains'
   registrars: 'Registrars'
   valid_to: 'Valid to'
@@ -411,15 +409,12 @@ en:
   record_created: 'Record created'
   failed_to_create_record: 'Failed to create record'
   record_updated: 'Record updated'
-  records_updated: 'Records updated'
   failed_to_update_record: 'Failed to update record'
   record_deleted: 'Record deleted'
   failed_to_delete_record: 'Failed to delete record'
 
   authentication_error: 'Authentication error'
 
-  setting: 'Setting'
-
   transfer_requested: 'Transfer requested.'
   message_was_not_found: 'Message was not found'
   host_obj_is_not_allowed: 'hostObj object is not allowed'
@@ -427,12 +422,6 @@ en:
   only_one_parameter_allowed: 'Only one parameter allowed: %{param_1} or %{param_2}'
   exactly_one_parameter_required: 'Exactly one parameter required: %{params}'
   required_parameter_missing_choice: 'Required parameter missing: %{param_1} or %{param_2}'
-  transfer_wait_time: 'Transfer wait time'
-  ns_min_count: 'Nameserver minimum count'
-  ns_max_count: 'Nameserver maximum count'
-  dnskeys_min_count: 'DNS keys minimum count'
-  dnskeys_max_count: 'DNS keys maximum count'
-  ds_data_allowed: 'DS data allowed'
   ds_data_with_key_allowed: 'Allow DS data with key'
   key_data_allowed: 'Allow key data'
   ds_digest_type: 'DS digest type'
@@ -565,7 +554,6 @@ en:
   public_key: 'Public key'
   ds_key_tag: 'DS key tag'
   ds_algorithm: 'DS algorithm'
-  ds_digest_type: 'DS digest type'
   ds_digest: 'DS digest'
   statuses: 'Statuses'
   status: 'Status'
@@ -621,7 +609,6 @@ en:
   index: 'Index'
   ident: 'Ident'
   email: 'E-mail'
-  value: 'Value'
   phone: 'Phone'
   org_name: Org name
   country: Country
@@ -809,32 +796,11 @@ en:
   tech: Tech contact
   valid: Valid
   object_is_not_eligible_for_renewal: 'Object is not eligible for renewal'
-  domain_expiring: 'Domain expiring'
-  domain_validation_rules: 'Domain validation rules'
   bank_statement_desc: 'Import file row will match only when matching following attributes: <b><br>ref number<br>payment amount<br>invoice number (the first numerical value in comment field)</b>.'
   create_bank_statement: 'Create bank statement'
   create_bank_transaction: 'Create bank transaction'
   create_new_invoice: 'Create new invoice'
   ip_is_not_whitelisted: 'IP is not whitelisted'
-  billing_settings: 'Billing settings'
-  registry_settings: 'Registry settings'
-  registry_billing_email: 'Billing e-mail'
-  registry_invoice_contact: 'Invoice contact'
-  registry_vat_no: 'VAT no.'
-  registry_vat_prc: 'VAT prc.'
-  registry_bank: 'Bank'
-  registry_iban: 'IBAN'
-  registry_swift: 'SWIFT'
-  registry_juridical_name: 'Juridical name'
-  registry_reg_no: 'Reg no.'
-  registry_email: 'E-mail'
-  registry_phone: 'Phone'
-  registry_url: 'URL'
-  registry_street: 'Street'
-  registry_city: 'City'
-  registry_state: 'State / Province'
-  registry_zip: 'Postcode'
-  registry_country_code: 'Country'
   billing_failure_credit_balance_low: 'Billing failure - credit balance low'
   create: 'Create'
   activity_type: 'Activity type'

config/routes.rb

@@ -220,7 +220,7 @@ Rails.application.routes.draw do
       end
     end
 
-    resources :settings
+    resources :settings, only: %i[index create]
 
     resources :blocked_domains do
       member do

spec/features/admin/settings/create_spec.rb

@@ -0,0 +1,13 @@
+require 'rails_helper'
+
+RSpec.feature 'Admin settings' do
+  background do
+    sign_in_to_admin_area
+  end
+
+  it 'saves settings' do
+    visit admin_settings_path
+    click_link_or_button 'Save'
+    expect(page).to have_text(t('admin.settings.create.saved'))
+  end
+end

spec/models/setting_spec.rb

@@ -1,11 +1,61 @@
 require 'rails_helper'
 
-describe Setting do
-  it 'returns value' do
-    expect(Setting.ns_min_count).to eq(2)
-    Setting.ns_min_count = '2'
-    expect(Setting.ns_min_count).to eq('2')
-    Setting.ns_min_count = true
-    expect(Setting.ns_min_count).to eq(true)
+RSpec.describe Setting do
+  describe 'integer_settings', db: false do
+    it 'returns integer settings' do
+      settings = %i[
+        admin_contacts_min_count
+        admin_contacts_max_count
+        tech_contacts_min_count
+        tech_contacts_max_count
+        orphans_contacts_in_months
+        ds_digest_type
+        dnskeys_min_count
+        dnskeys_max_count
+        ns_min_count
+        ns_max_count
+        transfer_wait_time
+        invoice_number_min
+        invoice_number_max
+        days_to_keep_business_registry_cache
+        days_to_keep_invoices_active
+        days_to_keep_overdue_invoices_active
+        days_to_renew_domain_before_expire
+        expire_warning_period
+        redemption_grace_period
+        expire_pending_confirmation
+      ]
+
+      expect(described_class.integer_settings).to eq(settings)
+    end
+  end
+
+  describe 'float_settings', db: false do
+    it 'returns float settings' do
+      settings = %i[
+        registry_vat_prc
+        minimum_deposit
+      ]
+
+      expect(described_class.float_settings).to eq(settings)
+    end
+  end
+
+  describe 'boolean_settings', db: false do
+    it 'returns boolean settings' do
+      settings = %i[
+        ds_data_allowed
+        key_data_allowed
+        client_side_status_editing_enabled
+        registrar_ip_whitelist_enabled
+        api_ip_whitelist_enabled
+        request_confrimation_on_registrant_change_enabled
+        request_confirmation_on_domain_deletion_enabled
+        nameserver_required
+        address_processing
+      ]
+
+      expect(described_class.boolean_settings).to eq(settings)
+    end
   end
 end

spec/requests/admin/settings/create_spec.rb

@@ -0,0 +1,35 @@
+require 'rails_helper'
+
+RSpec.describe 'Admin settings saving' do
+  before do
+    sign_in_to_admin_area
+  end
+
+  it 'saves integer setting' do
+    allow(Setting).to receive(:integer_settings) { %i[test_setting] }
+    post admin_settings_path, settings: { test_setting: '1' }
+    expect(Setting.test_setting).to eq(1)
+  end
+
+  it 'saves float setting' do
+    allow(Setting).to receive(:float_settings) { %i[test_setting] }
+    post admin_settings_path, settings: { test_setting: '1.2' }
+    expect(Setting.test_setting).to eq(1.2)
+  end
+
+  it 'saves boolean setting' do
+    allow(Setting).to receive(:boolean_settings) { %i[test_setting] }
+    post admin_settings_path, settings: { test_setting: 'true' }
+    expect(Setting.test_setting).to be true
+  end
+
+  it 'saves string setting' do
+    post admin_settings_path, settings: { test_setting: 'test' }
+    expect(Setting.test_setting).to eq('test')
+  end
+
+  it 'redirects to :index' do
+    post admin_settings_path, settings: { test: 'test' }
+    expect(response).to redirect_to admin_settings_path
+  end
+end

spec/support/matchers/epp/code.rb

@@ -11,7 +11,11 @@ module Matchers
       end
 
       def failure_message
-        "Expected EPP code of #{expected}, got #{actual} (#{description})"
+        "Expected EPP code of #{expected}, got #{actual} (#{code_description})"
+      end
+
+      def description
+        "should have EPP code of #{expected}"
       end
 
       private
@@ -23,7 +27,7 @@ module Matchers
         xml_document.xpath('//xmlns:result').first['code'].to_i
       end
 
-      def description
+      def code_description
         xml_document.css('result msg').text
       end
 

spec/support/settings.rb

@@ -6,7 +6,7 @@ RSpec.configure do |config|
       Setting.ds_algorithm = 2
       Setting.ds_data_allowed = true
       Setting.ds_data_with_key_allowed = true
-      Setting.key_data_allowed = true
+      Setting.key_data_allowed = false
 
       Setting.dnskeys_min_count = 0
       Setting.dnskeys_max_count = 9