"Reply to" and "From" Email addresses for SMTP Configurations (#16499)

* feat: create a from_email_address and a reply_to_email_address

* feat: update other mailers with the new SMTP email settings

* test: update all spec files

* fix: use the SMTP::Settings value

* fix tests

* spec: fix the comma

* feat: tighten some logic

* fix: test

* fix tests

* fix tests

* final fix test commit

* fix test

* fix test

* oops

* feat: add a reply_to for Devise Mailer

* chore: update the description

* use a proc for reply_to

* feat: update text

* Update spec/mailers/digest_mailer_spec.rb

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

* Update spec/mailers/verification_mailer_spec.rb

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

* Update spec/mailers/shared_examples/renders_proper_email_headers.rb

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

* refactor: move OPTIONS to a helper as its only being used in the view layer

Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>

app/helpers/settings_helper.rb

@@ -12,4 +12,8 @@ module SettingsHelper
   def user_experience_levels
     %w[1 3 5 8 10]
   end
+
+  def smpt_options
+    %i[user_name password address authentication domain port from_email_address reply_to_email_address].freeze
+  end
 end

app/lib/constants/settings/smtp.rb

@@ -27,6 +27,14 @@ module Constants
         domain: {
           description: "If you need to specify a HELO domain, you can do it here",
           placeholder: ""
+        },
+        from_email_address: {
+          description: "The email address that emails should be sent from",
+          placeholder: ""
+        },
+        reply_to_email_address: {
+          description: "The email address that users can reply to",
+          placeholder: ""
         }
       }.freeze
     end

app/mailers/application_mailer.rb

@@ -13,7 +13,7 @@ class ApplicationMailer < ActionMailer::Base
   default(
     from: -> { email_from },
     template_path: ->(mailer) { "mailers/#{mailer.class.name.underscore}" },
-    reply_to: -> { ForemInstance.email },
+    reply_to: -> { ForemInstance.reply_to_email_address },
   )
 
   def email_from(topic = "")
@@ -23,7 +23,7 @@ class ApplicationMailer < ActionMailer::Base
                        Settings::Community.community_name
                      end
 
-    "#{community_name} <#{ForemInstance.email}>"
+    "#{community_name} <#{ForemInstance.from_email_address}>"
   end
 
   def generate_unsubscribe_token(id, email_type)

app/mailers/devise_mailer.rb

@@ -1,11 +1,13 @@
 class DeviseMailer < Devise::Mailer
+  default reply_to: proc { ForemInstance.reply_to_email_address }
+
   include Deliverable
 
   before_action :use_settings_general_values
 
   def use_settings_general_values
     Devise.mailer_sender =
-      "#{Settings::Community.community_name} <#{ForemInstance.email}>"
+      "#{Settings::Community.community_name} <#{ForemInstance.from_email_address}>"
     ActionMailer::Base.default_url_options[:host] = Settings::General.app_domain
   end
 end

app/mailers/verification_mailer.rb

@@ -1,7 +1,7 @@
 class VerificationMailer < ApplicationMailer
   default from: lambda {
     I18n.t("mailers.verification_mailer.from", community: Settings::Community.community_name,
-                                               email: ForemInstance.email)
+                                               email: ForemInstance.from_email_address)
   }
 
   def account_ownership_verification_email

app/models/forem_instance.rb

@@ -17,6 +17,23 @@ class ForemInstance
     Settings::General.contact_email
   end
 
+  def self.reply_to_email_address
+    # Some business logic context:
+    # For a Forem Cloud Account, ApplicationConfig["DEFAULT_EMAIL"] will already be set to noreply@forem.com
+    # during the infrastructure setup and deploy.
+    # For a Forem Cloud Account that has Custom SMTP settings we want to use the Settings::SMTP.reply_to_email_address
+    # that the Forem will provide.
+    # For a selfhosted Forem we want to use the Settings::SMTP.reply_to_email_address, which will already have a
+    # default value of ApplicationConfig["DEFAULT_EMAIL"] set during their infrastructure setup even of they haven't
+    # provided the minimum settings as yet.
+    Settings::SMTP.provided_minimum_settings? ? Settings::SMTP.reply_to_email_address : email
+  end
+
+  def self.from_email_address
+    # same comment applies from self.reply_to_email_address
+    Settings::SMTP.provided_minimum_settings? ? Settings::SMTP.from_email_address : email
+  end
+
   # Return true if we are operating on a local installation, false otherwise
   def self.local?
     Settings::General.app_domain.include?("localhost")

app/models/settings/smtp.rb

@@ -1,8 +1,6 @@
 module Settings
   class SMTP < Base
     self.table_name = :settings_smtp
-
-    OPTIONS = %i[user_name password address authentication domain port].freeze
     AUTHENTICATION_METHODS = %w[plain login cram_md5].freeze
 
     setting :address, type: :string, default: ApplicationConfig["SMTP_ADDRESS"].presence
