From 6d5be0f9917eb061c3f5e3f4aee593401b38fc33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergei=20Ts=C3=B5ganov?= Date: Fri, 7 Jan 2022 14:06:13 +0200 Subject: [PATCH 1/7] Fix domain status history view --- app/controllers/admin/domains_controller.rb | 3 +++ app/views/admin/domains/versions.haml | 18 +++++++++++++----- config/locales/en.yml | 5 ++++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index 8946b006d..86af8e792 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -63,8 +63,11 @@ module Admin # rubocop:enable Metrics/MethodLength def versions + per_page = 10 @domain = Domain.where(id: params[:domain_id]).includes({ versions: :item }).first @versions = @domain.versions + @last_version = @versions.last + @old_versions = Kaminari.paginate_array(@versions.not_creates.reverse).page(params[:page]).per(per_page) end def keep diff --git a/app/views/admin/domains/versions.haml b/app/views/admin/domains/versions.haml index 7447ab6f8..e47463302 100644 --- a/app/views/admin/domains/versions.haml +++ b/app/views/admin/domains/versions.haml @@ -25,17 +25,25 @@ domain: @pending_domain, pending_user: @pending_user, statuses_link: true -# current version - - if @domain.versions.present? - %tr.small - = render 'admin/domains/partials/version', - domain: @domain, version: @domain.versions.last + - if @versions.present? + - if params[:page].blank? || (params[:page].present? && params[:page].to_i < 2) + %tr.small + = render 'admin/domains/partials/version', + domain: @domain, version: @last_version -# all other older versions - - @domain.versions.not_creates.reverse.each do |version| + - @old_versions.each do |version| %tr.small = render 'admin/domains/partials/version', domain: version.reify, version: version.previous +.row + .col-md-6 + = paginate @old_versions + .col-md-6.text-right + .pagination + = t(:result_count, count: @old_versions.total_count + 1) + :javascript window.addEventListener('load', function() { $(document).on('click', '.js-pending, .js-event', function(e) { diff --git a/config/locales/en.yml b/config/locales/en.yml index f80b14533..069f997ce 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -622,7 +622,10 @@ en: contact_ident: 'Contact ident' results_per_page: 'Results per page' nameserver_hostname: 'Nameserver hostname' - result_count: '%{count} results' + result_count: + zero: 'No results' + other: '%{count} results' + one: '1 result' failed_to_generate_invoice_invoice_number_limit_reached: 'Failed to generate invoice - invoice number limit reached' is_too_small_minimum_deposit_is: 'is too small. Minimum deposit is %{amount} %{currency}' poll_pending_update_confirmed_by_registrant: 'Registrant confirmed domain update' From 8c1fbb5d46aa12e4784c37945e8dbc38d98fa3bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergei=20Ts=C3=B5ganov?= Date: Fri, 7 Jan 2022 14:29:46 +0200 Subject: [PATCH 2/7] Made constant for per_page parameter --- app/controllers/admin/domains_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index 86af8e792..04b7bfbe3 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -1,5 +1,7 @@ module Admin class DomainsController < BaseController + DEFAULT_VERSIONS_PER_PAGE = 10 + before_action :set_domain, only: %i[show edit update keep] authorize_resource @@ -63,11 +65,10 @@ module Admin # rubocop:enable Metrics/MethodLength def versions - per_page = 10 @domain = Domain.where(id: params[:domain_id]).includes({ versions: :item }).first @versions = @domain.versions @last_version = @versions.last - @old_versions = Kaminari.paginate_array(@versions.not_creates.reverse).page(params[:page]).per(per_page) + @old_versions = Kaminari.paginate_array(@versions.not_creates.reverse).page(params[:page]).per(DEFAULT_VERSIONS_PER_PAGE) end def keep From 5d313f51fdde90c1e438f09723003fd6c3329ec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergei=20Ts=C3=B5ganov?= Date: Fri, 7 Jan 2022 14:36:01 +0200 Subject: [PATCH 3/7] Fixed codeclimate issue --- app/controllers/admin/domains_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index 04b7bfbe3..20f47da02 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -68,7 +68,9 @@ module Admin @domain = Domain.where(id: params[:domain_id]).includes({ versions: :item }).first @versions = @domain.versions @last_version = @versions.last - @old_versions = Kaminari.paginate_array(@versions.not_creates.reverse).page(params[:page]).per(DEFAULT_VERSIONS_PER_PAGE) + @old_versions = Kaminari.paginate_array(@versions.not_creates.reverse) + .page(params[:page]) + .per(DEFAULT_VERSIONS_PER_PAGE) end def keep From a7db42b944062cea47a7333edaa2b6f42a1f51e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergei=20Ts=C3=B5ganov?= Date: Tue, 11 Jan 2022 17:12:54 +0200 Subject: [PATCH 4/7] Prevent creating duplicate domain status history record --- app/controllers/admin/domains_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index 20f47da02..be20e83fe 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -49,7 +49,9 @@ module Admin rollback_history = @domain.json_statuses_history&.[]('admin_store_statuses_history') dp = ignore_empty_statuses @domain.is_admin = true - @domain.admin_status_update dp[:statuses] + PaperTrail.request(enabled: false) do + @domain.admin_status_update dp[:statuses] + end if @domain.update(dp) flash[:notice] = I18n.t('domain_updated') From 1f4733bdde8df3ea4939e9befc62c10ee9fbb5bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergei=20Ts=C3=B5ganov?= Date: Tue, 11 Jan 2022 20:15:00 +0200 Subject: [PATCH 5/7] Disabled PaperTrail request for admin_store_statuses_history --- app/controllers/admin/domains_controller.rb | 4 +--- app/models/domain.rb | 6 ++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/admin/domains_controller.rb b/app/controllers/admin/domains_controller.rb index be20e83fe..20f47da02 100644 --- a/app/controllers/admin/domains_controller.rb +++ b/app/controllers/admin/domains_controller.rb @@ -49,9 +49,7 @@ module Admin rollback_history = @domain.json_statuses_history&.[]('admin_store_statuses_history') dp = ignore_empty_statuses @domain.is_admin = true - PaperTrail.request(enabled: false) do - @domain.admin_status_update dp[:statuses] - end + @domain.admin_status_update dp[:statuses] if @domain.update(dp) flash[:notice] = I18n.t('domain_updated') diff --git a/app/models/domain.rb b/app/models/domain.rb index 2bab00eab..487c701a9 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -590,8 +590,10 @@ class Domain < ApplicationRecord # special handling for admin changing status def admin_status_update(update) - update_unless_locked_by_registrant(update) - update_not_by_locked_statuses(update) + PaperTrail.request(enabled: false) do + update_unless_locked_by_registrant(update) + update_not_by_locked_statuses(update) + end return unless update # check for deleted status From c83b99b6de4f9537008aa9b06f7f1dfb19c6866d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergei=20Ts=C3=B5ganov?= Date: Wed, 12 Jan 2022 10:41:56 +0200 Subject: [PATCH 6/7] Did small refactoring --- app/models/domain.rb | 20 ++++++++++++------- test/mailers/registrant_change_mailer_test.rb | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/models/domain.rb b/app/models/domain.rb index 487c701a9..873216cf1 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -469,6 +469,7 @@ class Domain < ApplicationRecord return false unless pending_update? return false unless registrant_verification_asked? return false unless registrant_verification_token == token + true end @@ -476,6 +477,7 @@ class Domain < ApplicationRecord return false unless pending_delete? return false unless registrant_verification_asked? return false unless registrant_verification_token == token + true end @@ -492,10 +494,12 @@ class Domain < ApplicationRecord def pending_delete! return true if pending_delete? + self.epp_pending_delete = true # for epp # TODO: if this were to ever return true, that would be wrong. EPP would report sucess pending return true unless registrant_verification_asked? + pending_delete_confirmation! save(validate: false) # should check if this did succeed @@ -560,6 +564,7 @@ class Domain < ApplicationRecord def pending_registrant return '' if pending_json.blank? return '' if pending_json['new_registrant_id'].blank? + Registrant.find_by(id: pending_json['new_registrant_id']) end @@ -590,11 +595,12 @@ class Domain < ApplicationRecord # special handling for admin changing status def admin_status_update(update) + return unless update + PaperTrail.request(enabled: false) do update_unless_locked_by_registrant(update) update_not_by_locked_statuses(update) end - return unless update # check for deleted status statuses.each do |s| @@ -660,7 +666,7 @@ class Domain < ApplicationRecord end def manage_automatic_statuses - if !self.class.nameserver_required? + unless self.class.nameserver_required? deactivate if nameservers.reject(&:marked_for_destruction?).empty? activate if nameservers.reject(&:marked_for_destruction?).size >= Setting.ns_min_count end @@ -681,11 +687,11 @@ class Domain < ApplicationRecord def children_log log = HashWithIndifferentAccess.new log[:admin_contacts] = admin_contact_ids - log[:tech_contacts] = tech_contact_ids - log[:nameservers] = nameserver_ids - log[:dnskeys] = dnskey_ids - log[:legal_documents]= [legal_document_id] - log[:registrant] = [registrant_id] + log[:tech_contacts] = tech_contact_ids + log[:nameservers] = nameserver_ids + log[:dnskeys] = dnskey_ids + log[:legal_documents] = [legal_document_id] + log[:registrant] = [registrant_id] log end diff --git a/test/mailers/registrant_change_mailer_test.rb b/test/mailers/registrant_change_mailer_test.rb index 5ddfc76db..21150ea7e 100644 --- a/test/mailers/registrant_change_mailer_test.rb +++ b/test/mailers/registrant_change_mailer_test.rb @@ -13,7 +13,7 @@ class RegistrantChangeMailerTest < ActionMailer::TestCase registrar: @domain.registrar, current_registrant: @domain.registrant, new_registrant: @domain.registrant) - .deliver_now + .deliver_now assert_emails 1 assert_equal ['john@inbox.test'], email.to From 9799752dfb47723bc833f407c6020859145d91a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergei=20Ts=C3=B5ganov?= Date: Wed, 12 Jan 2022 15:19:53 +0200 Subject: [PATCH 7/7] Did some more refactoring --- app/models/concerns/versions.rb | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index a1c872b56..46c19e436 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -64,19 +64,19 @@ module Versions module ClassMethods def all_versions_for(ids, time) - ver_klass = paper_trail.version_class - from_history = ver_klass.where(item_id: ids.to_a). - order(:item_id). - preceding(time + 1, true). - select("distinct on (item_id) #{ver_klass.table_name}.*"). - map do |ver| - valid_columns = ver.item_type.constantize&.column_names - o = new(ver.object&.slice(*valid_columns)) - o.version_loader = ver - changes = ver.object_changes.to_h&.slice(*valid_columns) - changes.each { |k, v| o.public_send("#{k}=", v[-1]) } - o - end + ver_klass = paper_trail.version_class + from_history = ver_klass.where(item_id: ids.to_a) + .order(:item_id) + .preceding(time + 1, true) + .select("distinct on (item_id) #{ver_klass.table_name}.*") + .map do |ver| + valid_columns = ver.item_type.constantize&.column_names + o = new(ver.object&.slice(*valid_columns)) + o.version_loader = ver + changes = ver.object_changes.to_h&.slice(*valid_columns) + changes.each { |k, v| o.public_send("#{k}=", v[-1]) } + o + end not_in_history = where(id: (ids.to_a - from_history.map(&:id))) from_history + not_in_history