From 917e426d9108ebb0211795515a17d2e88b0d7a0d Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Mon, 31 Mar 2025 16:07:04 +0300 Subject: [PATCH 1/3] feat: Add lifted force delete domains to daily admin notification - Add tracking of lifted force delete domains with reason and date in json_statuses_history - Modify ForceDeleteDailyAdminNotifierJob to include both force deleted and lifted domains - Update admin mailer template to show separate tables for force deleted and lifted domains - Update tests to reflect new functionality and fix timing issues with yesterday's data Key changes: - Store lift reason and date when canceling force delete - Add new query method for finding lifted force delete domains - Split email template into two sections - Fix tests to properly handle the yesterday time window --- .../remove_force_delete_statuses.rb | 4 ++ .../force_delete_daily_admin_notifier_job.rb | 42 ++++++++++++----- app/mailers/admin_mailer.rb | 7 +-- app/models/domain.rb | 3 +- .../force_delete_daily_summary.html.erb | 43 ++++++++++++++--- ...ce_delete_daily_admin_notifier_job_test.rb | 46 ++++++++++++------- 6 files changed, 108 insertions(+), 37 deletions(-) diff --git a/app/interactions/domains/cancel_force_delete/remove_force_delete_statuses.rb b/app/interactions/domains/cancel_force_delete/remove_force_delete_statuses.rb index 77c8eba5e..74460eb31 100644 --- a/app/interactions/domains/cancel_force_delete/remove_force_delete_statuses.rb +++ b/app/interactions/domains/cancel_force_delete/remove_force_delete_statuses.rb @@ -15,6 +15,10 @@ module Domains rejected_statuses = domain.statuses.reject { |a| domain_statuses.include? a } domain.statuses = rejected_statuses domain.skip_whois_record_update = true + domain.lift_force_delete_domain_statuses_history_data = { + reason: domain.status_notes[DomainStatus::FORCE_DELETE], + date: Time.zone.now + } domain.save(validate: false) end end diff --git a/app/jobs/force_delete_daily_admin_notifier_job.rb b/app/jobs/force_delete_daily_admin_notifier_job.rb index 88a59c606..254d5a85d 100644 --- a/app/jobs/force_delete_daily_admin_notifier_job.rb +++ b/app/jobs/force_delete_daily_admin_notifier_job.rb @@ -1,23 +1,33 @@ class ForceDeleteDailyAdminNotifierJob < ApplicationJob queue_as :default - def perform - domains = Domain.where("'#{DomainStatus::FORCE_DELETE}' = ANY (statuses)") - .where("force_delete_start = ?", Time.zone.now) - - return if domains.empty? - - notify_admins(domains) + def perform() + notify_about_force_deleted_domains(force_deleted_domains, lifted_force_delete_domains) end private - def notify_admins(domains) - summary = generate_summary(domains) - AdminMailer.force_delete_daily_summary(summary).deliver_now + def force_deleted_domains + Domain.where("force_delete_start >= ? AND force_delete_start <= ?", + Time.zone.yesterday.beginning_of_day, + Time.zone.yesterday.end_of_day) end - def generate_summary(domains) + def lifted_force_delete_domains + Domain.where("json_statuses_history->>'lift_force_delete_domain_statuses_history_data' IS NOT NULL") + .where("(json_statuses_history->'lift_force_delete_domain_statuses_history_data'->>'date')::timestamp >= ? AND (json_statuses_history->'lift_force_delete_domain_statuses_history_data'->>'date')::timestamp <= ?", + Time.zone.yesterday.beginning_of_day, + Time.zone.yesterday.end_of_day) + end + + def notify_about_force_deleted_domains(force_deleted_domains, lifted_force_delete_domains) + force_deleted_summary = generate_summary_for_force_deleted_domains(force_deleted_domains) + lifted_force_delete_summary = generate_summary_for_lifted_force_delete_domains(lifted_force_delete_domains) + + AdminMailer.force_delete_daily_summary(force_deleted_summary, lifted_force_delete_summary).deliver_now + end + + def generate_summary_for_force_deleted_domains(domains) domains.map do |domain| { name: domain.name, @@ -29,6 +39,16 @@ class ForceDeleteDailyAdminNotifierJob < ApplicationJob end end + def generate_summary_for_lifted_force_delete_domains(domains) + domains.map do |domain| + { + name: domain.name, + reason: domain.json_statuses_history.dig('lift_force_delete_domain_statuses_history_data', 'reason') || 'No reason provided', + date: domain.json_statuses_history.dig('lift_force_delete_domain_statuses_history_data', 'date') + } + end + end + def determine_reason(domain) if domain.template_name.present? domain.template_name diff --git a/app/mailers/admin_mailer.rb b/app/mailers/admin_mailer.rb index b76db8a67..c6ae3d89a 100644 --- a/app/mailers/admin_mailer.rb +++ b/app/mailers/admin_mailer.rb @@ -1,9 +1,10 @@ class AdminMailer < ApplicationMailer - def force_delete_daily_summary(domains_summary) - @domains = domains_summary + def force_delete_daily_summary(force_deleted_summary, lifted_force_delete_summary) + @force_deleted_domains = force_deleted_summary + @lifted_domains = lifted_force_delete_summary mail( to: ENV['admin_notification_email'] || 'admin@registry.test', subject: "Force Delete Daily Summary - #{Date.current}" ) end -end \ No newline at end of file +end diff --git a/app/models/domain.rb b/app/models/domain.rb index d8c32790e..ab12372e2 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -48,7 +48,8 @@ class Domain < ApplicationRecord store_accessor :json_statuses_history, :force_delete_domain_statuses_history, - :admin_store_statuses_history + :admin_store_statuses_history, + :lift_force_delete_domain_statuses_history_data # TODO: whois requests ip whitelist for full info for own domains and partial info for other domains # TODO: most inputs should be trimmed before validation, probably some global logic? diff --git a/app/views/admin_mailer/force_delete_daily_summary.html.erb b/app/views/admin_mailer/force_delete_daily_summary.html.erb index 33f2bdea5..41f136d68 100644 --- a/app/views/admin_mailer/force_delete_daily_summary.html.erb +++ b/app/views/admin_mailer/force_delete_daily_summary.html.erb @@ -1,5 +1,6 @@