@@ -12,6 +10,10 @@ module Settings
     setting :password, type: :string, default: ApplicationConfig["SMTP_PASSWORD"].presence
     setting :port, type: :integer, default: ApplicationConfig["SMTP_PORT"].presence || 25
     setting :user_name, type: :string, default: ApplicationConfig["SMTP_USER_NAME"].presence
+    setting :from_email_address, type: :string, default: ApplicationConfig["DEFAULT_EMAIL"].presence,
+                                 validates: { email: true, allow_blank: true }
+    setting :reply_to_email_address, type: :string, default: ApplicationConfig["DEFAULT_EMAIL"].presence,
+                                     validates: { email: true, allow_blank: true }
 
     class << self
       def settings

app/views/admin/settings/forms/_smtp.html.erb

@@ -30,7 +30,7 @@
         <% end %>
 
         <div class="js-custom-smtp-section <%= "hidden" if ForemInstance.only_sendgrid_enabled? %>">
-          <% Settings::SMTP::OPTIONS.each do |config_key| %>
+          <% smpt_options.each do |config_key| %>
             <div class="crayons-field mt-3">
               <%= admin_config_label config_key, model: Settings::SMTP %>
               <%= admin_config_description Constants::Settings::SMTP::DETAILS[config_key][:description] %>

spec/mailers/devise_mailer_spec.rb

@@ -5,16 +5,25 @@ RSpec.describe DeviseMailer, type: :mailer do
 
   describe "#reset_password_instructions" do
     let(:email) { described_class.reset_password_instructions(user, "test") }
+    let(:from_email_address) { "custom_noreply@forem.com" }
+    let(:reply_to_email_address) { "custom_reply@forem.com" }
 
     before do
       allow(Settings::General).to receive(:app_domain).and_return("funky-one-of-a-kind-domain-#{rand(100)}.com")
+      allow(Settings::SMTP).to receive(:provided_minimum_settings?).and_return(true)
+      allow(Settings::SMTP).to receive(:from_email_address).and_return(from_email_address)
+      allow(Settings::SMTP).to receive(:reply_to_email_address).and_return(reply_to_email_address)
     end
 
     it "renders sender" do
-      expected_from = "#{Settings::Community.community_name} <#{ForemInstance.email}>"
+      expected_from = "#{Settings::Community.community_name} <#{from_email_address}>"
       expect(email["from"].value).to eq(expected_from)
     end
 
+    it "renders a reply to email address" do
+      expect(email["reply_to"].value).to eq(reply_to_email_address)
+    end
+
     it "renders proper URL" do
       expect(email.to_s).to include(Settings::General.app_domain)
     end

spec/mailers/digest_mailer_spec.rb

@@ -3,19 +3,22 @@ require "rails_helper"
 RSpec.describe DigestMailer, type: :mailer do
   let(:user) { create(:user) }
   let(:article) { build_stubbed(:article) }
+  let(:from_email_address) { "custom_noreply@forem.com" }
 
   describe "#digest_email" do
     before do
       allow(article).to receive(:title).and_return("test title")
+      allow(Settings::SMTP).to receive(:provided_minimum_settings?).and_return(true)
+      allow(Settings::SMTP).to receive(:from_email_address).and_return(from_email_address)
     end
 
-    it "works correctly" do
+    it "works correctly", :aggregate_failures do
       email = described_class.with(user: user, articles: [article]).digest_email
 
       expect(email.subject).not_to be_nil
       expect(email.to).to eq([user.email])
-      expect(email.from).to eq([ForemInstance.email])
-      expected_from = "#{Settings::Community.community_name} Digest <#{ForemInstance.email}>"
+      expect(email.from).to eq([from_email_address])
+      expected_from = "#{Settings::Community.community_name} Digest <#{from_email_address}>"
       expect(email["from"].value).to eq(expected_from)
     end
   end

spec/mailers/shared_examples/renders_proper_email_headers.rb

@@ -1,11 +1,20 @@
 RSpec.shared_examples "#renders_proper_email_headers" do
-  it "renders proper sender" do
-    expect(email.from).to eq([ForemInstance.email])
-    expected_from = "#{Settings::Community.community_name} <#{ForemInstance.email}>"
+  let(:from_email_address) { "custom_noreply@forem.com" }
+  let(:reply_to_email_address) { "custom_reply@forem.com" }
+
+  before do
+    allow(Settings::SMTP).to receive(:provided_minimum_settings?).and_return(true)
+    allow(Settings::SMTP).to receive(:from_email_address).and_return(from_email_address)
+    allow(Settings::SMTP).to receive(:reply_to_email_address).and_return(reply_to_email_address)
+  end
+
+  it "renders proper sender", :aggregate_failures do
+    expect(email.from).to eq([from_email_address])
+    expected_from = "#{Settings::Community.community_name} <#{from_email_address}>"
     expect(email["from"].value).to eq(expected_from)
   end
 
   it "renders proper reply_to" do
-    expect(email["reply_to"].value).to eq(ForemInstance.email)
+    expect(email["reply_to"].value).to eq(reply_to_email_address)
   end
 end

spec/mailers/verification_mailer_spec.rb

