Fix errors when viewing version history of deleted domains by OlegPhenomenon · Pull Request #2901 · internetee/registry

OlegPhenomenon · 2026-02-25T10:13:46Z

Resolves two issues that occurred when viewing the history of a domain that has already been deleted in /admin/domain_versions/:id

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.
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

github-actions · 2026-04-09T08:34:35Z

🚀 Deploy Complete!

Property	Value
App	`registry`
Slot	`1`
URL	https://registry1-dev.cloud.tld.ee
Namespace	`registry1-dev`

(Environment ready for testing)

Copilot

Pull request overview

This PR fixes admin errors when viewing /admin/domain_versions/:id for domains that have already been deleted, by reconstructing missing Domain data from PaperTrail and hardening the view against missing timestamps/links.

Changes:

Updated Admin::DomainVersionsController#show to tolerate missing Domain records and reconstruct a domain object from PaperTrail (reify / object_changes).
Hardened admin/domain_versions/show to avoid nil timestamp crashes and to remove/disable links that would 404 for deleted domains.
Added deleted-domain handling for registrar rendering and the version sidebar “Current state” link.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
`app/controllers/admin/domain_versions_controller.rb`	Reconstructs `@domain` when the DB record is gone and adjusts version lookup to use `item_id` safely.
`app/views/admin/domain_versions/show.haml`	Adds nil-safe timestamp handling and adjusts UI/link behavior for deleted domains.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T14:25:50Z

+          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


When reconstructing a deleted domain for a "create" version, the code prefers next_version&.reify and only falls back to @version.object_changes if reify returns nil. In fixtures (e.g., a later destroy version) object may not contain all fields like name, so reify can return a non-nil but incomplete Domain, and the view will still show missing data (e.g., name as N/A). Prefer reconstructing from the create version’s object_changes (or merging object_changes into the reified object) so required attributes like name, registrar_id, registrant_id are reliably present.

Copilot · 2026-04-09T14:25:50Z

+                value = values.is_a?(Array) ? values.last : values
+                @domain.send("#{attr}=", value) if @domain.respond_to?("#{attr}=")
+              rescue StandardError
+                next


The per-attribute assignment loop rescues StandardError and silently skips failures. This can mask real issues (e.g., unexpected serialized types) and make reconstruction nondeterministic. Consider narrowing the rescued exceptions and/or recording enough context (e.g., attribute name) for troubleshooting instead of silently next-ing.

Suggested change

value = values.is_a?(Array) ? values.last : values

@domain.send("#{attr}=", value) if @domain.respond_to?("#{attr}=")

rescue StandardError

next

next unless @domain.respond_to?("#{attr}=")

value = values.is_a?(Array) ? values.last : values

@domain.public_send("#{attr}=", value)

rescue ArgumentError, TypeError => e

Rails.logger.warn(

"Failed to reconstruct Domain attribute #{attr.inspect} " \

"for version #{@version.id}: #{e.class}: #{e.message}"

)

Copilot · 2026-04-09T14:25:51Z

        @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


There are integration tests for the domain versions controller, but none cover the deleted-domain path introduced here (Domain.find_by returning nil and reconstructing from PaperTrail data). Please add a test using the existing log_domains fixtures (e.g., a create+destroy pair for a non-existent item_id) to assert the show page renders and includes key attributes like the domain name and registrar info.

Copilot · 2026-04-09T14:25:51Z

+          - 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']}"


This registrar fallback for deleted domains only checks @version.object['registrar_id'], but for create events object is often null (see test/fixtures/log_domains.yml), so the registrar will not be shown even if the ID exists in object_changes / the reconstructed @domain.registrar_id. Consider sourcing the registrar id from @domain.registrar_id and/or @version.object_changes as a fallback, and avoid doing the lookup in the view.

Copilot · 2026-04-09T14:25:51Z

+            - 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']}"


Registrar.find_by(...) introduces a database query in the view, which makes rendering harder to reason about and can hurt performance. Prefer resolving the registrar (or registrar name/ID display string) in the controller and passing it to the view, so the template stays presentation-only.

Suggested change

- 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']}"

%dt= t(:registrar_name)

%dd{class: changing_css_class(@version,"registrar_id")}

= "Registrar ID: #{@version.object['registrar_id']}"

Copilot · 2026-04-09T14:25:51Z

          %dt= t(:name)
          - if !@domain.name
-            - domain_name = Domain.find(@version.item_id).try(:name)
+            - domain_name = @version.try(:object).try(:[], 'name') || 'N/A'


For deleted domains, domain_name falls back to @version.object['name'], but for create events object can be null (fixtures have only object_changes), resulting in N/A even though the name is available. Consider falling back to @version.object_changes.dig('name', 1) (or equivalent) when object is missing.

Suggested change

- domain_name = @version.try(:object).try(:[], 'name') || 'N/A'

- domain_name = @version.try(:object).try(:[], 'name') || @version.try(:object_changes).try(:dig, 'name', 1) || 'N/A'

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

…-domain-in-domain-histroy

OlegPhenomenon requested review from vohmar and removed request for vohmar February 25, 2026 10:13

OlegPhenomenon assigned vohmar Mar 2, 2026

vohmar requested a review from maricavor March 12, 2026 15:17

vohmar requested a review from Copilot April 9, 2026 14:19

Copilot started reviewing on behalf of vohmar April 9, 2026 14:19 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

OlegPhenomenon force-pushed the 2897-admin-internal-error-for-any-deletion-domain-in-domain-histroy branch from fd2eff8 to 25f4633 Compare April 10, 2026 08:19

OlegPhenomenon added 3 commits May 26, 2026 15:41

refactor

bcabfd7

refactoring

0892b97

OlegPhenomenon force-pushed the 2897-admin-internal-error-for-any-deletion-domain-in-domain-histroy branch from 72a10a1 to 0892b97 Compare May 26, 2026 12:42

maricavor approved these changes May 27, 2026

View reviewed changes

Merge branch 'master' into 2897-admin-internal-error-for-any-deletion…

2ba544d

…-domain-in-domain-histroy

vohmar merged commit 90de774 into master May 27, 2026
4 of 5 checks passed

Navigation Menu

Uh oh!

Fix errors when viewing version history of deleted domains - #2901

Fix errors when viewing version history of deleted domains#2901
vohmar merged 4 commits into
masterfrom
2897-admin-internal-error-for-any-deletion-domain-in-domain-histroy

OlegPhenomenon commented Feb 25, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-                value = values.is_a?(Array) ? values.last : values
-                @domain.send("#{attr}=", value) if @domain.respond_to?("#{attr}=")
-              rescue StandardError
-                next
+                next unless @domain.respond_to?("#{attr}=")
+                value = values.is_a?(Array) ? values.last : values
+                @domain.public_send("#{attr}=", value)
+              rescue ArgumentError, TypeError => e
+                Rails.logger.warn(
+                  "Failed to reconstruct Domain attribute #{attr.inspect} " \
+                  "for version #{@version.id}: #{e.class}: #{e.message}"
+                )

	- domain_name = @version.try(:object).try(:[], 'name') \|\| 'N/A'
	- domain_name = @version.try(:object).try(:[], 'name') \|\| @version.try(:object_changes).try(:dig, 'name', 1) \|\| 'N/A'

Uh oh!

Conversation

OlegPhenomenon commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 9, 2026

🚀 Deploy Complete!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

OlegPhenomenon commented Feb 25, 2026 •

edited

Loading