From 1a0ba9cfd083f160942d70f5ef9e2eb697030fbf Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Mon, 15 Apr 2024 08:06:52 -0400 Subject: [PATCH] LG-12294: Fix asynchronous email delivery for aggregated sign-in email (#10421) * Fix asynchronous email delivery for aggregated sign-in email changelog: Upcoming Features, Sign In, Send single aggregated email notification for new device sign-in * Add exclusion for current sync-only mailers * Default synchronous_only to false --- .../alert_user_about_new_device.rb | 2 +- spec/mailers/report_mailer_spec.rb | 2 +- spec/mailers/user_mailer_spec.rb | 28 ++++++++++++++++++- spec/support/shared_examples_for_mailer.rb | 6 +++- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/app/services/user_alerts/alert_user_about_new_device.rb b/app/services/user_alerts/alert_user_about_new_device.rb index e4a8bfb4483..23bd3996464 100644 --- a/app/services/user_alerts/alert_user_about_new_device.rb +++ b/app/services/user_alerts/alert_user_about_new_device.rb @@ -33,7 +33,7 @@ def self.send_alert(user:, disavowal_event:, disavowal_token:) 'sign_in_unsuccessful_2fa', 'sign_in_after_2fa', ], - ).order(:created_at).includes(:device) + ).order(:created_at).includes(:device).to_a user.confirmed_email_addresses.each do |email_address| mailer = UserMailer.with(user:, email_address:) diff --git a/spec/mailers/report_mailer_spec.rb b/spec/mailers/report_mailer_spec.rb index 147661eedb8..de7988e5d81 100644 --- a/spec/mailers/report_mailer_spec.rb +++ b/spec/mailers/report_mailer_spec.rb @@ -15,7 +15,7 @@ ) end - it_behaves_like 'a system email' + it_behaves_like 'a system email', synchronous_only: true it 'sends to the current email' do expect(mail.to).to eq [email_address.email] diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index aaac96febaf..9c4063bd135 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.describe UserMailer, type: :mailer do - let(:user) { build(:user) } + let(:user) { create(:user) } let(:email_address) { user.email_addresses.first } let(:banned_email) { 'banned_email+123abc@gmail.com' } let(:banned_email_address) { create(:email_address, email: banned_email, user: user) } @@ -205,6 +205,32 @@ end end + describe '#new_device_sign_in_before_2fa' do + let(:event) { create(:event, event_type: :sign_in_before_2fa, user:, device: create(:device)) } + subject(:mail) do + UserMailer.with(user:, email_address:).new_device_sign_in_before_2fa( + events: user.events.where(event_type: 'sign_in_before_2fa').includes(:device).to_a, + disavowal_token: 'token', + ) + end + + it_behaves_like 'a system email' + it_behaves_like 'an email that respects user email locale preference' + end + + describe '#new_device_sign_in_after_2fa' do + let(:event) { create(:event, event_type: :sign_in_after_2fa, user:, device: create(:device)) } + subject(:mail) do + UserMailer.with(user:, email_address:).new_device_sign_in_after_2fa( + events: user.events.where(event_type: 'sign_in_after_2fa').includes(:device).to_a, + disavowal_token: 'token', + ) + end + + it_behaves_like 'a system email' + it_behaves_like 'an email that respects user email locale preference' + end + describe '#personal_key_regenerated' do let(:mail) do UserMailer.with(user: user, email_address: email_address).personal_key_regenerated diff --git a/spec/support/shared_examples_for_mailer.rb b/spec/support/shared_examples_for_mailer.rb index 5c2aaac7026..0ef1d70a3c9 100644 --- a/spec/support/shared_examples_for_mailer.rb +++ b/spec/support/shared_examples_for_mailer.rb @@ -1,4 +1,4 @@ -RSpec.shared_examples 'a system email' do +RSpec.shared_examples 'a system email' do |synchronous_only: false| it 'is from the default email' do expect(mail.from).to eq [IdentityConfig.store.email_from] expect(mail[:from].display_names).to eq [IdentityConfig.store.email_from_display_name] @@ -10,6 +10,10 @@ # https://www.caniemail.com/features/image-svg/ expect(body).not_to have_css('img[src$=".svg"]') end + + it 'does not error when delivered asynchronously', unless: synchronous_only do + mail.deliver_later + end end # expects there to be a let(:user) in scope