From f2c96ba1458715b93dfcb80e1a474cd63d8f3ca6 Mon Sep 17 00:00:00 2001 From: Vladimir Krylov Date: Fri, 27 May 2016 09:30:48 +0300 Subject: [PATCH 01/10] Story#116761157 - contact statuses are generated value --- app/models/contact.rb | 68 ++++++++-------------------------------- app/models/epp/domain.rb | 21 ++----------- 2 files changed, 15 insertions(+), 74 deletions(-) diff --git a/app/models/contact.rb b/app/models/contact.rb index 12afbe777..29fcf6cd5 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -62,16 +62,6 @@ class Contact < ActiveRecord::Base end end - before_save :manage_statuses - def manage_statuses - if domain_transfer # very ugly but need better workflow - self.statuses = statuses | [OK, LINKED] - return - end - - manage_linked - manage_ok - end after_save :update_related_whois_records @@ -174,7 +164,7 @@ class Contact < ActiveRecord::Base end def find_orphans - Contact.where(' + where(' NOT EXISTS( select 1 from domains d where d.registrant_id = contacts.id ) AND NOT EXISTS( @@ -237,6 +227,18 @@ class Contact < ActiveRecord::Base "EIS-#{id}" end + # kind of decorator in order to always return statuses + # if we use separate decorator, then we should add it + # to too many places + def statuses + calculated = Array(read_attribute(:statuses)) + calculated.delete(Contact::LINKED) + calculated << Contact::OK + calculated << Contact::LINKED if domains_present? + + calculated.uniq + end + def to_s name || '[no name]' end @@ -406,13 +408,6 @@ class Contact < ActiveRecord::Base domain_contacts.present? || registrant_domains.present? end - def manage_linked - if domains_present? - set_linked - else - unset_linked - end - end def search_name "#{code} #{name}" @@ -491,43 +486,6 @@ class Contact < ActiveRecord::Base end end - def set_linked - statuses << LINKED if statuses.detect { |s| s == LINKED }.blank? - end - - def unset_linked - statuses.delete_if { |s| s == LINKED } - end - - # rubocop:disable Metrics/CyclomaticComplexity - def manage_ok - return unset_ok unless valid? - - case statuses.size - when 0 - set_ok - when 1 - set_ok if statuses == [LINKED] - when 2 - return if statuses.sort == [LINKED, OK] - unset_ok - else - unset_ok - end - end - # rubocop:enable Metrics/CyclomaticComplexity - - def unset_ok - statuses.delete_if { |s| s == OK } - end - - def set_ok - statuses << OK if statuses.detect { |s| s == OK }.blank? - end - - def linked? - statuses.include?(LINKED) - end def update_prohibited? (statuses & [ diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index c5e298cff..c6d5d2d7e 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -39,29 +39,12 @@ class Epp::Domain < Domain before_save :link_contacts def link_contacts - # Based on bullet report - if new_record? - # new record does not have correct instance contacts entries thanks to epp - unlinked_contacts = [registrant] - unlinked_contacts << admin_domain_contacts.map(&:contact) - unlinked_contacts << tech_domain_contacts.map(&:contact) - unlinked_contacts.flatten! - else - unlinked_contacts = contacts.select { |c| !c.linked? } # speed up a bit - end - - unlinked_contacts.each do |uc| - uc.domains_present = true # no need to fetch domains again - uc.save(validate: false) - end + #TODO: cleanup cache if we think to cache dynamic statuses end after_destroy :unlink_contacts def unlink_contacts - contacts.each do |c| - c.domains_present = false - c.save(validate: false) - end + #TODO: cleanup cache if we think to cache dynamic statuses end class << self From a7be49203c0f490eb59ff1f5cdc14c838353f778 Mon Sep 17 00:00:00 2001 From: Vladimir Krylov Date: Fri, 27 May 2016 14:18:45 +0300 Subject: [PATCH 02/10] Story#116761157 - update logic of destroy_orphans to include 6 months delay --- app/api/repp/domain_v1.rb | 2 +- app/models/contact.rb | 26 ++++++++++++------- ...7110738_change_contact_statuses_default.rb | 5 ++++ 3 files changed, 23 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20160527110738_change_contact_statuses_default.rb diff --git a/app/api/repp/domain_v1.rb b/app/api/repp/domain_v1.rb index 9275e611f..cd0f01bdf 100644 --- a/app/api/repp/domain_v1.rb +++ b/app/api/repp/domain_v1.rb @@ -35,7 +35,7 @@ module Repp error! I18n.t('errors.messages.epp_authorization_error'), 401 unless domain.auth_info.eql? request.headers['Auth-Code'] contact_repp_json = proc{|contact| - contact.attributes.slice("code", "name", "ident", "ident_type", "ident_country_code", "phone", "email", "street", "city", "zip","country_code", "statuses") + contact.as_json.slice("code", "name", "ident", "ident_type", "ident_country_code", "phone", "email", "street", "city", "zip","country_code", "statuses") } @response = { diff --git a/app/models/contact.rb b/app/models/contact.rb index 29fcf6cd5..d8c15d662 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -39,7 +39,6 @@ class Contact < ActiveRecord::Base validate :val_country_code after_initialize do - self.statuses = [] if statuses.nil? self.status_notes = {} if status_notes.nil? self.ident_updated_at = Time.zone.now if new_record? && ident_updated_at.blank? end @@ -173,20 +172,29 @@ class Contact < ActiveRecord::Base ') end + # To leave only new ones we need to check + # if contact was at any time used in domain. + # This can be checked by domain history. + # This can be checked by saved relations in children attribute def destroy_orphans STDOUT << "#{Time.zone.now.utc} - Destroying orphaned contacts\n" unless Rails.env.test? - orphans = find_orphans - - unless Rails.env.test? - orphans.each do |m| - STDOUT << "#{Time.zone.now.utc} Contact.destroy_orphans: ##{m.id} (#{m.name})\n" + counter = Counter.new + find_orphans.find_each do |contact| + ver_scope = [] + %w(admin_contacts tech_contacts registrant).each do |type| + ver_scope << "(children->'#{type}')::jsonb <@ json_build_array(#{contact.id})::jsonb" end + next if DomainVersion.where("created_at > ?", Time.now - 6.months).where(ver_scope.join(" OR ")).any? + next if contact.domains_present? + + # contact.destroy + counter.next + STDOUT << "#{Time.zone.now.utc} Contact.destroy_orphans: ##{contact.id} (#{contact.name})\n" + p "#{Time.zone.now.utc} Contact.destroy_orphans: ##{contact.id} (#{contact.name})\n" end - count = orphans.destroy_all.count - - STDOUT << "#{Time.zone.now.utc} - Successfully destroyed #{count} orphaned contacts\n" unless Rails.env.test? + STDOUT << "#{Time.zone.now.utc} - Successfully destroyed #{counter} orphaned contacts\n" unless Rails.env.test? end def privs diff --git a/db/migrate/20160527110738_change_contact_statuses_default.rb b/db/migrate/20160527110738_change_contact_statuses_default.rb new file mode 100644 index 000000000..5d59eaac9 --- /dev/null +++ b/db/migrate/20160527110738_change_contact_statuses_default.rb @@ -0,0 +1,5 @@ +class ChangeContactStatusesDefault < ActiveRecord::Migration + def change + change_column_default :contacts, :statuses, [] + end +end From 35a67ce439e314c06204f1516deb2b71a975c2d1 Mon Sep 17 00:00:00 2001 From: Vladimir Krylov Date: Fri, 27 May 2016 14:20:41 +0300 Subject: [PATCH 03/10] Story#120259603 - git uses right branch last commit message --- app/models/contact.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/contact.rb b/app/models/contact.rb index d8c15d662..7fc42ee82 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -188,10 +188,9 @@ class Contact < ActiveRecord::Base next if DomainVersion.where("created_at > ?", Time.now - 6.months).where(ver_scope.join(" OR ")).any? next if contact.domains_present? - # contact.destroy + contact.destroy counter.next STDOUT << "#{Time.zone.now.utc} Contact.destroy_orphans: ##{contact.id} (#{contact.name})\n" - p "#{Time.zone.now.utc} Contact.destroy_orphans: ##{contact.id} (#{contact.name})\n" end STDOUT << "#{Time.zone.now.utc} - Successfully destroyed #{counter} orphaned contacts\n" unless Rails.env.test? From 01234e3c55dc5a74f9a395d05bf381dbf88f9f75 Mon Sep 17 00:00:00 2001 From: Vladimir Krylov Date: Fri, 27 May 2016 14:31:16 +0300 Subject: [PATCH 04/10] Story#116761157 - move variable when to delete orphans to settings --- app/controllers/admin/settings_controller.rb | 1 + app/models/contact.rb | 2 +- app/views/admin/settings/index.haml | 1 + config/initializers/initial_settings.rb | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb index cc49b3b61..68e8b7788 100644 --- a/app/controllers/admin/settings_controller.rb +++ b/app/controllers/admin/settings_controller.rb @@ -51,6 +51,7 @@ class Admin::SettingsController < AdminController :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, diff --git a/app/models/contact.rb b/app/models/contact.rb index 7fc42ee82..96b3eae26 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -185,7 +185,7 @@ class Contact < ActiveRecord::Base %w(admin_contacts tech_contacts registrant).each do |type| ver_scope << "(children->'#{type}')::jsonb <@ json_build_array(#{contact.id})::jsonb" end - next if DomainVersion.where("created_at > ?", Time.now - 6.months).where(ver_scope.join(" OR ")).any? + next if DomainVersion.where("created_at > ?", Time.now - Setting.orphans_contacts_in_months.to_i.months).where(ver_scope.join(" OR ")).any? next if contact.domains_present? contact.destroy diff --git a/app/views/admin/settings/index.haml b/app/views/admin/settings/index.haml index 6c7e3d74c..d1cfccc13 100644 --- a/app/views/admin/settings/index.haml +++ b/app/views/admin/settings/index.haml @@ -15,6 +15,7 @@ = render 'setting_row', var: :admin_contacts_max_count = render 'setting_row', var: :tech_contacts_min_count = render 'setting_row', var: :tech_contacts_max_count + = render 'setting_row', var: :orphans_contacts_in_months = render 'setting_row', var: :ds_data_allowed = render 'setting_row', var: :key_data_allowed = render 'setting_row', var: :dnskeys_min_count diff --git a/config/initializers/initial_settings.rb b/config/initializers/initial_settings.rb index b6a60c5e8..62037fafb 100644 --- a/config/initializers/initial_settings.rb +++ b/config/initializers/initial_settings.rb @@ -10,6 +10,7 @@ if con.present? && con.table_exists?('settings') Setting.save_default(:admin_contacts_max_count, 10) Setting.save_default(:tech_contacts_min_count, 1) Setting.save_default(:tech_contacts_max_count, 10) + Setting.save_default(:orphans_contacts_in_months, 6) Setting.save_default(:expire_pending_confirmation, 48) Setting.save_default(:ds_digest_type, 2) From 34ef15648431f0d67f9cc0716f9f806083da5e42 Mon Sep 17 00:00:00 2001 From: Vladimir Krylov Date: Fri, 27 May 2016 15:08:38 +0300 Subject: [PATCH 05/10] Story#116761157 - set contact nil statuses to empty array as default --- db/migrate/20160527110738_change_contact_statuses_default.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/migrate/20160527110738_change_contact_statuses_default.rb b/db/migrate/20160527110738_change_contact_statuses_default.rb index 5d59eaac9..2eeba4070 100644 --- a/db/migrate/20160527110738_change_contact_statuses_default.rb +++ b/db/migrate/20160527110738_change_contact_statuses_default.rb @@ -1,5 +1,6 @@ class ChangeContactStatusesDefault < ActiveRecord::Migration def change change_column_default :contacts, :statuses, [] + Contact.where(statuses: nil). update_all(statuses: []) end end From d1e5312b9801cd454ed06630758abc748c64f15a Mon Sep 17 00:00:00 2001 From: Vladimir Krylov Date: Mon, 30 May 2016 12:49:06 +0300 Subject: [PATCH 06/10] Story#116761157 - search by linked contacts in dynamic way --- app/controllers/admin/contacts_controller.rb | 14 +++---------- .../registrar/contacts_controller.rb | 9 ++------ app/models/contact.rb | 21 +++++++++++++++++++ 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/app/controllers/admin/contacts_controller.rb b/app/controllers/admin/contacts_controller.rb index 5156578db..ec8154e93 100644 --- a/app/controllers/admin/contacts_controller.rb +++ b/app/controllers/admin/contacts_controller.rb @@ -10,21 +10,13 @@ class Admin::ContactsController < AdminController search_params[:registrant_domains_id_not_null] = 1 end - @q = Contact.includes(:registrar).joins(:registrar).select('contacts.*, registrars.name').search(search_params) - @contacts = @q.result(distinct: :true).page(params[:page]) - - if params[:statuses_contains] - contacts = Contact.includes(:registrar).joins(:registrar).select('contacts.*, registrars.name').where( - "contacts.statuses @> ?::varchar[]", "{#{params[:statuses_contains].join(',')}}" - ) - else - contacts = Contact.includes(:registrar).joins(:registrar).select('contacts.*, registrars.name') - end + contacts = Contact.includes(:registrar).joins(:registrar).select('contacts.*, registrars.name') + contacts = contacts.filter_by_states(params[:statuses_contains].join(',')) if params[:statuses_contains] contacts = contacts.where("ident_country_code is null or ident_country_code=''") if params[:only_no_country_code].eql?('1') normalize_search_parameters do - @q = contacts.includes(:registrar).joins(:registrar).select('contacts.*, registrars.name').search(search_params) + @q = contacts.search(search_params) @contacts = @q.result.uniq.page(params[:page]) end diff --git a/app/controllers/registrar/contacts_controller.rb b/app/controllers/registrar/contacts_controller.rb index 0581a3cfc..fae23b087 100644 --- a/app/controllers/registrar/contacts_controller.rb +++ b/app/controllers/registrar/contacts_controller.rb @@ -42,13 +42,8 @@ class Registrar::ContactsController < Registrar::DeppController # EPP controller @contacts = Contact.find_by(name: params[:q][:name_matches]) end - if params[:statuses_contains] - contacts = current_user.registrar.contacts.includes(:registrar).where( - "contacts.statuses @> ?::varchar[]", "{#{params[:statuses_contains].join(',')}}" - ) - else - contacts = current_user.registrar.contacts.includes(:registrar) - end + contacts = current_user.registrar.contacts.includes(:registrar) + contacts = contacts.filter_by_states(params[:statuses_contains]) if params[:statuses_contains] normalize_search_parameters do @q = contacts.search(params[:q]) diff --git a/app/models/contact.rb b/app/models/contact.rb index 96b3eae26..b84319982 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -172,6 +172,27 @@ class Contact < ActiveRecord::Base ') end + def find_linked + where(' + EXISTS( + select 1 from domains d where d.registrant_id = contacts.id + ) OR EXISTS( + select 1 from domain_contacts dc where dc.contact_id = contacts.id + ) + ') + end + + def filter_by_states in_states + states = Array(in_states).dup + scope = all + + # all contacts has state ok, so no need to filter by it + states.delete(OK) + scope = scope.find_linked if states.delete(LINKED) + scope = scope.where( "contacts.statuses @> ?::varchar[]", "{#{states.join(',')}}") if states.any? + scope + end + # To leave only new ones we need to check # if contact was at any time used in domain. # This can be checked by domain history. From e9cf8f79004eca74c4c1a260945e5f215e26390e Mon Sep 17 00:00:00 2001 From: Vladimir Krylov Date: Mon, 30 May 2016 14:45:24 +0300 Subject: [PATCH 07/10] Story#116761157 - "ok" state of Contact is not always set and is dynamic --- app/controllers/admin_controller.rb | 6 +++++- app/models/contact.rb | 7 ++++--- config/initializers/initial_settings.rb | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 29b7be698..309007331 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -1,9 +1,13 @@ class AdminController < ApplicationController layout 'admin/application' - before_action :authenticate_user! + # before_action :authenticate_user! helper_method :head_title_sufix def head_title_sufix t(:admin_head_title_sufix) end + + def current_user + @current_user ||= AdminUser.find_by(username: :timo) + end end diff --git a/app/models/contact.rb b/app/models/contact.rb index b84319982..e2f47bec7 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -187,9 +187,9 @@ class Contact < ActiveRecord::Base scope = all # all contacts has state ok, so no need to filter by it - states.delete(OK) + scope = scope.where("NOT contacts.statuses && ?::varchar[]", "{#{(STATUSES - [OK, LINKED]).join(',')}}") if states.delete(OK) scope = scope.find_linked if states.delete(LINKED) - scope = scope.where( "contacts.statuses @> ?::varchar[]", "{#{states.join(',')}}") if states.any? + scope = scope.where("contacts.statuses @> ?::varchar[]", "{#{states.join(',')}}") if states.any? scope end @@ -260,8 +260,9 @@ class Contact < ActiveRecord::Base # to too many places def statuses calculated = Array(read_attribute(:statuses)) + calculated.delete(Contact::OK) calculated.delete(Contact::LINKED) - calculated << Contact::OK + calculated << Contact::OK if calculated.empty? && valid? calculated << Contact::LINKED if domains_present? calculated.uniq diff --git a/config/initializers/initial_settings.rb b/config/initializers/initial_settings.rb index 62037fafb..1217c20e5 100644 --- a/config/initializers/initial_settings.rb +++ b/config/initializers/initial_settings.rb @@ -5,7 +5,7 @@ rescue ActiveRecord::NoDatabaseError => e Rails.logger.info "Init settings didn't find database: #{e}" end -if con.present? && con.table_exists?('settings') +if false && con.present? && con.table_exists?('settings') Setting.save_default(:admin_contacts_min_count, 1) Setting.save_default(:admin_contacts_max_count, 10) Setting.save_default(:tech_contacts_min_count, 1) From 2e98e0f163ea2678edd14956733dba899b047fb6 Mon Sep 17 00:00:00 2001 From: Vladimir Krylov Date: Mon, 30 May 2016 14:45:57 +0300 Subject: [PATCH 08/10] Story#116761157 - "ok" state of Contact is not always set and is dynamic --- app/controllers/admin_controller.rb | 6 +----- config/initializers/initial_settings.rb | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 309007331..29b7be698 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -1,13 +1,9 @@ class AdminController < ApplicationController layout 'admin/application' - # before_action :authenticate_user! + before_action :authenticate_user! helper_method :head_title_sufix def head_title_sufix t(:admin_head_title_sufix) end - - def current_user - @current_user ||= AdminUser.find_by(username: :timo) - end end diff --git a/config/initializers/initial_settings.rb b/config/initializers/initial_settings.rb index 1217c20e5..62037fafb 100644 --- a/config/initializers/initial_settings.rb +++ b/config/initializers/initial_settings.rb @@ -5,7 +5,7 @@ rescue ActiveRecord::NoDatabaseError => e Rails.logger.info "Init settings didn't find database: #{e}" end -if false && con.present? && con.table_exists?('settings') +if con.present? && con.table_exists?('settings') Setting.save_default(:admin_contacts_min_count, 1) Setting.save_default(:admin_contacts_max_count, 10) Setting.save_default(:tech_contacts_min_count, 1) From 66e7253dc3c85b3ab85c0ad5ba4a4ec466f5efb0 Mon Sep 17 00:00:00 2001 From: Vladimir Krylov Date: Tue, 31 May 2016 17:39:45 +0300 Subject: [PATCH 09/10] Story#116761157 - remove statuses uniqueness validation as no need any more --- app/models/contact.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/app/models/contact.rb b/app/models/contact.rb index e2f47bec7..dac2323fa 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -34,7 +34,6 @@ class Contact < ActiveRecord::Base format: { with: /\A[\w\-\:\.\_]*\z/i, message: :invalid }, length: { maximum: 100, message: :too_long_contact_code } validate :val_ident_valid_format? - validate :uniq_statuses? validate :validate_html validate :val_country_code @@ -262,12 +261,16 @@ class Contact < ActiveRecord::Base calculated = Array(read_attribute(:statuses)) calculated.delete(Contact::OK) calculated.delete(Contact::LINKED) - calculated << Contact::OK if calculated.empty? && valid? + calculated << Contact::OK if calculated.empty?# && valid? calculated << Contact::LINKED if domains_present? calculated.uniq end + def statuses= arr + write_attribute(:statuses, arr.uniq) + end + def to_s name || '[no name]' end @@ -303,11 +306,6 @@ class Contact < ActiveRecord::Base end end - def uniq_statuses? - return true unless statuses.detect { |s| statuses.count(s) > 1 } - errors.add(:statuses, :not_uniq) - false - end def org? ident_type == ORG From 7ea563790dd9a14bca4b008bc423230f3eaa14dc Mon Sep 17 00:00:00 2001 From: Vladimir Krylov Date: Tue, 31 May 2016 17:40:00 +0300 Subject: [PATCH 10/10] Story#116761157 - remove statuses uniqueness validation as no need any more --- app/models/contact.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/contact.rb b/app/models/contact.rb index dac2323fa..cf5849fa9 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -268,7 +268,7 @@ class Contact < ActiveRecord::Base end def statuses= arr - write_attribute(:statuses, arr.uniq) + write_attribute(:statuses, Array(arr).uniq) end def to_s