From a8c75fb0e8a0f45afca5dd00afdfb331316ab515 Mon Sep 17 00:00:00 2001 From: Mac Siri Date: Wed, 23 Jun 2021 15:17:27 -0400 Subject: [PATCH] Re-add dynamic delivery_method for ApplicationMailer (#13994) --- app/mailers/application_mailer.rb | 15 +------------ app/mailers/concerns/deliverable.rb | 16 ++++++++++++++ app/mailers/devise_mailer.rb | 2 ++ .../follows/send_email_notification_worker.rb | 2 +- config/environments/production.rb | 21 +------------------ spec/mailers/application_mailer_spec.rb | 16 +++++++++----- 6 files changed, 32 insertions(+), 40 deletions(-) create mode 100644 app/mailers/concerns/deliverable.rb diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 6a17668a2..6cc05b641 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -6,13 +6,10 @@ class ApplicationMailer < ActionMailer::Base helper Rails.application.routes.url_helpers helper ApplicationHelper helper AuthenticationHelper + include Deliverable before_action :use_custom_host - # [@SRE] temporarily disabled - # before_action :set_delivery_options - # before_action :set_perform_deliveries - default( from: -> { email_from }, template_path: ->(mailer) { "mailers/#{mailer.class.name.underscore}" }, @@ -40,14 +37,4 @@ class ApplicationMailer < ActionMailer::Base def use_custom_host ActionMailer::Base.default_url_options[:host] = Settings::General.app_domain end - - def set_perform_deliveries - self.perform_deliveries = ForemInstance.smtp_enabled? - end - - protected - - def set_delivery_options - self.smtp_settings = Settings::SMTP.settings - end end diff --git a/app/mailers/concerns/deliverable.rb b/app/mailers/concerns/deliverable.rb new file mode 100644 index 000000000..8c0f156bf --- /dev/null +++ b/app/mailers/concerns/deliverable.rb @@ -0,0 +1,16 @@ +module Deliverable + extend ActiveSupport::Concern + + included do + before_action :set_perform_deliveries + after_action :set_delivery_options + end + + def set_perform_deliveries + self.perform_deliveries = ForemInstance.smtp_enabled? + end + + def set_delivery_options + mail.delivery_method.settings.merge!(Settings::SMTP.settings) + end +end diff --git a/app/mailers/devise_mailer.rb b/app/mailers/devise_mailer.rb index 4c695e947..6f591056c 100644 --- a/app/mailers/devise_mailer.rb +++ b/app/mailers/devise_mailer.rb @@ -1,4 +1,6 @@ class DeviseMailer < Devise::Mailer + include Deliverable + before_action :use_settings_general_values def use_settings_general_values diff --git a/app/workers/follows/send_email_notification_worker.rb b/app/workers/follows/send_email_notification_worker.rb index c107921b7..7eaa88e6f 100644 --- a/app/workers/follows/send_email_notification_worker.rb +++ b/app/workers/follows/send_email_notification_worker.rb @@ -9,7 +9,7 @@ module Follows return if EmailMessage.where(user_id: follow.followable_id) .where("sent_at > ?", rand(15..35).hours.ago) - .exists?(["subject LIKE ?", "%#{NotifyMailer.subjects[:new_follower_email]}"]) + .exists?(["subject LIKE ?", "%#{NotifyMailer.new.subjects[:new_follower_email]}"]) mailer.constantize.with(follow: follow).new_follower_email.deliver_now end diff --git a/config/environments/production.rb b/config/environments/production.rb index efa428577..b693317d4 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -127,27 +127,8 @@ Rails.application.configure do protocol = ENV["APP_PROTOCOL"] || "http://" config.action_mailer.delivery_method = :smtp - config.action_mailer.perform_deliveries = true + config.action_mailer.perform_deliveries = false config.action_mailer.default_url_options = { host: protocol + ENV["APP_DOMAIN"].to_s } - ActionMailer::Base.smtp_settings = if ENV["SENDGRID_API_KEY"].present? - { - address: "smtp.sendgrid.net", - port: 587, - authentication: :plain, - user_name: "apikey", - password: ENV["SENDGRID_API_KEY"], - domain: ENV["APP_DOMAIN"] - } - else - { - address: ENV["SMTP_ADDRESS"], - port: ENV["SMTP_PORT"], - authentication: ENV["SMTP_AUTHENTICATION"], - user_name: ENV["SMTP_USER_NAME"], - password: ENV["SMTP_PASSWORD"], - domain: ENV["SMTP_DOMAIN"] - } - end if ENV["HEROKU_APP_URL"].present? && ENV["HEROKU_APP_URL"] != ENV["APP_DOMAIN"] config.middleware.use Rack::HostRedirect, diff --git a/spec/mailers/application_mailer_spec.rb b/spec/mailers/application_mailer_spec.rb index 7f8e9970e..91a1974e9 100644 --- a/spec/mailers/application_mailer_spec.rb +++ b/spec/mailers/application_mailer_spec.rb @@ -4,7 +4,7 @@ RSpec.describe ApplicationMailer, type: :mailer do let(:user) { create(:user) } let(:email) { VerificationMailer.with(user_id: user.id).account_ownership_verification_email } - xdescribe "#set_perform_deliveries" do + describe "#set_perform_deliveries" do it "changes perform_deliveries from true to false if smtp is not enabled" do expect { email.deliver_now }.to change(described_class, :perform_deliveries).from(true).to(false) end @@ -17,11 +17,17 @@ RSpec.describe ApplicationMailer, type: :mailer do end end - xdescribe "#set_delivery_options" do - it "evoked Settings::SMTP.settings during callback" do - allow(Settings::SMTP).to receive(:settings) + describe "#set_delivery_options" do + after do + Settings::SMTP.clear_cache + end + + it "sets proper SMTP credential during callback" do + Settings::SMTP.user_name = Faker::Internet.username + Settings::SMTP.password = Faker::Internet.password email.deliver_now - expect(Settings::SMTP).to have_received(:settings) + + expect(described_class.deliveries.last.delivery_method.settings).to eq(Settings::SMTP.settings) end end end