Refactor contact mailer

- Send single email with new email in `to` and old email in `bcc` fields
- Remove `Que::Mailer` (#895)
- Add preview
- DRY templates
- Add tests
- Extract translations
- Remove useless specs

app/mailers/contact_mailer.rb

@@ -1,36 +1,12 @@
 class ContactMailer < ApplicationMailer
-  include Que::Mailer
   helper_method :address_processing
 
-  def email_updated(old_email, email, contact_id, should_deliver)
-    @contact   = Contact.find_by(id: contact_id)
+  def email_changed(contact:, old_email:)
+    @contact = contact
     @old_email = old_email
-    unless @contact
-      Rails.logger.info "Cannot send email in #{self.class.name}##{__method__} with contact_id #{contact_id}. It cannot be found"
-      return
-    end
 
-
-    return unless email || @contact
-    return if delivery_off?(@contact, should_deliver)
-
-    begin
-      mail(to: format(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
+    subject = default_i18n_subject(contact_code: contact.code)
+    mail(to: contact.email, bcc: old_email, subject: subject)
   end
 
   private
@@ -38,4 +14,4 @@ class ContactMailer < ApplicationMailer
   def address_processing
     Contact.address_processing?
   end
-end
+end

app/models/contact.rb

@@ -45,7 +45,6 @@ class Contact < ActiveRecord::Base
 
   before_validation :to_upcase_country_code
   before_validation :strip_email
-  before_update :manage_emails
 
   composed_of :identifier,
               class_name: 'Contact::Ident',
@@ -54,18 +53,6 @@ class Contact < ActiveRecord::Base
                                                                                 country_code: country_code) },
               mapping: [%w[ident code], %w[ident_type type], %w[ident_country_code country_code]]
 
-  def manage_emails
-    return nil unless email_changed?
-    return nil unless deliver_emails == true
-    emails = []
-    emails << [email, email_was]
-    emails = emails.flatten.uniq
-    emails.each do |e|
-      ContactMailer.email_updated(email_was, e, id, deliver_emails).deliver
-    end
-  end
-
-
   after_save :update_related_whois_records
 
   ORG = 'org'
@@ -77,8 +64,6 @@ class Contact < ActiveRecord::Base
   # From old registry software ("Fred"). No new contact can be created with this status
   PASSPORT = 'passport'
 
-  attr_accessor :deliver_emails
-
   #
   # STATUSES
   #

app/models/epp/contact.rb

@@ -145,8 +145,6 @@ class Epp::Contact < Contact
       self.legal_document_id = doc.id
     end
 
-    self.deliver_emails = true # turn on email delivery for epp
-
     ident_frame = frame.css('ident').first
 
     # https://github.com/internetee/registry/issues/576
@@ -175,7 +173,17 @@ class Epp::Contact < Contact
     self.upid = current_user.registrar.id if current_user.registrar
     self.up_date = Time.zone.now
 
-    super(at)
+    self.attributes = at
+
+    email_changed = email_changed?
+    old_email = email_was
+    updated = save
+
+    if updated && email_changed
+      ContactMailer.email_changed(contact: self, old_email: old_email).deliver_now
+    end
+
+    updated
   end
 
   def statuses_attrs(frame, action)

app/views/mailers/contact_mailer/email_changed.html.erb

@@ -20,10 +20,7 @@ Kontaktandmed:<br>
 <br><br>
 Palun veenduge, et muudatus on korrektne ning probleemide korral pöörduge oma registripidaja poole:
 <%= render 'mailers/shared/registrar/registrar.et.html', registrar: registrar %>
-<br><br>
-Lugupidamisega<br>
-Eesti Interneti Sihtasutus
-<br><br>
+<%= render 'mailers/shared/signatures/signature.et.html' %>
 <hr>
 <br><br>
 Hi <%= contact.name %>
@@ -44,6 +41,4 @@ Contact information:<br>
 <br><br>
 In case of problems please turn to your registrar:
 <%= render 'mailers/shared/registrar/registrar.en.html', registrar: registrar %>
-<br><br>
-Best Regards,<br>
-Estonian Internet Foundation
+<%= render 'mailers/shared/signatures/signature.en.html' %>

app/views/mailers/contact_mailer/email_changed.text.erb

@@ -20,9 +20,7 @@ Kontaktandmed:
 
 Palun veenduge, et muudatus on korrektne ning probleemide korral pöörduge oma registripidaja poole:
 <%= render 'mailers/shared/registrar/registrar.et.text', registrar: registrar %>
-
-Lugupidamisega
-Eesti Interneti Sihtasutus
+<%= render 'mailers/shared/signatures/signature.et.text' %>
 
 ----------------------------------------------------------------------------------
 
@@ -44,6 +42,4 @@ Contact information:
 
 In case of problems please turn to your registrar:
 <%= render 'mailers/shared/registrar/registrar.en.text', registrar: registrar %>
-
-Best Regards,
-Estonian Internet Foundation
+<%= render 'mailers/shared/signatures/signature.en.text' %>

config/locales/en.yml

@@ -574,7 +574,6 @@ en:
   minimum_total: 'Minimum total'
   maximum_total: 'Maximum total'
   unimplemented_object_service: 'Unimplemented object service'
-  contact_email_update_subject: 'Teie domeenide kontakt epostiaadress on muutunud / Contact e-mail addresses of your domains have changed'
   object_status_prohibits_operation: 'Object status prohibits operation'
   pending_delete_rejected_notification_subject: "Domeeni %{name} kustutamise taotlus tagasi lükatud / %{name} deletion declined"
   pending_delete_expired_notification_subject: "Domeeni %{name} kustutamise taotlus on tühistatud / %{name} deletion cancelled"

config/locales/mailers/contact.en.yml

@@ -0,0 +1,6 @@
+en:
+  contact_mailer:
+    email_changed:
+      subject: >-
+        Teie domeenide kontakt epostiaadress on muutunud
+        / Contact e-mail addresses of your domains have changed [%{contact_code}]

spec/views/mailers/contact_mailer/email_updated.html.erb_spec.rb

@@ -1,32 +0,0 @@
-require 'rails_helper'
-
-RSpec.describe 'mailers/contact_mailer/email_updated.html.erb' do
-  let(:contact) { instance_spy(Contact) }
-  let(:contact_presenter) { instance_spy(RegistrantPresenter) }
-  let(:registrar_presenter) { instance_spy(RegistrarPresenter) }
-
-  before :example do
-    allow(RegistrantPresenter).to receive(:new).and_return(contact_presenter)
-    allow(RegistrarPresenter).to receive(:new).and_return(registrar_presenter)
-
-    assign(:contact, contact)
-    assign(:old_email, 'test@test.com')
-
-    stub_template 'mailers/shared/registrant/_registrant.et.html' => ''
-    stub_template 'mailers/shared/registrant/_registrant.en.html' => ''
-    stub_template 'mailers/shared/registrar/_registrar.et.html' => ''
-    stub_template 'mailers/shared/registrar/_registrar.en.html' => ''
-  end
-
-  it 'has affected domain list in estonian' do
-    expect(contact_presenter).to receive(:domain_names_with_roles).with(locale: :et, line_break: '<br>').and_return('test domain list et')
-    render
-    expect(rendered).to have_text('test domain list et')
-  end
-
-  it 'has affected domain list in english' do
-    expect(contact_presenter).to receive(:domain_names_with_roles).with(line_break: '<br>').and_return('test domain list en')
-    render
-    expect(rendered).to have_text('test domain list en')
-  end
-end

spec/views/mailers/contact_mailer/email_updated.text.erb_spec.rb

@@ -1,32 +0,0 @@
-require 'rails_helper'
-
-RSpec.describe 'mailers/contact_mailer/email_updated.text.erb' do
-  let(:contact) { instance_spy(Contact) }
-  let(:contact_presenter) { instance_spy(RegistrantPresenter) }
-  let(:registrar_presenter) { instance_spy(RegistrarPresenter) }
-
-  before :example do
-    allow(RegistrantPresenter).to receive(:new).and_return(contact_presenter)
-    allow(RegistrarPresenter).to receive(:new).and_return(registrar_presenter)
-
-    assign(:contact, contact)
-    assign(:old_email, 'test@test.com')
-
-    stub_template 'mailers/shared/registrant/_registrant.et.text' => ''
-    stub_template 'mailers/shared/registrant/_registrant.en.text' => ''
-    stub_template 'mailers/shared/registrar/_registrar.et.text' => ''
-    stub_template 'mailers/shared/registrar/_registrar.en.text' => ''
-  end
-
-  it 'has affected domain list in estonian' do
-    expect(contact_presenter).to receive(:domain_names_with_roles).with(locale: :et).and_return('test domain list et')
-    render
-    expect(rendered).to have_text('test domain list et')
-  end
-
-  it 'has affected domain list in english' do
-    expect(contact_presenter).to receive(:domain_names_with_roles).and_return('test domain list en')
-    render
-    expect(rendered).to have_text('test domain list en')
-  end
-end

test/integration/epp/contact/update/base_test.rb

@@ -1,8 +1,11 @@
 require 'test_helper'
 
 class EppContactUpdateBaseTest < ActionDispatch::IntegrationTest
+  include ActionMailer::TestHelper
+
   setup do
     @contact = contacts(:john)
+    ActionMailer::Base.deliveries.clear
   end
 
   def test_updates_contact
@@ -45,6 +48,62 @@ class EppContactUpdateBaseTest < ActionDispatch::IntegrationTest
     assert_equal '+123.4', @contact.phone
   end
 
+  def test_notifies_a_contact_when_an_email_is_changed
+    assert_equal 'john-001', @contact.code
+    assert_not_equal 'new@inbox.test', @contact.email
+
+    # https://github.com/internetee/registry/issues/415
+    @contact.update_columns(code: @contact.code.upcase)
+
+    request_xml = <<-XML
+      <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+      <epp xmlns="https://epp.tld.ee/schema/epp-ee-1.0.xsd">
+        <command>
+          <update>
+            <contact:update xmlns:contact="https://epp.tld.ee/schema/contact-ee-1.1.xsd">
+              <contact:id>john-001</contact:id>
+              <contact:chg>
+                <contact:email>new@inbox.test</contact:email>
+              </contact:chg>
+            </contact:update>
+          </update>
+        </command>
+      </epp>
+    XML
+
+    post '/epp/command/update', { frame: request_xml }, 'HTTP_COOKIE' => 'session=api_bestnames'
+
+    assert_emails 1
+  end
+
+  def test_skips_notifying_a_contact_when_an_email_is_not_changed
+    assert_equal 'john-001', @contact.code
+    assert_equal 'john@inbox.test', @contact.email
+
+    # https://github.com/internetee/registry/issues/415
+    @contact.update_columns(code: @contact.code.upcase)
+
+    request_xml = <<-XML
+      <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+      <epp xmlns="https://epp.tld.ee/schema/epp-ee-1.0.xsd">
+        <command>
+          <update>
+            <contact:update xmlns:contact="https://epp.tld.ee/schema/contact-ee-1.1.xsd">
+              <contact:id>john-001</contact:id>
+              <contact:chg>
+                <contact:email>john@inbox.test</contact:email>
+              </contact:chg>
+            </contact:update>
+          </update>
+        </command>
+      </epp>
+    XML
+
+    post '/epp/command/update', { frame: request_xml }, 'HTTP_COOKIE' => 'session=api_bestnames'
+
+    assert_no_emails
+  end
+
   def test_non_existing_contact
     assert_nil Contact.find_by(code: 'non-existing')
 

test/mailers/contact_mailer_test.rb

@@ -0,0 +1,24 @@
+require 'test_helper'
+
+class ContactMailerTest < ActiveSupport::TestCase
+  include ActionMailer::TestHelper
+
+  setup do
+    @contact = contacts(:john)
+    ActionMailer::Base.deliveries.clear
+  end
+
+  def test_delivers_email_changed_email
+    assert_equal 'john-001', @contact.code
+    assert_equal 'john@inbox.test', @contact.email
+
+    email = ContactMailer.email_changed(contact: @contact, old_email: 'john-old@inbox.test')
+              .deliver_now
+
+    assert_emails 1
+    assert_equal %w[john@inbox.test], email.to
+    assert_equal %w[john-old@inbox.test], email.bcc
+    assert_equal 'Teie domeenide kontakt epostiaadress on muutunud' \
+                 ' / Contact e-mail addresses of your domains have changed [john-001]', email.subject
+  end
+end

test/mailers/previews/contact_mailer_preview.rb

@@ -0,0 +1,13 @@
+class ContactMailerPreview < ActionMailer::Preview
+  def email_changed
+    # Replace with `Contact.in_use` once https://github.com/internetee/registry/pull/1146 is merged
+    contact = Contact.where('EXISTS(SELECT 1 FROM domains WHERE domains.registrant_id = contacts.id)
+                             OR
+                             EXISTS(SELECT 1 FROM domain_contacts WHERE domain_contacts.contact_id =
+                             contacts.id)')
+
+    contact = contact.where.not(email: nil, country_code: nil, code: nil).first
+
+    ContactMailer.email_changed(contact: contact, old_email: 'old@inbox.test')
+  end
+end