@@ -2,15 +2,21 @@ require "rails_helper"
 
 RSpec.describe VerificationMailer, type: :mailer do
   let(:user) { create(:user) }
+  let(:from_email_address) { "custom_noreply@forem.com" }
 
   describe "#account_ownership_verification_email" do
-    it "works correctly" do
+    before do
+      allow(Settings::SMTP).to receive(:provided_minimum_settings?).and_return(true)
+      allow(Settings::SMTP).to receive(:from_email_address).and_return(from_email_address)
+    end
+
+    it "works correctly", :aggregate_failures do
       email = described_class.with(user_id: user.id).account_ownership_verification_email
 
       expect(email.subject).not_to be_nil
       expect(email.to).to eq([user.email])
-      expect(email.from).to eq([ForemInstance.email])
-      from = "#{Settings::Community.community_name} Email Verification <#{ForemInstance.email}>"
+      expect(email.from).to eq([from_email_address])
+      from = "#{Settings::Community.community_name} Email Verification <#{from_email_address}>"
       expect(email["from"].value).to eq(from)
     end
   end

spec/models/forem_instance_spec.rb

@@ -109,6 +109,68 @@ RSpec.describe ForemInstance, type: :model do
     end
   end
 
+  describe ".reply_to_email_address" do
+    context "when the minimum SMTP settings have been provided" do
+      let(:reply_to_email_address) { "custom_reply@forem.com" }
+
+      before do
+        allow(Settings::SMTP).to receive(:reply_to_email_address).and_return(reply_to_email_address)
+        allow(described_class).to receive(:email).and_return("noreply@forem.com")
+        allow(Settings::SMTP).to receive(:provided_minimum_settings?).and_return(true)
+      end
+
+      it "returns the correct email address" do
+        expect(described_class.reply_to_email_address).to be(reply_to_email_address)
+      end
+    end
+
+    context "when the minimum SMTP settings have not been provided" do
+      let(:reply_to_email_address) { "custom_reply@forem.com" }
+      let(:default_email_address) { "noreply@forem.com" }
+
+      before do
+        allow(Settings::SMTP).to receive(:reply_to_email_address).and_return(reply_to_email_address)
+        allow(described_class).to receive(:email).and_return(default_email_address)
+        allow(Settings::SMTP).to receive(:provided_minimum_settings?).and_return(false)
+      end
+
+      it "returns the correct email address" do
+        expect(described_class.reply_to_email_address).to be(default_email_address)
+      end
+    end
+  end
+
+  describe ".from_email_address" do
+    context "when the minimum SMTP settings have been provided" do
+      let(:from_email_address) { "custom_noreply@forem.com" }
+
+      before do
+        allow(Settings::SMTP).to receive(:from_email_address).and_return(from_email_address)
+        allow(described_class).to receive(:email).and_return("noreply@forem.com")
+        allow(Settings::SMTP).to receive(:provided_minimum_settings?).and_return(true)
+      end
+
+      it "returns the correct email address" do
+        expect(described_class.from_email_address).to be(from_email_address)
+      end
+    end
+
+    context "when the minimum SMTP settings have not been provided" do
+      let(:from_email_address) { "custom_noreply@forem.com" }
+      let(:default_email_address) { "noreply@forem.com" }
+
+      before do
+        allow(Settings::SMTP).to receive(:from_email_address).and_return(from_email_address)
+        allow(described_class).to receive(:email).and_return(default_email_address)
+        allow(Settings::SMTP).to receive(:provided_minimum_settings?).and_return(false)
+      end
+
+      it "returns the correct email address" do
+        expect(described_class.from_email_address).to be(default_email_address)
+      end
+    end
+  end
+
   describe ".only_sendgrid_enabled?" do
     it "returns false when the minimum SMTP settings are provided" do
       allow(Settings::SMTP).to receive(:user_name).and_return("something")

spec/models/settings/smtp_spec.rb

@@ -25,13 +25,19 @@ RSpec.describe Settings::SMTP do
                                              })
     end
 
+    # rubocop:disable RSpec/ExampleLength
     it "uses Settings::SMTP config if address is provided" do
+      from_email_address = "hello@forem.com"
+      reply_to_email_address = "reply@forem.com"
+
       described_class.address = "smtp.google.com"
       described_class.port = 25
       described_class.authentication = "plain"
       described_class.user_name = "username"
       described_class.password = "password"
       described_class.domain = "forem.local"
+      described_class.reply_to_email_address = reply_to_email_address
+      described_class.from_email_address = from_email_address
 
       expect(described_class.settings).to eq({
                                                address: "smtp.google.com",
@@ -39,10 +45,13 @@ RSpec.describe Settings::SMTP do
                                                authentication: "plain",
                                                user_name: "username",
                                                password: "password",
-                                               domain: "forem.local"
+                                               domain: "forem.local",
+                                               reply_to_email_address: reply_to_email_address,
+                                               from_email_address: from_email_address
                                              })
     end
   end
+  # rubocop:enable RSpec/ExampleLength
 
   describe "::provided_minimum_settings?" do
     it "returns true if addess, user_name, and password are provided" do