Refactor emails #2786

app/mailers/contact_mailer.rb

@@ -1,33 +1,27 @@
 class ContactMailer < ApplicationMailer
   # rubocop:disable Metrics/MethodLength
-  def email_updated(contact)
+  def email_updated(email, contact)
     return if delivery_off?(contact)
 
     @contact = contact
-    emails = []
-    emails << [@contact.email, @contact.email_was] if @contact.registrant_domains.present?
-    emails << @contact.domains.map(&:registrant_email) if @contact.domains.present?
-    emails = emails.uniq
 
-    return if whitelist_blocked?(emails)
-    emails.each do |email|
-      begin
-        mail(to: email, subject: "#{I18n.t(:contact_email_update_subject)} [#{@contact.code}]")
-      rescue EOFError,
-             IOError,
-             TimeoutError,
-             Errno::ECONNRESET,
-             Errno::ECONNABORTED,
-             Errno::EPIPE,
-             Errno::ETIMEDOUT,
-             Net::SMTPAuthenticationError,
-             Net::SMTPServerBusy,
-             Net::SMTPFatalError,
-             Net::SMTPSyntaxError,
-             Net::SMTPUnknownError,
-             OpenSSL::SSL::SSLError => e
-        logger.warn "EMAIL SENDING FAILED: #{email}: #{e}"
-      end
+    return if whitelist_blocked?(email)
+    begin
+      mail(to: email, subject: "#{I18n.t(:contact_email_update_subject)} [#{@contact.code}]")
+    rescue EOFError,
+           IOError,
+           TimeoutError,
+           Errno::ECONNRESET,
+           Errno::ECONNABORTED,
+           Errno::EPIPE,
+           Errno::ETIMEDOUT,
+           Net::SMTPAuthenticationError,
+           Net::SMTPServerBusy,
+           Net::SMTPFatalError,
+           Net::SMTPSyntaxError,
+           Net::SMTPUnknownError,
+           OpenSSL::SSL::SSLError => e
+      logger.info "EMAIL SENDING FAILED: #{email}: #{e}"
     end
   end
   # rubocop:enable Metrics/MethodLength

app/models/contact.rb

@@ -42,7 +42,14 @@ class Contact < ActiveRecord::Base
   before_update :manage_emails
   def manage_emails
     return nil unless email_changed?
-    ContactMailer.email_updated(self).deliver_now
+    return nil unless deliver_emails == true
+    emails = []
+    emails << [email, email_was]
+    emails << domains.map(&:registrant_email) if domains.present?
+    emails = emails.flatten.uniq
+    emails.each do |e|
+      ContactMailer.email_updated(e, contact).deliver_now
+    end
   end
 
   before_save :manage_statuses

config/initializers/settings.rb

@@ -13,4 +13,8 @@ TEST_EMAILS = %w(
   info@gitlab.eu
   test@example.com
   test@example.org
+  old@example.org
+  new@example.org
+  old@example.com
+  new@example.com
 )

spec/mailers/contact_mailer_spec.rb

@@ -4,8 +4,7 @@ describe ContactMailer do
   describe 'email changed notification when delivery turned off' do
     before :all do 
       @contact = Fabricate(:contact, email: 'test@example.ee')
-      @contact.email = 'test@example.com' # new email
-      @mail = ContactMailer.email_updated(@contact)
+      @mail = ContactMailer.email_updated('test@example.com', @contact)
     end
 
     it 'should not render email subject' do
@@ -31,8 +30,7 @@ describe ContactMailer do
       @contact = @domain.registrant
       @contact.reload # until figured out why registrant_domains not loaded
       @contact.deliver_emails = true
-      @contact.email = 'test@example.org' # new email
-      @mail = ContactMailer.email_updated(@contact)
+      @mail = ContactMailer.email_updated('info@example.org', @contact)
     end
 
     it 'should render email subject' do
@@ -43,9 +41,8 @@ describe ContactMailer do
       @mail.from.should == ["noreply@internet.ee"]
     end
 
-    it 'should have both old and new receiver email' do
-      @mail.to.size.should == 2 
-      @mail.to.include? "test@example.org"
+    it 'should send to info email' do
+      @mail.to.should == ['info@example.org']
     end
 
     it 'should render body' do