diff --git a/app/jobs/domain_expire_email_job.rb b/app/jobs/domain_expire_email_job.rb index d42d343ca..9b70a54e6 100644 --- a/app/jobs/domain_expire_email_job.rb +++ b/app/jobs/domain_expire_email_job.rb @@ -4,18 +4,6 @@ class DomainExpireEmailJob < Que::Job return if domain.registered? - log(domain) DomainExpireMailer.expired(domain: domain, registrar: domain.registrar).deliver_now end - - private - - def log(domain) - message = "Send DomainExpireMailer#expired email for domain ##{domain.id} to #{domain.primary_contact_emails.join(', ')}" - logger.info(message) - end - - def logger - Rails.logger - end end diff --git a/app/mailers/domain_expire_mailer.rb b/app/mailers/domain_expire_mailer.rb index 0b869a5d2..cf960e146 100644 --- a/app/mailers/domain_expire_mailer.rb +++ b/app/mailers/domain_expire_mailer.rb @@ -1,9 +1,38 @@ class DomainExpireMailer < ApplicationMailer def expired(domain:, registrar:) - @domain = DomainPresenter.new(domain: domain, view: view_context) - @registrar = RegistrarPresenter.new(registrar: registrar, view: view_context) + @domain = domain_presenter(domain: domain) + @registrar = registrar_presenter(registrar: registrar) + recipient = filter_invalid_emails(emails: domain.primary_contact_emails, domain: domain) subject = default_i18n_subject(domain_name: domain.name) - mail(to: domain.primary_contact_emails, subject: subject) + + logger.info("Send DomainExpireMailer#expired email for domain ##{domain.id}" \ + " to #{domain.primary_contact_emails.join(', ')}") + + mail(to: recipient, subject: subject) + end + + private + + def domain_presenter(domain:) + DomainPresenter.new(domain: domain, view: view_context) + end + + def registrar_presenter(registrar:) + RegistrarPresenter.new(registrar: registrar, view: view_context) + end + + # Needed because there are invalid emails in the database, which have been imported from legacy app + def filter_invalid_emails(emails:, domain:) + emails.keep_if do |email| + valid = EmailValidator.new(email).valid? + + unless valid + logger.info("Unable to send DomainExpireMailer#expired email for domain ##{domain.id}" \ + " to invalid recipient #{email}") + end + + valid + end end end diff --git a/spec/jobs/domain_expire_email_job_spec.rb b/spec/jobs/domain_expire_email_job_spec.rb index fb7ef9822..a25999b77 100644 --- a/spec/jobs/domain_expire_email_job_spec.rb +++ b/spec/jobs/domain_expire_email_job_spec.rb @@ -18,21 +18,11 @@ RSpec.describe DomainExpireEmailJob do before :example do allow(domain).to receive_messages( - id: 1, registrar: 'registrar', registered?: false, primary_contact_emails: %w(test@test.com test@test.com)) end - it 'creates log record' do - log_message = 'Send DomainExpireMailer#expired email for domain #1 to test@test.com, test@test.com' - - allow(DomainExpireMailer).to receive(:expired).and_return(message) - allow(message).to receive(:deliver_now) - - expect(Rails.logger).to receive(:info).with(log_message) - end - it 'sends email' do expect(DomainExpireMailer).to receive(:expired).with(domain: domain, registrar: 'registrar') .and_return(message) @@ -45,10 +35,6 @@ RSpec.describe DomainExpireEmailJob do allow(domain).to receive(:registered?).and_return(true) end - it 'does not create log record' do - expect(Rails.logger).to_not receive(:info) - end - it 'does not send email' do expect(DomainExpireMailer).to_not receive(:expired) end diff --git a/spec/mailers/domain_expire_mailer_spec.rb b/spec/mailers/domain_expire_mailer_spec.rb index e536493d8..1afe6e6ad 100644 --- a/spec/mailers/domain_expire_mailer_spec.rb +++ b/spec/mailers/domain_expire_mailer_spec.rb @@ -2,32 +2,64 @@ require 'rails_helper' RSpec.describe DomainExpireMailer do describe '#expired' do - let(:domain) { instance_spy(Domain, name: 'test.com') } - let(:registrar) { 'registrar' } + let(:domain) { instance_spy(Domain, + id: 1, + name: 'test.com', + primary_contact_emails: recipient) + } let(:domain_presenter) { instance_spy(DomainPresenter) } let(:registrar_presenter) { instance_spy(RegistrarPresenter) } - subject(:message) { described_class.expired(domain: domain, registrar: registrar) } + subject(:message) { described_class.expired(domain: domain, registrar: nil) } before :example do expect(DomainPresenter).to receive(:new).and_return(domain_presenter) expect(RegistrarPresenter).to receive(:new).and_return(registrar_presenter) end - it 'has sender' do - expect(message.from).to eq(['noreply@internet.ee']) + context 'when all recipients are valid' do + let(:recipient) { %w[recipient@test.com recipient@test.com] } + + it 'has sender' do + expect(message.from).to eq(['noreply@internet.ee']) + end + + it 'delivers to all recipients' do + expect(message.to).to match_array(%w[recipient@test.com recipient@test.com]) + end + + it 'has subject' do + expect(message.subject).to eq('The test.com domain has expired') + end + + it 'creates log record' do + log_message = 'Send DomainExpireMailer#expired email for domain #1 to recipient@test.com,' \ + ' recipient@test.com' + expect(described_class.logger).to receive(:info).with(log_message) + message.deliver_now + end + + it 'sends message' do + expect { message.deliver_now }.to change { ActionMailer::Base.deliveries.count }.by(1) + end end - it 'has recipient' do - expect(domain).to receive(:primary_contact_emails).and_return(['recipient@test.com']) - expect(message.to).to match_array(['recipient@test.com']) - end + context 'when some recipient is invalid' do + let(:recipient) { %w[invalid_email valid@test.com] } - it 'has subject' do - expect(message.subject).to eq('The test.com domain has expired') - end + before :example do + allow(described_class.logger).to receive(:info) + end - it 'sends message' do - expect { message.deliver! }.to change { ActionMailer::Base.deliveries.count }.by(1) + it 'does not deliver to invalid recipient' do + expect(message.to).to match_array(%w[valid@test.com]) + end + + it 'creates log record' do + log_message = 'Unable to send DomainExpireMailer#expired email for domain #1 to' \ + ' invalid recipient invalid_email' + expect(described_class.logger).to receive(:info).with(log_message) + message.deliver_now + end end end end