From 29c6c8ff44aac57ae8f9eacfb7d672c09b4478cf Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Thu, 6 Mar 2025 15:18:16 +0200 Subject: [PATCH 1/2] Fix ProcessClientHold tests and implementation This commit addresses several issues with the ProcessClientHold class and its tests: 1. Fixed the test_send_mail_delivers_email test by properly mocking the DomainDeleteMailer.forced method with correct parameter signatures and adding template_name to the domain. 2. Updated all tests to use Domain.stub_any_instance(:force_delete_scheduled?, true) to properly stub the force_delete_scheduled? method. 3. Improved test assertions to ensure proper behavior of the ProcessClientHold class, including notification creation and client hold status setting. 4. Added proper error handling in tests to ensure methods don't raise exceptions and restore original method implementations after testing. The changes ensure that the ProcessClientHold class correctly handles client hold status for domains in the force delete process, properly notifies registrars, and sends emails when required. --- .../client_hold/process_client_hold.rb | 2 +- config/locales/en.yml | 1 + .../client_hold/process_client_hold_test.rb | 195 ++++++++++++++++++ 3 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 test/interactions/domains/client_hold/process_client_hold_test.rb diff --git a/app/interactions/domains/client_hold/process_client_hold.rb b/app/interactions/domains/client_hold/process_client_hold.rb index 71396e87b..dd3840ec2 100644 --- a/app/interactions/domains/client_hold/process_client_hold.rb +++ b/app/interactions/domains/client_hold/process_client_hold.rb @@ -33,7 +33,7 @@ module Domains end def notify_client_hold - domain.registrar.notifications.create!(text: I18n.t('force_delete_set_on_domain', + domain.registrar.notifications.create!(text: I18n.t('hold_client_on_domain', domain_name: domain.name, outzone_date: domain.outzone_date, purge_date: domain.purge_date)) diff --git a/config/locales/en.yml b/config/locales/en.yml index 7cb720451..3b3cbb5b6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -653,6 +653,7 @@ en: created_at_until: 'Created at until' is_registrant: 'Is registrant' force_delete_set_on_domain: 'Force delete set on domain %{domain_name}. Outzone date: %{outzone_date}. Purge date: %{purge_date}' + hold_client_on_domain: 'clientHold status set on domain %{domain_name}. Outzone date: %{outzone_date}. Purge date: %{purge_date}' force_delete_auto_email: 'Force delete set on domain %{domain_name}. Outzone date: %{outzone_date}. Purge date: %{purge_date}. Invalid email: %{email}' grace_period_started_domain: 'For domain %{domain_name} started 45-days redemption grace period, ForceDelete will be in effect from %{date}' force_delete_cancelled: 'Force delete is cancelled on domain %{domain_name}' diff --git a/test/interactions/domains/client_hold/process_client_hold_test.rb b/test/interactions/domains/client_hold/process_client_hold_test.rb new file mode 100644 index 000000000..86134daee --- /dev/null +++ b/test/interactions/domains/client_hold/process_client_hold_test.rb @@ -0,0 +1,195 @@ +require 'test_helper' + +module Domains + module ClientHold + class ProcessClientHoldTest < ActiveSupport::TestCase + setup do + @domain = domains(:shop) + @registrar = registrars(:bestnames) + @domain.update!(registrar: @registrar) + @domain.force_delete_data = { 'force_delete_type' => 'soft' } + @domain.statuses = [] + @domain.force_delete_start = Time.zone.now - 1.day + @domain.contact_notification_sent_date = nil + @domain.valid_to = Time.zone.now + 30.days + @domain.save(validate: false) + end + + def test_notify_on_grace_period_when_should_notify + @domain.update!( + force_delete_start: Time.zone.now - 1.day, + contact_notification_sent_date: nil, + force_delete_data: { 'force_delete_type' => 'soft' } + ) + + Domain.stub_any_instance(:force_delete_scheduled?, true) do + assert_difference '@domain.registrar.notifications.count', 1 do + ProcessClientHold.run(domain: @domain) + end + + @domain.reload + assert_not_nil @domain.contact_notification_sent_date + end + end + + def test_execute_adds_client_hold_status_when_domain_is_holdable + @domain.update!( + force_delete_start: Time.zone.now - (Setting.expire_warning_period.days + 2.days), + force_delete_data: { 'force_delete_type' => 'soft' } + ) + + Domain.stub_any_instance(:force_delete_scheduled?, true) do + assert_not @domain.statuses.include?(DomainStatus::CLIENT_HOLD) + + ProcessClientHold.run(domain: @domain) + + @domain.reload + assert @domain.statuses.include?(DomainStatus::CLIENT_HOLD) + assert_equal ProcessClientHold::CLIENT_HOLD_SET_NOTE, @domain.force_delete_data['client_hold_mandatory'] + end + end + + def test_execute_does_not_add_client_hold_when_already_set + @domain.update!( + force_delete_start: Time.zone.now - (Setting.expire_warning_period.days + 2.days), + force_delete_data: { + 'force_delete_type' => 'soft', + 'client_hold_mandatory' => ProcessClientHold::CLIENT_HOLD_SET_NOTE + } + ) + + @domain.statuses << DomainStatus::CLIENT_HOLD + @domain.save(validate: false) + + Domain.stub_any_instance(:force_delete_scheduled?, true) do + assert_no_difference '@domain.registrar.notifications.count' do + ProcessClientHold.run(domain: @domain) + end + end + end + + def test_notify_client_hold_creates_notification + process = ProcessClientHold.new(domain: @domain) + + assert_difference '@domain.registrar.notifications.count', 1 do + process.notify_client_hold + end + + notification = @domain.registrar.notifications.last + assert_includes notification.text, @domain.name + assert_includes notification.text, @domain.outzone_date.to_s if @domain.outzone_date + assert_includes notification.text, @domain.purge_date.to_s if @domain.purge_date + end + + def test_send_mail_delivers_email + @domain.force_delete_data = {'force_delete_type' => 'soft'} + @domain.template_name = 'legal_person' + @domain.save! + + original_method = DomainDeleteMailer.method(:forced) + + DomainDeleteMailer.define_singleton_method(:forced) do |domain:, registrar:, registrant:, template_name:| + raise "Incorrect domain" unless domain.is_a?(Domain) + raise "Incorrect registrar" unless registrar.is_a?(Registrar) + raise "Incorrect registrant" unless registrant.is_a?(Contact) + raise "Incorrect template_name" unless template_name == 'legal_person' + + OpenStruct.new(deliver_now: true) + + Domain.stub_any_instance(:force_delete_scheduled?, true) do + assert_nothing_raised do + ProcessClientHold.new(domain: @domain).send_mail + end + end + ensure + DomainDeleteMailer.define_singleton_method(:forced, original_method) + end + end + + def test_should_notify_on_soft_force_delete + @domain.update!( + force_delete_start: Time.zone.now - 1.day, + contact_notification_sent_date: nil, + force_delete_data: { 'force_delete_type' => 'soft' } + ) + + Domain.stub_any_instance(:force_delete_scheduled?, true) do + process = ProcessClientHold.new(domain: @domain) + assert process.should_notify_on_soft_force_delete? + + @domain.update!(contact_notification_sent_date: Time.zone.today) + process = ProcessClientHold.new(domain: @domain) + assert_not process.should_notify_on_soft_force_delete? + end + + Domain.stub_any_instance(:force_delete_scheduled?, false) do + @domain.update!(force_delete_start: nil, contact_notification_sent_date: nil) + process = ProcessClientHold.new(domain: @domain) + assert_not process.should_notify_on_soft_force_delete? + end + + Domain.stub_any_instance(:force_delete_scheduled?, true) do + @domain.update!( + force_delete_start: Time.zone.now - 1.day, + force_delete_data: { 'force_delete_type' => 'hard' } + ) + process = ProcessClientHold.new(domain: @domain) + assert_not process.should_notify_on_soft_force_delete? + end + end + + def test_client_holdable + @domain.update!( + force_delete_start: Time.zone.now - (Setting.expire_warning_period.days + 2.days), + force_delete_data: { 'force_delete_type' => 'soft' } + ) + + Domain.stub_any_instance(:force_delete_scheduled?, true) do + process = ProcessClientHold.new(domain: @domain) + assert process.client_holdable? + + @domain.statuses << DomainStatus::CLIENT_HOLD + @domain.save(validate: false) + process = ProcessClientHold.new(domain: @domain) + assert_not process.client_holdable? + end + + Domain.stub_any_instance(:force_delete_scheduled?, false) do + @domain.statuses = [] + @domain.save(validate: false) + process = ProcessClientHold.new(domain: @domain) + assert_not process.client_holdable? + end + end + + def test_force_delete_lte_today + @domain.update!(force_delete_start: Time.zone.now - (Setting.expire_warning_period.days + 2.days)) + + process = ProcessClientHold.new(domain: @domain) + assert process.force_delete_lte_today + + @domain.update!(force_delete_start: Time.zone.now) + process = ProcessClientHold.new(domain: @domain) + assert_not process.force_delete_lte_today + end + + def test_force_delete_lte_valid_date + @domain.update!( + force_delete_start: Time.zone.now - (Setting.expire_warning_period.days + 2.days), + valid_to: Time.zone.now + 60.days + ) + + process = ProcessClientHold.new(domain: @domain) + assert process.force_delete_lte_valid_date + + @domain.update!( + force_delete_start: Time.zone.now, + valid_to: Time.zone.now + 5.days + ) + + process = ProcessClientHold.new(domain: @domain) + assert_not process.force_delete_lte_valid_date + end + end + end +end \ No newline at end of file From 95a6403595b1a8d159958b9248a466a8eae2de7b Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Thu, 6 Mar 2025 15:32:39 +0200 Subject: [PATCH 2/2] Fix ProcessClientHold tests and implementation This commit addresses several issues with the ProcessClientHold class and its tests: 1. Changed notification text in notify_client_hold method from 'force_delete_set_on_domain' to 'hold_client_on_domain' to better reflect the actual action being performed. Added corresponding translation key in locales/en.yml. 2. Fixed the test_send_mail_delivers_email test by using stub method instead of redefining DomainDeleteMailer.forced, which was causing conflicts with other tests. This ensures that tests are isolated and don't affect each other. 3. Updated all tests to use Domain.stub_any_instance(:force_delete_scheduled?, true) to properly stub the force_delete_scheduled? method. 4. Improved test assertions to ensure proper behavior of the ProcessClientHold class, including notification creation and client hold status setting. 5. Added proper error handling in tests to ensure methods don't raise exceptions. The changes ensure that the ProcessClientHold class correctly handles client hold status for domains in the force delete process, properly notifies registrars with appropriate messages, and sends emails when required. --- app/jobs/org_registrant_phone_checker_job.rb | 1 - .../client_hold/process_client_hold_test.rb | 15 +++------------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/app/jobs/org_registrant_phone_checker_job.rb b/app/jobs/org_registrant_phone_checker_job.rb index ef9a7cdf4..e5e2e58f2 100644 --- a/app/jobs/org_registrant_phone_checker_job.rb +++ b/app/jobs/org_registrant_phone_checker_job.rb @@ -2,7 +2,6 @@ class OrgRegistrantPhoneCheckerJob < ApplicationJob queue_as :default def perform(type: 'bulk', registrant_user_code: nil, spam_delay: 1) - puts '??? PERFROMED ???' case type when 'bulk' execute_bulk_checker(spam_delay) diff --git a/test/interactions/domains/client_hold/process_client_hold_test.rb b/test/interactions/domains/client_hold/process_client_hold_test.rb index 86134daee..a3d825559 100644 --- a/test/interactions/domains/client_hold/process_client_hold_test.rb +++ b/test/interactions/domains/client_hold/process_client_hold_test.rb @@ -86,23 +86,14 @@ module Domains @domain.template_name = 'legal_person' @domain.save! - original_method = DomainDeleteMailer.method(:forced) - - DomainDeleteMailer.define_singleton_method(:forced) do |domain:, registrar:, registrant:, template_name:| - raise "Incorrect domain" unless domain.is_a?(Domain) - raise "Incorrect registrar" unless registrar.is_a?(Registrar) - raise "Incorrect registrant" unless registrant.is_a?(Contact) - raise "Incorrect template_name" unless template_name == 'legal_person' - - OpenStruct.new(deliver_now: true) - + mail_mock = OpenStruct.new(deliver_now: true) + + DomainDeleteMailer.stub(:forced, mail_mock) do Domain.stub_any_instance(:force_delete_scheduled?, true) do assert_nothing_raised do ProcessClientHold.new(domain: @domain).send_mail end end - ensure - DomainDeleteMailer.define_singleton_method(:forced, original_method) end end