diff --git a/app/controllers/admin/contacts_controller.rb b/app/controllers/admin/contacts_controller.rb index 685a08c86..793fa1209 100644 --- a/app/controllers/admin/contacts_controller.rb +++ b/app/controllers/admin/contacts_controller.rb @@ -30,7 +30,7 @@ module Admin if params[:only_no_country_code].eql?('1') contacts = contacts.where("ident_country_code is null or ident_country_code=''") end - contacts = contacts.email_not_verified if params[:email_not_verified].eql?('1') + contacts = contacts.email_verification_failed if params[:email_verification_failed].eql?('1') contacts end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 8de3fdc70..5c742afce 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -108,4 +108,14 @@ module ApplicationHelper def body_css_class [controller_path.split('/').map!(&:dasherize), action_name.dasherize, 'page'].join('-') end + + def verified_email_span(verification) + content_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/models/contact.rb b/app/models/contact.rb index a24a53a72..ac64b059f 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -23,10 +23,9 @@ class Contact < ApplicationRecord accepts_nested_attributes_for :legal_documents - scope :email_not_verified, lambda { - joins('LEFT JOIN :email_address_verifications emv ON contacts.email = emv.email') - .where('verified_at IS NULL OR verified_at <= ?', - EmailAddressVerification.verification_period) + scope :email_verification_failed, lambda { + joins('LEFT JOIN email_address_verifications emv ON contacts.email = emv.email') + .where('success = false and verified_at IS NOT NULL') } validates :name, :email, presence: true diff --git a/app/models/email_address_verification.rb b/app/models/email_address_verification.rb index 412ce7f83..b478ab0f8 100644 --- a/app/models/email_address_verification.rb +++ b/app/models/email_address_verification.rb @@ -6,7 +6,11 @@ class EmailAddressVerification < ApplicationRecord } scope :verified_recently, lambda { - where('verified_at IS NOT NULL and verified_at >= ?', verification_period) + 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) } def recently_verified? @@ -22,16 +26,28 @@ class EmailAddressVerification < ApplicationRecord Time.zone.now - RECENTLY_VERIFIED_PERIOD end + def not_verified? + verified_at.blank? && !success + end + + def failed? + verified_at.present? && !success + end + + def verified? + success + end + def verify # media = success ? :mx : :smtp - media = :mx + media = :regex validation_request = Truemail.validate(email, with: media) if validation_request.result.success update(verified_at: Time.zone.now, success: true) else - update(verified_at: nil, + update(verified_at: Time.zone.now, success: false) end diff --git a/app/models/registrar.rb b/app/models/registrar.rb index 9abdfdb68..470d768b7 100644 --- a/app/models/registrar.rb +++ b/app/models/registrar.rb @@ -13,16 +13,6 @@ class Registrar < ApplicationRecord has_many :nameservers, through: :domains has_many :whois_records has_many :white_ips, dependent: :destroy - # belongs_to :email_address_verification, class_name: 'EmailAddressVerification', - # primary_key: 'email', - # foreign_key: 'email', - # optional: true, - # inverse_of: :registrar - # belongs_to :billing_email_address_verification, class_name: 'EmailAddressVerification', - # primary_key: 'email', - # foreign_key: 'billing_email', - # optional: true, - # inverse_of: :billing_registrar delegate :balance, to: :cash_account, allow_nil: true diff --git a/app/views/admin/contacts/index.haml b/app/views/admin/contacts/index.haml index bf3665664..cbd11d3fc 100644 --- a/app/views/admin/contacts/index.haml +++ b/app/views/admin/contacts/index.haml @@ -65,8 +65,8 @@ = check_box_tag :only_no_country_code, '1',params[:only_no_country_code].eql?('1'), style: 'width:auto;height:auto;float:right' .col-md-3 .form-group - = label_tag :email_not_verified, "Email not verified" - = check_box_tag :email_not_verified, '1',params[:email_not_verified].eql?('1'), style: 'width:auto;height:auto;float:right' + = label_tag :email_verification_failed, "Email verification failed" + = check_box_tag :email_verification_failed, '1',params[:email_verification_failed].eql?('1'), style: 'width:auto;height:auto;float:right' .row .col-md-3{style: 'padding-top: 25px;float:right;'} @@ -100,8 +100,7 @@ %td= link_to(contact, admin_contact_path(contact)) %td= contact.code %td= ident_for(contact) - %td{class: ('text-danger' unless contact.email_verification.success)} - = contact.email + %td= verified_email_span(contact.email_verification) %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 c4a151294..6568cd3d0 100644 --- a/app/views/admin/contacts/partials/_general.haml +++ b/app/views/admin/contacts/partials/_general.haml @@ -17,8 +17,7 @@ %dd= ident_for(@contact) %dt= t(:email) - %dd{class: ('text-danger' unless @contact.email_verification&.success)} - = @contact.email + %dd= verified_email_span(@contact.email_verification) %dt= t(:phone) %dd= @contact.phone diff --git a/app/views/admin/registrars/index.html.erb b/app/views/admin/registrars/index.html.erb index 6ed864972..e641f5294 100644 --- a/app/views/admin/registrars/index.html.erb +++ b/app/views/admin/registrars/index.html.erb @@ -49,13 +49,9 @@ <%= "#{x.test_registrar}" %> - > - <%= "#{x.email}" %> - + <%= verified_email_span(x.email_verification) %> <% if x[:billing_email].present? %> - > - <%= "#{x[:billing_email]}" %> - + <%= verified_email_span(x.billing_email_verification) %> <% end %> diff --git a/app/views/admin/registrars/show/_billing.html.erb b/app/views/admin/registrars/show/_billing.html.erb index ab731ce1d..07bccc7f4 100644 --- a/app/views/admin/registrars/show/_billing.html.erb +++ b/app/views/admin/registrars/show/_billing.html.erb @@ -15,8 +15,8 @@
<%= registrar.accounting_customer_code %>
<%= Registrar.human_attribute_name :billing_email %>
-
> - <%= registrar.billing_email %> +
+ <%= verified_email_span(registrar.billing_email_verification) %>
<%= 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 b5a222dd5..0ca1158d3 100644 --- a/app/views/admin/registrars/show/_contacts.html.erb +++ b/app/views/admin/registrars/show/_contacts.html.erb @@ -15,8 +15,8 @@
<%= @registrar.phone %>
<%= Registrar.human_attribute_name :email %>
-
> - <%= @registrar.email %> +
+ <%= verified_email_span(@registrar.email_verification) %>
diff --git a/db/data/20200608084321_fill_email_verifications.rb b/db/data/20200608084321_fill_email_verifications.rb index 68f02b133..073bf2b24 100644 --- a/db/data/20200608084321_fill_email_verifications.rb +++ b/db/data/20200608084321_fill_email_verifications.rb @@ -1,8 +1,8 @@ class FillEmailVerifications < ActiveRecord::Migration[6.0] def up - registrar_billing_emails = Registrar.pluck(:billing_email).uniq.reject(&:blank?) - registrar_emails = Registrar.pluck(:email).uniq.reject(&:blank?) - contact_emails = Contact.pluck(:email).uniq.reject(&:blank?) + registrar_billing_emails = Registrar.pluck(:billing_email).uniq.reject(&:blank?).map(&:downcase) + registrar_emails = Registrar.pluck(:email).uniq.reject(&:blank?).map(&:downcase) + contact_emails = Contact.pluck(:email).uniq.reject(&:blank?).map(&:downcase) emails = (contact_emails + registrar_emails + registrar_billing_emails).uniq diff --git a/lib/tasks/verify_email.rake b/lib/tasks/verify_email.rake index d069100f2..68dda6e60 100644 --- a/lib/tasks/verify_email.rake +++ b/lib/tasks/verify_email.rake @@ -2,9 +2,7 @@ namespace :verify_email do desc 'Stars verifying email jobs' task all_domains: :environment do verifications_by_domain = EmailAddressVerification.not_verified_recently.group_by(&:domain) - verifications_by_domain.each do |domain, verifications| - next if domain == 'not_found' - + verifications_by_domain.each do |_domain, verifications| ver = verifications.sample # Verify random email to not to clog the SMTP servers VerifyEmailsJob.enqueue(ver.id) next diff --git a/test/tasks/emails/verify_email_task_test.rb b/test/tasks/emails/verify_email_task_test.rb new file mode 100644 index 000000000..ff70730fd --- /dev/null +++ b/test/tasks/emails/verify_email_task_test.rb @@ -0,0 +1,49 @@ +require 'test_helper' + +class VerifyEmailTaskTest < ActiveSupport::TestCase + + def setup + @contact = contacts(:john) + @invalid_contact = contacts(:invalid_email) + @contact_verification = @contact.email_verification + @invalid_contact_verification = @invalid_contact.email_verification + + @default_whitelist = Truemail.configure.whitelisted_domains + @default_blacklist = Truemail.configure.blacklisted_domains + Truemail.configure.whitelisted_domains = whitelisted_domains + Truemail.configure.blacklisted_domains = blacklisted_domains + end + + def teardown + Truemail.configure.whitelisted_domains = @default_whitelist + Truemail.configure.blacklisted_domains = @default_blacklist + end + + def domain(email) + Mail::Address.new(email).domain + rescue Mail::Field::IncompleteParseError + nil + end + + def whitelisted_domains + [domain(@contact.email)].reject(&:blank?) + end + + def blacklisted_domains + [domain(@invalid_contact.email)].reject(&:blank?) + end + + def test_tasks_verifies_emails + capture_io { run_task } + + @contact_verification.reload + @invalid_contact_verification.reload + + assert @contact_verification.verified? + assert @invalid_contact_verification.failed? + end + + def run_task + Rake::Task['verify_email:all_domains'].execute + end +end