Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix domain status history view #2263

Merged
merged 7 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/controllers/admin/domains_controller.rb
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -65,6 +67,10 @@ def update
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
Expand Down
26 changes: 13 additions & 13 deletions app/models/concerns/versions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,19 @@ def touch_domains_version

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
Expand Down
24 changes: 16 additions & 8 deletions app/models/domain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -469,13 +469,15 @@ def registrant_update_confirmable?(token)
return false unless pending_update?
return false unless registrant_verification_asked?
return false unless registrant_verification_token == token

true
end

def registrant_delete_confirmable?(token)
return false unless pending_delete?
return false unless registrant_verification_asked?
return false unless registrant_verification_token == token

true
end

Expand All @@ -492,10 +494,12 @@ def registrant_verification_asked!(frame_str, current_user_id)

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

Expand Down Expand Up @@ -560,6 +564,7 @@ def to_s
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

Expand Down Expand Up @@ -590,10 +595,13 @@ def update_not_by_locked_statuses(update)

# 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
Expand Down Expand Up @@ -658,7 +666,7 @@ def set_server_hold
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
Expand All @@ -679,11 +687,11 @@ def manage_automatic_statuses
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

Expand Down
18 changes: 13 additions & 5 deletions app/views/admin/domains/versions.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion test/mailers/registrant_change_mailer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def test_delivers_confirmation_request_email
registrar: @domain.registrar,
current_registrant: @domain.registrant,
new_registrant: @domain.registrant)
.deliver_now
.deliver_now

assert_emails 1
assert_equal ['[email protected]'], email.to
Expand Down