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.

lib/email/sender.rb

@@ -12,7 +12,15 @@
 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 @@ def send
       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 @@ def send
       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)

spec/components/email/sender_spec.rb

@@ -52,6 +52,20 @@
         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