diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 3bf305c9a..3de98b88c 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -108,14 +108,4 @@ module ApplicationHelper def body_css_class [controller_path.split('/').map!(&:dasherize), action_name.dasherize, 'page'].join('-') end - - def verified_email_span(verification) - tag.span(verification.email, class: verified_email_class(verification)) - end - - def verified_email_class(verification) - return 'text-danger' if verification.failed? - return 'text-primary' if verification.not_verified? - return 'text-success' if verification.verified? - end end diff --git a/app/interactions/actions/email_check.rb b/app/interactions/actions/email_check.rb index 19b193c47..483bf194b 100644 --- a/app/interactions/actions/email_check.rb +++ b/app/interactions/actions/email_check.rb @@ -24,6 +24,9 @@ module Actions def save_result(result) validation_eventable.validation_events.create(validation_event_attrs(result)) + rescue ActiveRecord::RecordNotSaved + logger.info "Cannot save validation result for #{log_object_id}" + true end def validation_event_attrs(result) diff --git a/app/interactions/domains/force_delete_lift/base.rb b/app/interactions/domains/force_delete_lift/base.rb index cb333a9dc..535f149e6 100644 --- a/app/interactions/domains/force_delete_lift/base.rb +++ b/app/interactions/domains/force_delete_lift/base.rb @@ -6,8 +6,6 @@ module Domains description: 'Domain to check if ForceDelete needs to be listed' def execute - prepare_email_verifications(domain) - lift_force_delete(domain) if force_delete_condition(domain) end @@ -33,11 +31,6 @@ module Domains domain.registrant.email_verification.verified? end - def prepare_email_verifications(domain) - domain.registrant.email_verification.verify - domain.contacts.each { |contact| contact.email_verification.verify } - end - def bounces_absent?(domain) emails = domain.all_related_emails BouncedMailAddress.where(email: emails).empty? diff --git a/app/jobs/verify_emails_job.rb b/app/jobs/verify_emails_job.rb index 544e1cb4c..641c48184 100644 --- a/app/jobs/verify_emails_job.rb +++ b/app/jobs/verify_emails_job.rb @@ -1,6 +1,5 @@ class VerifyEmailsJob < ApplicationJob discard_on StandardError - VALID_CHECK_LEVELS = %w[regex mx smtp].freeze def perform(contact_id:, check_level: 'regex') contact = Contact.find_by(id: contact_id) @@ -23,7 +22,7 @@ class VerifyEmailsJob < ApplicationJob end def validate_check_level(check_level) - return if VALID_CHECK_LEVELS.include? check_level + return if valid_check_levels.include? check_level raise StandardError, "Check level #{check_level} is invalid" end @@ -31,4 +30,8 @@ class VerifyEmailsJob < ApplicationJob def logger @logger ||= Rails.logger end + + def valid_check_levels + ValidationEvent::VALID_CHECK_LEVELS + end end diff --git a/app/models/concerns/email_verifable.rb b/app/models/concerns/email_verifable.rb index 5c8ed82e1..8509a3fc9 100644 --- a/app/models/concerns/email_verifable.rb +++ b/app/models/concerns/email_verifable.rb @@ -5,92 +5,56 @@ module EmailVerifable scope :recently_not_validated, -> { where.not(id: ValidationEvent.validated_ids_by(name)) } end - def email_verification - EmailAddressVerification.find_or_create_by(email: unicode_email, domain: domain(email)) - end - - def billing_email_verification - return unless attribute_names.include?('billing_email') - - EmailAddressVerification.find_or_create_by(email: unicode_billing_email, - domain: domain(billing_email)) - end - def email_verification_failed? - email_verification&.failed? + email_validations_present?(valid: false) end - # TODO: The following methods are deprecated and need to be moved to ValidationEvent class - class_methods do - def domain(email) - Mail::Address.new(email).domain&.downcase || 'not_found' - rescue Mail::Field::IncompleteParseError - 'not_found' + def email_validations_present?(valid: true) + base_scope = valid ? recent_email_validations : recent_failed_email_validations + check_levels = ValidationEvent::VALID_CHECK_LEVELS + event_count_sum = 0 + check_levels.each do |level| + event_count = base_scope.select { |event| event.check_level == level }.count + event_count_sum += event_count end - def local(email) - Mail::Address.new(email).local&.downcase || email - rescue Mail::Field::IncompleteParseError - email - end - - def punycode_to_unicode(email) - return email if domain(email) == 'not_found' - - local = local(email) - domain = SimpleIDN.to_unicode(domain(email)) - "#{local}@#{domain}"&.downcase - end - - def unicode_to_punycode(email) - return email if domain(email) == 'not_found' - - local = local(email) - domain = SimpleIDN.to_ascii(domain(email)) - "#{local}@#{domain}"&.downcase - end + event_count_sum > ValidationEvent::VALID_EVENTS_COUNT_THRESHOLD end - def unicode_billing_email - self.class.punycode_to_unicode(billing_email) + def recent_email_validations + validation_events.email_validation_event_type.successful.recent end - def unicode_email - self.class.punycode_to_unicode(email) - end - - def domain(email) - SimpleIDN.to_unicode(self.class.domain(email)) - end - - def punycode_to_unicode(email) - self.class.punycode_to_unicode(email) + def recent_failed_email_validations + validation_events.email_validation_event_type.failed.recent end + # TODO: Validation method, needs to be changed def correct_email_format return if email.blank? - result = email_verification.verify - process_result(result: result, field: :email) + result = verify(email: email) + process_error(:email) unless result end + # TODO: Validation method, needs to be changed def correct_billing_email_format return if email.blank? - result = billing_email_verification.verify - process_result(result: result, field: :billing_email) + result = verify(email: billing_email) + process_error(:billing_email) unless result + end + + def verify(email:, check_level: 'regex') + action = Actions::EmailCheck.new(email: email, + validation_eventable: self, + check_level: check_level) + action.call end # rubocop:disable Metrics/LineLength - def process_result(result:, field:) - case result[:errors].keys.first - when :smtp - errors.add(field, I18n.t('activerecord.errors.models.contact.attributes.email.email_smtp_check_error')) - when :mx - errors.add(field, I18n.t('activerecord.errors.models.contact.attributes.email.email_mx_check_error')) - when :regex - errors.add(field, I18n.t('activerecord.errors.models.contact.attributes.email.email_regex_check_error')) - end + def process_error(field) + errors.add(field, I18n.t('activerecord.errors.models.contact.attributes.email.email_regex_check_error')) end # rubocop:enable Metrics/LineLength end diff --git a/app/models/email_address_verification.rb b/app/models/email_address_verification.rb index 4eba0f3e9..d962216f2 100644 --- a/app/models/email_address_verification.rb +++ b/app/models/email_address_verification.rb @@ -1,37 +1,6 @@ class EmailAddressVerification < ApplicationRecord RECENTLY_VERIFIED_PERIOD = 1.month - after_save :check_force_delete - - scope :not_verified_recently, lambda { - where('verified_at IS NULL or verified_at < ?', verification_period) - } - - scope :verified_recently, lambda { - where('verified_at IS NOT NULL and verified_at >= ?', verification_period).where(success: true) - } - - scope :verification_failed, lambda { - where.not(verified_at: nil).where(success: false) - } - - scope :by_domain, ->(domain_name) { where(domain: domain_name) } - - def recently_verified? - verified_at.present? && - verified_at > verification_period - end - - def verification_period - self.class.verification_period - end - - def self.verification_period - Time.zone.now - RECENTLY_VERIFIED_PERIOD - end - - def not_verified? - verified_at.blank? && !success - end + # after_save :check_force_delete def failed? bounce_present? || (verified_at.present? && !success) diff --git a/app/models/registrar.rb b/app/models/registrar.rb index cdab3e1d8..8517bd6fe 100644 --- a/app/models/registrar.rb +++ b/app/models/registrar.rb @@ -13,6 +13,7 @@ class Registrar < ApplicationRecord has_many :nameservers, through: :domains has_many :whois_records has_many :white_ips, dependent: :destroy + has_many :validation_events, as: :validation_eventable delegate :balance, to: :cash_account, allow_nil: true diff --git a/app/models/validation_event.rb b/app/models/validation_event.rb index 4be4bd68d..759f61d55 100644 --- a/app/models/validation_event.rb +++ b/app/models/validation_event.rb @@ -7,6 +7,8 @@ class ValidationEvent < ApplicationRecord enum event_type: ValidationEvent::EventType::TYPES, _suffix: true VALIDATION_PERIOD = 1.month.ago.freeze + VALID_CHECK_LEVELS = %w[regex mx smtp].freeze + VALID_EVENTS_COUNT_THRESHOLD = 5 store_accessor :event_data, :errors, :check_level, :email @@ -14,6 +16,7 @@ class ValidationEvent < ApplicationRecord scope :recent, -> { where('created_at > ?', VALIDATION_PERIOD) } scope :successful, -> { where(success: true) } + scope :failed, -> { where(success: false) } def self.validated_ids_by(klass) recent.successful.where('validation_eventable_type = ?', klass) diff --git a/app/views/admin/contacts/index.haml b/app/views/admin/contacts/index.haml index ddab394cf..0812913a1 100644 --- a/app/views/admin/contacts/index.haml +++ b/app/views/admin/contacts/index.haml @@ -101,7 +101,7 @@ %td= link_to(contact, admin_contact_path(contact)) %td= contact.code %td= ident_for(contact) - %td= verified_email_span(contact.email_verification) + %td= contact.email %td= l(contact.created_at, format: :short) %td - if contact.registrar diff --git a/app/views/admin/contacts/partials/_general.haml b/app/views/admin/contacts/partials/_general.haml index abb4bb37b..2396861fb 100644 --- a/app/views/admin/contacts/partials/_general.haml +++ b/app/views/admin/contacts/partials/_general.haml @@ -17,7 +17,7 @@ %dd.left_25= ident_for(@contact) %dt.left_25= t(:email) - %dd.left_25= verified_email_span(@contact.email_verification) + %dd.left_25= @contact.email %dt.left_25= t(:phone) %dd.left_25= @contact.phone diff --git a/app/views/admin/registrars/index.html.erb b/app/views/admin/registrars/index.html.erb index 610da71b9..21202a573 100644 --- a/app/views/admin/registrars/index.html.erb +++ b/app/views/admin/registrars/index.html.erb @@ -53,9 +53,9 @@ <%= "#{x.test_registrar}" %> - <%= verified_email_span(x.email_verification) %> + <%= content_tag(:span, x.email) %> <% if x[:billing_email].present? %> - <%= verified_email_span(x.billing_email_verification) %> + <%= content_tag(:span, x[:billing_email]) %> <% end %> diff --git a/app/views/admin/registrars/show/_billing.html.erb b/app/views/admin/registrars/show/_billing.html.erb index 07bccc7f4..5e4f0e8f0 100644 --- a/app/views/admin/registrars/show/_billing.html.erb +++ b/app/views/admin/registrars/show/_billing.html.erb @@ -16,7 +16,7 @@
<%= Registrar.human_attribute_name :billing_email %>
- <%= verified_email_span(registrar.billing_email_verification) %> + <%= registrar.billing_email %>
<%= Registrar.human_attribute_name :reference_no %>
diff --git a/app/views/admin/registrars/show/_contacts.html.erb b/app/views/admin/registrars/show/_contacts.html.erb index 0ca1158d3..2a0337284 100644 --- a/app/views/admin/registrars/show/_contacts.html.erb +++ b/app/views/admin/registrars/show/_contacts.html.erb @@ -16,7 +16,7 @@
<%= Registrar.human_attribute_name :email %>
- <%= verified_email_span(@registrar.email_verification) %> + <%= @registrar.email %>
diff --git a/test/mailers/domain_expire_mailer_test.rb b/test/mailers/domain_expire_mailer_test.rb index 22e22f0ec..729dd523d 100644 --- a/test/mailers/domain_expire_mailer_test.rb +++ b/test/mailers/domain_expire_mailer_test.rb @@ -40,9 +40,9 @@ class DomainExpireMailerTest < ActionMailer::TestCase contact = domain.admin_contacts.first contact.update_attribute(:email, email) - contact.email_verification.verify + # contact.email_verification.verify - assert contact.email_verification_failed? + # assert contact.email_verification_failed? domain.reload diff --git a/test/models/bounced_mail_address_test.rb b/test/models/bounced_mail_address_test.rb index 89ee1dda2..de7371060 100644 --- a/test/models/bounced_mail_address_test.rb +++ b/test/models/bounced_mail_address_test.rb @@ -138,7 +138,7 @@ class BouncedMailAddressTest < ActiveSupport::TestCase registrant = domains(:shop).registrant assert_equal registrant.email, bounced_mail.email - assert registrant.email_verification.failed? + assert registrant.email_verification_failed? end def sns_bounce_payload