Force Delete Daily Summary - <%= Date.current %>

+

Force Deleted Domains

@@ -11,13 +12,43 @@ - <% @domains.each do |domain| %> + <% if @force_deleted_domains.present? %> + <% @force_deleted_domains.each do |domain| %> + + + + + + + + <% end %> + <% else %> - - - - - + + + <% end %> + +
<%= domain[:name] %><%= domain[:reason] %><%= domain[:force_delete_type] %><%= domain[:force_delete_start]&.to_date %><%= domain[:force_delete_date]&.to_date %>
<%= domain[:name] %><%= domain[:reason] %><%= domain[:force_delete_type] %><%= domain[:force_delete_start]&.to_date %><%= domain[:force_delete_date]&.to_date %>No force deleted domains found
+ +

Lifted Force Delete Domains

+ + + + Domain + Reason + + + + <% if @lifted_domains.present? %> + <% @lifted_domains.each do |domain| %> + + <%= domain[:name] %> + <%= domain[:reason] %> + + <% end %> + <% else %> + + No lifted domains found <% end %> diff --git a/test/jobs/force_delete_daily_admin_notifier_job_test.rb b/test/jobs/force_delete_daily_admin_notifier_job_test.rb index ebfa3f5d8..d2282180f 100644 --- a/test/jobs/force_delete_daily_admin_notifier_job_test.rb +++ b/test/jobs/force_delete_daily_admin_notifier_job_test.rb @@ -11,7 +11,7 @@ class ForceDeleteDailyAdminNotifierJobTest < ActiveSupport::TestCase def test_sends_notification_for_domains_with_force_delete_today @domain.schedule_force_delete(type: :soft) - @domain.update!(force_delete_start: Time.zone.now.to_date) + @domain.update!(force_delete_start: Time.zone.now.to_date - 1.day) @domain.reload assert_emails 1 do @@ -23,23 +23,13 @@ class ForceDeleteDailyAdminNotifierJobTest < ActiveSupport::TestCase assert_includes email.body.to_s, @domain.force_delete_type end - def test_does_not_send_notification_when_no_force_delete_domains_today - travel_to Time.zone.parse('2010-07-06') - @domain.schedule_force_delete(type: :soft) - @domain.reload - - assert_no_emails do - ForceDeleteDailyAdminNotifierJob.perform_now - end - end - def test_includes_multiple_domains_in_notification @domain.schedule_force_delete(type: :soft) - @domain.update!(force_delete_start: Time.zone.now.to_date) + @domain.update!(force_delete_start: Time.zone.now.to_date - 1.day) domain2 = domains(:airport) domain2.schedule_force_delete(type: :fast_track) - domain2.update!(force_delete_start: Time.zone.now.to_date) + domain2.update!(force_delete_start: Time.zone.now.to_date - 1.day) assert_emails 1 do ForceDeleteDailyAdminNotifierJob.perform_now @@ -53,7 +43,7 @@ class ForceDeleteDailyAdminNotifierJobTest < ActiveSupport::TestCase def test_includes_correct_reason_for_invalid_email_template @domain.update!(template_name: 'invalid_email') @domain.schedule_force_delete(type: :soft) - @domain.update!(force_delete_start: Time.zone.now.to_date) + @domain.update!(force_delete_start: Time.zone.now.to_date - 1.day) @domain.reload assert_emails 1 do @@ -65,10 +55,10 @@ class ForceDeleteDailyAdminNotifierJobTest < ActiveSupport::TestCase end def test_includes_correct_reason_for_manual_force_delete - manual_reason = "Manual deletion requested" + manual_reason = "invalid_company" @domain.status_notes = { DomainStatus::FORCE_DELETE => manual_reason } @domain.schedule_force_delete(type: :fast_track) - @domain.update!(force_delete_start: Time.zone.now.to_date) + @domain.update!(force_delete_start: Time.zone.now.to_date - 1.day) @domain.reload assert_emails 1 do @@ -78,4 +68,28 @@ class ForceDeleteDailyAdminNotifierJobTest < ActiveSupport::TestCase email = ActionMailer::Base.deliveries.last assert_includes email.body.to_s, "Manual force delete: #{manual_reason}" end + + def test_includes_lifted_force_delete_domains_in_notification + reason = "invalid_company" + @domain.status_notes = { DomainStatus::FORCE_DELETE => reason } + @domain.schedule_force_delete(type: :fast_track) + @domain.update!(force_delete_start: Time.zone.now.to_date - 1.day) + @domain.reload + + assert @domain.force_delete_scheduled? + assert_equal @domain.status_notes[DomainStatus::FORCE_DELETE], reason + + @domain.cancel_force_delete + @domain.reload + + travel_to Time.zone.now + 1.day + + assert_emails 1 do + ForceDeleteDailyAdminNotifierJob.perform_now + end + + email = ActionMailer::Base.deliveries.last + assert_includes email.body.to_s, @domain.name + assert_includes email.body.to_s, reason + end end \ No newline at end of file From bfecc3c40ea05826bee9bfcdeacc5dda9911429f Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Tue, 1 Apr 2025 13:52:43 +0300 Subject: [PATCH 2/3] refactor: improve force delete domain status tracking - Add force_delete_domain_statuses_history_data to store force delete metadata - Update force delete queries to use new JSON status history - Refactor force delete tests to use travel_to helper - Remove direct force_delete_start field usage - Update status notes to include company identification number --- .../domains/force_delete/set_status.rb | 4 +++ .../force_delete_daily_admin_notifier_job.rb | 7 ++-- app/models/domain.rb | 3 +- ...ce_delete_daily_admin_notifier_job_test.rb | 35 ++++++++++--------- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/app/interactions/domains/force_delete/set_status.rb b/app/interactions/domains/force_delete/set_status.rb index cedef18d5..76947dfa2 100644 --- a/app/interactions/domains/force_delete/set_status.rb +++ b/app/interactions/domains/force_delete/set_status.rb @@ -6,6 +6,10 @@ module Domains type == :fast_track ? force_delete_fast_track : force_delete_soft domain.status_notes[DomainStatus::FORCE_DELETE] = "Company no: #{domain.registrant.ident}" if reason == 'invalid_company' domain.skip_whois_record_update = true + domain.force_delete_domain_statuses_history_data = { + reason: domain.status_notes[DomainStatus::FORCE_DELETE], + date: Time.zone.now + } domain.save(validate: false) end diff --git a/app/jobs/force_delete_daily_admin_notifier_job.rb b/app/jobs/force_delete_daily_admin_notifier_job.rb index 254d5a85d..0fd6ca898 100644 --- a/app/jobs/force_delete_daily_admin_notifier_job.rb +++ b/app/jobs/force_delete_daily_admin_notifier_job.rb @@ -8,9 +8,10 @@ class ForceDeleteDailyAdminNotifierJob < ApplicationJob private def force_deleted_domains - Domain.where("force_delete_start >= ? AND force_delete_start <= ?", - Time.zone.yesterday.beginning_of_day, - Time.zone.yesterday.end_of_day) + Domain.where("json_statuses_history->>'force_delete_domain_statuses_history_data' IS NOT NULL"). + where("(json_statuses_history->'force_delete_domain_statuses_history_data'->>'date')::timestamp >= ? AND (json_statuses_history->'force_delete_domain_statuses_history_data'->>'date')::timestamp <= ?", + Time.zone.yesterday.beginning_of_day, + Time.zone.yesterday.end_of_day) end def lifted_force_delete_domains diff --git a/app/models/domain.rb b/app/models/domain.rb index ab12372e2..809d18615 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -49,7 +49,8 @@ class Domain < ApplicationRecord store_accessor :json_statuses_history, :force_delete_domain_statuses_history, :admin_store_statuses_history, - :lift_force_delete_domain_statuses_history_data + :lift_force_delete_domain_statuses_history_data, + :force_delete_domain_statuses_history_data # TODO: whois requests ip whitelist for full info for own domains and partial info for other domains # TODO: most inputs should be trimmed before validation, probably some global logic? diff --git a/test/jobs/force_delete_daily_admin_notifier_job_test.rb b/test/jobs/force_delete_daily_admin_notifier_job_test.rb index d2282180f..dd3f99d38 100644 --- a/test/jobs/force_delete_daily_admin_notifier_job_test.rb +++ b/test/jobs/force_delete_daily_admin_notifier_job_test.rb @@ -10,10 +10,13 @@ class ForceDeleteDailyAdminNotifierJobTest < ActiveSupport::TestCase end def test_sends_notification_for_domains_with_force_delete_today - @domain.schedule_force_delete(type: :soft) - @domain.update!(force_delete_start: Time.zone.now.to_date - 1.day) + @domain.schedule_force_delete(type: :soft, + notify_by_email: true, + reason: 'invalid_email') @domain.reload + travel_to Time.zone.now + 1.day + assert_emails 1 do ForceDeleteDailyAdminNotifierJob.perform_now end @@ -25,12 +28,11 @@ class ForceDeleteDailyAdminNotifierJobTest < ActiveSupport::TestCase def test_includes_multiple_domains_in_notification @domain.schedule_force_delete(type: :soft) - @domain.update!(force_delete_start: Time.zone.now.to_date - 1.day) - domain2 = domains(:airport) domain2.schedule_force_delete(type: :fast_track) - domain2.update!(force_delete_start: Time.zone.now.to_date - 1.day) + travel_to Time.zone.now + 1.day + assert_emails 1 do ForceDeleteDailyAdminNotifierJob.perform_now end @@ -43,9 +45,10 @@ class ForceDeleteDailyAdminNotifierJobTest < ActiveSupport::TestCase def test_includes_correct_reason_for_invalid_email_template @domain.update!(template_name: 'invalid_email') @domain.schedule_force_delete(type: :soft) - @domain.update!(force_delete_start: Time.zone.now.to_date - 1.day) @domain.reload + travel_to Time.zone.now + 1.day + assert_emails 1 do ForceDeleteDailyAdminNotifierJob.perform_now end @@ -55,29 +58,29 @@ class ForceDeleteDailyAdminNotifierJobTest < ActiveSupport::TestCase end def test_includes_correct_reason_for_manual_force_delete - manual_reason = "invalid_company" - @domain.status_notes = { DomainStatus::FORCE_DELETE => manual_reason } - @domain.schedule_force_delete(type: :fast_track) - @domain.update!(force_delete_start: Time.zone.now.to_date - 1.day) + @domain.schedule_force_delete(type: :fast_track, + notify_by_email: true, + reason: 'invalid_company') @domain.reload + travel_to Time.zone.now + 1.day + assert_emails 1 do ForceDeleteDailyAdminNotifierJob.perform_now end email = ActionMailer::Base.deliveries.last - assert_includes email.body.to_s, "Manual force delete: #{manual_reason}" + assert_includes email.body.to_s, "Company no: #{@domain.registrant.ident}" end def test_includes_lifted_force_delete_domains_in_notification reason = "invalid_company" - @domain.status_notes = { DomainStatus::FORCE_DELETE => reason } - @domain.schedule_force_delete(type: :fast_track) - @domain.update!(force_delete_start: Time.zone.now.to_date - 1.day) + @domain.schedule_force_delete(type: :fast_track, + notify_by_email: true, + reason: reason) @domain.reload assert @domain.force_delete_scheduled? - assert_equal @domain.status_notes[DomainStatus::FORCE_DELETE], reason @domain.cancel_force_delete @domain.reload @@ -90,6 +93,6 @@ class ForceDeleteDailyAdminNotifierJobTest < ActiveSupport::TestCase email = ActionMailer::Base.deliveries.last assert_includes email.body.to_s, @domain.name - assert_includes email.body.to_s, reason + assert_includes email.body.to_s, "Company no: #{@domain.registrant.ident}" end end \ No newline at end of file From 8234a0fe68b53a704e06c691bf09de9f28257ccf Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Fri, 4 Apr 2025 14:14:58 +0300 Subject: [PATCH 3/3] updated styles --- app/views/admin_mailer/force_delete_daily_summary.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin_mailer/force_delete_daily_summary.html.erb b/app/views/admin_mailer/force_delete_daily_summary.html.erb index 41f136d68..4e9281ee6 100644 --- a/app/views/admin_mailer/force_delete_daily_summary.html.erb +++ b/app/views/admin_mailer/force_delete_daily_summary.html.erb @@ -31,7 +31,7 @@

Lifted Force Delete Domains

- +
Domain