FIX: Confirm new email not sent for staff if email disabled with "non-staff" option (#10794)

FIX: Confirm new email not sent for staff if email disabled with “non-staff” option (#10794)

See https://meta.discourse.org/t/email-address-change-confirmation-email-not-sent-but-every-other-notification-emails-are/165358

In short: with disable emails set to non-staff, email address change confirmation emails (those sent to the new address) are not sent for staff or admin members.

This was happening because we were looking up the staff user with the to_address of the email, but the to address was the new email address because we are sending a confirm email change email, and thus the user could not be found. We didn’t need to do this anyway because we are passing the user into the Email::Sender class anyway.

diff --git a/lib/email/sender.rb b/lib/email/sender.rb
index 027d858..0668d7e 100644
--- a/lib/email/sender.rb
+++ b/lib/email/sender.rb
@@ -12,7 +12,15 @@ require 'uri'
 require 'net/smtp'
 
 SMTP_CLIENT_ERRORS = [Net::SMTPFatalError, Net::SMTPSyntaxError]
-BYPASS_DISABLE_TYPES = ["admin_login", "test_message"]
+BYPASS_DISABLE_TYPES = %w(
+  admin_login
+  test_message
+  new_version
+  group_smtp
+  invite
+  download_backup_message
+  admin_confirmation_message
+)
 
 module Email
   class Sender
@@ -37,7 +45,7 @@ module Email
       return skip(SkippedEmailLog.reason_types[:sender_message_to_blank]) if @message.to.blank?
 
       if SiteSetting.disable_emails == "non-staff" && !bypass_disable
-        return unless User.find_by_email(to_address)&.staff?
+        return unless find_user&.staff?
       end
 
       return skip(SkippedEmailLog.reason_types[:sender_message_to_invalid]) if to_address.end_with?(".invalid")
@@ -232,6 +240,11 @@ module Email
       email_log
     end
 
+    def find_user
+      return @user if @user
+      User.find_by_email(to_address)
+    end
+
     def to_address
       @to_address ||= begin
         to = @message.try(:to)
diff --git a/spec/components/email/sender_spec.rb b/spec/components/email/sender_spec.rb
index d3c7741..6732bc1 100644
--- a/spec/components/email/sender_spec.rb
+++ b/spec/components/email/sender_spec.rb
@@ -52,6 +52,20 @@ describe Email::Sender do
         message = Mail::Message.new(to: moderator.email, body: "hello")
         Email::Sender.new(message, :hello).send
       end
+
+      it "delivers mail to staff user when confirming new email if user is provided" do
+        Mail::Message.any_instance.expects(:deliver_now).once
+        Fabricate(:email_change_request, {
+          user: moderator,
+          new_email: "newemail@testmoderator.com",
+          old_email: moderator.email,
+          change_state: EmailChangeRequest.states[:authorizing_new]
+        })
+        message = Mail::Message.new(
+          to: "newemail@testmoderator.com", body: "hello"
+        )
+        Email::Sender.new(message, :confirm_new_email, moderator).send
+      end
     end
   end
 

GitHub sha: f63da1c1

This commit appears in #10794 which was approved by danielwaterworth. It was merged by martin.