Merge pull request #2263 from internetee/834-fix-domain-history-records

Fix domain status history view

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
 
@@ -65,6 +67,10 @@ module Admin
     def versions
       @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)
     end
 
     def keep

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

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,10 +595,13 @@ 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)
     return unless update
 
+    PaperTrail.request(enabled: false) do
+      update_unless_locked_by_registrant(update)
+      update_not_by_locked_statuses(update)
+    end
+
     # check for deleted status
     statuses.each do |s|
       unless update.include? s
@@ -658,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
@@ -679,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
 

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) {

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'

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