diff --git a/app/mailers/contact_mailer.rb b/app/mailers/contact_mailer.rb index a036f8bef..d8d722844 100644 --- a/app/mailers/contact_mailer.rb +++ b/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 \ No newline at end of file diff --git a/app/models/contact.rb b/app/models/contact.rb index f73c0c6fe..8b1cfaa77 100644 --- a/app/models/contact.rb +++ b/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 # diff --git a/app/models/epp/contact.rb b/app/models/epp/contact.rb index 3803b3e25..38ddfa230 100644 --- a/app/models/epp/contact.rb +++ b/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) diff --git a/app/views/mailers/contact_mailer/email_updated.html.erb b/app/views/mailers/contact_mailer/email_changed.html.erb similarity index 93% rename from app/views/mailers/contact_mailer/email_updated.html.erb rename to app/views/mailers/contact_mailer/email_changed.html.erb index c3a22b063..65801f756 100644 --- a/app/views/mailers/contact_mailer/email_updated.html.erb +++ b/app/views/mailers/contact_mailer/email_changed.html.erb @@ -20,10 +20,7 @@ Kontaktandmed:


Palun veenduge, et muudatus on korrektne ning probleemide korral pöörduge oma registripidaja poole: <%= render 'mailers/shared/registrar/registrar.et.html', registrar: registrar %> -

-Lugupidamisega
-Eesti Interneti Sihtasutus -

+<%= render 'mailers/shared/signatures/signature.et.html' %>


Hi <%= contact.name %> @@ -44,6 +41,4 @@ Contact information:


In case of problems please turn to your registrar: <%= render 'mailers/shared/registrar/registrar.en.html', registrar: registrar %> -

-Best Regards,
-Estonian Internet Foundation +<%= render 'mailers/shared/signatures/signature.en.html' %> \ No newline at end of file diff --git a/app/views/mailers/contact_mailer/email_updated.text.erb b/app/views/mailers/contact_mailer/email_changed.text.erb similarity index 93% rename from app/views/mailers/contact_mailer/email_updated.text.erb rename to app/views/mailers/contact_mailer/email_changed.text.erb index d847f9f57..55d92d679 100644 --- a/app/views/mailers/contact_mailer/email_updated.text.erb +++ b/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' %> \ No newline at end of file diff --git a/config/locales/en.yml b/config/locales/en.yml index 12cc56ab9..4d6374ba6 100644 --- a/config/locales/en.yml +++ b/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" diff --git a/config/locales/mailers/contact.en.yml b/config/locales/mailers/contact.en.yml new file mode 100644 index 000000000..c9d653f22 --- /dev/null +++ b/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}] \ No newline at end of file diff --git a/spec/views/mailers/contact_mailer/email_updated.html.erb_spec.rb b/spec/views/mailers/contact_mailer/email_updated.html.erb_spec.rb deleted file mode 100644 index 47887c30d..000000000 --- a/spec/views/mailers/contact_mailer/email_updated.html.erb_spec.rb +++ /dev/null @@ -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: '
').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: '
').and_return('test domain list en') - render - expect(rendered).to have_text('test domain list en') - end -end diff --git a/spec/views/mailers/contact_mailer/email_updated.text.erb_spec.rb b/spec/views/mailers/contact_mailer/email_updated.text.erb_spec.rb deleted file mode 100644 index a74f16b28..000000000 --- a/spec/views/mailers/contact_mailer/email_updated.text.erb_spec.rb +++ /dev/null @@ -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 diff --git a/test/integration/epp/contact/update/base_test.rb b/test/integration/epp/contact/update/base_test.rb index c3ac3d2b0..1bcb64116 100644 --- a/test/integration/epp/contact/update/base_test.rb +++ b/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 + + + + + + john-001 + + new@inbox.test + + + + + + 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 + + + + + + john-001 + + john@inbox.test + + + + + + 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') diff --git a/test/mailers/contact_mailer_test.rb b/test/mailers/contact_mailer_test.rb new file mode 100644 index 000000000..0eadeccea --- /dev/null +++ b/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 \ No newline at end of file diff --git a/test/mailers/previews/contact_mailer_preview.rb b/test/mailers/previews/contact_mailer_preview.rb new file mode 100644 index 000000000..1e00ef673 --- /dev/null +++ b/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 \ No newline at end of file