From 5434df367601b890340894eea39f49b1fdc93e1f Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Wed, 25 Feb 2026 12:13:24 +0200 Subject: [PATCH 1/3] Fix errors when viewing version history of deleted domains Resolves two issues that occurred when viewing the history of a domain that has already been deleted in /admin/domain_versions/:id 1. ActiveRecord::RecordNotFound Domain.find was raising an error because the domain no longer exists in the database. Replaced find with find_by and added a fallback to reconstruct the domain object using PaperTrail version.reify. If it is a create event, where reify returns nil because there is no prior state, the object is reconstructed from the next available version or built directly from object_changes. 2. NoMethodError undefined method updated_at for nil:NilClass When viewing the create event version of a deleted domain, the reconstructed Domain.new object did not have an updated_at or created_at timestamp, causing the view to crash. Added safe navigator methods and fallback to version.created_at in the view to prevent this. Additionally, UI elements were adjusted for deleted domains: - Replaced dead links to the deleted domain with plain text - Hidden the Current state link in the version sidebar - Guarded the registrar link against being nil --- .../admin/domain_versions_controller.rb | 31 ++++++++++++- app/views/admin/domain_versions/show.haml | 44 ++++++++++++++----- 2 files changed, 61 insertions(+), 14 deletions(-) diff --git a/app/controllers/admin/domain_versions_controller.rb b/app/controllers/admin/domain_versions_controller.rb index fcada856e3..463f1155ff 100644 --- a/app/controllers/admin/domain_versions_controller.rb +++ b/app/controllers/admin/domain_versions_controller.rb @@ -61,10 +61,37 @@ def show @domain = Domain.find(params[:domain_id] || params[:id]) else @version = Version::DomainVersion.find(params[:id]) - @domain = Domain.find(@version.item_id) + @domain = Domain.find_by(id: @version.item_id) + + if @domain.nil? + @domain = @version.reify + @domain_deleted = true + + # For 'create' events, reify returns nil because there's no prior state. + # Try to reconstruct from a later version or from object_changes. + if @domain.nil? + next_version = Version::DomainVersion + .where(item_id: @version.item_id) + .where.not(object: nil) + .order(created_at: :asc, id: :asc) + .first + @domain = next_version&.reify + + if @domain.nil? + @domain = Domain.new + changes = @version.object_changes || {} + changes.each do |attr, values| + value = values.is_a?(Array) ? values.last : values + @domain.send("#{attr}=", value) if @domain.respond_to?("#{attr}=") + rescue StandardError + next + end + end + end + end end - @versions = Version::DomainVersion.where(item_id: @domain.id).order(created_at: :desc, id: :desc) + @versions = Version::DomainVersion.where(item_id: @version&.item_id || @domain.id).order(created_at: :desc, id: :desc) @versions_map = @versions.all.map(&:id) get_page if params[:page].blank? diff --git a/app/views/admin/domain_versions/show.haml b/app/views/admin/domain_versions/show.haml index 364e6f0257..d0c0b8811d 100644 --- a/app/views/admin/domain_versions/show.haml +++ b/app/views/admin/domain_versions/show.haml @@ -1,10 +1,11 @@ - if @version - children = HashWithIndifferentAccess.new(@version.children) - - nameservers = Nameserver.all_versions_for(children[:nameservers], @domain.updated_at) - - dnskeys = Dnskey.all_versions_for(children[:dnskeys], @domain.updated_at) - - tech_contacts = Contact.all_versions_for(children[:tech_contacts], @domain.updated_at) - - admin_contacts = Contact.all_versions_for(children[:admin_contacts], @domain.updated_at) - - registrant = Contact.all_versions_for(children[:registrant], @domain.updated_at) + - version_timestamp = @domain.try(:updated_at) || @version.created_at + - nameservers = Nameserver.all_versions_for(children[:nameservers], version_timestamp) + - dnskeys = Dnskey.all_versions_for(children[:dnskeys], version_timestamp) + - tech_contacts = Contact.all_versions_for(children[:tech_contacts], version_timestamp) + - admin_contacts = Contact.all_versions_for(children[:admin_contacts], version_timestamp) + - registrant = Contact.all_versions_for(children[:registrant], version_timestamp) - event = @version.event - creator = plain_username(@version.terminator) - else @@ -34,18 +35,27 @@ %dl.dl-horizontal %dt= t(:name) - if !@domain.name - - domain_name = Domain.find(@version.item_id).try(:name) + - domain_name = @version.try(:object).try(:[], 'name') || 'N/A' - else - domain_name = @domain.name - %dd= link_to(domain_name, admin_domain_path(@version ? @version.item_id : @domain.id)) + - if @domain_deleted + %dd= domain_name + - else + %dd= link_to(domain_name, admin_domain_path(@version ? @version.item_id : @domain.id)) %dt= t('.created') %dd - = l(@domain.created_at, format: :short) + - if @domain.try(:created_at) + = l(@domain.created_at, format: :short) + - else + = l(@version.created_at, format: :short) %dt= t('.updated') %dd - = l(@domain.updated_at, format: :short) + - if @domain.try(:updated_at) + = l(@domain.updated_at, format: :short) + - else + = l(@version.created_at, format: :short) %br @@ -112,11 +122,20 @@ \...#{ns[:public_key].to_s[-20,20]} %br - - if @domain.registrar + - if !@domain_deleted && @domain.registrar %dt= t(:registrar_name) %dd{class: changing_css_class(@version,"registrar_id")} = link_to admin_registrar_path(@domain.registrar), target: "registrar_#{@domain.registrar.id}" do = @domain.registrar.name + - elsif @domain_deleted && @version.try(:object).try(:[], 'registrar_id') + - registrar = Registrar.find_by(id: @version.object['registrar_id']) + %dt= t(:registrar_name) + %dd{class: changing_css_class(@version,"registrar_id")} + - if registrar + = link_to admin_registrar_path(registrar), target: "registrar_#{registrar.id}" do + = registrar.name + - else + = "Registrar ID: #{@version.object['registrar_id']}" %span{:style => "margin: 20px 20px; clear:both;"} - if @version && (prev = @versions_map[(@versions_map.index(@version.id) - 1)]) && @versions_map.index(@version.id) != 0 @@ -137,8 +156,9 @@ .col-md-4 .panel.panel-default{:style => "min-height:450px;"} %ul.nav.nav-pills.nav-stacked - %li{class: ('active' if @version.nil?)} - = link_to t('.current_state'), admin_domain_version_path(current: 1, domain_id: @domain.id) + - unless @domain_deleted + %li{class: ('active' if @version.nil?)} + = link_to t('.current_state'), admin_domain_version_path(current: 1, domain_id: @domain.id) - @versions.each do |vs| %li{class: (@version && vs.id == @version.id) && :active} = link_to admin_domain_version_path(vs) do From bcabfd72d07ec8b0e0890b3198a9e5e0c4930aeb Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Fri, 10 Apr 2026 11:19:03 +0300 Subject: [PATCH 2/3] refactor --- .gitignore | 2 +- .../admin/domain_versions_controller.rb | 39 ++------ app/services/admin/domain_version_resolver.rb | 91 +++++++++++++++++++ app/views/admin/domain_versions/show.haml | 19 ++-- .../domain_versions_controller_test.rb | 59 ++++++++++++ 5 files changed, 170 insertions(+), 40 deletions(-) create mode 100644 app/services/admin/domain_version_resolver.rb diff --git a/.gitignore b/.gitignore index e38abb012b..54e7fa9aae 100644 --- a/.gitignore +++ b/.gitignore @@ -35,4 +35,4 @@ CLAUDE.md .claude/ .memory-bank AGENT.md -own \ No newline at end of file +own diff --git a/app/controllers/admin/domain_versions_controller.rb b/app/controllers/admin/domain_versions_controller.rb index 463f1155ff..412e5995d0 100644 --- a/app/controllers/admin/domain_versions_controller.rb +++ b/app/controllers/admin/domain_versions_controller.rb @@ -57,41 +57,22 @@ def index end def show + @domain_deleted = false + if params[:current] @domain = Domain.find(params[:domain_id] || params[:id]) else @version = Version::DomainVersion.find(params[:id]) - @domain = Domain.find_by(id: @version.item_id) - - if @domain.nil? - @domain = @version.reify - @domain_deleted = true - - # For 'create' events, reify returns nil because there's no prior state. - # Try to reconstruct from a later version or from object_changes. - if @domain.nil? - next_version = Version::DomainVersion - .where(item_id: @version.item_id) - .where.not(object: nil) - .order(created_at: :asc, id: :asc) - .first - @domain = next_version&.reify - - if @domain.nil? - @domain = Domain.new - changes = @version.object_changes || {} - changes.each do |attr, values| - value = values.is_a?(Array) ? values.last : values - @domain.send("#{attr}=", value) if @domain.respond_to?("#{attr}=") - rescue StandardError - next - end - end - end - end + resolver = Admin::DomainVersionResolver.new(@version) + @domain = resolver.domain + @domain_deleted = resolver.deleted? + @deleted_domain_name = resolver.domain_name if @domain_deleted + @deleted_registrar = resolver.registrar if @domain_deleted + @deleted_registrar_id = resolver.registrar_id if @domain_deleted end - @versions = Version::DomainVersion.where(item_id: @version&.item_id || @domain.id).order(created_at: :desc, id: :desc) + item_id = @version ? @version.item_id : @domain.id + @versions = Version::DomainVersion.where(item_id: item_id).order(created_at: :desc, id: :desc) @versions_map = @versions.all.map(&:id) get_page if params[:page].blank? diff --git a/app/services/admin/domain_version_resolver.rb b/app/services/admin/domain_version_resolver.rb new file mode 100644 index 0000000000..d6902570b8 --- /dev/null +++ b/app/services/admin/domain_version_resolver.rb @@ -0,0 +1,91 @@ +module Admin + class DomainVersionResolver + attr_reader :version + + def initialize(version) + @version = version + end + + def domain + @domain ||= live_domain || reconstruct_domain + end + + def deleted? + live_domain.nil? + end + + def registrar + return @registrar if defined?(@registrar) + + @registrar = Registrar.find_by(id: registrar_id) + end + + def registrar_id + domain&.registrar_id || changes_value('registrar_id') || object_value('registrar_id') + end + + def domain_name + domain&.name || object_value('name') || changes_value('name') + end + + private + + def live_domain + return @live_domain if defined?(@live_domain) + + @live_domain = Domain.find_by(id: version.item_id) + end + + def reconstruct_domain + reify_with_changes || reify_from_next_version || build_from_changes + end + + def reify_with_changes + reified = version.reify + return nil unless reified + + apply_changes(reified) + reified + end + + def reify_from_next_version + next_version = Version::DomainVersion + .where(item_id: version.item_id) + .where.not(object: nil) + .order(created_at: :asc, id: :asc) + .first + next_version&.reify + end + + def build_from_changes + domain = Domain.new + apply_changes(domain) + domain + end + + def apply_changes(domain) + changes = version.object_changes || {} + allowed = Domain.column_names + changes.slice(*allowed).each do |attr, values| + value = values.is_a?(Array) ? values.last : values + domain.public_send("#{attr}=", value) + rescue ArgumentError, TypeError => e + Rails.logger.warn( + "DomainVersionResolver: failed to assign #{attr.inspect} " \ + "for version #{version.id}: #{e.class}: #{e.message}" + ) + end + end + + def object_value(key) + version.object.is_a?(Hash) ? version.object[key] : nil + end + + def changes_value(key) + changes = version.object_changes + return nil unless changes.is_a?(Hash) && changes[key].is_a?(Array) + + changes[key].last + end + end +end diff --git a/app/views/admin/domain_versions/show.haml b/app/views/admin/domain_versions/show.haml index d0c0b8811d..f5e4a2a450 100644 --- a/app/views/admin/domain_versions/show.haml +++ b/app/views/admin/domain_versions/show.haml @@ -34,10 +34,10 @@ .panel-body %dl.dl-horizontal %dt= t(:name) - - if !@domain.name - - domain_name = @version.try(:object).try(:[], 'name') || 'N/A' - - else + - if @domain.name.present? - domain_name = @domain.name + - else + - domain_name = @deleted_domain_name || 'N/A' - if @domain_deleted %dd= domain_name - else @@ -127,16 +127,15 @@ %dd{class: changing_css_class(@version,"registrar_id")} = link_to admin_registrar_path(@domain.registrar), target: "registrar_#{@domain.registrar.id}" do = @domain.registrar.name - - elsif @domain_deleted && @version.try(:object).try(:[], 'registrar_id') - - registrar = Registrar.find_by(id: @version.object['registrar_id']) + - elsif @domain_deleted && @deleted_registrar_id %dt= t(:registrar_name) %dd{class: changing_css_class(@version,"registrar_id")} - - if registrar - = link_to admin_registrar_path(registrar), target: "registrar_#{registrar.id}" do - = registrar.name + - if @deleted_registrar + = link_to admin_registrar_path(@deleted_registrar), target: "registrar_#{@deleted_registrar.id}" do + = @deleted_registrar.name - else - = "Registrar ID: #{@version.object['registrar_id']}" - %span{:style => "margin: 20px 20px; clear:both;"} + = "Registrar ID: #{@deleted_registrar_id}" + %span{:style => "margin: 20px 20px; clear:both;"} - if @version && (prev = @versions_map[(@versions_map.index(@version.id) - 1)]) && @versions_map.index(@version.id) != 0 = link_to(t(:previous), diff --git a/test/integration/admin_area/domain_versions_controller_test.rb b/test/integration/admin_area/domain_versions_controller_test.rb index e64b811cef..4b4a01f2f4 100644 --- a/test/integration/admin_area/domain_versions_controller_test.rb +++ b/test/integration/admin_area/domain_versions_controller_test.rb @@ -137,4 +137,63 @@ def test_show_with_invalid_domain_id get admin_domain_version_path(999999), params: { current: true } end end + + def test_show_create_version_of_deleted_domain + deleted_item_id = 9_999_001 + create_version = Version::DomainVersion.create!( + item_type: 'Domain', + item_id: deleted_item_id, + event: 'create', + whodunnit: users(:admin).id.to_s, + object: nil, + object_changes: { + 'name' => [nil, 'ghost.test'], + 'registrar_id' => [nil, @registrar.id], + 'registrant_id' => [nil, contacts(:john).id], + }, + created_at: Time.zone.parse('2024-01-01') + ) + + get admin_domain_version_path(create_version.id) + + assert_response :ok + assert_includes response.body, 'ghost.test' + assert_includes response.body, @registrar.name + end + + def test_show_update_version_of_deleted_domain + deleted_item_id = 9_999_002 + Version::DomainVersion.create!( + item_type: 'Domain', + item_id: deleted_item_id, + event: 'create', + whodunnit: users(:admin).id.to_s, + object: nil, + object_changes: { + 'name' => [nil, 'gone.test'], + 'registrar_id' => [nil, @registrar.id], + }, + created_at: Time.zone.parse('2024-01-01') + ) + update_version = Version::DomainVersion.create!( + item_type: 'Domain', + item_id: deleted_item_id, + event: 'update', + whodunnit: users(:admin).id.to_s, + object: { + 'name' => 'gone.test', + 'registrar_id' => @registrar.id, + }, + object_changes: { + 'statuses' => [[], ['serverHold']], + }, + created_at: Time.zone.parse('2024-02-01') + ) + + get admin_domain_version_path(update_version.id) + + assert_response :ok + assert_includes response.body, 'gone.test' + assert_includes response.body, @registrar.name + end end From 0892b97eb60e682538b9a5eb8fe844db13263fce Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Fri, 10 Apr 2026 13:58:41 +0300 Subject: [PATCH 3/3] refactoring --- .gitignore | 2 + .../admin/domain_versions_controller.rb | 12 +- app/models/version/domain_version/resolver.rb | 102 +++++++++++++ app/services/admin/domain_version_resolver.rb | 91 ------------ app/views/admin/domain_versions/show.haml | 40 ++---- config/locales/admin/domain_versions.en.yml | 1 + .../domain_versions_controller_test.rb | 49 ++++++- .../version/domain_version/resolver_test.rb | 135 ++++++++++++++++++ 8 files changed, 304 insertions(+), 128 deletions(-) create mode 100644 app/models/version/domain_version/resolver.rb delete mode 100644 app/services/admin/domain_version_resolver.rb create mode 100644 test/models/version/domain_version/resolver_test.rb diff --git a/.gitignore b/.gitignore index 54e7fa9aae..cb4bf2c3b5 100644 --- a/.gitignore +++ b/.gitignore @@ -36,3 +36,5 @@ CLAUDE.md .memory-bank AGENT.md own +AGENT.md + diff --git a/app/controllers/admin/domain_versions_controller.rb b/app/controllers/admin/domain_versions_controller.rb index 412e5995d0..285be85f89 100644 --- a/app/controllers/admin/domain_versions_controller.rb +++ b/app/controllers/admin/domain_versions_controller.rb @@ -57,21 +57,15 @@ def index end def show - @domain_deleted = false - if params[:current] @domain = Domain.find(params[:domain_id] || params[:id]) else @version = Version::DomainVersion.find(params[:id]) - resolver = Admin::DomainVersionResolver.new(@version) - @domain = resolver.domain - @domain_deleted = resolver.deleted? - @deleted_domain_name = resolver.domain_name if @domain_deleted - @deleted_registrar = resolver.registrar if @domain_deleted - @deleted_registrar_id = resolver.registrar_id if @domain_deleted + @resolver = Version::DomainVersion::Resolver.new(@version) + @domain = @resolver.domain end - item_id = @version ? @version.item_id : @domain.id + item_id = @resolver&.item_id || @domain.id @versions = Version::DomainVersion.where(item_id: item_id).order(created_at: :desc, id: :desc) @versions_map = @versions.all.map(&:id) diff --git a/app/models/version/domain_version/resolver.rb b/app/models/version/domain_version/resolver.rb new file mode 100644 index 0000000000..ab0c4c1847 --- /dev/null +++ b/app/models/version/domain_version/resolver.rb @@ -0,0 +1,102 @@ +class Version::DomainVersion < PaperTrail::Version + class Resolver + attr_reader :version + + def initialize(version) + @version = version + end + + def domain + @domain ||= live_domain || reconstruct_domain + end + + def deleted? + live_domain.nil? + end + + def domain_name + domain.name.presence || object_value('name') || changes_value('name') + end + + def registrar + return @registrar if defined?(@registrar) + + @registrar = registrar_id && Registrar.find_by(id: registrar_id) + end + + def registrar_id + domain.registrar_id || changes_value('registrar_id') || object_value('registrar_id') + end + + def item_id + version.item_id + end + + private + + def live_domain + return @live_domain if defined?(@live_domain) + + @live_domain = Domain.find_by(id: version.item_id) + end + + def reconstruct_domain + reify_with_changes || earliest_reifiable_version&.reify || build_from_changes + end + + def reify_with_changes + reified = version.reify + return nil unless reified + + apply_changes(reified) + stamp_timestamps(reified) + reified + end + + def earliest_reifiable_version + sibling_versions.where.not(object: nil).first + end + + def build_from_changes + record = Domain.new + apply_changes(record) + stamp_timestamps(record) + record + end + + def apply_changes(record) + changes = version.object_changes || {} + changes.slice(*Domain.column_names).each do |attr, values| + value = values.is_a?(Array) ? values.last : values + record.public_send("#{attr}=", value) + end + end + + def stamp_timestamps(record) + record.created_at ||= earliest_version_created_at + record.updated_at ||= version.created_at + end + + def earliest_version_created_at + @earliest_version_created_at ||= + sibling_versions.pluck(:created_at).first || version.created_at + end + + def sibling_versions + Version::DomainVersion + .where(item_id: version.item_id) + .order(created_at: :asc, id: :asc) + end + + def object_value(key) + version.object.is_a?(Hash) ? version.object[key] : nil + end + + def changes_value(key) + changes = version.object_changes + return nil unless changes.is_a?(Hash) && changes[key].is_a?(Array) + + changes[key].last + end + end +end diff --git a/app/services/admin/domain_version_resolver.rb b/app/services/admin/domain_version_resolver.rb deleted file mode 100644 index d6902570b8..0000000000 --- a/app/services/admin/domain_version_resolver.rb +++ /dev/null @@ -1,91 +0,0 @@ -module Admin - class DomainVersionResolver - attr_reader :version - - def initialize(version) - @version = version - end - - def domain - @domain ||= live_domain || reconstruct_domain - end - - def deleted? - live_domain.nil? - end - - def registrar - return @registrar if defined?(@registrar) - - @registrar = Registrar.find_by(id: registrar_id) - end - - def registrar_id - domain&.registrar_id || changes_value('registrar_id') || object_value('registrar_id') - end - - def domain_name - domain&.name || object_value('name') || changes_value('name') - end - - private - - def live_domain - return @live_domain if defined?(@live_domain) - - @live_domain = Domain.find_by(id: version.item_id) - end - - def reconstruct_domain - reify_with_changes || reify_from_next_version || build_from_changes - end - - def reify_with_changes - reified = version.reify - return nil unless reified - - apply_changes(reified) - reified - end - - def reify_from_next_version - next_version = Version::DomainVersion - .where(item_id: version.item_id) - .where.not(object: nil) - .order(created_at: :asc, id: :asc) - .first - next_version&.reify - end - - def build_from_changes - domain = Domain.new - apply_changes(domain) - domain - end - - def apply_changes(domain) - changes = version.object_changes || {} - allowed = Domain.column_names - changes.slice(*allowed).each do |attr, values| - value = values.is_a?(Array) ? values.last : values - domain.public_send("#{attr}=", value) - rescue ArgumentError, TypeError => e - Rails.logger.warn( - "DomainVersionResolver: failed to assign #{attr.inspect} " \ - "for version #{version.id}: #{e.class}: #{e.message}" - ) - end - end - - def object_value(key) - version.object.is_a?(Hash) ? version.object[key] : nil - end - - def changes_value(key) - changes = version.object_changes - return nil unless changes.is_a?(Hash) && changes[key].is_a?(Array) - - changes[key].last - end - end -end diff --git a/app/views/admin/domain_versions/show.haml b/app/views/admin/domain_versions/show.haml index f5e4a2a450..648e3f97a0 100644 --- a/app/views/admin/domain_versions/show.haml +++ b/app/views/admin/domain_versions/show.haml @@ -1,6 +1,7 @@ +- deleted = @resolver&.deleted? - if @version - children = HashWithIndifferentAccess.new(@version.children) - - version_timestamp = @domain.try(:updated_at) || @version.created_at + - version_timestamp = @domain.updated_at || @version.created_at - nameservers = Nameserver.all_versions_for(children[:nameservers], version_timestamp) - dnskeys = Dnskey.all_versions_for(children[:dnskeys], version_timestamp) - tech_contacts = Contact.all_versions_for(children[:tech_contacts], version_timestamp) @@ -34,28 +35,17 @@ .panel-body %dl.dl-horizontal %dt= t(:name) - - if @domain.name.present? - - domain_name = @domain.name - - else - - domain_name = @deleted_domain_name || 'N/A' - - if @domain_deleted + - domain_name = (@resolver ? @resolver.domain_name : @domain.name).presence || t('.unknown_name') + - if deleted %dd= domain_name - else %dd= link_to(domain_name, admin_domain_path(@version ? @version.item_id : @domain.id)) %dt= t('.created') - %dd - - if @domain.try(:created_at) - = l(@domain.created_at, format: :short) - - else - = l(@version.created_at, format: :short) + %dd= l(@domain.created_at, format: :short) %dt= t('.updated') - %dd - - if @domain.try(:updated_at) - = l(@domain.updated_at, format: :short) - - else - = l(@version.created_at, format: :short) + %dd= l(@domain.updated_at, format: :short) %br @@ -122,19 +112,17 @@ \...#{ns[:public_key].to_s[-20,20]} %br - - if !@domain_deleted && @domain.registrar + - registrar = @resolver ? @resolver.registrar : @domain.registrar + - registrar_id = @resolver ? @resolver.registrar_id : @domain.registrar&.id + - if registrar %dt= t(:registrar_name) %dd{class: changing_css_class(@version,"registrar_id")} - = link_to admin_registrar_path(@domain.registrar), target: "registrar_#{@domain.registrar.id}" do - = @domain.registrar.name - - elsif @domain_deleted && @deleted_registrar_id + = link_to admin_registrar_path(registrar), target: "registrar_#{registrar.id}" do + = registrar.name + - elsif registrar_id %dt= t(:registrar_name) %dd{class: changing_css_class(@version,"registrar_id")} - - if @deleted_registrar - = link_to admin_registrar_path(@deleted_registrar), target: "registrar_#{@deleted_registrar.id}" do - = @deleted_registrar.name - - else - = "Registrar ID: #{@deleted_registrar_id}" + = "Registrar ID: #{registrar_id}" %span{:style => "margin: 20px 20px; clear:both;"} - if @version && (prev = @versions_map[(@versions_map.index(@version.id) - 1)]) && @versions_map.index(@version.id) != 0 @@ -155,7 +143,7 @@ .col-md-4 .panel.panel-default{:style => "min-height:450px;"} %ul.nav.nav-pills.nav-stacked - - unless @domain_deleted + - unless deleted %li{class: ('active' if @version.nil?)} = link_to t('.current_state'), admin_domain_version_path(current: 1, domain_id: @domain.id) - @versions.each do |vs| diff --git a/config/locales/admin/domain_versions.en.yml b/config/locales/admin/domain_versions.en.yml index 513f1f33b6..72cf9610db 100644 --- a/config/locales/admin/domain_versions.en.yml +++ b/config/locales/admin/domain_versions.en.yml @@ -14,3 +14,4 @@ en: admin_contacts: Admin. contacts tech_contacts: Tech. contacts current_state: Current state + unknown_name: N/A diff --git a/test/integration/admin_area/domain_versions_controller_test.rb b/test/integration/admin_area/domain_versions_controller_test.rb index 4b4a01f2f4..53a116788e 100644 --- a/test/integration/admin_area/domain_versions_controller_test.rb +++ b/test/integration/admin_area/domain_versions_controller_test.rb @@ -139,7 +139,7 @@ def test_show_with_invalid_domain_id end def test_show_create_version_of_deleted_domain - deleted_item_id = 9_999_001 + deleted_item_id = next_unused_domain_id create_version = Version::DomainVersion.create!( item_type: 'Domain', item_id: deleted_item_id, @@ -159,10 +159,12 @@ def test_show_create_version_of_deleted_domain assert_response :ok assert_includes response.body, 'ghost.test' assert_includes response.body, @registrar.name + assert_no_match %r{href="#{admin_domain_path(deleted_item_id)}"}, response.body + assert_no_match %r{admin_domain_version[^"]*current=1}, response.body end def test_show_update_version_of_deleted_domain - deleted_item_id = 9_999_002 + deleted_item_id = next_unused_domain_id Version::DomainVersion.create!( item_type: 'Domain', item_id: deleted_item_id, @@ -195,5 +197,48 @@ def test_show_update_version_of_deleted_domain assert_response :ok assert_includes response.body, 'gone.test' assert_includes response.body, @registrar.name + assert_no_match %r{href="#{admin_domain_path(deleted_item_id)}"}, response.body + end + + def test_show_destroy_version_of_deleted_domain + deleted_item_id = next_unused_domain_id + Version::DomainVersion.create!( + item_type: 'Domain', + item_id: deleted_item_id, + event: 'create', + whodunnit: users(:admin).id.to_s, + object: nil, + object_changes: { + 'name' => [nil, 'bye.test'], + 'registrar_id' => [nil, @registrar.id], + }, + created_at: Time.zone.parse('2024-01-01') + ) + destroy_version = Version::DomainVersion.create!( + item_type: 'Domain', + item_id: deleted_item_id, + event: 'destroy', + whodunnit: users(:admin).id.to_s, + object: { + 'name' => 'bye.test', + 'registrar_id' => @registrar.id, + }, + object_changes: nil, + created_at: Time.zone.parse('2024-03-01') + ) + + get admin_domain_version_path(destroy_version.id) + + assert_response :ok + assert_includes response.body, 'bye.test' + end + + private + + def next_unused_domain_id + @next_unused_domain_id ||= Domain.maximum(:id).to_i + + Version::DomainVersion.maximum(:item_id).to_i + + 1000 + @next_unused_domain_id += 1 end end diff --git a/test/models/version/domain_version/resolver_test.rb b/test/models/version/domain_version/resolver_test.rb new file mode 100644 index 0000000000..2cfb8e0d7d --- /dev/null +++ b/test/models/version/domain_version/resolver_test.rb @@ -0,0 +1,135 @@ +require 'test_helper' + +class Version::DomainVersion::ResolverTest < ActiveSupport::TestCase + setup do + @registrar = registrars(:bestnames) + @admin = users(:admin) + end + + def test_resolves_live_domain_without_reconstruction + domain = domains(:shop) + version = Version::DomainVersion.where(item_id: domain.id).first + skip 'no fixtures versions' unless version + + resolver = Version::DomainVersion::Resolver.new(version) + + assert_equal domain.id, resolver.domain.id + refute resolver.deleted? + assert_equal domain.name, resolver.domain_name + end + + def test_reconstructs_domain_from_create_version_with_nil_object + item_id = next_unused_item_id + version = build_version( + item_id: item_id, + event: 'create', + object: nil, + object_changes: { + 'name' => [nil, 'ghost.test'], + 'registrar_id' => [nil, @registrar.id], + } + ) + + resolver = Version::DomainVersion::Resolver.new(version) + + assert resolver.deleted? + assert_equal 'ghost.test', resolver.domain_name + assert_equal @registrar.id, resolver.registrar_id + assert_equal @registrar, resolver.registrar + assert_not_nil resolver.domain.created_at + assert_not_nil resolver.domain.updated_at + end + + def test_reconstructs_domain_from_update_version_via_reify + item_id = next_unused_item_id + build_version( + item_id: item_id, + event: 'create', + object: nil, + object_changes: { 'name' => [nil, 'gone.test'], 'registrar_id' => [nil, @registrar.id] } + ) + update_version = build_version( + item_id: item_id, + event: 'update', + object: { 'name' => 'gone.test', 'registrar_id' => @registrar.id }, + object_changes: { 'statuses' => [[], ['serverHold']] } + ) + + resolver = Version::DomainVersion::Resolver.new(update_version) + + assert resolver.deleted? + assert_equal 'gone.test', resolver.domain_name + assert_equal @registrar.id, resolver.registrar_id + end + + def test_domain_name_falls_back_through_object_and_changes + item_id = next_unused_item_id + version = build_version( + item_id: item_id, + event: 'destroy', + object: { 'name' => 'from-object.test' }, + object_changes: nil + ) + + resolver = Version::DomainVersion::Resolver.new(version) + + assert_equal 'from-object.test', resolver.domain_name + end + + def test_domain_name_returns_nil_when_no_data_available + item_id = next_unused_item_id + version = build_version( + item_id: item_id, + event: 'update', + object: nil, + object_changes: nil + ) + + resolver = Version::DomainVersion::Resolver.new(version) + + assert_nil resolver.domain_name + end + + def test_registrar_memoizes_missing_lookup + item_id = next_unused_item_id + missing_registrar_id = Registrar.maximum(:id).to_i + 9999 + version = build_version( + item_id: item_id, + event: 'create', + object: nil, + object_changes: { 'name' => [nil, 'x.test'], 'registrar_id' => [nil, missing_registrar_id] } + ) + resolver = Version::DomainVersion::Resolver.new(version) + resolver.registrar # prime memoization + + assert_queries(0) { 3.times { resolver.registrar } } + assert_nil resolver.registrar + end + + private + + def build_version(attrs) + Version::DomainVersion.create!( + item_type: 'Domain', + whodunnit: @admin.id.to_s, + created_at: Time.zone.now, + **attrs + ) + end + + def next_unused_item_id + @next_unused_item_id ||= Domain.maximum(:id).to_i + + Version::DomainVersion.maximum(:item_id).to_i + + 5000 + @next_unused_item_id += 1 + end + + def assert_queries(expected) + count = 0 + counter = ->(_name, _start, _finish, _id, payload) do + count += 1 unless payload[:name] == 'SCHEMA' || payload[:sql].include?('TRANSACTION') + end + ActiveSupport::Notifications.subscribed(counter, 'sql.active_record') { yield } + assert_equal expected, count, "expected #{expected} queries, got #{count}" + end +end