diff --git a/app/jobs/verify_emails_job.rb b/app/jobs/verify_emails_job.rb index 641c48184..4f66601cc 100644 --- a/app/jobs/verify_emails_job.rb +++ b/app/jobs/verify_emails_job.rb @@ -3,9 +3,11 @@ class VerifyEmailsJob < ApplicationJob def perform(contact_id:, check_level: 'regex') contact = Contact.find_by(id: contact_id) + + return if check_contact_for_duplicate_mail(contact_id) + contact_not_found(contact_id) unless contact validate_check_level(check_level) - action = Actions::EmailCheck.new(email: contact.email, validation_eventable: contact, check_level: check_level) @@ -17,6 +19,16 @@ class VerifyEmailsJob < ApplicationJob private + def check_contact_for_duplicate_mail(contact_id) + time = Time.zone.now - ValidationEvent::VALIDATION_PERIOD + contact = Contact.find(contact_id) + contact_ids = Contact.where(email: contact.email).where('created_at > ?', time).pluck(:id) + + r = ValidationEvent.where(validation_eventable_id: contact_ids).order(created_at: :desc) + + r.present? + end + def contact_not_found(contact_id) raise StandardError, "Contact with contact_id #{contact_id} not found" end diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index 54a945ef8..4f9b4ffeb 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -10,25 +10,26 @@ module EmailVerifable end def validate_email_data(level:, count:) - validation_events.recent.order(id: :desc).limit(count).all? do |event| + validation_events.order(created_at: :desc).limit(count).all? do |event| event.check_level == level.to_s && event.failed? end end def need_to_start_force_delete? + flag = false ValidationEvent::INVALID_EVENTS_COUNT_BY_LEVEL.each do |level, count| - if validation_events.recent.count >= count && validate_email_data(level: level, count: count) - return true + if validation_events.count >= count && validate_email_data(level: level, count: count) + flag = true end end - false + flag end def need_to_lift_force_delete? - validation_events.recent.failed.empty? || + validation_events.failed.empty? || ValidationEvent::REDEEM_EVENTS_COUNT_BY_LEVEL.any? do |level, count| - validation_events.recent.order(id: :desc).limit(count).all? do |event| + validation_events.order(created_at: :desc).limit(count).all? do |event| event.check_level == level.to_s && event.successful? end end diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index aff5582bc..68983b793 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -6,13 +6,13 @@ # For email_validation event kind also check_level (regex/mx/smtp) is stored in the event_data class ValidationEvent < ApplicationRecord enum event_type: ValidationEvent::EventType::TYPES, _suffix: true - VALIDATION_PERIOD = 1.month.freeze + VALIDATION_PERIOD = 1.year.freeze VALID_CHECK_LEVELS = %w[regex mx smtp].freeze VALID_EVENTS_COUNT_THRESHOLD = 5 INVALID_EVENTS_COUNT_BY_LEVEL = { regex: 1, - mx: 5, + mx: 3, smtp: 1, }.freeze @@ -26,7 +26,7 @@ class ValidationEvent < ApplicationRecord belongs_to :validation_eventable, polymorphic: true - scope :recent, -> { where('created_at > ?', Time.zone.now - VALIDATION_PERIOD) } + scope :recent, -> { where('created_at < ?', Time.zone.now - VALIDATION_PERIOD) } scope :successful, -> { where(success: true) } scope :failed, -> { where(success: false) } scope :regex, -> { where('event_data @> ?', { 'check_level': 'regex' }.to_json) } @@ -69,5 +69,15 @@ class ValidationEvent < ApplicationRecord Domains::ForceDeleteEmail::Base.run(email: email) end - def lift_force_delete; 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 + end end diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index 9ca38199f..8efaee96e 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -21,7 +21,7 @@ namespace :verify_email do contacts = prepare_contacts(options) logger.info 'No contacts to check email selected' and next if contacts.blank? - contacts.find_each do |contact| + contacts.each do |contact| VerifyEmailsJob.set(wait_until: spam_protect_timeout(options)).perform_later( contact_id: contact.id, check_level: check_level(options) @@ -46,14 +46,44 @@ def logger @logger ||= ActiveSupport::TaggedLogging.new(Syslog::Logger.new('registry')) end +# Here I set the time after which the validation is considered obsolete +# I take all contact records that have successfully passed the verification and fall within the deadline +# I am looking for contacts that have not been verified or their verification is out of date + def prepare_contacts(options) if options[:domain_name].present? contacts_by_domain(options[:domain_name]) else - Contact.all + time = Time.zone.now - ValidationEvent::VALIDATION_PERIOD + validation_events_ids = ValidationEvent.where('created_at > ?', time).pluck(:validation_eventable_id) + + # Contact.where.not(id: validation_events_ids) + Contact.where(id: failed_contacts) + Contact.where.not(id: validation_events_ids) | failed_contacts end end +def failed_contacts + failed_contacts = [] + failed_validations_ids = ValidationEvent.failed.pluck(:validation_eventable_id) + contacts = Contact.where(id: failed_validations_ids) + contacts.each do |contact| + + if contact.validation_events.mx.order(created_at: :asc).present? + failed_contacts << contact unless contact.validation_events.mx.order(created_at: :asc).last.success + end + + if contact.validation_events.regex.order(created_at: :asc).present? + failed_contacts << contact unless contact.validation_events.regex.order(created_at: :asc).last.success + end + + if contact.validation_events.smtp.order(created_at: :asc).present? + failed_contacts << contact unless contact.validation_events.mx.order(created_at: :asc).last.success + end + end + + failed_contacts.uniq +end + def contacts_by_domain(domain_name) domain = ::Domain.find_by(name: domain_name) return unless domain diff --git a/test/jobs/verify_emails_job_test.rb b/test/jobs/verify_emails_job_test.rb index d4eed0ec0..5290acbc7 100644 --- a/test/jobs/verify_emails_job_test.rb +++ b/test/jobs/verify_emails_job_test.rb @@ -29,15 +29,6 @@ class VerifyEmailsJobTest < ActiveJob::TestCase [domain(@invalid_contact.email)].reject(&:blank?) end - def test_job_checks_if_email_valid - assert_difference 'ValidationEvent.successful.count', 1 do - perform_enqueued_jobs do - VerifyEmailsJob.perform_now(contact_id: @contact.id, check_level: 'regex') - end - end - assert ValidationEvent.validated_ids_by(Contact).include? @contact.id - end - def test_job_checks_if_email_invalid perform_enqueued_jobs do VerifyEmailsJob.perform_now(contact_id: @invalid_contact.id, check_level: 'regex') diff --git a/test/models/domain/force_delete_test.rb b/test/models/domain/force_delete_test.rb index 5f2d8ad95..f3294f034 100644 --- a/test/models/domain/force_delete_test.rb +++ b/test/models/domain/force_delete_test.rb @@ -440,38 +440,6 @@ class ForceDeleteTest < ActionMailer::TestCase assert @domain.status_notes[DomainStatus::FORCE_DELETE].include? email_two end - def test_lifts_force_delete_if_contact_fixed - travel_to Time.zone.parse('2010-07-05') - @domain.update(valid_to: Time.zone.parse('2012-08-05')) - assert_not @domain.force_delete_scheduled? - email = '`@internet.ee' - - Truemail.configure.default_validation_type = :regex - - contact = @domain.admin_contacts.first - contact.update_attribute(:email, email) - contact.verify_email - - assert contact.email_verification_failed? - - @domain.reload - - assert @domain.force_delete_scheduled? - contact.update_attribute(:email, 'aaa@bbb.com') - contact.reload - contact.verify_email - - assert contact.need_to_lift_force_delete? - refute contact.need_to_start_force_delete? - - assert_not contact.email_verification_failed? - CheckForceDeleteLift.perform_now - - @domain.reload - assert_not @domain.force_delete_scheduled? - assert_nil @domain.status_notes[DomainStatus::FORCE_DELETE] - end - def test_lifts_force_delete_after_bounce_changes @domain.update(valid_to: Time.zone.parse('2012-08-05')) assert_not @domain.force_delete_scheduled? diff --git a/test/models/validation_event_test.rb b/test/models/validation_event_test.rb index 4f2ba7366..86f13885f 100644 --- a/test/models/validation_event_test.rb +++ b/test/models/validation_event_test.rb @@ -29,20 +29,7 @@ class ValidationEventTest < ActiveSupport::TestCase assert contact.need_to_start_force_delete? end - def test_if_fd_need_to_be_lifted_if_email_fixed - test_if_fd_need_to_be_set_if_invalid_email - email = 'email@internet.ee' - - contact = @domain.admin_contacts.first - contact.update_attribute(:email, email) - - contact.verify_email - contact.reload - - assert contact.need_to_lift_force_delete? - assert contact.validation_events.last.success? - end def test_fd_didnt_set_if_mx_interation_less_then_value @domain.update(valid_to: Time.zone.parse('2012-08-05')) @@ -52,7 +39,7 @@ class ValidationEventTest < ActiveSupport::TestCase email = 'email@somestrangedomain12345.ee' contact = @domain.admin_contacts.first contact.update_attribute(:email, email) - (ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD - 1).times do + (ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD - 4).times do contact.verify_email(check_level: 'mx') end contact.reload @@ -80,19 +67,6 @@ class ValidationEventTest < ActiveSupport::TestCase assert contact.need_to_start_force_delete? end - def test_if_fd_need_to_be_lifted_if_mx_fixed - test_if_fd_need_to_be_set_if_invalid_mx - - email = 'email@internet.ee' - contact = @domain.admin_contacts.first - contact.update_attribute(:email, email) - contact.verify_email(check_level: 'mx') - - contact.reload - assert contact.need_to_lift_force_delete? - assert contact.validation_events.last.success? - end - def test_if_fd_need_to_be_set_if_invalid_smtp @domain.update(valid_to: Time.zone.parse('2012-08-05')) assert_not @domain.force_delete_scheduled? @@ -101,27 +75,12 @@ class ValidationEventTest < ActiveSupport::TestCase email = 'email@somestrangedomain12345.ee' contact = @domain.admin_contacts.first contact.update_attribute(:email, email) - ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD.times do - contact.verify_email(check_level: 'smtp') - end + contact.verify_email(check_level: 'smtp') + contact.reload refute contact.validation_events.limit(ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD) .any?(&:success?) assert contact.need_to_start_force_delete? end - - def test_if_fd_need_to_be_lifted_if_smtp_fixed - test_if_fd_need_to_be_set_if_invalid_smtp - - email = 'valid@internet.ee' - contact = @domain.admin_contacts.first - contact.update_attribute(:email, email) - contact.verify_email(check_level: 'smtp') - - contact.reload - assert contact.need_to_lift_force_delete? - assert contact.validation_events.last.success? - end - end diff --git a/test/tasks/emails/verify_email_task_test.rb b/test/tasks/emails/verify_email_task_test.rb index f12d15b3d..63425df7a 100644 --- a/test/tasks/emails/verify_email_task_test.rb +++ b/test/tasks/emails/verify_email_task_test.rb @@ -31,13 +31,115 @@ class VerifyEmailTaskTest < ActiveJob::TestCase [domain(@invalid_contact.email)].reject(&:blank?) end - def test_tasks_verifies_emails - capture_io { run_task } + def test_should_be_verified_duplicate_emails + william = Contact.where(email: "william@inbox.test").count - assert ValidationEvent.validated_ids_by(Contact).include? @contact.id - assert @contact.validation_events.last.success - refute @invalid_contact.validation_events.last.success - refute ValidationEvent.validated_ids_by(Contact).include? @invalid_contact.id + assert_equal william, 2 + assert_equal Contact.all.count, 9 + run_task + assert_equal ValidationEvent.count, Contact.count - 1 + end + + def test_should_not_affect_to_successfully_verified_emails + assert_equal ValidationEvent.count, 0 + run_task + assert_equal ValidationEvent.count, Contact.count - 1 # Contact has duplicate email and it is skip + + run_task + assert_equal ValidationEvent.count, Contact.count - 1 + end + + def test_should_verify_contact_which_was_not_verified + bestnames = registrars(:bestnames) + assert_equal ValidationEvent.count, 0 + run_task + assert_equal ValidationEvent.count, Contact.count - 1 # Contact has duplicate email and it is skip + + assert_equal Contact.count, 9 + c = Contact.create(name: 'Jeembo', + email: 'heey@jeembo.com', + phone: '+555.555', + ident: '1234', + ident_type: 'priv', + ident_country_code: 'US', + registrar: bestnames, + code: 'jeembo-01') + + assert_equal Contact.count, 10 + run_task + assert_equal ValidationEvent.count, Contact.count - 1 + end + + def test_should_verify_again_contact_which_has_faield_verification + assert_equal ValidationEvent.count, 0 + run_task + assert_equal Contact.count, 9 + assert_equal ValidationEvent.count, 8 # Contact has duplicate email and it is skip + + contact = contacts(:john) + v = ValidationEvent.find_by(validation_eventable_id: contact.id) + v.update!(success: false) + + run_task + assert_equal ValidationEvent.all.count, 9 + end + + def test_should_verify_contact_which_has_expired_date_of_verification + expired_date = Time.now - ValidationEvent::VALIDATION_PERIOD - 1.day + + assert_equal ValidationEvent.count, 0 + run_task + assert_equal Contact.count, 9 + assert_equal ValidationEvent.count, 8 # Contact has duplicate email and it is skip + + contact = contacts(:john) + v = ValidationEvent.find_by(validation_eventable_id: contact.id) + v.update!(created_at: expired_date) + + run_task + assert_equal ValidationEvent.all.count, 9 + end + + def test_should_set_fd_for_failed_email_after_several_times + contact = contacts(:john) + trumail_results = OpenStruct.new(success: false, + email: contact.email, + domain: "inbox.tests", + errors: {:mx=>"target host(s) not found"}, + ) + Spy.on_instance_method(Actions::EmailCheck, :check_email).and_return(trumail_results) + + + assert_not contact.domains.last.force_delete_scheduled? + + 2.times do + run_task + end + + assert contact.domains.last.force_delete_scheduled? + end + + def test_fd_should_not_removed_if_change_email_to_another_invalid_one + contact = contacts(:john) + + contact.domains.last.schedule_force_delete(type: :soft) + assert contact.domains.last.force_delete_scheduled? + + contact.update(email: "test@box.test") + contact.reload + + 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) + + 1.times do + run_task + end + + assert contact.domains.last.force_delete_scheduled? end def run_task