diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index 6631569f87..e064589ab3 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -11,7 +11,6 @@ def initialize(email:, validation_eventable:, check_level: nil) def call result = check_email(email) save_result(result) - filtering_old_failed_records(result) result.success ? log_success : log_failure(result) result.success end @@ -26,27 +25,25 @@ def calculate_check_level Rails.env.test? && check_level == 'smtp' ? :mx : check_level.to_sym end - def filtering_old_failed_records(result) - if @check_level == "mx" && !result.success && validation_eventable.validation_events.count > 3 - validation_eventable.validation_events.order!(created_at: :asc) - while validation_eventable.validation_events.count > 3 - validation_eventable.validation_events.first.destroy - end + def filtering_old_failed_records(result, contact) + ValidationEvent::INVALID_EVENTS_COUNT_BY_LEVEL.each do |level, limit| + handle_failed_records(contact: contact, check_level: level, limit: limit, success: result.success) end + end - if @check_level == "mx" && result.success && validation_eventable.validation_events.count > 1 - validation_eventable.validation_events.order!(created_at: :asc) - while validation_eventable.validation_events.count > 1 - validation_eventable.validation_events.first.destroy + def handle_failed_records(contact:, check_level:, limit:, success:) + if @check_level.to_sym == check_level && !success && contact.validation_events.count > limit + contact.validation_events.order!(created_at: :asc) + while contact.validation_events.count > limit + contact.validation_events.first.destroy end end + end - if @check_level == "smtp" && validation_eventable.validation_events.count > 1 - validation_eventable.validation_events.order!(created_at: :asc) - while validation_eventable.validation_events.count > 1 - validation_eventable.validation_events.first.destroy - end - end + def filtering_old_records(contact:, success:) + return unless success + + contact.validation_events.destroy_all end def save_result(result) @@ -63,7 +60,10 @@ def save_result(result) contacts.find_in_batches(batch_size: 500) do |contact_batches| contact_batches.each do |contact| + # methods should be in this order! + filtering_old_records(contact: contact, success: result.success) contact.validation_events.create(validation_event_attrs(result)) + filtering_old_failed_records(result, contact) end end rescue ActiveRecord::RecordNotSaved diff --git a/test/interactions/email_check_test.rb b/test/interactions/email_check_test.rb index 4e77a5beee..0c704ba72b 100644 --- a/test/interactions/email_check_test.rb +++ b/test/interactions/email_check_test.rb @@ -95,4 +95,46 @@ def test_should_remove_valid_validation_record_if_there_count_more_than_one assert_equal @contact.validation_events.count, 1 assert @contact.validation_events.last.success end + + def test_should_remove_old_record_if_validation_pass_the_limit + trumail_results = OpenStruct.new(success: false, + email: @contact.email, + domain: "box.tests", + errors: {:mx=>"target host(s) not found"}) + + action = Actions::EmailCheck.new(email: @contact.email, + validation_eventable: @contact, + check_level: 'mx') + + action.stub :check_email, trumail_results do + 4.times do + action.call + end + end + + assert_equal @contact.validation_events.count, 3 + end + + def test_should_remove_old_record_if_multiple_contacts_has_the_same_email + contact_two = contacts(:william) + contact_two.update(email: @contact.email) + contact_two.reload + trumail_results = OpenStruct.new(success: false, + email: @contact.email, + domain: "box.tests", + errors: {:mx=>"target host(s) not found"}) + + action = Actions::EmailCheck.new(email: @contact.email, + validation_eventable: @contact, + check_level: 'mx') + + action.stub :check_email, trumail_results do + 4.times do + action.call + end + end + + assert_equal @contact.validation_events.count, 3 + assert_equal contact_two.validation_events.count, 3 + end end