From 626edbd4c3d016a9597bbff73ec5656b618e759c Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Wed, 27 Apr 2022 13:45:12 +0300 Subject: [PATCH 1/9] Add validation event check force delete to a job --- ...validation_event_check_force_delete_job.rb | 13 ++ app/models/validation_event.rb | 25 +-- test/models/domain/force_delete_test.rb | 168 +++++++++--------- 3 files changed, 106 insertions(+), 100 deletions(-) create mode 100644 app/jobs/validation_event_check_force_delete_job.rb diff --git a/app/jobs/validation_event_check_force_delete_job.rb b/app/jobs/validation_event_check_force_delete_job.rb new file mode 100644 index 000000000..3a0ee561b --- /dev/null +++ b/app/jobs/validation_event_check_force_delete_job.rb @@ -0,0 +1,13 @@ +class ValidationEventCheckForceDeleteJob < ApplicationJob + def perform(event_id, contact_id) + event = ValidationEvent.find(event_id) + contact = Contact.find(contact_id) + + if contact.need_to_start_force_delete? + event.start_force_delete + elsif contact.need_to_lift_force_delete? + event.refresh_status_notes + event.lift_force_delete + end + end +end diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 53a76e049..121c0e805 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -35,7 +35,7 @@ class ValidationEvent < ApplicationRecord scope :smtp, -> { where('event_data @> ?', { 'check_level': 'smtp' }.to_json) } scope :by_object, ->(object) { where(validation_eventable: object) } - # after_create :check_for_force_delete + after_create :check_for_force_delete def self.validated_ids_by(klass) recent.successful.where('validation_eventable_type = ?', klass) @@ -59,12 +59,7 @@ class ValidationEvent < ApplicationRecord end def check_for_force_delete - if object.need_to_start_force_delete? - start_force_delete - elsif object.need_to_lift_force_delete? - refresh_status_notes - lift_force_delete - end + ValidationEventCheckForceDeleteJob.perform_later(id, object.id) end def start_force_delete @@ -99,14 +94,12 @@ class ValidationEvent < ApplicationRecord end def lift_force_delete - # domain_contacts = Contact.where(email: email).map(&:domain_contacts).flatten - # registrant_ids = Registrant.where(email: email).pluck(:id) - # - # domains = domain_contacts.map(&:domain).flatten + - # Domain.where(registrant_id: registrant_ids) - # - # domains.each do |domain| - # Domains::ForceDeleteLift::Base.run(domain: domain) - # end + domain_contacts = Contact.where(email: email).map(&:domain_contacts).flatten + registrant_domains = Domain.where(registrant_id: Registrant.where(email: email).pluck(:id)) + domains = domain_contacts.map(&:domain).flatten + registrant_domains + + domains.each do |domain| + Domains::ForceDeleteLift::Base.run(domain: domain) + end end end diff --git a/test/models/domain/force_delete_test.rb b/test/models/domain/force_delete_test.rb index 799ac8169..f0dcb1007 100644 --- a/test/models/domain/force_delete_test.rb +++ b/test/models/domain/force_delete_test.rb @@ -380,115 +380,115 @@ class ForceDeleteTest < ActionMailer::TestCase assert notification.text.include? asserted_text end - # def test_schedules_force_delete_invalid_contact - # @domain.update(valid_to: Time.zone.parse('2012-08-05')) - # assert_not @domain.force_delete_scheduled? - # travel_to Time.zone.parse('2010-07-05') - # email = '`@internet.ee' - # asserted_text = "Invalid email: #{email}" + def test_schedules_force_delete_invalid_contact + @domain.update(valid_to: Time.zone.parse('2012-08-05')) + assert_not @domain.force_delete_scheduled? + travel_to Time.zone.parse('2010-07-05') + email = '`@internet.ee' + asserted_text = "Invalid email: #{email}" - # Truemail.configure.default_validation_type = :regex + Truemail.configure.default_validation_type = :regex - # contact = @domain.admin_contacts.first - # contact.update_attribute(:email, email) + contact = @domain.admin_contacts.first + contact.update_attribute(:email, email) - # ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD.times do - # contact.verify_email - # end + ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD.times do + contact.verify_email + end - # @domain.reload + @domain.reload - # assert @domain.force_delete_scheduled? - # assert_equal Date.parse('2010-09-19'), @domain.force_delete_date.to_date - # assert_equal Date.parse('2010-08-05'), @domain.force_delete_start.to_date - # assert_equal @domain.status_notes[DomainStatus::FORCE_DELETE], email - # notification = @domain.registrar.notifications.last - # assert notification.text.include? asserted_text - # end + assert @domain.force_delete_scheduled? + assert_equal Date.parse('2010-09-19'), @domain.force_delete_date.to_date + assert_equal Date.parse('2010-08-05'), @domain.force_delete_start.to_date + assert_equal @domain.status_notes[DomainStatus::FORCE_DELETE], email + notification = @domain.registrar.notifications.last + assert notification.text.include? asserted_text + end - # def test_add_invalid_email_to_domain_status_notes - # domain = domains(:airport) - # domain.update(valid_to: Time.zone.parse('2012-08-05'), - # statuses: %w[serverForceDelete serverRenewProhibited serverTransferProhibited], - # force_delete_data: { 'template_name': 'invalid_email', 'force_delete_type': 'soft' }, - # status_notes: { "serverForceDelete": '`@internet2.ee' }) + def test_add_invalid_email_to_domain_status_notes + domain = domains(:airport) + domain.update(valid_to: Time.zone.parse('2012-08-05'), + statuses: %w[serverForceDelete serverRenewProhibited serverTransferProhibited], + force_delete_data: { 'template_name': 'invalid_email', 'force_delete_type': 'soft' }, + status_notes: { "serverForceDelete": '`@internet2.ee' }) - # travel_to Time.zone.parse('2010-07-05') - # email = '`@internet.ee' - # invalid_emails = '`@internet2.ee `@internet.ee' - # asserted_text = "Invalid email: #{invalid_emails}" + travel_to Time.zone.parse('2010-07-05') + email = '`@internet.ee' + invalid_emails = '`@internet2.ee `@internet.ee' + asserted_text = "Invalid email: #{invalid_emails}" - # Truemail.configure.default_validation_type = :regex + Truemail.configure.default_validation_type = :regex - # contact_first = domain.admin_contacts.first - # contact_first.update_attribute(:email_history, 'john@inbox.test') - # contact_first.update_attribute(:email, email) + contact_first = domain.admin_contacts.first + contact_first.update_attribute(:email_history, 'john@inbox.test') + contact_first.update_attribute(:email, email) - # ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD.times do - # contact_first.verify_email - # end + ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD.times do + contact_first.verify_email + end - # domain.reload + domain.reload - # assert_equal domain.status_notes[DomainStatus::FORCE_DELETE], invalid_emails - # notification = domain.registrar.notifications.last - # assert_not notification.text.include? asserted_text - # end + assert_equal domain.status_notes[DomainStatus::FORCE_DELETE], invalid_emails + notification = domain.registrar.notifications.last + assert_not notification.text.include? asserted_text + end - # def test_remove_invalid_email_from_domain_status_notes - # domain = domains(:airport) - # domain.update(valid_to: Time.zone.parse('2012-08-05'), - # statuses: %w[serverForceDelete serverRenewProhibited serverTransferProhibited], - # force_delete_data: { 'template_name': 'invalid_email', 'force_delete_type': 'soft' }, - # status_notes: { "serverForceDelete": '`@internet2.ee `@internet.ee' }) + def test_remove_invalid_email_from_domain_status_notes + domain = domains(:airport) + domain.update(valid_to: Time.zone.parse('2012-08-05'), + statuses: %w[serverForceDelete serverRenewProhibited serverTransferProhibited], + force_delete_data: { 'template_name': 'invalid_email', 'force_delete_type': 'soft' }, + status_notes: { "serverForceDelete": '`@internet2.ee `@internet.ee' }) - # travel_to Time.zone.parse('2010-07-05') - # email = '`@internet2.ee' - # invalid_email = '`@internet.ee' - # asserted_text = "Invalid email: #{invalid_email}" + travel_to Time.zone.parse('2010-07-05') + email = '`@internet2.ee' + invalid_email = '`@internet.ee' + asserted_text = "Invalid email: #{invalid_email}" - # Truemail.configure.default_validation_type = :regex + Truemail.configure.default_validation_type = :regex - # contact_first = domain.admin_contacts.first - # contact_first.update_attribute(:email_history, email) - # contact_first.update_attribute(:email, 'john@inbox.test') + contact_first = domain.admin_contacts.first + contact_first.update_attribute(:email_history, email) + contact_first.update_attribute(:email, 'john@inbox.test') - # travel_to Time.zone.parse('2010-07-05 0:00:03') - # contact_first.verify_email - # domain.reload + travel_to Time.zone.parse('2010-07-05 0:00:03') + contact_first.verify_email + domain.reload - # assert_equal domain.status_notes[DomainStatus::FORCE_DELETE], invalid_email - # notification = domain.registrar.notifications.last - # assert notification.text.include? asserted_text - # end + assert_equal domain.status_notes[DomainStatus::FORCE_DELETE], invalid_email + notification = domain.registrar.notifications.last + assert notification.text.include? asserted_text + end - # def test_domain_should_have_several_bounced_emails - # @domain.update(valid_to: Time.zone.parse('2012-08-05')) - # assert_not @domain.force_delete_scheduled? - # travel_to Time.zone.parse('2010-07-05') - # email_one = '`@internet.ee' - # email_two = '@@internet.ee' + def test_domain_should_have_several_bounced_emails + @domain.update(valid_to: Time.zone.parse('2012-08-05')) + assert_not @domain.force_delete_scheduled? + travel_to Time.zone.parse('2010-07-05') + email_one = '`@internet.ee' + email_two = '@@internet.ee' - # contact_one = @domain.admin_contacts.first - # contact_one.update_attribute(:email, email_one) - # contact_one.verify_email + contact_one = @domain.admin_contacts.first + contact_one.update_attribute(:email, email_one) + contact_one.verify_email - # assert contact_one.need_to_start_force_delete? + assert contact_one.need_to_start_force_delete? - # contact_two = @domain.admin_contacts.first - # contact_two.update_attribute(:email, email_two) - # contact_two.verify_email + contact_two = @domain.admin_contacts.first + contact_two.update_attribute(:email, email_two) + contact_two.verify_email - # assert contact_two.need_to_start_force_delete? + assert contact_two.need_to_start_force_delete? - # @domain.reload + @domain.reload - # assert @domain.force_delete_scheduled? - # assert_equal Date.parse('2010-09-19'), @domain.force_delete_date.to_date - # assert_equal Date.parse('2010-08-05'), @domain.force_delete_start.to_date - # assert @domain.status_notes[DomainStatus::FORCE_DELETE].include? email_one - # assert @domain.status_notes[DomainStatus::FORCE_DELETE].include? email_two - # end + assert @domain.force_delete_scheduled? + assert_equal Date.parse('2010-09-19'), @domain.force_delete_date.to_date + assert_equal Date.parse('2010-08-05'), @domain.force_delete_start.to_date + assert @domain.status_notes[DomainStatus::FORCE_DELETE].include? email_one + assert @domain.status_notes[DomainStatus::FORCE_DELETE].include? email_two + end def test_lifts_force_delete_after_bounce_changes @domain.update(valid_to: Time.zone.parse('2012-08-05')) From c246bbaa6d0d2c694ade103aaf78d770f6988f64 Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Thu, 28 Apr 2022 12:29:24 +0300 Subject: [PATCH 2/9] Remove legacy code --- app/jobs/validation_event_check_force_delete_job.rb | 1 - app/models/validation_event.rb | 10 ---------- test/models/domain/force_delete_test.rb | 8 ++++++++ 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/app/jobs/validation_event_check_force_delete_job.rb b/app/jobs/validation_event_check_force_delete_job.rb index 3a0ee561b..1b914ae9a 100644 --- a/app/jobs/validation_event_check_force_delete_job.rb +++ b/app/jobs/validation_event_check_force_delete_job.rb @@ -7,7 +7,6 @@ class ValidationEventCheckForceDeleteJob < ApplicationJob event.start_force_delete elsif contact.need_to_lift_force_delete? event.refresh_status_notes - event.lift_force_delete end end end diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 121c0e805..27e69ad44 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -92,14 +92,4 @@ class ValidationEvent < ApplicationRecord purge_date: domain.purge_date, email: domain.status_notes[DomainStatus::FORCE_DELETE])) end - - def lift_force_delete - domain_contacts = Contact.where(email: email).map(&:domain_contacts).flatten - registrant_domains = Domain.where(registrant_id: Registrant.where(email: email).pluck(:id)) - domains = domain_contacts.map(&:domain).flatten + registrant_domains - - domains.each do |domain| - Domains::ForceDeleteLift::Base.run(domain: domain) - end - end end diff --git a/test/models/domain/force_delete_test.rb b/test/models/domain/force_delete_test.rb index f0dcb1007..21c6da229 100644 --- a/test/models/domain/force_delete_test.rb +++ b/test/models/domain/force_delete_test.rb @@ -1,6 +1,8 @@ require 'test_helper' class ForceDeleteTest < ActionMailer::TestCase + include ActiveJob::TestHelper + setup do @domain = domains(:shop) Setting.redemption_grace_period = 30 @@ -396,6 +398,7 @@ class ForceDeleteTest < ActionMailer::TestCase contact.verify_email end + perform_enqueued_jobs @domain.reload assert @domain.force_delete_scheduled? @@ -428,6 +431,7 @@ class ForceDeleteTest < ActionMailer::TestCase contact_first.verify_email end + perform_enqueued_jobs domain.reload assert_equal domain.status_notes[DomainStatus::FORCE_DELETE], invalid_emails @@ -455,6 +459,8 @@ class ForceDeleteTest < ActionMailer::TestCase travel_to Time.zone.parse('2010-07-05 0:00:03') contact_first.verify_email + + perform_enqueued_jobs domain.reload assert_equal domain.status_notes[DomainStatus::FORCE_DELETE], invalid_email @@ -472,12 +478,14 @@ class ForceDeleteTest < ActionMailer::TestCase contact_one = @domain.admin_contacts.first contact_one.update_attribute(:email, email_one) contact_one.verify_email + perform_enqueued_jobs assert contact_one.need_to_start_force_delete? contact_two = @domain.admin_contacts.first contact_two.update_attribute(:email, email_two) contact_two.verify_email + perform_enqueued_jobs assert contact_two.need_to_start_force_delete? From e159821a864211ea04085fdd774d8e909921debc Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Mon, 2 May 2022 11:50:03 +0300 Subject: [PATCH 3/9] Create check force delete rake task --- ...validation_event_check_force_delete_job.rb | 38 +++++++++++++-- app/models/concerns/email_verifable.rb | 7 +-- app/models/validation_event.rb | 37 --------------- lib/tasks/check_force_delete.rake | 12 +++++ test/tasks/check_force_delete_test.rb | 46 +++++++++++++++++++ 5 files changed, 93 insertions(+), 47 deletions(-) create mode 100644 lib/tasks/check_force_delete.rake create mode 100644 test/tasks/check_force_delete_test.rb diff --git a/app/jobs/validation_event_check_force_delete_job.rb b/app/jobs/validation_event_check_force_delete_job.rb index 1b914ae9a..d779bb1a6 100644 --- a/app/jobs/validation_event_check_force_delete_job.rb +++ b/app/jobs/validation_event_check_force_delete_job.rb @@ -1,12 +1,42 @@ class ValidationEventCheckForceDeleteJob < ApplicationJob - def perform(event_id, contact_id) - event = ValidationEvent.find(event_id) + def perform(contact_id) contact = Contact.find(contact_id) + email = contact.email if contact.need_to_start_force_delete? - event.start_force_delete + Domains::ForceDeleteEmail::Base.run(email: email) elsif contact.need_to_lift_force_delete? - event.refresh_status_notes + refresh_status_notes(domain_list(email)) end end + + private + + def refresh_status_notes(domain_list) + domain_list.each do |domain| + force_delete_emails = domain.status_notes[DomainStatus::FORCE_DELETE] + next unless force_delete_emails + + force_delete_emails.slice!(object.email_history) + force_delete_emails.lstrip! + domain.save(validate: false) + + notify_registrar(domain) unless domain.status_notes[DomainStatus::FORCE_DELETE].empty? + end + end + + def domain_list(email) + domain_contacts = Contact.where(email: email).map(&:domain_contacts).flatten + registrant_ids = Registrant.where(email: email).pluck(:id) + + (domain_contacts.map(&:domain).flatten + Domain.where(registrant_id: registrant_ids)).uniq + end + + def notify_registrar(domain) + domain.registrar.notifications.create!(text: I18n.t('force_delete_auto_email', + domain_name: domain.name, + outzone_date: domain.outzone_date, + purge_date: domain.purge_date, + email: domain.status_notes[DomainStatus::FORCE_DELETE])) + end end diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index 77d457e9e..208061110 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -12,14 +12,9 @@ module EmailVerifable end def need_to_start_force_delete? - flag = false ValidationEvent::INVALID_EVENTS_COUNT_BY_LEVEL.each do |level, count| - if validation_events.count >= count && validate_email_data(level: level, count: count) - flag = true - end + validation_events.count >= count && validate_email_data(level: level, count: count) end - - flag end def need_to_lift_force_delete? diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 27e69ad44..42a0c0038 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -35,8 +35,6 @@ class ValidationEvent < ApplicationRecord scope :smtp, -> { where('event_data @> ?', { 'check_level': 'smtp' }.to_json) } scope :by_object, ->(object) { where(validation_eventable: object) } - after_create :check_for_force_delete - def self.validated_ids_by(klass) recent.successful.where('validation_eventable_type = ?', klass) .pluck(:validation_eventable_id) @@ -57,39 +55,4 @@ class ValidationEvent < ApplicationRecord def object validation_eventable end - - def check_for_force_delete - ValidationEventCheckForceDeleteJob.perform_later(id, object.id) - end - - def start_force_delete - Domains::ForceDeleteEmail::Base.run(email: email) - end - - def refresh_status_notes - domain_list.each do |domain| - next unless domain.status_notes[DomainStatus::FORCE_DELETE] - - domain.status_notes[DomainStatus::FORCE_DELETE].slice!(object.email_history) - domain.status_notes[DomainStatus::FORCE_DELETE].lstrip! - domain.save(validate: false) - - notify_registrar(domain) unless domain.status_notes[DomainStatus::FORCE_DELETE].empty? - end - end - - def domain_list - domain_contacts = Contact.where(email: email).map(&:domain_contacts).flatten - registrant_ids = Registrant.where(email: email).pluck(:id) - - (domain_contacts.map(&:domain).flatten + Domain.where(registrant_id: registrant_ids)).uniq - end - - def notify_registrar(domain) - domain.registrar.notifications.create!(text: I18n.t('force_delete_auto_email', - domain_name: domain.name, - outzone_date: domain.outzone_date, - purge_date: domain.purge_date, - email: domain.status_notes[DomainStatus::FORCE_DELETE])) - end end diff --git a/lib/tasks/check_force_delete.rake b/lib/tasks/check_force_delete.rake new file mode 100644 index 000000000..277ce2e5b --- /dev/null +++ b/lib/tasks/check_force_delete.rake @@ -0,0 +1,12 @@ +desc 'Check Force Delete' +task :check_force_delete do + + invalid_contacts = Contact.joins(:validation_events).select do |contact| + events = contact.validation_events + events.mx.count >= 3 || events.regex.present? + end.uniq(&:id) + + invalid_contacts.each do |contact| + ValidationEventCheckForceDeleteJob.perform_later(contact.id) + end +end diff --git a/test/tasks/check_force_delete_test.rb b/test/tasks/check_force_delete_test.rb new file mode 100644 index 000000000..c1d3ebc61 --- /dev/null +++ b/test/tasks/check_force_delete_test.rb @@ -0,0 +1,46 @@ +require 'test_helper' + +class CheckForceDeleteTaskTest < ActiveSupport::TestCase + include ActiveJob::TestHelper + + def setup + @contact = contacts(:john) + @invalid_contact = contacts(:invalid_email) + end + + def test_enque_force_delete_when_three_invalid_records_by_mx + trumail_results = OpenStruct.new(success: false, + email: @contact.email, + domain: 'box.tests', + errors: { mx: 'target host(s) not found' }) + + Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) + Spy.on_instance_method(Actions::AAndAaaaEmailValidation, :call).and_return([]) + + action = Actions::EmailCheck.new(email: @contact.email, + validation_eventable: @contact, + check_level: 'mx') + 3.times do + action.call + end + + run_task + + assert_enqueued_jobs 1 + assert_enqueued_with(job: ValidationEventCheckForceDeleteJob, args: [@contact.id]) + end + + def test_enque_force_delete_when_invalid_record_by_regex + @invalid_contact.verify_email + run_task + + assert_enqueued_jobs 1 + assert_enqueued_with(job: ValidationEventCheckForceDeleteJob, args: [@invalid_contact.id]) + end + + private + + def run_task + Rake::Task['check_force_delete'].execute + end +end From 944aba563cf1cb3212ed1779a355f89847db30ff Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Mon, 2 May 2022 12:52:32 +0300 Subject: [PATCH 4/9] Fix tests --- ...validation_event_check_force_delete_job.rb | 27 +++++++++---------- app/models/concerns/email_verifable.rb | 5 +++- lib/tasks/check_force_delete.rake | 3 +-- test/models/domain/force_delete_test.rb | 18 +++++++++---- 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/app/jobs/validation_event_check_force_delete_job.rb b/app/jobs/validation_event_check_force_delete_job.rb index d779bb1a6..5330a4f35 100644 --- a/app/jobs/validation_event_check_force_delete_job.rb +++ b/app/jobs/validation_event_check_force_delete_job.rb @@ -1,33 +1,32 @@ class ValidationEventCheckForceDeleteJob < ApplicationJob def perform(contact_id) - contact = Contact.find(contact_id) - email = contact.email + @contact = Contact.find(contact_id) + @email = @contact.email - if contact.need_to_start_force_delete? - Domains::ForceDeleteEmail::Base.run(email: email) - elsif contact.need_to_lift_force_delete? - refresh_status_notes(domain_list(email)) + if @contact.need_to_start_force_delete? + Domains::ForceDeleteEmail::Base.run(email: @email) + elsif @contact.need_to_lift_force_delete? + refresh_status_notes end end private - def refresh_status_notes(domain_list) + def refresh_status_notes domain_list.each do |domain| - force_delete_emails = domain.status_notes[DomainStatus::FORCE_DELETE] - next unless force_delete_emails + next unless domain.status_notes[DomainStatus::FORCE_DELETE] - force_delete_emails.slice!(object.email_history) - force_delete_emails.lstrip! + domain.status_notes[DomainStatus::FORCE_DELETE].slice!(@contact.email_history) + domain.status_notes[DomainStatus::FORCE_DELETE].lstrip! domain.save(validate: false) notify_registrar(domain) unless domain.status_notes[DomainStatus::FORCE_DELETE].empty? end end - def domain_list(email) - domain_contacts = Contact.where(email: email).map(&:domain_contacts).flatten - registrant_ids = Registrant.where(email: email).pluck(:id) + def domain_list + domain_contacts = Contact.where(email: @email).map(&:domain_contacts).flatten + registrant_ids = Registrant.where(email: @email).pluck(:id) (domain_contacts.map(&:domain).flatten + Domain.where(registrant_id: registrant_ids)).uniq end diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index 208061110..be9603031 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -12,9 +12,12 @@ module EmailVerifable end def need_to_start_force_delete? + flag = false ValidationEvent::INVALID_EVENTS_COUNT_BY_LEVEL.each do |level, count| - validation_events.count >= count && validate_email_data(level: level, count: count) + flag = true if validation_events.count >= count && validate_email_data(level: level, count: count) end + + flag end def need_to_lift_force_delete? diff --git a/lib/tasks/check_force_delete.rake b/lib/tasks/check_force_delete.rake index 277ce2e5b..74e0643c8 100644 --- a/lib/tasks/check_force_delete.rake +++ b/lib/tasks/check_force_delete.rake @@ -1,6 +1,5 @@ desc 'Check Force Delete' -task :check_force_delete do - +task :check_force_delete, :environment do invalid_contacts = Contact.joins(:validation_events).select do |contact| events = contact.validation_events events.mx.count >= 3 || events.regex.present? diff --git a/test/models/domain/force_delete_test.rb b/test/models/domain/force_delete_test.rb index 21c6da229..91f61cb6a 100644 --- a/test/models/domain/force_delete_test.rb +++ b/test/models/domain/force_delete_test.rb @@ -398,7 +398,7 @@ class ForceDeleteTest < ActionMailer::TestCase contact.verify_email end - perform_enqueued_jobs + perform_check_force_delete_job(contact.id) @domain.reload assert @domain.force_delete_scheduled? @@ -431,7 +431,7 @@ class ForceDeleteTest < ActionMailer::TestCase contact_first.verify_email end - perform_enqueued_jobs + perform_check_force_delete_job(contact_first.id) domain.reload assert_equal domain.status_notes[DomainStatus::FORCE_DELETE], invalid_emails @@ -460,7 +460,7 @@ class ForceDeleteTest < ActionMailer::TestCase travel_to Time.zone.parse('2010-07-05 0:00:03') contact_first.verify_email - perform_enqueued_jobs + perform_check_force_delete_job(contact_first.id) domain.reload assert_equal domain.status_notes[DomainStatus::FORCE_DELETE], invalid_email @@ -478,14 +478,14 @@ class ForceDeleteTest < ActionMailer::TestCase contact_one = @domain.admin_contacts.first contact_one.update_attribute(:email, email_one) contact_one.verify_email - perform_enqueued_jobs + perform_check_force_delete_job(contact_one.id) assert contact_one.need_to_start_force_delete? contact_two = @domain.admin_contacts.first contact_two.update_attribute(:email, email_two) contact_two.verify_email - perform_enqueued_jobs + perform_check_force_delete_job(contact_two.id) assert contact_two.need_to_start_force_delete? @@ -535,4 +535,12 @@ class ForceDeleteTest < ActionMailer::TestCase @bounced_mail.diagnostic = 'smtp; 550 5.1.1 user unknown' @bounced_mail.save! end + + private + + def perform_check_force_delete_job(contact_id) + perform_enqueued_jobs do + ValidationEventCheckForceDeleteJob.perform_now(contact_id) + end + end end From be0ac715b109b18962f743c3f0ba4aee25324007 Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Mon, 2 May 2022 15:54:46 +0300 Subject: [PATCH 5/9] Fix code climate --- ...validation_event_check_force_delete_job.rb | 27 +++++++++---------- lib/tasks/check_force_delete.rake | 2 +- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/app/jobs/validation_event_check_force_delete_job.rb b/app/jobs/validation_event_check_force_delete_job.rb index 5330a4f35..6441b271f 100644 --- a/app/jobs/validation_event_check_force_delete_job.rb +++ b/app/jobs/validation_event_check_force_delete_job.rb @@ -1,32 +1,31 @@ class ValidationEventCheckForceDeleteJob < ApplicationJob def perform(contact_id) @contact = Contact.find(contact_id) - @email = @contact.email + email = @contact.email if @contact.need_to_start_force_delete? - Domains::ForceDeleteEmail::Base.run(email: @email) + Domains::ForceDeleteEmail::Base.run(email: email) elsif @contact.need_to_lift_force_delete? - refresh_status_notes + domain_list(email).each { |domain| refresh_status_notes(domain) } end end private - def refresh_status_notes - domain_list.each do |domain| - next unless domain.status_notes[DomainStatus::FORCE_DELETE] + def refresh_status_notes(domain) + force_delete_emails = domain.status_notes[DomainStatus::FORCE_DELETE] + return unless force_delete_emails - domain.status_notes[DomainStatus::FORCE_DELETE].slice!(@contact.email_history) - domain.status_notes[DomainStatus::FORCE_DELETE].lstrip! - domain.save(validate: false) + force_delete_emails.slice!(@contact.email_history) + force_delete_emails.lstrip! + domain.save(validate: false) - notify_registrar(domain) unless domain.status_notes[DomainStatus::FORCE_DELETE].empty? - end + notify_registrar(domain) unless force_delete_emails.empty? end - def domain_list - domain_contacts = Contact.where(email: @email).map(&:domain_contacts).flatten - registrant_ids = Registrant.where(email: @email).pluck(:id) + def domain_list(email) + domain_contacts = Contact.where(email: email).map(&:domain_contacts).flatten + registrant_ids = Registrant.where(email: email).pluck(:id) (domain_contacts.map(&:domain).flatten + Domain.where(registrant_id: registrant_ids)).uniq end diff --git a/lib/tasks/check_force_delete.rake b/lib/tasks/check_force_delete.rake index 74e0643c8..6c79b7039 100644 --- a/lib/tasks/check_force_delete.rake +++ b/lib/tasks/check_force_delete.rake @@ -1,5 +1,5 @@ desc 'Check Force Delete' -task :check_force_delete, :environment do +task check_force_delete: :environment do invalid_contacts = Contact.joins(:validation_events).select do |contact| events = contact.validation_events events.mx.count >= 3 || events.regex.present? From b1afdbf50e65f96a1452f9eed2778cdbbf4c22e8 Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Tue, 3 May 2022 13:43:19 +0300 Subject: [PATCH 6/9] Add rake task optional args --- ...elete_job.rb => check_force_delete_job.rb} | 23 ++++++++------- lib/tasks/check_force_delete.rake | 28 +++++++++++++++---- test/models/domain/force_delete_test.rb | 2 +- test/tasks/check_force_delete_test.rb | 4 +-- 4 files changed, 39 insertions(+), 18 deletions(-) rename app/jobs/{validation_event_check_force_delete_job.rb => check_force_delete_job.rb} (67%) diff --git a/app/jobs/validation_event_check_force_delete_job.rb b/app/jobs/check_force_delete_job.rb similarity index 67% rename from app/jobs/validation_event_check_force_delete_job.rb rename to app/jobs/check_force_delete_job.rb index 6441b271f..5fb3408f1 100644 --- a/app/jobs/validation_event_check_force_delete_job.rb +++ b/app/jobs/check_force_delete_job.rb @@ -1,22 +1,25 @@ -class ValidationEventCheckForceDeleteJob < ApplicationJob - def perform(contact_id) - @contact = Contact.find(contact_id) - email = @contact.email +class CheckForceDeleteJob < ApplicationJob + def perform(contact_ids) + contacts = Contact.find(contact_ids) - if @contact.need_to_start_force_delete? - Domains::ForceDeleteEmail::Base.run(email: email) - elsif @contact.need_to_lift_force_delete? - domain_list(email).each { |domain| refresh_status_notes(domain) } + contacts.each do |contact| + email = contact.email + + if contact.need_to_start_force_delete? + Domains::ForceDeleteEmail::Base.run(email: email) + elsif contact.need_to_lift_force_delete? + domain_list(email).each { |domain| refresh_status_notes(contact, domain) } + end end end private - def refresh_status_notes(domain) + def refresh_status_notes(contact, domain) force_delete_emails = domain.status_notes[DomainStatus::FORCE_DELETE] return unless force_delete_emails - force_delete_emails.slice!(@contact.email_history) + force_delete_emails.slice!(contact.email_history) force_delete_emails.lstrip! domain.save(validate: false) diff --git a/lib/tasks/check_force_delete.rake b/lib/tasks/check_force_delete.rake index 6c79b7039..4fb989729 100644 --- a/lib/tasks/check_force_delete.rake +++ b/lib/tasks/check_force_delete.rake @@ -1,11 +1,29 @@ desc 'Check Force Delete' -task check_force_delete: :environment do +task :check_force_delete, %i[batch batch_size batches_delay] => :environment do |_t, args| + args.with_defaults(batch: false, batch_size: 1_000, batches_delay: 15) + + batch = ActiveModel::Type::Boolean.new.cast(args[:batch]) + batch_size = args[:batch_size].to_i + batches_delay = args[:batches_delay].to_i.minutes + invalid_contacts = Contact.joins(:validation_events).select do |contact| events = contact.validation_events - events.mx.count >= 3 || events.regex.present? - end.uniq(&:id) + mx = events.mx.select(&:failed?).count >= ValidationEvent::MX_CHECK + regex = events.regex.select(&:failed?).present? - invalid_contacts.each do |contact| - ValidationEventCheckForceDeleteJob.perform_later(contact.id) + (contact.need_to_start_force_delete? || contact.need_to_lift_force_delete?) && (mx || regex) + end.uniq + + if batch + waiting_minutes = 0.minutes + + invalid_contacts.find_in_batches(batch_size: batch_size) do |contact_batches| + CheckForceDeleteJob.set(wait: waiting_minutes).perform_later(contact_batches) + waiting_minutes += batches_delay + end + else + invalid_contacts.each do |contact| + CheckForceDeleteJob.perform_later([contact.id]) + end end end diff --git a/test/models/domain/force_delete_test.rb b/test/models/domain/force_delete_test.rb index 91f61cb6a..c333e5938 100644 --- a/test/models/domain/force_delete_test.rb +++ b/test/models/domain/force_delete_test.rb @@ -540,7 +540,7 @@ class ForceDeleteTest < ActionMailer::TestCase def perform_check_force_delete_job(contact_id) perform_enqueued_jobs do - ValidationEventCheckForceDeleteJob.perform_now(contact_id) + ValidationEventCheckForceDeleteJob.perform_now([contact_id]) end end end diff --git a/test/tasks/check_force_delete_test.rb b/test/tasks/check_force_delete_test.rb index c1d3ebc61..692e03ac1 100644 --- a/test/tasks/check_force_delete_test.rb +++ b/test/tasks/check_force_delete_test.rb @@ -27,7 +27,7 @@ class CheckForceDeleteTaskTest < ActiveSupport::TestCase run_task assert_enqueued_jobs 1 - assert_enqueued_with(job: ValidationEventCheckForceDeleteJob, args: [@contact.id]) + assert_enqueued_with(job: CheckForceDeleteJob, args: [@contact.id]) end def test_enque_force_delete_when_invalid_record_by_regex @@ -35,7 +35,7 @@ class CheckForceDeleteTaskTest < ActiveSupport::TestCase run_task assert_enqueued_jobs 1 - assert_enqueued_with(job: ValidationEventCheckForceDeleteJob, args: [@invalid_contact.id]) + assert_enqueued_with(job: CheckForceDeleteJob, args: [@invalid_contact.id]) end private From ee76deaf52bc223e2efd53c7c78674087fa5945a Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Tue, 3 May 2022 14:10:48 +0300 Subject: [PATCH 7/9] Fix tests --- test/models/domain/force_delete_test.rb | 2 +- test/tasks/check_force_delete_test.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/models/domain/force_delete_test.rb b/test/models/domain/force_delete_test.rb index c333e5938..48372210b 100644 --- a/test/models/domain/force_delete_test.rb +++ b/test/models/domain/force_delete_test.rb @@ -540,7 +540,7 @@ class ForceDeleteTest < ActionMailer::TestCase def perform_check_force_delete_job(contact_id) perform_enqueued_jobs do - ValidationEventCheckForceDeleteJob.perform_now([contact_id]) + CheckForceDeleteJob.perform_now([contact_id]) end end end diff --git a/test/tasks/check_force_delete_test.rb b/test/tasks/check_force_delete_test.rb index 692e03ac1..c52937eaf 100644 --- a/test/tasks/check_force_delete_test.rb +++ b/test/tasks/check_force_delete_test.rb @@ -27,7 +27,7 @@ class CheckForceDeleteTaskTest < ActiveSupport::TestCase run_task assert_enqueued_jobs 1 - assert_enqueued_with(job: CheckForceDeleteJob, args: [@contact.id]) + assert_enqueued_with(job: CheckForceDeleteJob, args: [[@contact.id]]) end def test_enque_force_delete_when_invalid_record_by_regex @@ -35,7 +35,7 @@ class CheckForceDeleteTaskTest < ActiveSupport::TestCase run_task assert_enqueued_jobs 1 - assert_enqueued_with(job: CheckForceDeleteJob, args: [@invalid_contact.id]) + assert_enqueued_with(job: CheckForceDeleteJob, args: [[@invalid_contact.id]]) end private From 27fbf224f97e96146399f9725e110f7892b322ba Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Tue, 10 May 2022 14:35:12 +0300 Subject: [PATCH 8/9] Improve check force delete query --- lib/tasks/check_force_delete.rake | 29 ++++----------------------- test/tasks/check_force_delete_test.rb | 19 ++++++++++++++++++ 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/lib/tasks/check_force_delete.rake b/lib/tasks/check_force_delete.rake index 4fb989729..2fed5ddeb 100644 --- a/lib/tasks/check_force_delete.rake +++ b/lib/tasks/check_force_delete.rake @@ -1,29 +1,8 @@ desc 'Check Force Delete' -task :check_force_delete, %i[batch batch_size batches_delay] => :environment do |_t, args| - args.with_defaults(batch: false, batch_size: 1_000, batches_delay: 15) +task check_force_delete: :environment do - batch = ActiveModel::Type::Boolean.new.cast(args[:batch]) - batch_size = args[:batch_size].to_i - batches_delay = args[:batches_delay].to_i.minutes + validations = ValidationEvent.select(:validation_eventable_id).failed.where(validation_eventable_type: 'Contact').group(:validation_eventable_id) + invalid_contact_ids = (validations.mx.having("count(event_data ->> 'success') > 2") + validations.regex).pluck(:validation_eventable_id) - invalid_contacts = Contact.joins(:validation_events).select do |contact| - events = contact.validation_events - mx = events.mx.select(&:failed?).count >= ValidationEvent::MX_CHECK - regex = events.regex.select(&:failed?).present? - - (contact.need_to_start_force_delete? || contact.need_to_lift_force_delete?) && (mx || regex) - end.uniq - - if batch - waiting_minutes = 0.minutes - - invalid_contacts.find_in_batches(batch_size: batch_size) do |contact_batches| - CheckForceDeleteJob.set(wait: waiting_minutes).perform_later(contact_batches) - waiting_minutes += batches_delay - end - else - invalid_contacts.each do |contact| - CheckForceDeleteJob.perform_later([contact.id]) - end - end + CheckForceDeleteJob.perform_later(invalid_contact_ids) end diff --git a/test/tasks/check_force_delete_test.rb b/test/tasks/check_force_delete_test.rb index c52937eaf..dbe2f40aa 100644 --- a/test/tasks/check_force_delete_test.rb +++ b/test/tasks/check_force_delete_test.rb @@ -38,6 +38,25 @@ class CheckForceDeleteTaskTest < ActiveSupport::TestCase assert_enqueued_with(job: CheckForceDeleteJob, args: [[@invalid_contact.id]]) end + def test_not_enque_force_delete + trumail_results = OpenStruct.new(success: false, + email: @contact.email, + domain: 'box.tests', + errors: { mx: 'target host(s) not found' }) + + Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) + Spy.on_instance_method(Actions::AAndAaaaEmailValidation, :call).and_return([]) + + action = Actions::EmailCheck.new(email: @contact.email, + validation_eventable: @contact, + check_level: 'mx') + 2.times do + action.call + end + + assert_enqueued_jobs 0 + end + private def run_task From 58c51820df1c6e61244055775b1ae1318d070f52 Mon Sep 17 00:00:00 2001 From: Thiago Youssef Date: Tue, 10 May 2022 16:19:13 +0300 Subject: [PATCH 9/9] Fix code climate --- lib/tasks/check_force_delete.rake | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/tasks/check_force_delete.rake b/lib/tasks/check_force_delete.rake index 2fed5ddeb..7bb66b743 100644 --- a/lib/tasks/check_force_delete.rake +++ b/lib/tasks/check_force_delete.rake @@ -1,8 +1,12 @@ desc 'Check Force Delete' task check_force_delete: :environment do + validations = ValidationEvent.select(:validation_eventable_id) + .failed.where(validation_eventable_type: 'Contact') + .group(:validation_eventable_id) - validations = ValidationEvent.select(:validation_eventable_id).failed.where(validation_eventable_type: 'Contact').group(:validation_eventable_id) - invalid_contact_ids = (validations.mx.having("count(event_data ->> 'success') > 2") + validations.regex).pluck(:validation_eventable_id) + invalid_contact_ids = ( + validations.mx.having("count(event_data ->> 'success') > 2") + validations.regex + ).pluck(:validation_eventable_id) CheckForceDeleteJob.perform_later(invalid_contact_ids) end